Skip to content

Diagnose ';' statements #910

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

Merged
merged 1 commit into from
Oct 10, 2022
Merged

Diagnose ';' statements #910

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
2 changes: 1 addition & 1 deletion Sources/SwiftParser/Diagnostics/DiagnosticExtensions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ extension FixIt.Changes {
newNode: Syntax(TokenSyntax(node.tokenKind, leadingTrivia: [], trailingTrivia: [], presence: .missing))
)
]
if !node.leadingTrivia.isEmpty, let nextToken = node.nextToken(viewMode: .sourceAccurate) {
if !node.leadingTrivia.isEmpty, let nextToken = node.nextToken(viewMode: .sourceAccurate), !nextToken.leadingTrivia.contains(where: { $0.isNewline }) {
changes.append(.replaceLeadingTrivia(token: nextToken, newTrivia: node.leadingTrivia))
}
return FixIt.Changes(changes: changes)
Expand Down
7 changes: 7 additions & 0 deletions Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,13 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
handledNodes.append(semicolon.id)
}
}
if let semicolon = node.semicolon, semicolon.presence == .present, node.item.isMissingAllTokens {
addDiagnostic(node, .stanaloneSemicolonStatement, fixIts: [
FixIt(message: RemoveTokensFixIt(tokensToRemove: [semicolon]), changes: [
.makeMissing(node: semicolon)
])
], handledNodes: [node.item.id])
}
return .visitChildren
}

Expand Down
1 change: 1 addition & 0 deletions Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public enum StaticParserError: String, DiagnosticMessage {
case editorPlaceholderInSourceFile = "editor placeholder in source file"
case missingColonInTernaryExprDiagnostic = "expected ':' after '? ...' in ternary expression"
case missingFunctionParameterClause = "expected argument list in function declaration"
case stanaloneSemicolonStatement = "';' statements are not allowed"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
case stanaloneSemicolonStatement = "';' statements are not allowed"
case standaloneSemicolonStatement = "';' statements are not allowed"

Also I know this is what the old diagnostic was, but could we add standalone to the start? We could also make some better suggestions depending on the context, ie. in a switch suggest break. Possibly fallthrough as well, though that's semantically different.

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 typo, also adding “standalone” to the error message makes sense to me. The Fix-It to replace it with break doesn’t really seem worth it. I don’t think there are very many C developers switching to Swift anymore that are expecting ';' statements to work around the requirement that switch cases must have an executable statement.

case throwsInReturnPosition = "'throws' may only occur before '->'"
case tryMustBePlacedOnReturnedExpr = "'try' must be placed on the returned expression"
case tryMustBePlacedOnThrownExpr = "'try' must be placed on the thrown expression"
Expand Down
17 changes: 11 additions & 6 deletions Tests/SwiftParserTest/translated/SwitchTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,17 +228,22 @@ final class SwitchTests: XCTestCase {
func testSwitch15() {
AssertParse(
"""
switch x {
case 0:1️⃣
;
switch x {
case 0:
1️⃣;
case 1:
x = 0
}
""",
diagnostics: [
DiagnosticSpec(message: "expected expression in switch case"),
// TODO: Old parser expected error on line 3: ';' statements are not allowed, Fix-It replacements: 3 - 5 = ''
]
DiagnosticSpec(message: "';' statements are not allowed", fixIts: ["remove ';'"]),
], fixedSource: """
switch x {
case 0:
case 1:
x = 0
}
"""
)
}

Expand Down