Skip to content

Commit 97f0a75

Browse files
ahoppenahoppen
authored
Merge pull request #870 from ahoppen/ahoppen/single-statement-per-line
Disallow multiple statements on the same line
2 parents 4f2e153 + 9448d79 commit 97f0a75

32 files changed

+1603
-580
lines changed

‎Sources/SwiftDiagnostics/FixIt.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ public struct FixIt {
2929
public enum Change {
3030
/// Replace `oldNode` by `newNode`.
3131
case replace(oldNode: Syntax, newNode: Syntax)
32+
/// Remove the trailing trivia of the given token
33+
case removeTrailingTrivia(TokenSyntax)
3234
}
3335

3436
/// A description of what this Fix-It performs.

‎Sources/SwiftParser/Diagnostics/DiagnosticExtensions.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,20 @@ extension FixIt.Change {
3636
)
3737
}
3838
}
39+
40+
extension Array where Element == FixIt.Change {
41+
/// Makes the `token` present, moving it in front of the previous token's trivia.
42+
static func makePresentBeforeTrivia(token: TokenSyntax) -> [FixIt.Change] {
43+
if let previousToken = token.previousToken(viewMode: .sourceAccurate) {
44+
let presentToken = PresentMaker().visit(token).withTrailingTrivia(previousToken.trailingTrivia)
45+
return [
46+
.removeTrailingTrivia(previousToken),
47+
.replace(oldNode: Syntax(token), newNode: presentToken),
48+
]
49+
} else {
50+
return [
51+
.makePresent(node: token)
52+
]
53+
}
54+
}
55+
}

‎Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,16 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
131131

132132
// MARK: - Specialized diagnostic generation
133133

134+
public override func visit(_ node: CodeBlockItemSyntax) -> SyntaxVisitorContinueKind {
135+
if let semicolon = node.semicolon, semicolon.presence == .missing {
136+
let position = semicolon.previousToken(viewMode: .sourceAccurate)?.endPositionBeforeTrailingTrivia
137+
addDiagnostic(semicolon, position: position, .consecutiveStatementsOnSameLine, fixIts: [
138+
FixIt(message: .insertSemicolon, changes: .makePresentBeforeTrivia(token: semicolon))
139+
], handledNodes: [semicolon.id])
140+
}
141+
return .visitChildren
142+
}
143+
134144
public override func visit(_ node: MissingDeclSyntax) -> SyntaxVisitorContinueKind {
135145
return handleMissingSyntax(node)
136146
}

