Skip to content

Commit ee4b4ac

Browse files
kimdvkimdv
committed
Add newline fix-it after consecutive declarations
1 parent a65a0c3 commit ee4b4ac

File tree

7 files changed

+343
-28
lines changed

7 files changed

+343
-28
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: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,23 @@ fileprivate func getTokens(between first: TokenSyntax, and second: TokenSyntax)
5050
return tokens
5151
}
5252

53+
fileprivate func indentation(for token: TokenSyntax) -> Trivia {
54+
var ancestor: Syntax = Syntax(token)
55+
while let parent = ancestor.parent {
56+
ancestor = parent
57+
58+
if let codeBlockItemSyntax = ancestor.as(CodeBlockItemSyntax.self) {
59+
return codeBlockItemSyntax.leadingTrivia.indentation(isOnNewline: true) ?? []
60+
}
61+
62+
if let memberDeclListItemSyntax = ancestor.as(MemberDeclListItemSyntax.self) {
63+
return memberDeclListItemSyntax.leadingTrivia.indentation(isOnNewline: true) ?? []
64+
}
65+
}
66+
67+
return []
68+
}
69+
5370
fileprivate extension TokenSyntax {
5471
/// Assuming this token is a `poundAvailableKeyword` or `poundUnavailableKeyword`
5572
/// returns the opposite keyword.
@@ -1160,8 +1177,16 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
11601177
position: position,
11611178
.consecutiveDeclarationsOnSameLine,
11621179
fixIts: [
1163-
FixIt(message: .insertSemicolon, changes: .makePresent(semicolon))
1164-
],
1180+
node.lastToken(viewMode: .sourceAccurate).map {
1181+
FixIt(
1182+
message: .insertNewline,
1183+
changes: [
1184+
.replaceTrailingTrivia(token: $0, newTrivia: $0.trailingTrivia + .newlines(1) + indentation(for: $0))
1185+
]
1186+
)
1187+
},
1188+
FixIt(message: .insertSemicolon, changes: .makePresent(semicolon)),
1189+
].compactMap { $0 },
11651190
handledNodes: [semicolon.id]
11661191
)
11671192
} else {

‎Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ 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 {
112112
.init("consecutive statements on a line must be separated by ';'")

‎Tests/SwiftParserTest/Assertions.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ 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 }
278+
let messages = messages ?? diagnostics.compactMap { $0.fixIts.first?.message.message }
279279

280280
self.changes =
281281
diagnostics

‎Tests/SwiftParserTest/translated/ConsecutiveStatementsTests.swift‎

Lines changed: 134 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,16 @@ final class ConsecutiveStatementsTests: XCTestCase {
102102
}
103103
""",
104104
diagnostics: [
105-
DiagnosticSpec(locationMarker: "1️⃣", message: "consecutive declarations on a line must be separated by ';'", fixIts: ["insert ';'"]),
106-
DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive declarations on a line must be separated by ';'", fixIts: ["insert ';'"]),
105+
DiagnosticSpec(
106+
locationMarker: "1️⃣",
107+
message: "consecutive declarations on a line must be separated by newline or ';'",
108+
fixIts: ["insert newline", "insert ';'"]
109+
),
110+
DiagnosticSpec(
111+
locationMarker: "2️⃣",
112+
message: "consecutive declarations on a line must be separated by newline or ';'",
113+
fixIts: ["insert newline", "insert ';'"]
114+
),
107115
DiagnosticSpec(locationMarker: "3️⃣", message: "consecutive statements on a line must be separated by ';'", fixIts: ["insert ';'"]),
108116
DiagnosticSpec(locationMarker: "4️⃣", message: "consecutive statements on a line must be separated by ';'", fixIts: ["insert ';'"]),
109117
DiagnosticSpec(locationMarker: "5️⃣", message: "consecutive statements on a line must be separated by ';'", fixIts: ["insert ';'"]),
@@ -129,7 +137,7 @@ final class ConsecutiveStatementsTests: XCTestCase {
129137
)
130138
}
131139

132-
func testConsecutiveStatements4() {
140+
func testConsecutiveStatements4a() {
133141
assertParse(
134142
"""
135143
class C {
@@ -142,8 +150,39 @@ final class ConsecutiveStatementsTests: XCTestCase {
142150
}
143151
""",
144152
diagnostics: [
145-
DiagnosticSpec(message: "consecutive declarations on a line must be separated by ';'", fixIts: ["insert ';'"])
153+
DiagnosticSpec(message: "consecutive declarations on a line must be separated by newline or ';'", fixIts: ["insert newline", "insert ';'"])
146154
],
155+
applyFixIts: ["insert newline"],
156+
fixedSource: """
157+
class C {
158+
// In a sequence of declarations.
159+
var a, b : Int
160+
func d() -> Int {}
161+
init() {
162+
a = 0
163+
b = 0
164+
}
165+
}
166+
"""
167+
)
168+
}
169+
170+
func testConsecutiveStatements4b() {
171+
assertParse(
172+
"""
173+
class C {
174+
// In a sequence of declarations.
175+
var a, b : Int1️⃣ func d() -> Int {}
176+
init() {
177+
a = 0
178+
b = 0
179+
}
180+
}
181+
""",
182+
diagnostics: [
183+
DiagnosticSpec(message: "consecutive declarations on a line must be separated by newline or ';'", fixIts: ["insert newline", "insert ';'"])
184+
],
185+
applyFixIts: ["insert ';'"],
147186
fixedSource: """
148187
class C {
149188
// In a sequence of declarations.
@@ -157,16 +196,37 @@ final class ConsecutiveStatementsTests: XCTestCase {
157196
)
158197
}
159198

160-
func testConsecutiveStatements5() {
199+
func testConsecutiveStatements5a() {
200+
assertParse(
201+
"""
202+
protocol P {
203+
func a()1️⃣ func b()
204+
}
205+
""",
206+
diagnostics: [
207+
DiagnosticSpec(message: "consecutive declarations on a line must be separated by newline or ';'", fixIts: ["insert newline", "insert ';'"])
208+
],
209+
applyFixIts: ["insert newline"],
210+
fixedSource: """
211+
protocol P {
212+
func a()
213+
func b()
214+
}
215+
"""
216+
)
217+
}
218+
219+
func testConsecutiveStatements5b() {
161220
assertParse(
162221
"""
163222
protocol P {
164223
func a()1️⃣ func b()
165224
}
166225
""",
167226
diagnostics: [
168-
DiagnosticSpec(message: "consecutive declarations on a line must be separated by ';'", fixIts: ["insert ';'"])
227+
DiagnosticSpec(message: "consecutive declarations on a line must be separated by newline or ';'", fixIts: ["insert newline", "insert ';'"])
169228
],
229+
applyFixIts: ["insert ';'"],
170230
fixedSource: """
171231
protocol P {
172232
func a(); func b()
@@ -175,7 +235,39 @@ final class ConsecutiveStatementsTests: XCTestCase {
175235
)
176236
}
177237

178-
func testConsecutiveStatements6() {
238+
func testConsecutiveStatements6a() {
239+
assertParse(
240+
"""
241+
enum Color {
242+
case Red1️⃣ case Blue
243+
func a() {}2️⃣ func b() {}
244+
}
245+
""",
246+
diagnostics: [
247+
DiagnosticSpec(
248+
locationMarker: "1️⃣",
249+
message: "consecutive declarations on a line must be separated by newline or ';'",
250+
fixIts: ["insert newline", "insert ';'"]
251+
),
252+
DiagnosticSpec(
253+
locationMarker: "2️⃣",
254+
message: "consecutive declarations on a line must be separated by newline or ';'",
255+
fixIts: ["insert newline", "insert ';'"]
256+
),
257+
],
258+
applyFixIts: ["insert newline"],
259+
fixedSource: """
260+
enum Color {
261+
case Red
262+
case Blue
263+
func a() {}
264+
func b() {}
265+
}
266+
"""
267+
)
268+
}
269+
270+
func testConsecutiveStatements6b() {
179271
assertParse(
180272
"""
181273
enum Color {
@@ -184,9 +276,18 @@ final class ConsecutiveStatementsTests: XCTestCase {
184276
}
185277
""",
186278
diagnostics: [
187-
DiagnosticSpec(locationMarker: "1️⃣", message: "consecutive declarations on a line must be separated by ';'", fixIts: ["insert ';'"]),
188-
DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive declarations on a line must be separated by ';'", fixIts: ["insert ';'"]),
279+
DiagnosticSpec(
280+
locationMarker: "1️⃣",
281+
message: "consecutive declarations on a line must be separated by newline or ';'",
282+
fixIts: ["insert newline", "insert ';'"]
283+
),
284+
DiagnosticSpec(
285+
locationMarker: "2️⃣",
286+
message: "consecutive declarations on a line must be separated by newline or ';'",
287+
fixIts: ["insert newline", "insert ';'"]
288+
),
189289
],
290+
applyFixIts: ["insert ';'"],
190291
fixedSource: """
191292
enum Color {
192293
case Red; case Blue
@@ -212,4 +313,28 @@ final class ConsecutiveStatementsTests: XCTestCase {
212313
"""
213314
)
214315
}
316+
317+
func testConsecutiveStatements8() {
318+
assertParse(
319+
"""
320+
class Foo {
321+
func a() {}1️⃣/* some comment */ func b() {}
322+
}
323+
""",
324+
diagnostics: [
325+
DiagnosticSpec(
326+
locationMarker: "1️⃣",
327+
message: "consecutive declarations on a line must be separated by newline or ';'",
328+
fixIts: ["insert newline", "insert ';'"]
329+
)
330+
],
331+
applyFixIts: ["insert newline"],
332+
fixedSource: """
333+
class Foo {
334+
func a() {}/* some comment */
335+
func b() {}
336+
}
337+
"""
338+
)
339+
}
215340
}

‎Tests/SwiftParserTest/translated/RecoveryTests.swift‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -938,7 +938,7 @@ final class RecoveryTests: XCTestCase {
938938
)
939939
}
940940

941-
func testRecovery62() {
941+
func testRecovery62a() {
942942
assertParse(
943943
"""
944944
enum EE 1️⃣EE<T> where T : Multi {
@@ -954,11 +954,44 @@ final class RecoveryTests: XCTestCase {
954954
),
955955
DiagnosticSpec(
956956
locationMarker: "2️⃣",
957-
message: "consecutive declarations on a line must be separated by ';'",
958-
fixIts: ["insert ';'"]
957+
message: "consecutive declarations on a line must be separated by newline or ';'",
958+
fixIts: ["insert newline", "insert ';'"]
959+
),
960+
DiagnosticSpec(locationMarker: "3️⃣", message: "unexpected code 'a' before enum case"),
961+
],
962+
applyFixIts: ["join the identifiers together", "insert newline"],
963+
fixedSource: """
964+
enum EEEE<T> where T : Multi {
965+
case a
966+
a
967+
case b
968+
}
969+
"""
970+
)
971+
}
972+
973+
func testRecovery62b() {
974+
assertParse(
975+
"""
976+
enum EE 1️⃣EE<T> where T : Multi {
977+
case a2️⃣ 3️⃣a
978+
case b
979+
}
980+
""",
981+
diagnostics: [
982+
DiagnosticSpec(
983+
locationMarker: "1️⃣",
984+
message: "found an unexpected second identifier in enum; is there an accidental break?",
985+
fixIts: ["join the identifiers together"]
986+
),
987+
DiagnosticSpec(
988+
locationMarker: "2️⃣",
989+
message: "consecutive declarations on a line must be separated by newline or ';'",
990+
fixIts: ["insert newline", "insert ';'"]
959991
),
960992
DiagnosticSpec(locationMarker: "3️⃣", message: "unexpected code 'a' before enum case"),
961993
],
994+
applyFixIts: ["join the identifiers together", "insert ';'"],
962995
fixedSource: """
963996
enum EEEE<T> where T : Multi {
964997
case a;a
@@ -1565,8 +1598,8 @@ final class RecoveryTests: XCTestCase {
15651598
diagnostics: [
15661599
DiagnosticSpec(
15671600
locationMarker: "1️⃣",
1568-
message: "consecutive declarations on a line must be separated by ';'",
1569-
fixIts: ["insert ';'"]
1601+
message: "consecutive declarations on a line must be separated by newline or ';'",
1602+
fixIts: ["insert newline", "insert ';'"]
15701603
),
15711604
DiagnosticSpec(
15721605
locationMarker: "1️⃣",
@@ -1605,7 +1638,8 @@ final class RecoveryTests: XCTestCase {
16051638
],
16061639
fixedSource: """
16071640
struct ErrorTypeInVarDeclDictionaryType {
1608-
let a1: String;:
1641+
let a1: String
1642+
:
16091643
let a2: [String: Int]
16101644
let a3: [String: [Int]]
16111645
let a4: [String: Int]

0 commit comments

Comments
 (0)