Skip to content

Fix parser failures found by source alteration in test cases #1538

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 39 additions & 2 deletions CodeGeneration/Sources/SyntaxSupport/DeclNodes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ public let DECL_NODES: [Node] = [
),
Child(
name: "Detail",
kind: .token(choices: [.token(tokenKind: "IdentifierToken"), .keyword(text: "set")])
kind: .token(choices: [.token(tokenKind: "IdentifierToken")])
),
Child(
name: "RightParen",
Expand All @@ -348,7 +348,44 @@ public let DECL_NODES: [Node] = [
children: [
Child(
name: "Name",
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")]),
kind: .token(choices: [
.keyword(text: "__consuming"),
.keyword(text: "__setter_access"),
.keyword(text: "_const"),
.keyword(text: "_local"),
.keyword(text: "actor"),
.keyword(text: "async"),
.keyword(text: "borrowing"),
.keyword(text: "class"),
.keyword(text: "consuming"),
.keyword(text: "convenience"),
.keyword(text: "distributed"),
.keyword(text: "dynamic"),
.keyword(text: "fileprivate"),
.keyword(text: "final"),
.keyword(text: "indirect"),
.keyword(text: "infix"),
.keyword(text: "internal"),
.keyword(text: "isolated"),
.keyword(text: "lazy"),
.keyword(text: "mutating"),
.keyword(text: "nonisolated"),
.keyword(text: "nonmutating"),
.keyword(text: "open"),
.keyword(text: "optional"),
.keyword(text: "override"),
.keyword(text: "package"),
.keyword(text: "postfix"),
.keyword(text: "prefix"),
.keyword(text: "private"),
.keyword(text: "public"),
.keyword(text: "reasync"),
.keyword(text: "required"),
.keyword(text: "setter_access"),
.keyword(text: "static"),
.keyword(text: "unowned"),
.keyword(text: "weak"),
]),
classification: "Attribute"
),
Child(
Expand Down
3 changes: 2 additions & 1 deletion Sources/SwiftParser/Modifiers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,12 @@ extension Parser {
extension Parser {
mutating func parseModifierDetail() -> RawDeclModifierDetailSyntax {
let (unexpectedBeforeLeftParen, leftParen) = self.expect(.leftParen)
let detailToken = self.consumeAnyToken()
let (unexpectedBeforeDetailToken, detailToken) = self.expect(.identifier, TokenSpec(.set, remapping: .identifier), default: .identifier)
let (unexpectedBeforeRightParen, rightParen) = self.expect(.rightParen)
return RawDeclModifierDetailSyntax(
unexpectedBeforeLeftParen,
leftParen: leftParen,
unexpectedBeforeDetailToken,
detail: detailToken,
unexpectedBeforeRightParen,
rightParen: rightParen,
Expand Down
4 changes: 2 additions & 2 deletions Sources/SwiftParser/Patterns.swift
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,8 @@ extension Parser {

/// 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()
if label == nil, colon == nil, self.at(.identifier), peek().rawTokenKind == .identifier, peek().tokenText.isStartingWithUppercase {
label = consume(if: .identifier)
colon = self.missingToken(.colon)
}

Expand Down
26 changes: 17 additions & 9 deletions Sources/SwiftParser/Specifiers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -412,11 +412,11 @@ extension RawAccessorEffectSpecifiersSyntax: RawEffectSpecifiersTrait {
}

extension TokenConsumer {
mutating func at<SpecSet1: TokenSpecSet, SpecSet2: TokenSpecSet>(anyIn specSet1: SpecSet1.Type, or specSet2: SpecSet2.Type) -> (TokenSpec, TokenConsumptionHandle)? {
mutating func at<SpecSet1: TokenSpecSet, SpecSet2: TokenSpecSet>(anyIn specSet1: SpecSet1.Type, or specSet2: SpecSet2.Type) -> (spec: TokenSpec, handle: TokenConsumptionHandle, matchedSubset: Any.Type)? {
if let (spec, handle) = self.at(anyIn: specSet1) {
return (spec.spec, handle)
return (spec.spec, handle, SpecSet1.self)
} else if let (spec, handle) = self.at(anyIn: specSet2) {
return (spec.spec, handle)
return (spec.spec, handle, SpecSet2.self)
} else {
return nil
}
Expand Down Expand Up @@ -470,14 +470,14 @@ extension Parser {

var unexpectedAfterThrowsLoopProgress = LoopProgressCondition()
while unexpectedAfterThrowsLoopProgress.evaluate(self.currentToken) {
if let (_, handle) = self.at(anyIn: S.MisspelledAsyncSpecifiers.self, or: S.CorrectAsyncTokenKinds.self) {
if let (_, handle, _) = self.at(anyIn: S.MisspelledAsyncSpecifiers.self, or: S.CorrectAsyncTokenKinds.self) {
let misspelledAsync = self.eat(handle)
unexpectedAfterThrows.append(RawSyntax(misspelledAsync))
if asyncKeyword == nil {
// Handle `async` after `throws`
asyncKeyword = missingToken(.keyword(.async))
}
} else if let (_, handle) = self.at(anyIn: S.MisspelledThrowsTokenKinds.self, or: S.CorrectThrowsTokenKinds.self) {
} else if let (_, handle, _) = self.at(anyIn: S.MisspelledThrowsTokenKinds.self, or: S.CorrectThrowsTokenKinds.self) {
let misspelledThrows = self.eat(handle)
unexpectedAfterThrows.append(RawSyntax(misspelledThrows))
} else {
Expand Down Expand Up @@ -521,17 +521,25 @@ extension Parser {
var unexpected: [RawTokenSyntax] = []
var loopProgress = LoopProgressCondition()
while loopProgress.evaluate(self.currentToken) {
if let (spec, handle) = self.at(anyIn: S.MisspelledAsyncSpecifiers.self, or: S.CorrectAsyncTokenKinds.self) {
if let (spec, handle, matchedSubset) = self.at(anyIn: S.MisspelledAsyncSpecifiers.self, or: S.CorrectAsyncTokenKinds.self) {
let misspelledAsync = self.eat(handle)
unexpected.append(misspelledAsync)
if effectSpecifiers?.asyncSpecifier == nil {
synthesizedAsync = missingToken(spec)
if matchedSubset == S.CorrectAsyncTokenKinds.self {
synthesizedAsync = missingToken(spec)
} else {
synthesizedAsync = missingToken(.async)
}
}
} else if let (spec, handle) = self.at(anyIn: S.MisspelledThrowsTokenKinds.self, or: S.CorrectThrowsTokenKinds.self) {
} else if let (spec, handle, matchedSubset) = self.at(anyIn: S.MisspelledThrowsTokenKinds.self, or: S.CorrectThrowsTokenKinds.self) {
let misspelledThrows = self.eat(handle)
unexpected.append(misspelledThrows)
if effectSpecifiers?.throwsSpecifier == nil {
synthesizedThrows = missingToken(spec)
if matchedSubset == S.CorrectThrowsTokenKinds.self {
synthesizedThrows = missingToken(spec)
} else {
synthesizedThrows = missingToken(.throws)
}
}
} else {
break
Expand Down
10 changes: 7 additions & 3 deletions Sources/SwiftParser/Statements.swift
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,6 @@ extension Parser {
repeat {
let condition = self.parseConditionElement(lastBindingKind: elements.last?.condition.as(RawOptionalBindingConditionSyntax.self)?.bindingKeyword)
var unexpectedBeforeKeepGoing: RawUnexpectedNodesSyntax? = nil
if let equalOperator = self.consumeIfContextualPunctuator("=="), let falseKeyword = self.consume(if: .keyword(.false)) {
unexpectedBeforeKeepGoing = RawUnexpectedNodesSyntax([equalOperator, falseKeyword], arena: self.arena)
}
keepGoing = self.consume(if: .comma)
if keepGoing == nil, let andOperator = self.consumeIfContextualPunctuator("&&") {
unexpectedBeforeKeepGoing = RawUnexpectedNodesSyntax(combining: unexpectedBeforeKeepGoing, andOperator, arena: self.arena)
Expand Down Expand Up @@ -342,6 +339,12 @@ extension Parser {
let (unexpectedBeforeLParen, lparen) = self.expect(.leftParen)
let spec = self.parseAvailabilitySpecList()
let (unexpectedBeforeRParen, rparen) = self.expect(.rightParen)
let unexpectedAfterRParen: RawUnexpectedNodesSyntax?
if let (equalOperator, falseKeyword) = self.consume(if: { $0.isContextualPunctuator("==") }, followedBy: { TokenSpec.keyword(.false) ~= $0 }) {
unexpectedAfterRParen = RawUnexpectedNodesSyntax([equalOperator, falseKeyword], arena: self.arena)
} else {
unexpectedAfterRParen = nil
}
return .availability(
RawAvailabilityConditionSyntax(
availabilityKeyword: keyword,
Expand All @@ -350,6 +353,7 @@ extension Parser {
availabilitySpec: spec,
unexpectedBeforeRParen,
rightParen: rparen,
unexpectedAfterRParen,
arena: self.arena
)
)
Expand Down
64 changes: 35 additions & 29 deletions Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,41 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
return .visitChildren
}

public override func visit(_ node: AvailabilityConditionSyntax) -> SyntaxVisitorContinueKind {
if shouldSkip(node) {
return .skipChildren
}

if let unexpectedAfterRightParen = node.unexpectedAfterRightParen,
let (_, falseKeyword) = unexpectedAfterRightParen.twoTokens(
firstSatisfying: { $0.tokenKind == .binaryOperator("==") },
secondSatisfying: { $0.tokenKind == .keyword(.false) }
)
{
// Diagnose #available used as an expression
let negatedAvailabilityKeyword = node.availabilityKeyword.negatedAvailabilityKeyword
let negatedAvailability =
node
.with(\.availabilityKeyword, negatedAvailabilityKeyword)
.with(\.unexpectedAfterRightParen, nil)
addDiagnostic(
unexpectedAfterRightParen,
AvailabilityConditionAsExpression(availabilityToken: node.availabilityKeyword, negatedAvailabilityToken: negatedAvailabilityKeyword),
fixIts: [
FixIt(
message: ReplaceTokensFixIt(replaceTokens: getTokens(between: node.availabilityKeyword, and: falseKeyword), replacements: getTokens(between: negatedAvailability.availabilityKeyword, and: negatedAvailability.rightParen)),
changes: [
.replace(oldNode: Syntax(node), newNode: Syntax(negatedAvailability))
]
)
],
handledNodes: [unexpectedAfterRightParen.id]
)
}

return .visitChildren
}

public override func visit(_ node: AvailabilityVersionRestrictionSyntax) -> SyntaxVisitorContinueKind {
if shouldSkip(node) {
return .skipChildren
Expand All @@ -483,35 +518,6 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
if shouldSkip(node) {
return .skipChildren
}
if let unexpected = node.unexpectedBetweenConditionAndTrailingComma,
let availability = node.condition.as(AvailabilityConditionSyntax.self),
let (_, falseKeyword) = unexpected.twoTokens(
firstSatisfying: { $0.tokenKind == .binaryOperator("==") },
secondSatisfying: { $0.tokenKind == .keyword(.false) }
)
{
// Diagnose #available used as an expression
let negatedAvailabilityKeyword = availability.availabilityKeyword.negatedAvailabilityKeyword
let negatedCoditionElement = ConditionElementSyntax(
condition: .availability(availability.with(\.availabilityKeyword, negatedAvailabilityKeyword)),
trailingComma: node.trailingComma
)
if let negatedAvailability = negatedCoditionElement.condition.as(AvailabilityConditionSyntax.self) {
addDiagnostic(
unexpected,
AvailabilityConditionAsExpression(availabilityToken: availability.availabilityKeyword, negatedAvailabilityToken: negatedAvailabilityKeyword),
fixIts: [
FixIt(
message: ReplaceTokensFixIt(replaceTokens: getTokens(between: availability.availabilityKeyword, and: falseKeyword), replacements: getTokens(between: negatedAvailability.availabilityKeyword, and: negatedAvailability.rightParen)),
changes: [
.replace(oldNode: Syntax(node), newNode: Syntax(negatedCoditionElement))
]
)
],
handledNodes: [unexpected.id]
)
}
}
if let trailingComma = node.trailingComma {
exchangeTokens(
unexpected: node.unexpectedBetweenConditionAndTrailingComma,
Expand Down
43 changes: 23 additions & 20 deletions Sources/SwiftSyntax/generated/raw/RawSyntaxValidation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -799,39 +799,42 @@ func validateLayout(layout: RawSyntaxBuffer, as kind: SyntaxKind) {
assert(layout.count == 5)
assertNoError(kind, 0, verify(layout[0], as: RawUnexpectedNodesSyntax?.self))
assertNoError(kind, 1, verify(layout[1], as: RawTokenSyntax.self, tokenChoices: [
.keyword("__consuming"),
.keyword("__setter_access"),
.keyword("_const"),
.keyword("_local"),
.keyword("actor"),
.keyword("async"),
.keyword("borrowing"),
.keyword("class"),
.keyword("consuming"),
.keyword("convenience"),
.keyword("distributed"),
.keyword("dynamic"),
.keyword("fileprivate"),
.keyword("final"),
.keyword("indirect"),
.keyword("infix"),
.keyword("internal"),
.keyword("isolated"),
.keyword("lazy"),
.keyword("mutating"),
.keyword("nonisolated"),
.keyword("nonmutating"),
.keyword("open"),
.keyword("optional"),
.keyword("override"),
.keyword("package"),
.keyword("postfix"),
.keyword("prefix"),
.keyword("private"),
.keyword("public"),
.keyword("reasync"),
.keyword("required"),
.keyword("setter_access"),
.keyword("static"),
.keyword("unowned"),
.keyword("weak"),
.keyword("private"),
.keyword("fileprivate"),
.keyword("internal"),
.keyword("public"),
.keyword("open"),
.keyword("mutating"),
.keyword("nonmutating"),
.keyword("indirect"),
.keyword("__consuming"),
.keyword("borrowing"),
.keyword("consuming"),
.keyword("actor"),
.keyword("async"),
.keyword("distributed"),
.keyword("isolated"),
.keyword("nonisolated"),
.keyword("_const"),
.keyword("_local"),
.keyword("package")
.keyword("weak")
]))
assertNoError(kind, 2, verify(layout[2], as: RawUnexpectedNodesSyntax?.self))
assertNoError(kind, 3, verify(layout[3], as: RawDeclModifierDetailSyntax?.self))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -436,29 +436,33 @@ final class AvailabilityQueryUnavailabilityTests: XCTestCase {
func testAvailabilityQueryUnavailability34a() {
assertParse(
"""
// Diagnose wrong spellings of unavailability
if #available(*) 1️⃣== false {
}
""",
diagnostics: [
DiagnosticSpec(message: "#available cannot be used as an expression, did you mean to use '#unavailable'?", fixIts: ["replace '#available(*) == false' with '#unavailable(*)'"])
]
],
fixedSource: """
if #unavailable(*) {
}
"""
)
}

func testAvailabilityQueryUnavailability34b() {
assertParse(
"""
// Diagnose wrong spellings of unavailability
if #available(*) 1️⃣== false && 2️⃣true {
if #available(*) 1️⃣== false 2️⃣&& true {
}
""",
diagnostics: [
DiagnosticSpec(locationMarker: "1️⃣", message: "unexpected code '== false &&' in 'if' statement"),
DiagnosticSpec(locationMarker: "2️⃣", message: "expected ',' in 'if' statement", fixIts: ["insert ','"]),
// 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 = ''
// TODO: Old parser expected error on line 2: expected ',' joining parts of a multi-clause condition, Fix-It replacements: 27 - 28 = ','
]
DiagnosticSpec(locationMarker: "1️⃣", message: "#available cannot be used as an expression, did you mean to use '#unavailable'?", fixIts: ["replace '#available(*) == false' with '#unavailable(*)'"]),
DiagnosticSpec(locationMarker: "2️⃣", message: "expected ',' joining parts of a multi-clause condition", fixIts: ["replace '&&' with ','"]),
],
fixedSource: """
if #unavailable(*) , true {
}
"""
)
}

Expand All @@ -470,6 +474,22 @@ final class AvailabilityQueryUnavailabilityTests: XCTestCase {
""",
diagnostics: [
DiagnosticSpec(message: "availability condition cannot be used in an expression; did you mean '#unavailable'?", fixIts: ["replace '!#available' with '#unavailable'"])
],
fixedSource: """
if #unavailable(*) {
}
"""
)
}

func testAvailabilityQueryUnavailability34d() {
assertParse(
"""
if #available(*) 1️⃣== {
}
""",
diagnostics: [
DiagnosticSpec(locationMarker: "1️⃣", message: "unexpected code '==' in 'if' statement")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should add a fixit if we start to require it in #1467

Same for fixedSource below

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reminder

]
)
}
Expand Down