Skip to content

Improve recovery if variable or subscript is missing brace wrapping getter and setter #979

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
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
54 changes: 29 additions & 25 deletions Sources/SwiftParser/Declarations.swift
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,10 @@ extension Parser {
///
/// declarations → declaration declarations?
///
/// If `allowMissingFuncOrVarKeywordRecovery` is `true`, this methods tries to
/// synthesize `func` or `var` keywords where necessary.
/// If `inMemberDeclList` is `true`, we know that the next item must be a
/// declaration and thus start with a keyword. This allows futher recovery.
@_spi(RawSyntax)
public mutating func parseDeclaration(allowMissingFuncOrVarKeywordRecovery: Bool = false) -> RawDeclSyntax {
public mutating func parseDeclaration(inMemberDeclList: Bool = false) -> RawDeclSyntax {
switch self.at(anyIn: PoundDeclarationStart.self) {
case (.poundIfKeyword, _)?:
let directive = self.parsePoundIfDirective { parser in
Expand Down Expand Up @@ -207,7 +207,7 @@ extension Parser {
case (.subscriptKeyword, let handle)?:
return RawDeclSyntax(self.parseSubscriptDeclaration(attrs, handle))
case (.letKeyword, let handle)?, (.varKeyword, let handle)?:
return RawDeclSyntax(self.parseLetOrVarDeclaration(attrs, handle))
return RawDeclSyntax(self.parseLetOrVarDeclaration(attrs, handle, inMemberDeclList: inMemberDeclList))
case (.initKeyword, let handle)?:
return RawDeclSyntax(self.parseInitializerDeclaration(attrs, handle))
case (.deinitKeyword, let handle)?:
Expand All @@ -219,7 +219,7 @@ extension Parser {
case (.actorContextualKeyword, let handle)?:
return RawDeclSyntax(self.parseActorDeclaration(attrs, handle))
case nil:
if allowMissingFuncOrVarKeywordRecovery {
if inMemberDeclList {
let isProbablyVarDecl = self.at(any: [.identifier, .wildcardKeyword]) && self.peek().tokenKind.is(any: [.colon, .equal, .comma])
let isProbablyTupleDecl = self.at(.leftParen) && self.peek().tokenKind.is(any: [.identifier, .wildcardKeyword])

Expand Down Expand Up @@ -608,7 +608,7 @@ extension Parser {
if self.at(.poundSourceLocationKeyword) {
decl = RawDeclSyntax(self.parsePoundSourceLocationDirective())
} else {
decl = self.parseDeclaration(allowMissingFuncOrVarKeywordRecovery: true)
decl = self.parseDeclaration(inMemberDeclList: true)
}

let semi = self.consume(if: .semicolon)
Expand Down Expand Up @@ -1690,7 +1690,7 @@ extension Parser {

// Parse getter and setter.
let accessor: RawSyntax?
if self.at(.leftBrace) {
if self.at(.leftBrace) || self.at(anyIn: AccessorKind.self) != nil {
accessor = self.parseGetSet()
} else {
accessor = nil
Expand Down Expand Up @@ -1721,10 +1721,22 @@ extension Parser {
/// pattern-initializer-list → pattern-initializer | pattern-initializer ',' pattern-initializer-list
/// pattern-initializer → pattern initializer?
/// initializer → = expression
///
/// If `inMemberDeclList` is `true`, we know that the next item needs to be a
/// declaration that is started by a keyword. Thus, we in the following case
/// we know that `set` can't start a new declaration and we can thus recover
/// by synthesizing a missing `{` in front of `set`.
/// ```
/// var x: Int
/// set {
/// }
/// }
/// ```
@_spi(RawSyntax)
public mutating func parseLetOrVarDeclaration(
_ attrs: DeclAttributes,
_ handle: RecoveryConsumptionHandle
_ handle: RecoveryConsumptionHandle,
inMemberDeclList: Bool = false
) -> RawVariableDeclSyntax {
let (unexpectedBeforeIntroducer, introducer) = self.eat(handle)
let hasTryBeforeIntroducer = unexpectedBeforeIntroducer?.containsToken(where: { $0.tokenKind == .tryKeyword }) ?? false
Expand Down Expand Up @@ -1786,7 +1798,7 @@ extension Parser {
}

let accessor: RawSyntax?
if self.at(.leftBrace) {
if self.at(.leftBrace) || (inMemberDeclList && self.at(anyIn: AccessorKind.self) != nil) {
accessor = self.parseGetSet()
} else {
accessor = nil
Expand All @@ -1812,21 +1824,6 @@ extension Parser {
arena: self.arena)
}

enum AccessorKind: SyntaxText, ContextualKeywords, Equatable {
case `get` = "get"
case `set` = "set"
case `didSet` = "didSet"
case `willSet` = "willSet"
case unsafeAddress = "unsafeAddress"
case addressWithOwner = "addressWithOwner"
case addressWithNativeOwner = "addressWithNativeOwner"
case unsafeMutableAddress = "unsafeMutableAddress"
case mutableAddressWithOwner = "mutableAddressWithOwner"
case mutableAddressWithNativeOwner = "mutableAddressWithNativeOwner"
case _read = "_read"
case _modify = "_modify"
}

struct AccessorIntroducer {
var attributes: RawAttributeListSyntax?
var modifier: RawDeclModifierSyntax?
Expand Down Expand Up @@ -1921,7 +1918,14 @@ extension Parser {
@_spi(RawSyntax)
public mutating func parseGetSet() -> RawSyntax {
// Parse getter and setter.
let (unexpectedBeforeLBrace, lbrace) = self.expect(.leftBrace)
let unexpectedBeforeLBrace: RawUnexpectedNodesSyntax?
let lbrace: RawTokenSyntax
if self.at(anyIn: AccessorKind.self) != nil {
unexpectedBeforeLBrace = nil
lbrace = missingToken(.leftBrace, text: nil)
} else {
(unexpectedBeforeLBrace, lbrace) = self.expect(.leftBrace)
}
// Collect all explicit accessors to a list.
var elements = [RawAccessorDeclSyntax]()
do {
Expand Down
15 changes: 15 additions & 0 deletions Sources/SwiftParser/RawTokenKindSubset.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,21 @@ extension ContextualKeywords where RawValue == SyntaxText {

// MARK: - Subsets

enum AccessorKind: SyntaxText, ContextualKeywords, Equatable {
case `get` = "get"
case `set` = "set"
case `didSet` = "didSet"
case `willSet` = "willSet"
case unsafeAddress = "unsafeAddress"
case addressWithOwner = "addressWithOwner"
case addressWithNativeOwner = "addressWithNativeOwner"
case unsafeMutableAddress = "unsafeMutableAddress"
case mutableAddressWithOwner = "mutableAddressWithOwner"
case mutableAddressWithNativeOwner = "mutableAddressWithNativeOwner"
case _read = "_read"
case _modify = "_modify"
}

enum BinaryOperator: RawTokenKindSubset {
case spacedBinaryOperator
case unspacedBinaryOperator
Expand Down
51 changes: 51 additions & 0 deletions Tests/SwiftParserTest/Declarations.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,57 @@ final class DeclarationTests: XCTestCase {
"""
)
}

func testVariableDeclWithGetSetButNoBrace() {
AssertParse(
"""
struct Foo {
var x: Int 1️⃣
get {
4
}
set {
x = newValue
}
}
}
""",
diagnostics: [
DiagnosticSpec(message: "expected '{' in variable")
]
)
}

func testVariableDeclWithSetGetButNoBrace() {
AssertParse(
"""
struct Foo {
var x: Int 1️⃣
set {
x = newValue
}
get {
4
}
}
}
""",
diagnostics: [
DiagnosticSpec(message: "expected '{' in variable")
]
)
}

func testVariableFollowedByReferenceToSet() {
AssertParse(
"""
func bar() {
let a = b
set.c
}
"""
)
}
}

extension Parser.DeclAttributes {
Expand Down
14 changes: 6 additions & 8 deletions Tests/SwiftParserTest/translated/SubscriptingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -362,20 +362,18 @@ final class SubscriptingTests: XCTestCase {
AssertParse(
"""
struct A8 {
subscript(i : Int) -> Int
1️⃣get 2️⃣{
subscript(i : Int) -> Int1️⃣
get {
return stored
}
3️⃣set 4️⃣{
set {
stored = value
}
}
}2️⃣
""",
diagnostics: [
DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'func' in function"),
DiagnosticSpec(locationMarker: "2️⃣", message: "expected parameter clause in function signature"),
DiagnosticSpec(locationMarker: "3️⃣", message: "expected 'func' in function"),
DiagnosticSpec(locationMarker: "4️⃣", message: "expected parameter clause in function signature"),
DiagnosticSpec(locationMarker: "1️⃣", message: "expected '{' in subscript"),
DiagnosticSpec(locationMarker: "2️⃣", message: "expected '}' to end struct"),
]
)
}
Expand Down