Skip to content

Commit 59d0170

Browse files
ahoppenahoppen
committed
Add diagnostic to move 'async' and 'throws' in 'ArrowExpr' in front of '->'
1 parent 833e365 commit 59d0170

File tree

9 files changed

+347
-106
lines changed

9 files changed

+347
-106
lines changed

‎CodeGeneration/Sources/generate-swiftbasicformat/BasicFormatFile.swift

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ private func createTokenFormatFunction() -> FunctionDecl {
168168
) {
169169
VariableDecl("var node = node")
170170
SwitchStmt(expression: MemberAccessExpr(base: "node", name: "tokenKind")) {
171-
for token in SYNTAX_TOKENS {
171+
for token in SYNTAX_TOKENS where token.name != "ContextualKeyword" {
172172
SwitchCase(label: SwitchCaseLabel(caseItems: CaseItem(pattern: ExpressionPattern(expression: MemberAccessExpr(name: token.swiftKind))))) {
173173
if token.requiresLeadingSpace {
174174
IfStmt(
@@ -196,6 +196,20 @@ private func createTokenFormatFunction() -> FunctionDecl {
196196
SwitchCase(label: SwitchCaseLabel(caseItems: CaseItem(pattern: ExpressionPattern(expression: MemberAccessExpr(name: "eof"))))) {
197197
BreakStmt("break")
198198
}
199+
SwitchCase(label: SwitchCaseLabel(caseItems: CaseItem(pattern: ExpressionPattern(expression: MemberAccessExpr(name: "contextualKeyword"))))) {
200+
SwitchStmt(
201+
"""
202+
switch node.text {
203+
case "async":
204+
if node.trailingTrivia.isEmpty {
205+
node.trailingTrivia += .space
206+
}
207+
default:
208+
break
209+
}
210+
"""
211+
)
212+
}
199213
}
200214
SequenceExpr("node.leadingTrivia = node.leadingTrivia.indented(indentation: indentation)")
201215
SequenceExpr("node.trailingTrivia = node.trailingTrivia.indented(indentation: indentation)")

‎Sources/SwiftBasicFormat/generated/BasicFormat.swift

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3161,8 +3161,6 @@ open class BasicFormat: SyntaxRewriter {
31613161
break
31623162
case .dollarIdentifier:
31633163
break
3164-
case .contextualKeyword:
3165-
break
31663164
case .rawStringDelimiter:
31673165
break
31683166
case .stringSegment:
@@ -3173,6 +3171,15 @@ open class BasicFormat: SyntaxRewriter {
31733171
break
31743172
case .eof:
31753173
break
3174+
case .contextualKeyword:
3175+
switch node.text {
3176+
case "async":
3177+
if node.trailingTrivia.isEmpty {
3178+
node.trailingTrivia += .space
3179+
}
3180+
default:
3181+
break
3182+
}
31763183
}
31773184
node.leadingTrivia = node.leadingTrivia.indented(indentation: indentation)
31783185
node.trailingTrivia = node.trailingTrivia.indented(indentation: indentation)

‎Sources/SwiftParser/Diagnostics/MissingNodesError.swift

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@ import SwiftBasicFormat
1717
// MARK: - Shared code
1818

1919
/// Returns a string that describes `missingNodes`.
20-
/// `missingNodes` are expected to all be children of `commonParent`.
21-
private func missingNodesDescription(missingNodes: [Syntax], commonParent: Syntax?) -> String {
22-
assert(missingNodes.allSatisfy({ $0.parent == commonParent }))
20+
/// If `commonParent` is not `nil`, `missingNodes` are expected to all be children of `commonParent`.
21+
func missingNodesDescription(missingNodes: [Syntax], commonParent: Syntax?) -> String {
22+
if commonParent != nil {
23+
assert(missingNodes.allSatisfy({ $0.parent == commonParent }))
24+
}
2325

2426
// If all tokens in the parent are missing, return the parent type name.
2527
if let commonParent = commonParent,

‎Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,31 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
170170
return .visitChildren
171171
}
172172

173+
public override func visit(_ node: ArrowExprSyntax) -> SyntaxVisitorContinueKind {
174+
if shouldSkip(node) {
175+
return .skipChildren
176+
}
177+
if let unexpectedAfterArrowToken = node.unexpectedAfterArrowToken {
178+
let modifiersAfterArrow = unexpectedAfterArrowToken.compactMap { $0.as(TokenSyntax.self) }.filter {
179+
$0.tokenKind == .contextualKeyword("async") || $0.tokenKind == .throwsKeyword
180+
}
181+
if modifiersAfterArrow.count == unexpectedAfterArrowToken.count {
182+
// All unexpected tokens after '->' are effect modifiers. Map them to the identifiers in front of '->'.
183+
var fixItChanges = modifiersAfterArrow.map { FixIt.Change.makeMissing(node: $0) }
184+
if let asyncKeyword = node.asyncKeyword, asyncKeyword.presence == .missing {
185+
fixItChanges.append(.makePresent(node: asyncKeyword))
186+
}
187+
if let throwsToken = node.throwsToken, throwsToken.presence == .missing {
188+
fixItChanges.append(.makePresent(node: throwsToken))
189+
}
190+
addDiagnostic(unexpectedAfterArrowToken, EffectsSpecifierAfterArrow(effectsSpecifiersAfterArrow: modifiersAfterArrow), fixIts: [
191+
FixIt(message: MoveEffectsSpecifiersBeforeArrowFixIt(effectsSpecifiersAfterArrow: modifiersAfterArrow), changes: fixItChanges)
192+
], handledNodes: [unexpectedAfterArrowToken.id, node.asyncKeyword?.id, node.throwsToken?.id].compactMap({ $0 }))
193+
}
194+
}
195+
return .visitChildren
196+
}
197+
173198
public override func visit(_ node: ForInStmtSyntax) -> SyntaxVisitorContinueKind {
174199
if shouldSkip(node) {
175200
return .skipChildren

‎Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,14 @@ public enum StaticParserError: String, DiagnosticMessage {
8282

8383
// MARK: - Diagnostics (please sort alphabetically)
8484

85+
public struct EffectsSpecifierAfterArrow: ParserError {
86+
public let effectsSpecifiersAfterArrow: [TokenSyntax]
87+
88+
public var message: String {
89+
"\(missingNodesDescription(missingNodes: effectsSpecifiersAfterArrow.map(Syntax.init), commonParent: nil)) may only occur before '->'"
90+
}
91+
}
92+
8593
public struct ExtaneousCodeAtTopLevel: ParserError {
8694
public let extraneousCode: UnexpectedNodesSyntax
8795

@@ -147,3 +155,11 @@ public enum StaticParserFixIt: String, FixItMessage {
147155
MessageID(domain: diagnosticDomain, id: "\(type(of: self)).\(self)")
148156
}
149157
}
158+
159+
public struct MoveEffectsSpecifiersBeforeArrowFixIt: ParserFixIt {
160+
public let effectsSpecifiersAfterArrow: [TokenSyntax]
161+
162+
public var message: String {
163+
"move \(missingNodesDescription(missingNodes: effectsSpecifiersAfterArrow.map(Syntax.init), commonParent: nil)) in front of '->'"
164+
}
165+
}

‎Sources/SwiftParser/Expressions.swift

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -319,17 +319,42 @@ extension Parser {
319319
return nil
320320
}
321321
case (.arrow, _)?, (.throwsKeyword, _)?:
322-
let asyncKeyword = self.consumeIfContextualKeyword("async")
322+
var asyncKeyword = self.consumeIfContextualKeyword("async")
323+
324+
var throwsKeyword = self.consume(if: .throwsKeyword)
323325

324-
let throwsKeyword = self.consume(if: .throwsKeyword)
325326
let (unexpectedBeforeArrow, arrow) = self.expect(.arrow)
326327

328+
// Recover if effect modifiers are specified after '->' by eating them into
329+
// unexpectedAfterArrow and inserting the corresponding effects modifier as
330+
// missing into the ArrowExprSyntax. This reflect the semantics the user
331+
// originally intended.
332+
var effectModifiersAfterArrow: [RawTokenSyntax] = []
333+
if let asyncAfterArrow = self.consumeIfContextualKeyword("async") {
334+
effectModifiersAfterArrow.append(asyncAfterArrow)
335+
if asyncKeyword == nil {
336+
asyncKeyword = missingToken(.contextualKeyword, text: "async")
337+
}
338+
}
339+
if let throwsAfterArrow = self.consume(if: .throwsKeyword) {
340+
effectModifiersAfterArrow.append(throwsAfterArrow)
341+
if throwsKeyword == nil {
342+
throwsKeyword = missingToken(.throwsKeyword, text: nil)
343+
}
344+
}
345+
let unexpectedAfterArrow = effectModifiersAfterArrow.isEmpty ? nil : RawUnexpectedNodesSyntax(
346+
elements: effectModifiersAfterArrow.map { RawSyntax($0) },
347+
arena: self.arena
348+
)
349+
327350
let op = RawArrowExprSyntax(
328351
asyncKeyword: asyncKeyword,
329352
throwsToken: throwsKeyword,
330353
unexpectedBeforeArrow,
331354
arrowToken: arrow,
332-
arena: self.arena)
355+
unexpectedAfterArrow,
356+
arena: self.arena
357+
)
333358

334359
return (RawExprSyntax(op), nil)
335360

‎Tests/SwiftParserTest/Expressions.swift

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -579,10 +579,17 @@ final class ExpressionTests: XCTestCase {
579579
AssertParse(
580580
"[(Int) -> #^DIAG^#throws Int]()",
581581
diagnostics: [
582-
// FIXME: We should suggest to move 'throws' in front of '->'
583-
DiagnosticSpec(message: "expected expression in array element"),
584-
DiagnosticSpec(message: "unexpected text 'throws Int' in array"),
585-
]
582+
DiagnosticSpec(message: "'throws' may only occur before '->'", fixIts: ["move 'throws' in front of '->'"]),
583+
],
584+
fixedSource: "[(Int) throws -> Int]()"
585+
)
586+
587+
AssertParse(
588+
"[(Int) -> #^DIAG^#async throws Int]()",
589+
diagnostics: [
590+
DiagnosticSpec(message: "'async throws' may only occur before '->'", fixIts: ["move 'async throws' in front of '->'"]),
591+
],
592+
fixedSource: "[(Int) async throws -> Int]()"
586593
)
587594

588595
AssertParse(

‎Tests/SwiftParserTest/translated/AsyncTests.swift

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,18 @@ final class AsyncTests: XCTestCase {
8686
func testAsync8() {
8787
AssertParse(
8888
"""
89-
func asyncGlobal8() async throws async -> async Int async {}
89+
func asyncGlobal8() async throws async -> #^DIAG^#async Int async {}
9090
""",
9191
diagnostics: [
92+
DiagnosticSpec(message: "'async' may only occur before '->'")
9293
// TODO: Old parser expected error on line 1: 'async' has already been specified, Fix-It replacements: 34 - 40 = ''
9394
// TODO: Old parser expected error on line 1: 'async' has already been specified, Fix-It replacements: 43 - 49 = ''
9495
// TODO: Old parser expected error on line 1: 'async' has already been specified, Fix-It replacements: 53 - 59 = ''
95-
]
96+
],
97+
// TODO: This should not insert another 'async'
98+
fixedSource: """
99+
func asyncGlobal8() async throws async async -> Int async {}
100+
"""
96101
)
97102
}
98103

@@ -127,16 +132,29 @@ final class AsyncTests: XCTestCase {
127132
)
128133
}
129134

130-
func testAsync10() {
135+
func testAsync10a() {
131136
AssertParse(
132137
"""
133-
// Parsing function types with 'async'.
134138
typealias AsyncFunc1 = () async -> ()
139+
"""
140+
)
141+
}
142+
143+
func testAsync10b() {
144+
AssertParse(
145+
"""
135146
typealias AsyncFunc2 = () async throws -> ()
147+
"""
148+
)
149+
}
150+
151+
func testAsync10c() {
152+
AssertParse(
153+
"""
136154
typealias AsyncFunc3 = () throws async -> ()
137155
""",
138156
diagnostics: [
139-
// TODO: Old parser expected error on line 4: 'async' must precede 'throws', Fix-It replacements: 34 - 40 = '', 27 - 27 = 'async '
157+
// TODO: Old parser expected error on line 1: 'async' must precede 'throws', Fix-It replacements: 34 - 40 = '', 27 - 27 = 'async '
140158
]
141159
)
142160
}
@@ -148,14 +166,14 @@ final class AsyncTests: XCTestCase {
148166
func testTypeExprs() {
149167
let _ = [() async -> ()]()
150168
let _ = [() async throws -> ()]()
151-
let _ = [() throws #^DIAG^#async -> ()]()
152-
let _ = [() -> async ()]()
169+
let _ = [() throws #^DIAG_1^#async -> ()]()
170+
let _ = [() -> #^DIAG_2^#async ()]()
153171
}
154172
""",
155173
diagnostics: [
156174
// TODO: Old parser expected error on line 5: 'async' must precede 'throws', Fix-It replacements: 22 - 28 = '', 15 - 15 = 'async '
157-
DiagnosticSpec(message: "unexpected text 'async' in array element"),
158-
// TODO: Old parser expected error on line 6: 'async' may only occur before '->', Fix-It replacements: 18 - 24 = '', 15 - 15 = 'async '
175+
DiagnosticSpec(locationMarker: "DIAG_1", message: "unexpected text 'async' in array element"),
176+
DiagnosticSpec(locationMarker: "DIAG_2", message: "'async' may only occur before '->'"),
159177
]
160178
)
161179
}

0 commit comments

Comments
 (0)