Skip to content

Commit 1f95a65

Browse files
ahoppenahoppen
committed
Fix parser failures found by source alteration in test cases
These parser failures were found by swiftlang#1340. - There was one round-trip failure introduced by swiftlang#1464 - A few cases were just consume tokens as the wrong child - The list of expected token kinds of `DeclModifierSynax` didn’t contain one modifier that was actually parsed
1 parent e92c961 commit 1f95a65

File tree

8 files changed

+154
-75
lines changed

8 files changed

+154
-75
lines changed

‎CodeGeneration/Sources/SyntaxSupport/DeclNodes.swift

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ public let DECL_NODES: [Node] = [
332332
),
333333
Child(
334334
name: "Detail",
335-
kind: .token(choices: [.token(tokenKind: "IdentifierToken"), .keyword(text: "set")])
335+
kind: .token(choices: [.token(tokenKind: "IdentifierToken")])
336336
),
337337
Child(
338338
name: "RightParen",
@@ -348,7 +348,44 @@ public let DECL_NODES: [Node] = [
348348
children: [
349349
Child(
350350
name: "Name",
351-
kind: .token(choices: [.keyword(text: "class"), .keyword(text: "convenience"), .keyword(text: "dynamic"), .keyword(text: "final"), .keyword(text: "infix"), .keyword(text: "lazy"), .keyword(text: "optional"), .keyword(text: "override"), .keyword(text: "postfix"), .keyword(text: "prefix"), .keyword(text: "required"), .keyword(text: "static"), .keyword(text: "unowned"), .keyword(text: "weak"), .keyword(text: "private"), .keyword(text: "fileprivate"), .keyword(text: "internal"), .keyword(text: "public"), .keyword(text: "open"), .keyword(text: "mutating"), .keyword(text: "nonmutating"), .keyword(text: "indirect"), .keyword(text: "__consuming"), .keyword(text: "borrowing"), .keyword(text: "consuming"), .keyword(text: "actor"), .keyword(text: "async"), .keyword(text: "distributed"), .keyword(text: "isolated"), .keyword(text: "nonisolated"), .keyword(text: "_const"), .keyword(text: "_local"), .keyword(text: "package")]),
351+
kind: .token(choices: [
352+
.keyword(text: "__consuming"),
353+
.keyword(text: "__setter_access"),
354+
.keyword(text: "_const"),
355+
.keyword(text: "_local"),
356+
.keyword(text: "actor"),
357+
.keyword(text: "async"),
358+
.keyword(text: "borrowing"),
359+
.keyword(text: "class"),
360+
.keyword(text: "consuming"),
361+
.keyword(text: "convenience"),
362+
.keyword(text: "distributed"),
363+
.keyword(text: "dynamic"),
364+
.keyword(text: "fileprivate"),
365+
.keyword(text: "final"),
366+
.keyword(text: "indirect"),
367+
.keyword(text: "infix"),
368+
.keyword(text: "internal"),
369+
.keyword(text: "isolated"),
370+
.keyword(text: "lazy"),
371+
.keyword(text: "mutating"),
372+
.keyword(text: "nonisolated"),
373+
.keyword(text: "nonmutating"),
374+
.keyword(text: "open"),
375+
.keyword(text: "optional"),
376+
.keyword(text: "override"),
377+
.keyword(text: "package"),
378+
.keyword(text: "postfix"),
379+
.keyword(text: "prefix"),
380+
.keyword(text: "private"),
381+
.keyword(text: "public"),
382+
.keyword(text: "reasync"),
383+
.keyword(text: "required"),
384+
.keyword(text: "setter_access"),
385+
.keyword(text: "static"),
386+
.keyword(text: "unowned"),
387+
.keyword(text: "weak"),
388+
]),
352389
classification: "Attribute"
353390
),
354391
Child(

‎Sources/SwiftParser/Modifiers.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,12 @@ extension Parser {
9999
extension Parser {
100100
mutating func parseModifierDetail() -> RawDeclModifierDetailSyntax {
101101
let (unexpectedBeforeLeftParen, leftParen) = self.expect(.leftParen)
102-
let detailToken = self.consumeAnyToken()
102+
let (unexpectedBeforeDetailToken, detailToken) = self.expect(.identifier, TokenSpec(.set, remapping: .identifier), default: .identifier)
103103
let (unexpectedBeforeRightParen, rightParen) = self.expect(.rightParen)
104104
return RawDeclModifierDetailSyntax(
105105
unexpectedBeforeLeftParen,
106106
leftParen: leftParen,
107+
unexpectedBeforeDetailToken,
107108
detail: detailToken,
108109
unexpectedBeforeRightParen,
109110
rightParen: rightParen,

‎Sources/SwiftParser/Patterns.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,8 @@ extension Parser {
221221

222222
/// If we have something like `x SomeType`, use the indication that `SomeType` starts with a capital letter (and is thus probably a type name)
223223
/// as an indication that the user forgot to write the colon instead of forgetting to write the comma to separate two elements.
224-
if label == nil, colon == nil, peek().rawTokenKind == .identifier, peek().tokenText.isStartingWithUppercase {
225-
label = consumeAnyToken()
224+
if label == nil, colon == nil, self.at(.identifier), peek().rawTokenKind == .identifier, peek().tokenText.isStartingWithUppercase {
225+
label = consume(if: .identifier)
226226
colon = self.missingToken(.colon)
227227
}
228228

‎Sources/SwiftParser/Specifiers.swift

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -412,11 +412,11 @@ extension RawAccessorEffectSpecifiersSyntax: RawEffectSpecifiersTrait {
412412
}
413413

414414
extension TokenConsumer {
415-
mutating func at<SpecSet1: TokenSpecSet, SpecSet2: TokenSpecSet>(anyIn specSet1: SpecSet1.Type, or specSet2: SpecSet2.Type) -> (TokenSpec, TokenConsumptionHandle)? {
415+
mutating func at<SpecSet1: TokenSpecSet, SpecSet2: TokenSpecSet>(anyIn specSet1: SpecSet1.Type, or specSet2: SpecSet2.Type) -> (spec: TokenSpec, handle: TokenConsumptionHandle, matchedSubset: Any.Type)? {
416416
if let (spec, handle) = self.at(anyIn: specSet1) {
417-
return (spec.spec, handle)
417+
return (spec.spec, handle, SpecSet1.self)
418418
} else if let (spec, handle) = self.at(anyIn: specSet2) {
419-
return (spec.spec, handle)
419+
return (spec.spec, handle, SpecSet2.self)
420420
} else {
421421
return nil
422422
}
@@ -470,14 +470,14 @@ extension Parser {
470470

471471
var unexpectedAfterThrowsLoopProgress = LoopProgressCondition()
472472
while unexpectedAfterThrowsLoopProgress.evaluate(self.currentToken) {
473-
if let (_, handle) = self.at(anyIn: S.MisspelledAsyncSpecifiers.self, or: S.CorrectAsyncTokenKinds.self) {
473+
if let (_, handle, _) = self.at(anyIn: S.MisspelledAsyncSpecifiers.self, or: S.CorrectAsyncTokenKinds.self) {
474474
let misspelledAsync = self.eat(handle)
475475
unexpectedAfterThrows.append(RawSyntax(misspelledAsync))
476476
if asyncKeyword == nil {
477477
// Handle `async` after `throws`
478478
asyncKeyword = missingToken(.keyword(.async))
479479
}
480-
} else if let (_, handle) = self.at(anyIn: S.MisspelledThrowsTokenKinds.self, or: S.CorrectThrowsTokenKinds.self) {
480+
} else if let (_, handle, _) = self.at(anyIn: S.MisspelledThrowsTokenKinds.self, or: S.CorrectThrowsTokenKinds.self) {
481481
let misspelledThrows = self.eat(handle)
482482
unexpectedAfterThrows.append(RawSyntax(misspelledThrows))
483483
} else {
@@ -521,17 +521,25 @@ extension Parser {
521521
var unexpected: [RawTokenSyntax] = []
522522
var loopProgress = LoopProgressCondition()
523523
while loopProgress.evaluate(self.currentToken) {
524-
if let (spec, handle) = self.at(anyIn: S.MisspelledAsyncSpecifiers.self, or: S.CorrectAsyncTokenKinds.self) {
524+
if let (spec, handle, matchedSubset) = self.at(anyIn: S.MisspelledAsyncSpecifiers.self, or: S.CorrectAsyncTokenKinds.self) {
525525
let misspelledAsync = self.eat(handle)
526526
unexpected.append(misspelledAsync)
527527
if effectSpecifiers?.asyncSpecifier == nil {
528-
synthesizedAsync = missingToken(spec)
528+
if matchedSubset == S.CorrectAsyncTokenKinds.self {
529+
synthesizedAsync = missingToken(spec)
530+
} else {
531+
synthesizedAsync = missingToken(.async)
532+
}
529533
}
530-
} else if let (spec, handle) = self.at(anyIn: S.MisspelledThrowsTokenKinds.self, or: S.CorrectThrowsTokenKinds.self) {
534+
} else if let (spec, handle, matchedSubset) = self.at(anyIn: S.MisspelledThrowsTokenKinds.self, or: S.CorrectThrowsTokenKinds.self) {
531535
let misspelledThrows = self.eat(handle)
532536
unexpected.append(misspelledThrows)
533537
if effectSpecifiers?.throwsSpecifier == nil {
534-
synthesizedThrows = missingToken(spec)
538+
if matchedSubset == S.CorrectThrowsTokenKinds.self {
539+
synthesizedThrows = missingToken(spec)
540+
} else {
541+
synthesizedThrows = missingToken(.throws)
542+
}
535543
}
536544
} else {
537545
break

‎Sources/SwiftParser/Statements.swift

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,9 +188,6 @@ extension Parser {
188188
repeat {
189189
let condition = self.parseConditionElement(lastBindingKind: elements.last?.condition.as(RawOptionalBindingConditionSyntax.self)?.bindingKeyword)
190190
var unexpectedBeforeKeepGoing: RawUnexpectedNodesSyntax? = nil
191-
if let equalOperator = self.consumeIfContextualPunctuator("=="), let falseKeyword = self.consume(if: .keyword(.false)) {
192-
unexpectedBeforeKeepGoing = RawUnexpectedNodesSyntax([equalOperator, falseKeyword], arena: self.arena)
193-
}
194191
keepGoing = self.consume(if: .comma)
195192
if keepGoing == nil, let andOperator = self.consumeIfContextualPunctuator("&&") {
196193
unexpectedBeforeKeepGoing = RawUnexpectedNodesSyntax(combining: unexpectedBeforeKeepGoing, andOperator, arena: self.arena)
@@ -342,6 +339,12 @@ extension Parser {
342339
let (unexpectedBeforeLParen, lparen) = self.expect(.leftParen)
343340
let spec = self.parseAvailabilitySpecList()
344341
let (unexpectedBeforeRParen, rparen) = self.expect(.rightParen)
342+
let unexpectedAfterRParen: RawUnexpectedNodesSyntax?
343+
if let (equalOperator, falseKeyword) = self.consume(if: { $0.isContextualPunctuator("==") }, followedBy: { TokenSpec.keyword(.false) ~= $0 }) {
344+
unexpectedAfterRParen = RawUnexpectedNodesSyntax([equalOperator, falseKeyword], arena: self.arena)
345+
} else {
346+
unexpectedAfterRParen = nil
347+
}
345348
return .availability(
346349
RawAvailabilityConditionSyntax(
347350
availabilityKeyword: keyword,
@@ -350,6 +353,7 @@ extension Parser {
350353
availabilitySpec: spec,
351354
unexpectedBeforeRParen,
352355
rightParen: rparen,
356+
unexpectedAfterRParen,
353357
arena: self.arena
354358
)
355359
)

‎Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 35 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,41 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
460460
return .visitChildren
461461
}
462462

463+
public override func visit(_ node: AvailabilityConditionSyntax) -> SyntaxVisitorContinueKind {
464+
if shouldSkip(node) {
465+
return .skipChildren
466+
}
467+
468+
if let unexpectedAfterRightParen = node.unexpectedAfterRightParen,
469+
let (_, falseKeyword) = unexpectedAfterRightParen.twoTokens(
470+
firstSatisfying: { $0.tokenKind == .binaryOperator("==") },
471+
secondSatisfying: { $0.tokenKind == .keyword(.false) }
472+
)
473+
{
474+
// Diagnose #available used as an expression
475+
let negatedAvailabilityKeyword = node.availabilityKeyword.negatedAvailabilityKeyword
476+
let negatedAvailability =
477+
node
478+
.with(\.availabilityKeyword, negatedAvailabilityKeyword)
479+
.with(\.unexpectedAfterRightParen, nil)
480+
addDiagnostic(
481+
unexpectedAfterRightParen,
482+
AvailabilityConditionAsExpression(availabilityToken: node.availabilityKeyword, negatedAvailabilityToken: negatedAvailabilityKeyword),
483+
fixIts: [
484+
FixIt(
485+
message: ReplaceTokensFixIt(replaceTokens: getTokens(between: node.availabilityKeyword, and: falseKeyword), replacements: getTokens(between: negatedAvailability.availabilityKeyword, and: negatedAvailability.rightParen)),
486+
changes: [
487+
.replace(oldNode: Syntax(node), newNode: Syntax(negatedAvailability))
488+
]
489+
)
490+
],
491+
handledNodes: [unexpectedAfterRightParen.id]
492+
)
493+
}
494+
495+
return .visitChildren
496+
}
497+
463498
public override func visit(_ node: AvailabilityVersionRestrictionSyntax) -> SyntaxVisitorContinueKind {
464499
if shouldSkip(node) {
465500
return .skipChildren
@@ -483,35 +518,6 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
483518
if shouldSkip(node) {
484519
return .skipChildren
485520
}
486-
if let unexpected = node.unexpectedBetweenConditionAndTrailingComma,
487-
let availability = node.condition.as(AvailabilityConditionSyntax.self),
488-
let (_, falseKeyword) = unexpected.twoTokens(
489-
firstSatisfying: { $0.tokenKind == .binaryOperator("==") },
490-
secondSatisfying: { $0.tokenKind == .keyword(.false) }
491-
)
492-
{
493-
// Diagnose #available used as an expression
494-
let negatedAvailabilityKeyword = availability.availabilityKeyword.negatedAvailabilityKeyword
495-
let negatedCoditionElement = ConditionElementSyntax(
496-
condition: .availability(availability.with(\.availabilityKeyword, negatedAvailabilityKeyword)),
497-
trailingComma: node.trailingComma
498-
)
499-
if let negatedAvailability = negatedCoditionElement.condition.as(AvailabilityConditionSyntax.self) {
500-
addDiagnostic(
501-
unexpected,
502-
AvailabilityConditionAsExpression(availabilityToken: availability.availabilityKeyword, negatedAvailabilityToken: negatedAvailabilityKeyword),
503-
fixIts: [
504-
FixIt(
505-
message: ReplaceTokensFixIt(replaceTokens: getTokens(between: availability.availabilityKeyword, and: falseKeyword), replacements: getTokens(between: negatedAvailability.availabilityKeyword, and: negatedAvailability.rightParen)),
506-
changes: [
507-
.replace(oldNode: Syntax(node), newNode: Syntax(negatedCoditionElement))
508-
]
509-
)
510-
],
511-
handledNodes: [unexpected.id]
512-
)
513-
}
514-
}
515521
if let trailingComma = node.trailingComma {
516522
exchangeTokens(
517523
unexpected: node.unexpectedBetweenConditionAndTrailingComma,

‎Sources/SwiftSyntax/generated/raw/RawSyntaxValidation.swift

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -799,39 +799,42 @@ func validateLayout(layout: RawSyntaxBuffer, as kind: SyntaxKind) {
799799
assert(layout.count == 5)
800800
assertNoError(kind, 0, verify(layout[0], as: RawUnexpectedNodesSyntax?.self))
801801
assertNoError(kind, 1, verify(layout[1], as: RawTokenSyntax.self, tokenChoices: [
802+
.keyword("__consuming"),
803+
.keyword("__setter_access"),
804+
.keyword("_const"),
805+
.keyword("_local"),
806+
.keyword("actor"),
807+
.keyword("async"),
808+
.keyword("borrowing"),
802809
.keyword("class"),
810+
.keyword("consuming"),
803811
.keyword("convenience"),
812+
.keyword("distributed"),
804813
.keyword("dynamic"),
814+
.keyword("fileprivate"),
805815
.keyword("final"),
816+
.keyword("indirect"),
806817
.keyword("infix"),
818+
.keyword("internal"),
819+
.keyword("isolated"),
807820
.keyword("lazy"),
821+
.keyword("mutating"),
822+
.keyword("nonisolated"),
823+
.keyword("nonmutating"),
824+
.keyword("open"),
808825
.keyword("optional"),
809826
.keyword("override"),
827+
.keyword("package"),
810828
.keyword("postfix"),
811829
.keyword("prefix"),
830+
.keyword("private"),
831+
.keyword("public"),
832+
.keyword("reasync"),
812833
.keyword("required"),
834+
.keyword("setter_access"),
813835
.keyword("static"),
814836
.keyword("unowned"),
815-
.keyword("weak"),
816-
.keyword("private"),
817-
.keyword("fileprivate"),
818-
.keyword("internal"),
819-
.keyword("public"),
820-
.keyword("open"),
821-
.keyword("mutating"),
822-
.keyword("nonmutating"),
823-
.keyword("indirect"),
824-
.keyword("__consuming"),
825-
.keyword("borrowing"),
826-
.keyword("consuming"),
827-
.keyword("actor"),
828-
.keyword("async"),
829-
.keyword("distributed"),
830-
.keyword("isolated"),
831-
.keyword("nonisolated"),
832-
.keyword("_const"),
833-
.keyword("_local"),
834-
.keyword("package")
837+
.keyword("weak")
835838
]))
836839
assertNoError(kind, 2, verify(layout[2], as: RawUnexpectedNodesSyntax?.self))
837840
assertNoError(kind, 3, verify(layout[3], as: RawDeclModifierDetailSyntax?.self))

‎Tests/SwiftParserTest/translated/AvailabilityQueryUnavailabilityTests.swift

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -436,29 +436,33 @@ final class AvailabilityQueryUnavailabilityTests: XCTestCase {
436436
func testAvailabilityQueryUnavailability34a() {
437437
assertParse(
438438
"""
439-
// Diagnose wrong spellings of unavailability
440439
if #available(*) 1️⃣== false {
441440
}
442441
""",
443442
diagnostics: [
444443
DiagnosticSpec(message: "#available cannot be used as an expression, did you mean to use '#unavailable'?", fixIts: ["replace '#available(*) == false' with '#unavailable(*)'"])
445-
]
444+
],
445+
fixedSource: """
446+
if #unavailable(*) {
447+
}
448+
"""
446449
)
447450
}
448451

449452
func testAvailabilityQueryUnavailability34b() {
450453
assertParse(
451454
"""
452-
// Diagnose wrong spellings of unavailability
453-
if #available(*) 1️⃣== false && 2️⃣true {
455+
if #available(*) 1️⃣== false 2️⃣&& true {
454456
}
455457
""",
456458
diagnostics: [
457-
DiagnosticSpec(locationMarker: "1️⃣", message: "unexpected code '== false &&' in 'if' statement"),
458-
DiagnosticSpec(locationMarker: "2️⃣", message: "expected ',' in 'if' statement", fixIts: ["insert ','"]),
459-
// TODO: Old parser expected error on line 2: #available cannot be used as an expression, did you mean to use '#unavailable'?, Fix-It replacements: 4 - 14 = '#unavailable', 18 - 27 = ''
460-
// TODO: Old parser expected error on line 2: expected ',' joining parts of a multi-clause condition, Fix-It replacements: 27 - 28 = ','
461-
]
459+
DiagnosticSpec(locationMarker: "1️⃣", message: "#available cannot be used as an expression, did you mean to use '#unavailable'?", fixIts: ["replace '#available(*) == false' with '#unavailable(*)'"]),
460+
DiagnosticSpec(locationMarker: "2️⃣", message: "expected ',' joining parts of a multi-clause condition", fixIts: ["replace '&&' with ','"]),
461+
],
462+
fixedSource: """
463+
if #unavailable(*) , true {
464+
}
465+
"""
462466
)
463467
}
464468

@@ -470,6 +474,22 @@ final class AvailabilityQueryUnavailabilityTests: XCTestCase {
470474
""",
471475
diagnostics: [
472476
DiagnosticSpec(message: "availability condition cannot be used in an expression; did you mean '#unavailable'?", fixIts: ["replace '!#available' with '#unavailable'"])
477+
],
478+
fixedSource: """
479+
if #unavailable(*) {
480+
}
481+
"""
482+
)
483+
}
484+
485+
func testAvailabilityQueryUnavailability34d() {
486+
assertParse(
487+
"""
488+
if #available(*) 1️⃣== {
489+
}
490+
""",
491+
diagnostics: [
492+
DiagnosticSpec(locationMarker: "1️⃣", message: "unexpected code '==' in 'if' statement")
473493
]
474494
)
475495
}

0 commit comments

Comments
 (0)