-
Notifications
You must be signed in to change notification settings - Fork 441
Implement a few more diagnostics in SwiftParser #1124
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,21 +12,47 @@ | |
|
||
// This test file has been translated from swift/test/Parse/hashbang_library.swift | ||
|
||
import SwiftSyntax | ||
|
||
import XCTest | ||
|
||
final class HashbangLibraryTests: XCTestCase { | ||
func testHashbangLibrary1() { | ||
// Check that we diagnose and skip the hashbang at the beginning of the file | ||
// when compiling in library mode. | ||
AssertParse( | ||
""" | ||
#!/usr/bin/swift | ||
#!/usr/bin/swift | ||
class Foo {} | ||
// Check that we diagnose and skip the hashbang at the beginning of the file | ||
// when compiling in library mode. | ||
""", | ||
diagnostics: [ | ||
// TODO: Old parser expected error on line 1: hashbang line is allowed only in the main file | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No diagnostic... what tree do we produce here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We parse the shebang as trivia. I would like to parse it as part of the source file at some point (rdar://98309158) but the current implementation isn’t too bad. Added a substructure test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW you don't need all those There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, the PR is so old it still required the |
||
] | ||
substructure: Syntax( | ||
SourceFileSyntax( | ||
statements: CodeBlockItemListSyntax([ | ||
CodeBlockItemSyntax( | ||
item: .decl( | ||
DeclSyntax( | ||
ClassDeclSyntax( | ||
classKeyword: .keyword( | ||
.class, | ||
leadingTrivia: [ | ||
.shebang("#!/usr/bin/swift"), | ||
.newlines(1), | ||
], | ||
trailingTrivia: .space | ||
), | ||
identifier: .identifier("Foo", trailingTrivia: .space), | ||
members: MemberDeclBlockSyntax( | ||
members: MemberDeclListSyntax([]) | ||
) | ||
) | ||
) | ||
) | ||
) | ||
]), | ||
eofToken: .eof() | ||
) | ||
), | ||
substructureCheckTrivia: true | ||
) | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -110,11 +110,29 @@ final class IdentifiersTests: XCTestCase { | |
func testIdentifiers8b() { | ||
AssertParse( | ||
""" | ||
struct Self {} | ||
struct 1️⃣Self {} | ||
""", | ||
diagnostics: [ | ||
// TODO: Old parser expected error on line 3: keyword 'Self' cannot be used as an identifier here | ||
// TODO: Old parser expected note on line 3: if this name is unavoidable, use backticks to escape it, Fix-It replacements: 8 - 12 = '`Self`' | ||
DiagnosticSpec(message: "keyword 'Self' cannot be used as an identifier here") | ||
] | ||
) | ||
} | ||
|
||
func testStructNamedLowercaseAny() { | ||
AssertParse( | ||
""" | ||
struct any {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was the point of this test to check a keyword or to check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Damn, I hate that we have capital any and lowercase any. I also added a test case for capital There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Heh :) All the more reason to merge keywords! |
||
""" | ||
) | ||
} | ||
|
||
func testStructNamedCapitalAny() { | ||
AssertParse( | ||
""" | ||
struct 1️⃣Any {} | ||
""", | ||
diagnostics: [ | ||
DiagnosticSpec(message: "keyword 'Any' cannot be used as an identifier here") | ||
] | ||
) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we do the keyword merging it would be nice to have sets for all the different "keyword" tokens we can have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey we can totally do this now :D. But fine not to for this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe in a follow-up PR.