diff --git a/Sources/SwiftParser/Patterns.swift b/Sources/SwiftParser/Patterns.swift index d4fce79e88f..81007bbd5ff 100644 --- a/Sources/SwiftParser/Patterns.swift +++ b/Sources/SwiftParser/Patterns.swift @@ -217,9 +217,22 @@ extension Parser { while !self.at(.eof, .rightParen) && keepGoing && loopProgress.evaluate(currentToken) { // If the tuple element has a label, parse it. let labelAndColon = self.consume(if: .identifier, followedBy: .colon) - let (label, colon) = (labelAndColon?.0, labelAndColon?.1) + var (label, colon) = (labelAndColon?.0, labelAndColon?.1) + + /// If we have something like `x SomeType`, use the indication that `SomeType` starts with a capital letter (and is thus probably a type name) + /// as an indication that the user forgot to write the colon instead of forgetting to write the comma to separate two elements. + if label == nil, colon == nil, peek().rawTokenKind == .identifier, peek().tokenText.isStartingWithUppercase { + label = consumeAnyToken() + colon = self.missingToken(.colon) + } + let pattern = self.parsePattern() - let trailingComma = self.consume(if: .comma) + var trailingComma = self.consume(if: .comma) + + if trailingComma == nil && self.at(TokenSpec(.identifier, allowAtStartOfLine: false)) { + trailingComma = self.missingToken(RawTokenKind.comma) + } + keepGoing = trailingComma != nil elements.append( RawTuplePatternElementSyntax( diff --git a/Sources/SwiftParser/SyntaxUtils.swift b/Sources/SwiftParser/SyntaxUtils.swift index c6860e0d8bd..7db11897f93 100644 --- a/Sources/SwiftParser/SyntaxUtils.swift +++ b/Sources/SwiftParser/SyntaxUtils.swift @@ -88,6 +88,14 @@ extension SyntaxText { var isEditorPlaceholder: Bool { return self.starts(with: SyntaxText("<#")) && self.hasSuffix(SyntaxText("#>")) } + + var isStartingWithUppercase: Bool { + if !self.isEmpty, let firstCharacterByte = self.baseAddress?.pointee { + return 65 <= firstCharacterByte && firstCharacterByte <= 90 + } else { + return false + } + } } extension RawTokenKind { diff --git a/Sources/SwiftParser/Types.swift b/Sources/SwiftParser/Types.swift index 8ddab9caa8b..83f2f2096c1 100644 --- a/Sources/SwiftParser/Types.swift +++ b/Sources/SwiftParser/Types.swift @@ -536,7 +536,7 @@ extension Parser { if let first = first, second == nil, colon?.isMissing == true, - first.tokenText.description.first?.isUppercase == true + first.tokenText.isStartingWithUppercase { elements.append( RawTupleTypeElementSyntax( diff --git a/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift b/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift index f428cfc9da4..17424b8eb42 100644 --- a/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift +++ b/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift @@ -164,10 +164,10 @@ extension FixIt.MultiNodeChange { } } - /// Transfers the leading and trivia trivia of `nodes` to the trailing trivia - /// of the previous token. While doing this, it tries to be smart, merging trivia - /// where it makes sense and refusing to add e.g. a space after punctuation, - /// where it usually doesn't make sense. + /// Transfers the leading and trailing trivia of `nodes` to the previous token + /// While doing this, it tries to be smart, merging trivia where it makes sense + /// and refusing to add e.g. a space after punctuation, where it usually + /// doesn't make sense. static func transferTriviaAtSides(from nodes: [SyntaxType]) -> Self { let removedTriviaAtSides = (nodes.first?.leadingTrivia ?? []).merging(nodes.last?.trailingTrivia ?? []) if !removedTriviaAtSides.isEmpty, let previousToken = nodes.first?.previousToken(viewMode: .sourceAccurate) { diff --git a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift index e559a211d47..204bd91acf4 100644 --- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift +++ b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift @@ -327,7 +327,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { FixIt( message: .joinIdentifiers, changes: [ - FixIt.MultiNodeChange(.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))), + FixIt.MultiNodeChange(.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), trailingTrivia: tokens.last?.trailingTrivia ?? [], presence: .present)))), .makeMissing(tokens), ] ) @@ -338,7 +338,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { FixIt( message: .joinIdentifiersWithCamelCase, changes: [ - FixIt.MultiNodeChange(.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))), + FixIt.MultiNodeChange(.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), trailingTrivia: tokens.last?.trailingTrivia ?? [], presence: .present)))), .makeMissing(tokens), ] ) diff --git a/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift b/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift index 3cd88f30a2a..0a908e807a8 100644 --- a/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift +++ b/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift @@ -383,9 +383,9 @@ public struct SpaceSeparatedIdentifiersError: ParserError { if let name = firstToken.parent?.ancestorOrSelf(mapping: { $0.nodeTypeNameForDiagnostics(allowBlockNames: false) }) { - return "found an unexpected second identifier in \(name)" + return "found an unexpected second identifier in \(name); is there an accidental break?" } else { - return "found an unexpected second identifier" + return "found an unexpected second identifier; is there an accidental break?" } } } diff --git a/Tests/SwiftParserTest/ParserTests.swift b/Tests/SwiftParserTest/ParserTests.swift index 189e69df57c..87a91163393 100644 --- a/Tests/SwiftParserTest/ParserTests.swift +++ b/Tests/SwiftParserTest/ParserTests.swift @@ -54,7 +54,7 @@ public class ParserTests: XCTestCase { } } - /// Run parsr tests on all of the Swift files in the given path, recursively. + /// Run parser tests on all of the Swift files in the given path, recursively. func runParserTests( name: String, path: URL, diff --git a/Tests/SwiftParserTest/TypeTests.swift b/Tests/SwiftParserTest/TypeTests.swift index a40e771da4a..273d0f5382b 100644 --- a/Tests/SwiftParserTest/TypeTests.swift +++ b/Tests/SwiftParserTest/TypeTests.swift @@ -15,15 +15,17 @@ import XCTest final class TypeTests: XCTestCase { - func testMissingColonInType() { assertParse( """ var foo 1️⃣Bar = 1 """, diagnostics: [ - DiagnosticSpec(message: "expected ':' in type annotation") - ] + DiagnosticSpec(message: "expected ':' in type annotation", fixIts: ["insert ':'"]) + ], + fixedSource: """ + var foo: Bar = 1 + """ ) } @@ -65,7 +67,6 @@ final class TypeTests: XCTestCase { } func testClosureSignatures() { - assertParse( """ simple { [] str in diff --git a/Tests/SwiftParserTest/translated/InvalidTests.swift b/Tests/SwiftParserTest/translated/InvalidTests.swift index de836932453..8019ee30749 100644 --- a/Tests/SwiftParserTest/translated/InvalidTests.swift +++ b/Tests/SwiftParserTest/translated/InvalidTests.swift @@ -388,92 +388,85 @@ final class InvalidTests: XCTestCase { ) } - func testInvalid23a() { - assertParse( - """ - func dog 1️⃣cow() {} - """, - diagnostics: [ - DiagnosticSpec( - message: "found an unexpected second identifier in function", - fixIts: [ - "join the identifiers together", - "join the identifiers together with camel-case", - ] - ) - ], - applyFixIts: ["join the identifiers together"], - fixedSource: "func dogcow() {}" - ) - } - - func testInvalid23b() { - assertParse( - """ - func dog 1️⃣cow() {} - """, - diagnostics: [ - DiagnosticSpec( - message: "found an unexpected second identifier in function", - fixIts: [ - "join the identifiers together", - "join the identifiers together with camel-case", - ] - ) - ], - applyFixIts: ["join the identifiers together with camel-case"], - fixedSource: "func dogCow() {}" - ) + func testInvalid23() { + let testCases: [UInt: (fixIt: String, fixedSource: String)] = [ + #line: ("join the identifiers together", "func dogcow() {}"), + #line: ("join the identifiers together with camel-case", "func dogCow() {}"), + ] + + for (line, testCase) in testCases { + assertParse( + """ + func dog 1️⃣cow() {} + """, + diagnostics: [ + DiagnosticSpec( + message: "found an unexpected second identifier in function; is there an accidental break?", + fixIts: [ + "join the identifiers together", + "join the identifiers together with camel-case", + ] + ) + ], + applyFixIts: [testCase.fixIt], + fixedSource: testCase.fixedSource, + line: line + ) + } } func testThreeIdentifersForFunctionName() { - assertParse( - """ - func dog 1️⃣cow sheep() {} - """, - diagnostics: [ - DiagnosticSpec( - message: "found an unexpected second identifier in function", - fixIts: [ - "join the identifiers together", - "join the identifiers together with camel-case", - ] - ) - ], - applyFixIts: ["join the identifiers together with camel-case"], - fixedSource: "func dogCowSheep() {}" - ) - } + let testCases: [UInt: (fixIt: String, fixedSource: String)] = [ + #line: ("join the identifiers together", "func dogcowsheep() {}"), + #line: ("join the identifiers together with camel-case", "func dogCowSheep() {}"), + ] - func testInvalid24() { - assertParse( - """ - func cat 1️⃣Mouse() {} - """, - diagnostics: [ - DiagnosticSpec(message: "found an unexpected second identifier in function", fixIts: ["join the identifiers together"]) - ], - fixedSource: "func catMouse() {}" - ) + for (line, testCase) in testCases { + assertParse( + """ + func dog 1️⃣cow sheep() {} + """, + diagnostics: [ + DiagnosticSpec( + message: "found an unexpected second identifier in function; is there an accidental break?", + fixIts: [ + "join the identifiers together", + "join the identifiers together with camel-case", + ] + ) + ], + applyFixIts: [testCase.fixIt], + fixedSource: testCase.fixedSource, + line: line + ) + } } func testInvalid25() { - assertParse( - """ - func friend 1️⃣ship(x: T) {} - """, - diagnostics: [ - DiagnosticSpec( - message: "found an unexpected second identifier in function", - fixIts: [ - "join the identifiers together", - "join the identifiers together with camel-case", - ] - ) - ], - applyFixIts: ["join the identifiers together with camel-case"], - fixedSource: "func friendShip(x: T) {}" - ) + let testCases: [UInt: (fixIt: String, fixedSource: String)] = [ + #line: ("join the identifiers together", "func friendship(x: T) {}"), + #line: ("join the identifiers together with camel-case", "func friendShip(x: T) {}"), + ] + + for (line, testCase) in testCases { + assertParse( + """ + func friend 1️⃣ship(x: T) {} + """, + diagnostics: [ + DiagnosticSpec( + message: "found an unexpected second identifier in function; is there an accidental break?", + fixIts: [ + "join the identifiers together", + "join the identifiers together with camel-case", + ] + ) + ], + applyFixIts: [testCase.fixIt], + fixedSource: testCase.fixedSource, + line: line + ) + } } func testInvalid26() { diff --git a/Tests/SwiftParserTest/translated/RecoveryTests.swift b/Tests/SwiftParserTest/translated/RecoveryTests.swift index a3c04523276..92be5a53967 100644 --- a/Tests/SwiftParserTest/translated/RecoveryTests.swift +++ b/Tests/SwiftParserTest/translated/RecoveryTests.swift @@ -742,14 +742,30 @@ final class RecoveryTests: XCTestCase { //===--- Recovery for multiple identifiers in decls func testRecovery58() { - assertParse( - """ - protocol Multi 1️⃣ident {} - """, - diagnostics: [ - DiagnosticSpec(message: "found an unexpected second identifier in protocol", highlight: "ident") - ] - ) + let testCases: [UInt: (fixIt: String, fixedSource: String)] = [ + #line: ("join the identifiers together", "protocol Multiident {}"), + #line: ("join the identifiers together with camel-case", "protocol MultiIdent {}"), + ] + + for (line, testCase) in testCases { + assertParse( + """ + protocol Multi 1️⃣ident {} + """, + diagnostics: [ + DiagnosticSpec( + message: "found an unexpected second identifier in protocol; is there an accidental break?", + highlight: "ident", + fixIts: ["join the identifiers together", "join the identifiers together with camel-case"], + line: line + ) + ], + applyFixIts: [testCase.fixIt], + fixedSource: testCase.fixedSource, + line: line + ) + } + } func testRecovery60() { @@ -758,7 +774,11 @@ final class RecoveryTests: XCTestCase { class CCC 1️⃣CCC {} """, diagnostics: [ - DiagnosticSpec(message: "found an unexpected second identifier in class") + DiagnosticSpec( + message: "found an unexpected second identifier in class; is there an accidental break?", + highlight: "CCC", + fixIts: ["join the identifiers together"] + ) ], fixedSource: """ class CCCCCC {} @@ -775,7 +795,7 @@ final class RecoveryTests: XCTestCase { } """, diagnostics: [ - DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in enum"), + DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in enum; is there an accidental break?", fixIts: ["join the identifiers together"]), DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive declarations on a line must be separated by ';'"), DiagnosticSpec(locationMarker: "3️⃣", message: "unexpected code 'a' before enum case"), ] @@ -794,24 +814,39 @@ final class RecoveryTests: XCTestCase { } """#, diagnostics: [ - DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in struct"), + DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in struct; is there an accidental break?", fixIts: ["join the identifiers together"]), DiagnosticSpec(locationMarker: "2️⃣", message: "expected ':' in type annotation"), DiagnosticSpec(locationMarker: "3️⃣", message: #"unexpected code ': Int = ""' before function"#), - // TODO: (good first issue) Old parser expected error on line 4: found an unexpected second identifier in variable declaration; is there an accidental break? DiagnosticSpec(locationMarker: "4️⃣", message: "expected ':' in type annotation"), ] ) } - func testRecovery64() { + func testRecovery64a() { assertParse( """ let (efg 1️⃣hij, foobar) = (5, 6) """, diagnostics: [ - // TODO: (good first issue) Old parser expected error on line 1: found an unexpected second identifier in constant declaration; is there an accidental break? - DiagnosticSpec(message: "unexpected code 'hij, foobar' in tuple pattern") - ] + DiagnosticSpec(message: "expected ',' in tuple pattern", fixIts: ["insert ','"]) + ], + fixedSource: """ + let (efg, hij, foobar) = (5, 6) + """ + ) + } + + func testRecovery64b() { + assertParse( + """ + let (efg 1️⃣Hij, foobar) = (5, 6) + """, + diagnostics: [ + DiagnosticSpec(message: "expected ':' in tuple pattern", fixIts: ["insert ':'"]) + ], + fixedSource: """ + let (efg: Hij, foobar) = (5, 6) + """ ) }