Skip to content

Detect missing comma in function declaration #886

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Detect missing comma in function declaration #886

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 7 additions & 1 deletion Sources/SwiftParser/Declarations.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1357,7 +1357,13 @@ extension Parser {
defaultArgument = nil
}

let trailingComma = self.consume(if: .comma)
var trailingComma = self.consume(if: .comma)
if trailingComma == nil {
var lookahead = self.lookahead()
if lookahead.consume(if: { $0.canBeArgumentLabel }, followedBy: { $0.tokenKind == .colon }) != nil {
Comment on lines +1362 to +1363
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I’m not sure if this is the right approach because it break for anything but the most simple arguments. E.g. we wouldn’t enter this recovery case if the argument has modifiers or a secondName. I think you would need to implement a little more sophisticated logic in Lookahead and call that from here.

Also adding test a test case for func foo(first second: Int third fourth: Int) would be good.

trailingComma = missingToken(.comma, text: nil)
}
}
keepGoing = trailingComma != nil
elements.append(RawFunctionParameterSyntax(
attributes: attrs,
Expand Down
102 changes: 62 additions & 40 deletions Tests/SwiftParserTest/Declarations.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,27 +799,36 @@ final class DeclarationTests: XCTestCase {
)
}

func testDontRecoverFromDeclKeyword() {
AssertParse(
"func foo(first second #^MISSING_COLON^#third#^STMTS^# #^MISSING_RPAREN^#struct#^MISSING_IDENTIFIER^##^BRACES^#: Int) {}",
substructure: Syntax(FunctionParameterSyntax(
attributes: nil,
modifiers: nil,
firstName: .identifier("first"),
secondName: .identifier("second"),
colon: .colonToken(presence: .missing),
type: TypeSyntax(SimpleTypeIdentifierSyntax(name: .identifier("third"), genericArgumentClause: nil)),
ellipsis: nil,
defaultArgument: nil,
trailingComma: nil
)),
func testRecoverFromDeclKeyword() {
AssertParse(
"func foo(first second #^MISSING_COLON^#third #^MISSING_COMMA^#struct: Int) {}",
substructure: Syntax(FunctionParameterListSyntax([
FunctionParameterSyntax(
attributes: nil,
modifiers: nil,
firstName: .identifier("first"),
secondName: .identifier("second"),
colon: .colonToken(presence: .missing),
type: TypeSyntax(SimpleTypeIdentifierSyntax(name: .identifier("third"), genericArgumentClause: nil)),
ellipsis: nil,
defaultArgument: nil,
trailingComma: .commaToken(presence: .missing)
),
FunctionParameterSyntax(
attributes: nil,
modifiers: nil,
firstName: .structKeyword(),
secondName: nil,
colon: .colonToken(),
type: TypeSyntax(SimpleTypeIdentifierSyntax(name: .identifier("Int"), genericArgumentClause: nil)),
ellipsis: nil,
defaultArgument: nil,
trailingComma: nil
)
])),
diagnostics: [
DiagnosticSpec(locationMarker: "MISSING_COLON", message: "expected ':' in function parameter"),
DiagnosticSpec(locationMarker: "STMTS", message: "consecutive statements on a line must be separated by ';'"),
DiagnosticSpec(locationMarker: "MISSING_RPAREN", message: "expected ')' to end parameter clause"),
DiagnosticSpec(locationMarker: "MISSING_IDENTIFIER", message: "expected identifier in struct"),
DiagnosticSpec(locationMarker: "BRACES", message: "expected member block in struct"),
DiagnosticSpec(locationMarker: "BRACES", message: "extraneous ': Int) {}' at top level"),
DiagnosticSpec(locationMarker: "MISSING_COMMA", message: "expected ',' in function parameter"),
]
)
}
Expand Down Expand Up @@ -851,28 +860,41 @@ final class DeclarationTests: XCTestCase {
)
}

func testDontRecoverFromUnbalancedParens() {
func testRecoverFromUnbalancedParens() {
AssertParse(
"func foo(first second #^COLON^#[third #^END_ARRAY^#fourth: Int) {}",
substructure: Syntax(FunctionParameterSyntax(
attributes: nil,
modifiers: nil,
firstName: TokenSyntax.identifier("first"),
secondName: TokenSyntax.identifier("second"),
colon: TokenSyntax(.colon, presence: .missing),
type: TypeSyntax(ArrayTypeSyntax(
leftSquareBracket: TokenSyntax.leftSquareBracketToken(),
elementType: TypeSyntax(SimpleTypeIdentifierSyntax(name: TokenSyntax.identifier("third"), genericArgumentClause: nil)),
rightSquareBracket: TokenSyntax(.rightSquareBracket, presence: .missing)
)),
ellipsis: nil,
defaultArgument: nil,
trailingComma: nil
)),
substructure: Syntax(FunctionParameterListSyntax([
FunctionParameterSyntax(
attributes: nil,
modifiers: nil,
firstName: TokenSyntax.identifier("first"),
secondName: TokenSyntax.identifier("second"),
colon: TokenSyntax(.colon, presence: .missing),
type: TypeSyntax(ArrayTypeSyntax(
leftSquareBracket: TokenSyntax.leftSquareBracketToken(),
elementType: TypeSyntax(SimpleTypeIdentifierSyntax(name: TokenSyntax.identifier("third"), genericArgumentClause: nil)),
rightSquareBracket: TokenSyntax(.rightSquareBracket, presence: .missing)
)),
ellipsis: nil,
defaultArgument: nil,
trailingComma: .commaToken(presence: .missing)
),
FunctionParameterSyntax(
attributes: nil,
modifiers: nil,
firstName: TokenSyntax.identifier("fourth"),
secondName: nil,
colon: .colonToken(),
type: TypeSyntax(SimpleTypeIdentifierSyntax(name: .identifier("Int"), genericArgumentClause: nil)),
ellipsis: nil,
defaultArgument: nil,
trailingComma: nil
)
])),
diagnostics: [
DiagnosticSpec(locationMarker: "COLON", message: "expected ':' in function parameter"),
DiagnosticSpec(locationMarker: "END_ARRAY" , message: "expected ']' to end array type"),
DiagnosticSpec(locationMarker: "END_ARRAY", message: "unexpected text 'fourth: Int' in parameter clause")
DiagnosticSpec(locationMarker: "END_ARRAY", message: "expected ']' to end array type"),
DiagnosticSpec(locationMarker: "END_ARRAY", message: "expected ',' in function parameter")
]
)
}
Expand Down Expand Up @@ -1041,7 +1063,7 @@ final class DeclarationTests: XCTestCase {
""",
diagnostics: [
// TODO: Old parser expected error on line 1: 'isolated' may only be used on parameters
DiagnosticSpec(message: "unexpected text 'map: String' in parameter clause"),
DiagnosticSpec(message: "expected ',' in function parameter"),
]
)

Expand All @@ -1051,7 +1073,7 @@ final class DeclarationTests: XCTestCase {
""",
diagnostics: [
// TODO: Old parser expected error on line 1: 'isolated' may only be used on parameters
DiagnosticSpec(message: "unexpected text 'map: String' in parameter clause"),
DiagnosticSpec(message: "expected ',' in function parameter"),
]
)

Expand All @@ -1061,7 +1083,7 @@ final class DeclarationTests: XCTestCase {
""",
diagnostics: [
// TODO: Old parser expected error on line 1: 'isolated' may only be used on parameters
DiagnosticSpec(message: "unexpected text 'map: String' in parameter clause"),
DiagnosticSpec(message: "expected ',' in function parameter"),
]
)

Expand Down