From 7fb3e7f1c5731a95731dc5a598939183101348fe Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Fri, 21 Jul 2023 10:29:50 +0200 Subject: [PATCH] Add correct diagnostic for missing parentheses --- Sources/SwiftParser/Expressions.swift | 53 +++++----- Tests/SwiftParserTest/ExpressionTests.swift | 110 ++++++++++++++++++++ 2 files changed, 136 insertions(+), 27 deletions(-) diff --git a/Sources/SwiftParser/Expressions.swift b/Sources/SwiftParser/Expressions.swift index bc43255679c..0a94e8b35fa 100644 --- a/Sources/SwiftParser/Expressions.swift +++ b/Sources/SwiftParser/Expressions.swift @@ -1746,7 +1746,9 @@ extension Parser { var effectSpecifiers: RawTypeEffectSpecifiersSyntax? var returnClause: RawReturnClauseSyntax? = nil if !self.at(.keyword(.in)) { - if self.at(.leftParen) { + // If the next token is ':', then it looks like the code contained a non-shorthand closure parameter with a type annotation. + // These need to be wrapped in parentheses. + if self.at(.leftParen) || self.peek(isAt: .colon) { // Parse the closure arguments. let params = self.parseParameterClause(RawClosureParameterClauseSyntax.self) { parser in parser.parseClosureParameter() @@ -2430,34 +2432,28 @@ extension Parser.Lookahead { } // Parse pattern-tuple func-signature-result? 'in'. - if lookahead.consume(if: .leftParen) != nil { // Consume the ')'. - + if lookahead.at(.leftParen) { // Consume the '('. // While we don't have '->' or ')', eat balanced tokens. var skipProgress = LoopProgressCondition() - while !lookahead.at(.endOfFile, .rightParen) && lookahead.hasProgressed(&skipProgress) { + while !lookahead.at(.endOfFile, .rightBrace, .keyword(.in)) && !lookahead.at(.arrow) && lookahead.hasProgressed(&skipProgress) { lookahead.skipSingle() } - - // Consume the ')', if it's there. - if lookahead.consume(if: .rightParen) != nil { - lookahead.consumeEffectsSpecifiers() - - // Parse the func-signature-result, if present. - if lookahead.consume(if: .arrow) != nil { - guard lookahead.canParseType() else { - return false - } - - lookahead.consumeEffectsSpecifiers() - } - } - // Okay, we have a closure signature. } else if lookahead.at(.identifier) || lookahead.at(.wildcard) { // Parse identifier (',' identifier)* lookahead.consumeAnyToken() + /// If the next token is a colon, interpret is as a type annotation and consume a type after it. + /// While type annotations aren’t allowed in shorthand closure parameters, we consume them to improve recovery. + func consumeOptionalTypeAnnotation() -> Bool { + if lookahead.consume(if: .colon) != nil { + return lookahead.canParseType() + } else { + return true + } + } + var parametersProgress = LoopProgressCondition() - while lookahead.consume(if: .comma) != nil && lookahead.hasProgressed(¶metersProgress) { + while consumeOptionalTypeAnnotation() && lookahead.consume(if: .comma) != nil && lookahead.hasProgressed(¶metersProgress) { if lookahead.at(.identifier) || lookahead.at(.wildcard) { lookahead.consumeAnyToken() continue @@ -2465,17 +2461,20 @@ extension Parser.Lookahead { return false } + } - lookahead.consumeEffectsSpecifiers() + // Consume the ')', if it's there. + lookahead.consume(if: .rightParen) - // Parse the func-signature-result, if present. - if lookahead.consume(if: .arrow) != nil { - guard lookahead.canParseType() else { - return false - } + lookahead.consumeEffectsSpecifiers() - lookahead.consumeEffectsSpecifiers() + // Parse the func-signature-result, if present. + if lookahead.consume(if: .arrow) != nil { + guard lookahead.canParseType() else { + return false } + + lookahead.consumeEffectsSpecifiers() } // Parse the 'in' at the end. diff --git a/Tests/SwiftParserTest/ExpressionTests.swift b/Tests/SwiftParserTest/ExpressionTests.swift index 574bee52da2..b8e29d1ffb7 100644 --- a/Tests/SwiftParserTest/ExpressionTests.swift +++ b/Tests/SwiftParserTest/ExpressionTests.swift @@ -2632,5 +2632,115 @@ final class StatementExpressionTests: ParserTestCase { assertParse("_ = { (@_noImplicitCopy _ x: Int) -> () in }") assertParse("_ = { (@Wrapper x) in }") + + assertParse( + """ + withInvalidOrderings { (comparisonPredicate: @escaping (Int, Int) -> Bool) in + } + """ + ) + } + + func testClosureWithMissingParentheses() { + assertParse( + """ + _ = { 1️⃣a: Int, b: Int 2️⃣in + } + """, + diagnostics: [ + DiagnosticSpec( + locationMarker: "1️⃣", + message: "expected '(' to start parameter clause", + fixIts: ["insert '('"] + ), + DiagnosticSpec( + locationMarker: "2️⃣", + message: "expected ')' to end parameter clause", + fixIts: ["insert ')'"] + ), + ], + fixedSource: """ + _ = { (a: Int, b: Int) in + } + """ + ) + } + + func testClosureWithReturnArrowAndMissingParentheses() { + assertParse( + """ + _ = { 1️⃣a: Int, b: 2️⃣Int 3️⃣-> Int 4️⃣in + } + """, + diagnostics: [ + DiagnosticSpec( + locationMarker: "1️⃣", + message: "expected '(' to start parameter clause", + fixIts: ["insert '('"] + ), + DiagnosticSpec( + locationMarker: "2️⃣", + message: "expected '(' to start function type", + fixIts: ["insert '('"] + ), + DiagnosticSpec( + locationMarker: "3️⃣", + message: "expected ')' in function type", + fixIts: ["insert ')'"] + ), + DiagnosticSpec( + locationMarker: "4️⃣", + message: "expected ')' to end parameter clause", + fixIts: ["insert ')'"] + ), + ], + fixedSource: """ + _ = { (a: Int, b: (Int) -> Int) in + } + """ + ) + } + + func testClosureWithMissingLeftParenthese() { + assertParse( + """ + _ = { 1️⃣a: Int, b: Int) in + } + """, + diagnostics: [ + DiagnosticSpec( + locationMarker: "1️⃣", + message: "expected '(' to start parameter clause", + fixIts: ["insert '('"] + ) + ], + fixedSource: """ + _ = { (a: Int, b: Int) in + } + """ + ) + } + + func testClosureWithDollarIdentifier() { + assertParse( + """ + let (ids, (actions, tracking)) = state.withCriticalRegion { ($0.valueObservers(for: keyPath), $0.didSet(keyPath: keyPath)) } + """ + ) + + assertParse( + """ + let (ids, (actions, tracking)) = state.withCriticalRegion { ($0.valueObservers(for: keyPath), $0.didSet(keyPath: keyPath)) } + """ + ) + + assertParse( + """ + state.withCriticalRegion { (1 + 2) } + for action in tracking { + action() + } + """ + ) } }