Skip to content

Commit cf0c99a

Browse files
kimdvkimdv
authored
Merge pull request swiftlang#1698 from kimdv/kimdv/add-newline-after-declarations
Add newline after consecutive declarations
2 parents 34453e6 + fc780bc commit cf0c99a

16 files changed

+1620
-124
lines changed

‎Sources/SwiftBasicFormat/Trivia+FormatExtensions.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ extension Trivia {
4545

4646
/// Returns the indentation of the last trivia piece in this trivia that is
4747
/// not a whitespace.
48-
func indentation(isOnNewline: Bool) -> Trivia? {
48+
/// - Parameter isOnNewline: Specifies if the character before this trivia is a newline character, i.e. if this trivia already starts on a new line.
49+
/// - Returns: An optional ``Trivia`` with indentation of the last trivia piece.
50+
public func indentation(isOnNewline: Bool) -> Trivia? {
4951
let lastNonWhitespaceTriviaPieceIndex = self.pieces.lastIndex(where: { !$0.isWhitespace }) ?? self.pieces.endIndex
5052
let piecesBeforeLastNonWhitespace = self.pieces[..<lastNonWhitespaceTriviaPieceIndex]
5153
let indentation: ArraySlice<TriviaPiece>

‎Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 61 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,31 @@ fileprivate func getTokens(between first: TokenSyntax, and second: TokenSyntax)
5151
}
5252

5353
fileprivate extension TokenSyntax {
54+
/// The indentation of this token
55+
///
56+
/// In contrast to `indentation`, this does not walk to previous tokens to
57+
/// find the indentation of the line this token occurs on.
58+
private var indentation: Trivia? {
59+
let previous = self.previousToken(viewMode: .sourceAccurate)
60+
return ((previous?.trailingTrivia ?? []) + leadingTrivia).indentation(isOnNewline: false)
61+
}
62+
63+
/// Returns the indentation of the line this token occurs on
64+
var indentationOfLine: Trivia {
65+
var token: TokenSyntax = self
66+
if let indentation = token.indentation {
67+
return indentation
68+
}
69+
while let previous = token.previousToken(viewMode: .sourceAccurate) {
70+
token = previous
71+
if let indentation = token.indentation {
72+
return indentation
73+
}
74+
}
75+
76+
return []
77+
}
78+
5479
/// Assuming this token is a `poundAvailableKeyword` or `poundUnavailableKeyword`
5580
/// returns the opposite keyword.
5681
var negatedAvailabilityKeyword: TokenSyntax {
@@ -702,13 +727,28 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
702727
// Only diagnose the missing semicolon if the item doesn't contain any errors.
703728
// If the item contains errors, the root cause is most likely something different and not the missing semicolon.
704729
let position = semicolon.previousToken(viewMode: .sourceAccurate)?.endPositionBeforeTrailingTrivia
730+
var fixIts: [FixIt] = [
731+
FixIt(message: .insertSemicolon, changes: .makePresent(semicolon))
732+
]
733+
if let firstToken = node.firstToken(viewMode: .sourceAccurate),
734+
let lastToken = node.lastToken(viewMode: .sourceAccurate)
735+
{
736+
fixIts.insert(
737+
FixIt(
738+
message: .insertNewline,
739+
changes: [
740+
.replaceTrailingTrivia(token: lastToken, newTrivia: lastToken.trailingTrivia + .newlines(1) + firstToken.indentationOfLine)
741+
]
742+
),
743+
at: 0
744+
)
745+
}
746+
705747
addDiagnostic(
706748
semicolon,
707749
position: position,
708750
.consecutiveStatementsOnSameLine,
709-
fixIts: [
710-
FixIt(message: .insertSemicolon, changes: .makePresent(semicolon))
711-
],
751+
fixIts: fixIts,
712752
handledNodes: [semicolon.id]
713753
)
714754
} else {
@@ -1155,13 +1195,28 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
11551195
// Only diagnose the missing semicolon if the decl doesn't contain any errors.
11561196
// If the decl contains errors, the root cause is most likely something different and not the missing semicolon.
11571197
let position = semicolon.previousToken(viewMode: .sourceAccurate)?.endPositionBeforeTrailingTrivia
1198+
var fixIts: [FixIt] = [
1199+
FixIt(message: .insertSemicolon, changes: .makePresent(semicolon))
1200+
]
1201+
if let firstToken = node.firstToken(viewMode: .sourceAccurate),
1202+
let lastToken = node.lastToken(viewMode: .sourceAccurate)
1203+
{
1204+
fixIts.insert(
1205+
FixIt(
1206+
message: .insertNewline,
1207+
changes: [
1208+
.replaceTrailingTrivia(token: lastToken, newTrivia: lastToken.trailingTrivia + .newlines(1) + firstToken.indentationOfLine)
1209+
]
1210+
),
1211+
at: 0
1212+
)
1213+
}
1214+
11581215
addDiagnostic(
11591216
semicolon,
11601217
position: position,
11611218
.consecutiveDeclarationsOnSameLine,
1162-
fixIts: [
1163-
FixIt(message: .insertSemicolon, changes: .makePresent(semicolon))
1164-
],
1219+
fixIts: fixIts,
11651220
handledNodes: [semicolon.id]
11661221
)
11671222
} else {

‎Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,10 @@ extension DiagnosticMessage where Self == StaticParserError {
106106
.init("'class' constraint can only appear on protocol declarations")
107107
}
108108
public static var consecutiveDeclarationsOnSameLine: Self {
109-
.init("consecutive declarations on a line must be separated by ';'")
109+
.init("consecutive declarations on a line must be separated by newline or ';'")
110110
}
111111
public static var consecutiveStatementsOnSameLine: Self {
112-
.init("consecutive statements on a line must be separated by ';'")
112+
.init("consecutive statements on a line must be separated by newline or ';'")
113113
}
114114
public static var cStyleForLoop: Self {
115115
.init("C-style for statement has been removed in Swift 3")

‎Tests/SwiftParserTest/Assertions.swift

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -275,15 +275,13 @@ class FixItApplier: SyntaxRewriter {
275275
var changes: [FixIt.Change]
276276

277277
init(diagnostics: [Diagnostic], withMessages messages: [String]?) {
278+
let messages = messages ?? diagnostics.compactMap { $0.fixIts.first?.message.message }
279+
278280
self.changes =
279281
diagnostics
280282
.flatMap { $0.fixIts }
281283
.filter {
282-
if let messages {
283-
return messages.contains($0.message.message)
284-
} else {
285-
return true
286-
}
284+
return messages.contains($0.message.message)
287285
}
288286
.flatMap { $0.changes }
289287
}

‎Tests/SwiftParserTest/DeclarationTests.swift

Lines changed: 128 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,54 @@ final class DeclarationTests: XCTestCase {
329329
internal(set) var defaultProp = 0
330330
""",
331331
diagnostics: [
332-
DiagnosticSpec(locationMarker: "1️⃣", message: "consecutive statements on a line must be separated by ';'", fixIts: ["insert ';'"]),
333-
DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive statements on a line must be separated by ';'", fixIts: ["insert ';'"]),
332+
DiagnosticSpec(
333+
locationMarker: "1️⃣",
334+
message: "consecutive statements on a line must be separated by newline or ';'",
335+
fixIts: ["insert newline", "insert ';'"]
336+
),
337+
DiagnosticSpec(
338+
locationMarker: "2️⃣",
339+
message: "consecutive statements on a line must be separated by newline or ';'",
340+
fixIts: ["insert newline", "insert ';'"]
341+
),
342+
],
343+
applyFixIts: ["insert newline"],
344+
fixedSource: """
345+
open
346+
open(set)
347+
var openProp = 0
348+
public public(set) var publicProp = 0
349+
package package(set) var packageProp = 0
350+
internal internal(set) var internalProp = 0
351+
fileprivate fileprivate(set) var fileprivateProp = 0
352+
private private(set) var privateProp = 0
353+
internal(set) var defaultProp = 0
354+
"""
355+
)
356+
357+
assertParse(
358+
"""
359+
open1️⃣ open(set)2️⃣ var openProp = 0
360+
public public(set) var publicProp = 0
361+
package package(set) var packageProp = 0
362+
internal internal(set) var internalProp = 0
363+
fileprivate fileprivate(set) var fileprivateProp = 0
364+
private private(set) var privateProp = 0
365+
internal(set) var defaultProp = 0
366+
""",
367+
diagnostics: [
368+
DiagnosticSpec(
369+
locationMarker: "1️⃣",
370+
message: "consecutive statements on a line must be separated by newline or ';'",
371+
fixIts: ["insert newline", "insert ';'"]
372+
),
373+
DiagnosticSpec(
374+
locationMarker: "2️⃣",
375+
message: "consecutive statements on a line must be separated by newline or ';'",
376+
fixIts: ["insert newline", "insert ';'"]
377+
),
334378
],
379+
applyFixIts: ["insert ';'"],
335380
fixedSource: """
336381
open; open(set); var openProp = 0
337382
public public(set) var publicProp = 0
@@ -1025,18 +1070,74 @@ final class DeclarationTests: XCTestCase {
10251070
)
10261071
}
10271072

1028-
func testTextRecovery() {
1073+
func testTextRecovery1() {
1074+
assertParse(
1075+
"""
1076+
Lorem1️⃣ ipsum2️⃣ dolor3️⃣ sit4️⃣ amet5️⃣, consectetur adipiscing elit
1077+
""",
1078+
diagnostics: [
1079+
DiagnosticSpec(
1080+
locationMarker: "1️⃣",
1081+
message: "consecutive statements on a line must be separated by newline or ';'",
1082+
fixIts: ["insert newline", "insert ';'"]
1083+
),
1084+
DiagnosticSpec(
1085+
locationMarker: "2️⃣",
1086+
message: "consecutive statements on a line must be separated by newline or ';'",
1087+
fixIts: ["insert newline", "insert ';'"]
1088+
),
1089+
DiagnosticSpec(
1090+
locationMarker: "3️⃣",
1091+
message: "consecutive statements on a line must be separated by newline or ';'",
1092+
fixIts: ["insert newline", "insert ';'"]
1093+
),
1094+
DiagnosticSpec(
1095+
locationMarker: "4️⃣",
1096+
message: "consecutive statements on a line must be separated by newline or ';'",
1097+
fixIts: ["insert newline", "insert ';'"]
1098+
),
1099+
DiagnosticSpec(locationMarker: "5️⃣", message: "extraneous code ', consectetur adipiscing elit' at top level"),
1100+
],
1101+
applyFixIts: ["insert newline"],
1102+
fixedSource: """
1103+
Lorem
1104+
ipsum
1105+
dolor
1106+
sit
1107+
amet, consectetur adipiscing elit
1108+
"""
1109+
)
1110+
}
1111+
1112+
func testTextRecovery2() {
10291113
assertParse(
10301114
"""
10311115
Lorem1️⃣ ipsum2️⃣ dolor3️⃣ sit4️⃣ amet5️⃣, consectetur adipiscing elit
10321116
""",
10331117
diagnostics: [
1034-
DiagnosticSpec(locationMarker: "1️⃣", message: "consecutive statements on a line must be separated by ';'", fixIts: ["insert ';'"]),
1035-
DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive statements on a line must be separated by ';'", fixIts: ["insert ';'"]),
1036-
DiagnosticSpec(locationMarker: "3️⃣", message: "consecutive statements on a line must be separated by ';'", fixIts: ["insert ';'"]),
1037-
DiagnosticSpec(locationMarker: "4️⃣", message: "consecutive statements on a line must be separated by ';'", fixIts: ["insert ';'"]),
1118+
DiagnosticSpec(
1119+
locationMarker: "1️⃣",
1120+
message: "consecutive statements on a line must be separated by newline or ';'",
1121+
fixIts: ["insert newline", "insert ';'"]
1122+
),
1123+
DiagnosticSpec(
1124+
locationMarker: "2️⃣",
1125+
message: "consecutive statements on a line must be separated by newline or ';'",
1126+
fixIts: ["insert newline", "insert ';'"]
1127+
),
1128+
DiagnosticSpec(
1129+
locationMarker: "3️⃣",
1130+
message: "consecutive statements on a line must be separated by newline or ';'",
1131+
fixIts: ["insert newline", "insert ';'"]
1132+
),
1133+
DiagnosticSpec(
1134+
locationMarker: "4️⃣",
1135+
message: "consecutive statements on a line must be separated by newline or ';'",
1136+
fixIts: ["insert newline", "insert ';'"]
1137+
),
10381138
DiagnosticSpec(locationMarker: "5️⃣", message: "extraneous code ', consectetur adipiscing elit' at top level"),
10391139
],
1140+
applyFixIts: ["insert ';'"],
10401141
fixedSource: """
10411142
Lorem; ipsum; dolor; sit; amet, consectetur adipiscing elit
10421143
"""
@@ -2409,4 +2510,24 @@ final class DeclarationTests: XCTestCase {
24092510
fixedSource: "protocol X<<#identifier#>> {}"
24102511
)
24112512
}
2513+
2514+
func testCorrectIndentationWithNewline() {
2515+
assertParse(
2516+
"""
2517+
func
2518+
test() {}1️⃣ var other: Int
2519+
""",
2520+
diagnostics: [
2521+
DiagnosticSpec(
2522+
message: "consecutive statements on a line must be separated by newline or ';'",
2523+
fixIts: ["insert newline", "insert ';'"]
2524+
)
2525+
],
2526+
fixedSource: """
2527+
func
2528+
test() {}
2529+
var other: Int
2530+
"""
2531+
)
2532+
}
24122533
}

0 commit comments

Comments
 (0)