‎Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ public extension ParserFixIt {
6666

6767
/// Please order the cases in this enum alphabetically by case name.
6868
public enum StaticParserError: String, DiagnosticMessage {
69+
case consecutiveStatementsOnSameLine = "consecutive statements on a line must be separated by ';'"
6970
case cStyleForLoop = "C-style for statement has been removed in Swift 3"
7071
case missingColonInTernaryExprDiagnostic = "expected ':' after '? ...' in ternary expression"
7172
case missingFunctionParameterClause = "expected argument list in function declaration"
@@ -138,6 +139,7 @@ public struct UnexpectedNodesError: ParserError {
138139
// MARK: - Fix-Its (please sort alphabetically)
139140

140141
public enum StaticParserFixIt: String, FixItMessage {
142+
case insertSemicolon = "insert ';'"
141143
case insertAttributeArguments = "insert attribute argument"
142144
case moveThrowBeforeArrow = "move 'throws' before '->'"
143145

‎Sources/SwiftParser/TopLevel.swift

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -37,22 +37,38 @@ extension Parser {
3737
}
3838

3939
extension Parser {
40+
mutating func parseCodeBlockItemList(context: ItemContext?, stopCondition: (inout Parser) -> Bool) -> RawCodeBlockItemListSyntax {
41+
var elements = [RawCodeBlockItemSyntax]()
42+
var loopProgress = LoopProgressCondition()
43+
while !stopCondition(&self), loopProgress.evaluate(currentToken) {
44+
let newItemAtStartOfLine = self.currentToken.isAtStartOfLine
45+
guard let newElement = self.parseCodeBlockItem(context: context) else {
46+
break
47+
}
48+
if let lastItem = elements.last, lastItem.semicolon == nil && !newItemAtStartOfLine {
49+
elements[elements.count - 1] = RawCodeBlockItemSyntax(
50+
lastItem.unexpectedBeforeItem,
51+
item: lastItem.item,
52+
lastItem.unexpectedBetweenItemAndSemicolon,
53+
semicolon: missingToken(.semicolon, text: nil),
54+
lastItem.unexpectedBetweenSemicolonAndErrorTokens,
55+
errorTokens: lastItem.errorTokens,
56+
arena: self.arena
57+
)
58+
}
59+
elements.append(newElement)
60+
}
61+
return .init(elements: elements, arena: self.arena)
62+
}
63+
4064
/// Parse the top level items in a source file.
4165
///
4266
/// Grammar
4367
/// =======
4468
///
4569
/// top-level-declaration → statements?
4670
mutating func parseTopLevelCodeBlockItems() -> RawCodeBlockItemListSyntax {
47-
var elements = [RawCodeBlockItemSyntax]()
48-
var loopProgress = LoopProgressCondition()
49-
while
50-
let newElement = self.parseCodeBlockItem(context: .topLevel),
51-
loopProgress.evaluate(currentToken)
52-
{
53-
elements.append(newElement)
54-
}
55-
return .init(elements: elements, arena: self.arena)
71+
return parseCodeBlockItemList(context: .topLevel, stopCondition: { _ in false })
5672
}
5773

5874
/// The optional form of `parseCodeBlock` that checks to see if the parser has
@@ -75,23 +91,9 @@ extension Parser {
7591
/// code-block → '{' statements? '}'
7692
mutating func parseCodeBlock(context: ItemContext? = nil) -> RawCodeBlockSyntax {
7793
let (unexpectedBeforeLBrace, lbrace) = self.expect(.leftBrace)
78-
var items = [RawCodeBlockItemSyntax]()
79-
var loopProgress = LoopProgressCondition()
80-
while
81-
!self.at(.rightBrace),
82-
let newItem = self.parseCodeBlockItem(context: context),
83-
loopProgress.evaluate(currentToken)
84-
{
85-
items.append(newItem)
86-
}
94+
let itemList = parseCodeBlockItemList(context: context, stopCondition: { $0.at(.rightBrace) })
8795
let (unexpectedBeforeRBrace, rbrace) = self.expect(.rightBrace)
8896

89-
let itemList: RawCodeBlockItemListSyntax
90-
if items.isEmpty && (lbrace.isMissing || rbrace.isMissing) {
91-
itemList = .init(elements: [], arena: self.arena)
92-
} else {
93-
itemList = .init(elements: items, arena: self.arena)
94-
}
9597
return .init(
9698
unexpectedBeforeLBrace,
9799
leftBrace: lbrace,

‎Tests/SwiftParserTest/Assertions.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,18 @@ class FixItApplier: SyntaxRewriter {
135135
return nil
136136
}
137137

138+
override func visit(_ node: TokenSyntax) -> Syntax {
139+
for change in changes {
140+
switch change {
141+
case .removeTrailingTrivia(let changeNode) where changeNode.id == node.id:
142+
return Syntax(node.withTrailingTrivia([]))
143+
default:
144+
break
145+
}
146+
}
147+
return Syntax(node)
148+
}
149+
138150
/// Applies all Fix-Its in `diagnostics` to `tree` and returns the fixed syntax tree.
139151
public static func applyFixes<T: SyntaxProtocol>(in diagnostics: [Diagnostic], to tree: T) -> Syntax {
140152
let applier = FixItApplier(diagnostics: diagnostics)

‎Tests/SwiftParserTest/Declarations.swift

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -243,13 +243,18 @@ final class DeclarationTests: XCTestCase {
243243
func testAccessLevelModifier() {
244244
AssertParse(
245245
"""
246-
open open(set) var openProp = 0
246+
open#^DIAG_1^# open(set)#^DIAG_2^# var openProp = 0
247247
public public(set) var publicProp = 0
248248
internal internal(set) var internalProp = 0
249249
fileprivate fileprivate(set) var fileprivateProp = 0
250250
private private(set) var privateProp = 0
251251
internal(set) var defaultProp = 0
252-
"""
252+
""",
253+
diagnostics: [
254+
// TODO: This test case should not produce any errors
255+
DiagnosticSpec(locationMarker: "DIAG_1", message: "consecutive statements on a line must be separated by ';'"),
256+
DiagnosticSpec(locationMarker: "DIAG_2", message: "consecutive statements on a line must be separated by ';'"),
257+
]
253258
)
254259

255260
AssertParse(
@@ -738,10 +743,14 @@ final class DeclarationTests: XCTestCase {
738743
func testTextRecovery() {
739744
AssertParse(
740745
"""
741-
Lorem ipsum dolor sit amet#^DIAG^#, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
746+
Lorem#^DIAG_1^# ipsum#^DIAG_2^# dolor#^DIAG_3^# sit#^DIAG_4^# amet#^DIAG_5^#, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
742747
""",
743748
diagnostics: [
744-
DiagnosticSpec(message: "extraneous code at top level"),
749+
DiagnosticSpec(locationMarker: "DIAG_1", message: "consecutive statements on a line must be separated by ';'"),
750+
DiagnosticSpec(locationMarker: "DIAG_2", message: "consecutive statements on a line must be separated by ';'"),
751+
DiagnosticSpec(locationMarker: "DIAG_3", message: "consecutive statements on a line must be separated by ';'"),
752+
DiagnosticSpec(locationMarker: "DIAG_4", message: "consecutive statements on a line must be separated by ';'"),
753+
DiagnosticSpec(locationMarker: "DIAG_5", message: "extraneous code at top level"),
745754
]
746755
)
747756
}
@@ -792,7 +801,7 @@ final class DeclarationTests: XCTestCase {
792801

793802
func testDontRecoverFromDeclKeyword() {
794803
AssertParse(
795-
"func foo(first second #^MISSING_COLON^#third #^MISSING_RPAREN^#struct#^MISSING_IDENTIFIER^##^BRACES^#: Int) {}",
804+
"func foo(first second #^MISSING_COLON^#third#^STMTS^# #^MISSING_RPAREN^#struct#^MISSING_IDENTIFIER^##^BRACES^#: Int) {}",
796805
substructure: Syntax(FunctionParameterSyntax(
797806
attributes: nil,
798807
modifiers: nil,
@@ -806,6 +815,7 @@ final class DeclarationTests: XCTestCase {
806815
)),
807816
diagnostics: [
808817
DiagnosticSpec(locationMarker: "MISSING_COLON", message: "expected ':' in function parameter"),
818+
DiagnosticSpec(locationMarker: "STMTS", message: "consecutive statements on a line must be separated by ';'"),
809819
DiagnosticSpec(locationMarker: "MISSING_RPAREN", message: "expected ')' to end parameter clause"),
810820
DiagnosticSpec(locationMarker: "MISSING_IDENTIFIER", message: "expected identifier in struct"),
811821
DiagnosticSpec(locationMarker: "BRACES", message: "expected member block in struct"),

‎Tests/SwiftParserTest/Expressions.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,7 @@ final class ExpressionTests: XCTestCase {
498498
"""##,
499499
diagnostics: [
500500
DiagnosticSpec(locationMarker: "AFTER_SLASH", message: "expected root in key path"),
501+
DiagnosticSpec(locationMarker: "AFTER_SLASH", message: "consecutive statements on a line must be separated by ';'"),
501502
DiagnosticSpec(locationMarker: "AFTER_PAREN", message: "expected ')' to end tuple type"),
502503
]
503504
)
@@ -558,9 +559,10 @@ final class ExpressionTests: XCTestCase {
558559
\n #^KEY_PATH_1^#
559560
if false != true {
560561
\n #^KEY_PATH_2^#
561-
print "\(i)\"\n#^END^#
562+
print#^STMTS^# "\(i)\"\n#^END^#
562563
"""#,
563564
diagnostics: [
565+
DiagnosticSpec(locationMarker: "STMTS", message: "consecutive statements on a line must be separated by ';'"),
564566
DiagnosticSpec(locationMarker: "END", message: #"expected '"' to end string literal"#),
565567
DiagnosticSpec(locationMarker: "END", message: "expected '}' to end 'if' statement"),
566568
DiagnosticSpec(locationMarker: "END", message: "expected '}' to end function"),
@@ -597,12 +599,13 @@ final class ExpressionTests: XCTestCase {
597599
AssertParse(
598600
"""
599601
do {
600-
true ? () : #^DIAG^#throw opaque_error()
602+
true ? () :#^DIAG_1^# #^DIAG_2^#throw opaque_error()
601603
} catch _ {
602604
}
603605
""",
604606
diagnostics: [
605-
DiagnosticSpec(message: "expected expression in 'do' statement")
607+
DiagnosticSpec(locationMarker: "DIAG_1", message: "consecutive statements on a line must be separated by ';'"),
608+
DiagnosticSpec(locationMarker: "DIAG_2", message: "expected expression in 'do' statement"),
606609
]
607610
)
608611
}

‎Tests/SwiftParserTest/Statements.swift

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ final class StatementTests: XCTestCase {
5858
for index in (0...nest) {
5959
let indent = String(repeating: " ", count: index + 1)
6060
source += indent + "if false != true {\n"
61-
source += indent + " print \"\\(i)\"\n"
61+
source += indent + " print(\"\\(i)\")\n"
6262
}
6363

6464
for index in (0...nest).reversed() {
@@ -367,13 +367,15 @@ final class StatementTests: XCTestCase {
367367
}
368368
}
369369
""",
370-
substructure: Syntax(YieldStmtSyntax(
371-
yieldKeyword: .contextualKeyword("yield"),
372-
yields: Syntax(YieldListSyntax(
373-
leftParen: .leftParenToken(),
374-
elementList: YieldExprListSyntax([]),
375-
rightParen: .rightParenToken())))),
376-
substructureAfterMarker: "YIELD")
370+
substructure: Syntax(YieldStmtSyntax(
371+
yieldKeyword: .contextualKeyword("yield"),
372+
yields: Syntax(YieldListSyntax(
373+
leftParen: .leftParenToken(),
374+
elementList: YieldExprListSyntax([]),
375+
rightParen: .rightParenToken())
376+
))
377+
),
378+
substructureAfterMarker: "YIELD")
377379

378380
// Make sure these are not.
379381
AssertParse(

‎Tests/SwiftParserTest/translated/AsyncTests.swift

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,22 @@ final class AsyncTests: XCTestCase {
1616
func testAsync2() {
1717
AssertParse(
1818
"""
19-
func asyncGlobal3() throws async { }
19+
func asyncGlobal3() throws#^DIAG^# async { }
2020
""",
2121
diagnostics: [
2222
// TODO: Old parser expected error on line 1: 'async' must precede 'throws', Fix-It replacements: 28 - 34 = '', 21 - 21 = 'async '
23+
DiagnosticSpec(message: "consecutive statements on a line must be separated by ';'")
2324
]
2425
)
2526
}
2627

2728
func testAsync3() {
2829
AssertParse(
2930
"""
30-
func asyncGlobal3(fn: () throws -> Int) rethrows async { }
31+
func asyncGlobal3(fn: () throws -> Int) rethrows#^DIAG^# async { }
3132
""",
3233
diagnostics: [
34+
DiagnosticSpec(message: "consecutive statements on a line must be separated by ';'")
3335
// TODO: Old parser expected error on line 1: 'async' must precede 'rethrows', Fix-It replacements: 50 - 56 = '', 41 - 41 = 'async '
3436
]
3537
)
@@ -38,9 +40,10 @@ final class AsyncTests: XCTestCase {
3840
func testAsync4() {
3941
AssertParse(
4042
"""
41-
func asyncGlobal4() -> Int async { }
43+
func asyncGlobal4() -> Int#^DIAG^# async { }
4244
""",
4345
diagnostics: [
46+
DiagnosticSpec(message: "consecutive statements on a line must be separated by ';'")
4447
// TODO: Old parser expected error on line 1: 'async' may only occur before '->', Fix-It replacements: 28 - 34 = '', 21 - 21 = 'async '
4548
]
4649
)
@@ -49,11 +52,12 @@ final class AsyncTests: XCTestCase {
4952
func testAsync5() {
5053
AssertParse(
5154
"""
52-
func asyncGlobal5() -> Int async throws #^DIAG^#{ }
55+
func asyncGlobal5() -> Int#^STMTS^# async throws #^DIAG^#{ }
5356
""",
5457
diagnostics: [
5558
// TODO: Old parser expected error on line 1: 'async' may only occur before '->', Fix-It replacements: 28 - 34 = '', 21 - 21 = 'async '
5659
// TODO: Old parser expected error on line 1: 'throws' may only occur before '->', Fix-It replacements: 34 - 41 = '', 21 - 21 = 'throws '
60+
DiagnosticSpec(locationMarker: "STMTS", message: "consecutive statements on a line must be separated by ';'"),
5761
DiagnosticSpec(message: "expected '->'"),
5862
]
5963
)
@@ -75,9 +79,10 @@ final class AsyncTests: XCTestCase {
7579
func testAsync7() {
7680
AssertParse(
7781
"""
78-
func asyncGlobal7() throws -> Int async { }
82+
func asyncGlobal7() throws -> Int#^DIAG^# async { }
7983
""",
8084
diagnostics: [
85+
DiagnosticSpec(message: "consecutive statements on a line must be separated by ';'")
8186
// TODO: Old parser expected error on line 1: 'async' may only occur before '->', Fix-It replacements: 35 - 41 = '', 21 - 21 = 'async '
8287
]
8388
)
@@ -86,9 +91,12 @@ final class AsyncTests: XCTestCase {
8691
func testAsync8() {
8792
AssertParse(
8893
"""
89-
func asyncGlobal8() async throws async -> async Int async {}
94+
func asyncGlobal8() async throws#^DIAG_1^# async -> async#^DIAG_2^# Int#^DIAG_3^# async {}
9095
""",
9196
diagnostics: [
97+
DiagnosticSpec(locationMarker: "DIAG_1", message: "consecutive statements on a line must be separated by ';'"),
98+
DiagnosticSpec(locationMarker: "DIAG_2", message: "consecutive statements on a line must be separated by ';'"),
99+
DiagnosticSpec(locationMarker: "DIAG_3", message: "consecutive statements on a line must be separated by ';'"),
92100
// TODO: Old parser expected error on line 1: 'async' has already been specified, Fix-It replacements: 34 - 40 = ''
93101
// TODO: Old parser expected error on line 1: 'async' has already been specified, Fix-It replacements: 43 - 49 = ''
94102
// TODO: Old parser expected error on line 1: 'async' has already been specified, Fix-It replacements: 53 - 59 = ''

‎Tests/SwiftParserTest/translated/AvailabilityQueryUnavailabilityTests.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ final class AvailabilityQueryUnavailabilityTests: XCTestCase {
2020
if #unavailable(OSX 10.51, *) {}
2121
// Disallow use as an expression.
2222
if (#^DIAG_1^##unavailable(OSX 10.51)) {}
23-
let x = #^DIAG_2^##unavailable(OSX 10.51)
23+
let x =#^DIAG_2A^# #^DIAG_2^##unavailable(OSX 10.51)
2424
(#unavailable(OSX 10.51) ? 1 : 0)
2525
if !#unavailable(OSX 10.52) {
2626
}
27-
if let _ = Optional(5), #^DIAG_3^#!#^DIAG_4^##unavailable(OSX 10.52) {
27+
if let _ = Optional(5),#^DIAG_3A^# #^DIAG_3^#!#^DIAG_4^##unavailable(OSX 10.52) {
2828
}
2929
""",
3030
diagnostics: [
@@ -33,11 +33,13 @@ final class AvailabilityQueryUnavailabilityTests: XCTestCase {
3333
DiagnosticSpec(locationMarker: "DIAG_1", message: "expected value in tuple"),
3434
DiagnosticSpec(locationMarker: "DIAG_1", message: "unexpected text '#unavailable(OSX 10.51)' in tuple"),
3535
// TODO: Old parser expected error on line 5: #unavailable may only be used as condition of
36+
DiagnosticSpec(locationMarker: "DIAG_2A", message: "consecutive statements on a line must be separated by ';'"),
3637
DiagnosticSpec(locationMarker: "DIAG_2", message: "expected expression in variable"),
3738
DiagnosticSpec(locationMarker: "DIAG_2", message: "unexpected text before variable"),
3839
// TODO: Old parser expected error on line 6: #unavailable may only be used as condition of an
3940
// TODO: Old parser expected error on line 7: #unavailable may only be used as condition of an
4041
// TODO: Old parser expected error on line 9: #unavailable may only be used as condition
42+
DiagnosticSpec(locationMarker: "DIAG_3A", message: "consecutive statements on a line must be separated by ';'"),
4143
DiagnosticSpec(locationMarker: "DIAG_3", message: "expected pattern in variable"),
4244
DiagnosticSpec(locationMarker: "DIAG_4", message: "expected expression in prefix operator expression"),
4345
DiagnosticSpec(locationMarker: "DIAG_4", message: "extraneous code at top level"),

0 commit comments

Comments
 (0)