Skip to content

Diagnose if a number is used in place of an identifier #941

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 13, 2022
Merged

Diagnose if a number is used in place of an identifier #941

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
6 changes: 2 additions & 4 deletions Sources/SwiftParser/Diagnostics/MissingNodesError.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,12 @@ fileprivate enum NodesDescriptionPart {
}
return "'\(tokenContents.trimmingWhitespace())'"
case .tokenWithoutDefaultText(let token):
if let parent = token.parent,
let childName = parent.childNameForDiagnostics(token.index) {
if let childName = token.childNameInParent {
return childName
}
return token.tokenKind.decomposeToRaw().rawKind.nameForDiagnostics
case .node(let node):
if let parent = node.parent,
let childName = parent.childNameForDiagnostics(node.index) {
if let childName = node.childNameInParent {
return childName
} else {
return node.nodeTypeNameForDiagnostics(allowBlockNames: true)
Expand Down
7 changes: 6 additions & 1 deletion Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,12 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
fixIts = []
}

addDiagnostic(invalidIdentifier, InvalidIdentifierError(invalidIdentifier: invalidIdentifier), fixIts: fixIts, handledNodes: [previousParent.id])
addDiagnostic(
invalidIdentifier,
InvalidIdentifierError(invalidIdentifier: invalidIdentifier, missingIdentifier: node),
fixIts: fixIts,
handledNodes: [previousParent.id]
)
} else {
return handleMissingSyntax(node)
}
Expand Down
6 changes: 5 additions & 1 deletion Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,15 @@ public struct IdentifierNotAllowedInOperatorName: ParserError {

public struct InvalidIdentifierError: ParserError {
public let invalidIdentifier: TokenSyntax
public let missingIdentifier: TokenSyntax

public var message: String {
switch invalidIdentifier.tokenKind {
case .floatingLiteral(let text), .integerLiteral(let text):
fallthrough
case .unknown(let text) where text.first?.isNumber == true:
return "identifier can only start with a letter or underscore, not a number"
let name = missingIdentifier.childNameInParent ?? "identifier"
return "\(name) can only start with a letter or underscore, not a number"
case .wildcardKeyword:
return "'\(invalidIdentifier.text)' cannot be used as an identifier here"
case let tokenKind where tokenKind.isKeyword:
Expand Down
7 changes: 6 additions & 1 deletion Sources/SwiftParser/Parser.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,12 @@ extension Parser {
self.missingToken(.identifier, text: nil)
)
}
if keywordRecovery, self.currentToken.tokenKind.isKeyword, !self.currentToken.isAtStartOfLine {
if let number = self.consume(ifAny: [.integerLiteral, .floatingLiteral]) {
return (
RawUnexpectedNodesSyntax(elements: [RawSyntax(number)], arena: self.arena),
self.missingToken(.identifier, text: nil)
)
} else if keywordRecovery, self.currentToken.tokenKind.isKeyword, !self.currentToken.isAtStartOfLine {
let keyword = self.consumeAnyToken()
return (
RawUnexpectedNodesSyntax(elements: [RawSyntax(keyword)], arena: self.arena),
Expand Down
14 changes: 14 additions & 0 deletions Sources/SwiftSyntax/Syntax.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,23 @@ public protocol SyntaxProtocol: CustomStringConvertible,

/// Return a name with which the child at the given `index` can be referred to
/// in diagnostics.
/// Typically, you want to use `childNameInParent` on the child instead of
/// calling this method on the parent.
func childNameForDiagnostics(_ index: SyntaxChildrenIndex) -> String?
}

public extension SyntaxProtocol {
/// If the parent has a dedicated "name for diagnostics" for this node, return it.
/// Otherwise, return `nil`.
var childNameInParent: String? {
if let parent = self.parent, let childName = parent.childNameForDiagnostics(self.index) {
return childName
} else {
return nil
}
}
}

extension SyntaxProtocol {
var data: SyntaxData {
return _syntaxNode.data
Expand Down
6 changes: 3 additions & 3 deletions Tests/SwiftParserTest/Declarations.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -690,8 +690,8 @@ final class DeclarationTests: XCTestCase {
}
""",
diagnostics: [
DiagnosticSpec(locationMarker: "1️⃣", message: "identifier can only start with a letter or underscore, not a number"),
DiagnosticSpec(locationMarker: "2️⃣", message: "identifier can only start with a letter or underscore, not a number"),
DiagnosticSpec(locationMarker: "1️⃣", message: "name can only start with a letter or underscore, not a number"),
Copy link
Contributor

Choose a reason for hiding this comment

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

As in the other PR, IMO identifier is more descriptive than name here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Once I’ve merged my PR backlog, I will set up a PR that checks what happens if we remove the name_for_diagnostics. It’s probably easiest to discuss all the implications this has on there.

DiagnosticSpec(locationMarker: "2️⃣", message: "name can only start with a letter or underscore, not a number"),
]
)
}
Expand Down Expand Up @@ -1023,7 +1023,7 @@ final class DeclarationTests: XCTestCase {
AssertParse(
"associatedtype 1️⃣5s",
diagnostics: [
DiagnosticSpec(message: "identifier can only start with a letter or underscore, not a number"),
DiagnosticSpec(message: "name can only start with a letter or underscore, not a number"),
]
)
}
Expand Down
6 changes: 3 additions & 3 deletions Tests/SwiftParserTest/translated/EnumTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,13 +259,13 @@ final class EnumTests: XCTestCase {
AssertParse(
"""
enum SwitchEnvy {
case 1️⃣0:
case 1️⃣02️⃣:
}
""",
diagnostics: [
// TODO: Old parser expected error on line 2: 'case' label can only appear inside a 'switch' statement
DiagnosticSpec(message: "expected name in enum case"),
DiagnosticSpec(message: "unexpected code '0:' in enum"),
DiagnosticSpec(locationMarker: "1️⃣", message: "name can only start with a letter or underscore, not a number"),
DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code ':' in enum"),
]
)
}
Expand Down
135 changes: 67 additions & 68 deletions Tests/SwiftParserTest/translated/NumberIdentifierErrorsTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,40 @@ final class NumberIdentifierErrorsTests: XCTestCase {
)
}

func testNumberIdentifierErrors2() {
func testNumberIdentifierErrors2a() {
AssertParse(
"""
func 1️⃣1() {}
func 2️⃣2.0() {}
func 3️⃣3func() {}
""",
diagnostics: [
// TODO: Old parser expected error on line 1: function name can only start with a letter or underscore, not a number
DiagnosticSpec(locationMarker: "1️⃣", message: "expected name in function"),
DiagnosticSpec(locationMarker: "1️⃣", message: "unexpected code '1' before parameter clause"),
// TODO: Old parser expected error on line 2: function name can only start with a letter or underscore, not a number
DiagnosticSpec(locationMarker: "2️⃣", message: "expected name in function"),
DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code '2.0' before parameter clause"),
// TODO: Old parser expected error on line 3: function name can only start with a letter or underscore, not a number
// TODO: Old parser expected error on line 3: 'f' is not a valid digit in integer literal
DiagnosticSpec(locationMarker: "3️⃣", message: "identifier can only start with a letter or underscore, not a number"),
DiagnosticSpec(message: "name can only start with a letter or underscore, not a number"),
]
)
}

func testNumberIdentifierErrors2b() {
AssertParse(
"""
func 1️⃣2.0() {}
""",
diagnostics: [
DiagnosticSpec(message: "name can only start with a letter or underscore, not a number"),
]
)
}

func testNumberIdentifierErrors2c() {
AssertParse(
"""
func 1️⃣3func() {}
""",
diagnostics: [
DiagnosticSpec(message: "name can only start with a letter or underscore, not a number"),
]
)
}


func testNumberIdentifierErrors3a() {
AssertParse(
"""
Expand All @@ -42,10 +55,8 @@ final class NumberIdentifierErrorsTests: XCTestCase {
}
""",
diagnostics: [
// TODO: Old parser expected error on line 1: protocol name can only start with a letter or underscore, not a number
DiagnosticSpec(locationMarker: "1️⃣", message: "expected name and member block in protocol"),
// TODO: Old parser expected error on line 2: associatedtype name can only start with a letter or underscore, not a number
DiagnosticSpec(locationMarker: "2️⃣", message: "expected name in associatedtype declaration"),
DiagnosticSpec(locationMarker: "1️⃣", message: "name can only start with a letter or underscore, not a number"),
DiagnosticSpec(locationMarker: "2️⃣", message: "name can only start with a letter or underscore, not a number"),
]
)
}
Expand All @@ -58,10 +69,8 @@ final class NumberIdentifierErrorsTests: XCTestCase {
}
""",
diagnostics: [
// TODO: Old parser expected error on line 1: protocol name can only start with a letter or underscore, not a number
DiagnosticSpec(locationMarker: "1️⃣", message: "expected name and member block in protocol"),
// TODO: Old parser expected error on line 2: associatedtype name can only start with a letter or underscore, not a number
DiagnosticSpec(locationMarker: "2️⃣", message: "expected name in associatedtype declaration"),
DiagnosticSpec(locationMarker: "1️⃣", message: "name can only start with a letter or underscore, not a number"),
DiagnosticSpec(locationMarker: "2️⃣", message: "name can only start with a letter or underscore, not a number"),
]
)
}
Expand All @@ -74,34 +83,42 @@ final class NumberIdentifierErrorsTests: XCTestCase {
}
""",
diagnostics: [
// TODO: Old parser expected error on line 1: protocol name can only start with a letter or underscore, not a number
// TODO: Old parser expected error on line 1: 'p' is not a valid digit in integer literal
DiagnosticSpec(locationMarker: "1️⃣", message: "identifier can only start with a letter or underscore, not a number"),
// TODO: Old parser expected error on line 2: associatedtype name can only start with a letter or underscore, not a number
// TODO: Old parser expected error on line 2: 'a' is not a valid digit in integer literal
DiagnosticSpec(locationMarker: "2️⃣", message: "identifier can only start with a letter or underscore, not a number"),
DiagnosticSpec(locationMarker: "1️⃣", message: "name can only start with a letter or underscore, not a number"),
DiagnosticSpec(locationMarker: "2️⃣", message: "name can only start with a letter or underscore, not a number"),
]
)
}


func testNumberIdentifierErrors4() {
func testNumberIdentifierErrors4a() {
AssertParse(
"""
typealias 1️⃣10 = Int
typealias 2️⃣11.0 = Int
typealias 3️⃣12typealias = Int
""",
diagnostics: [
// TODO: Old parser expected error on line 1: typealias name can only start with a letter or underscore, not a number
DiagnosticSpec(locationMarker: "1️⃣", message: "expected name in typealias declaration"),
DiagnosticSpec(locationMarker: "1️⃣", message: "unexpected code '10' in typealias declaration"),
// TODO: Old parser expected error on line 2: typealias name can only start with a letter or underscore, not a number
DiagnosticSpec(locationMarker: "2️⃣", message: "expected name in typealias declaration"),
DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code '11.0' in typealias declaration"),
// TODO: Old parser expected error on line 3: typealias name can only start with a letter or underscore, not a number
// TODO: Old parser expected error on line 3: 't' is not a valid digit in integer literal
DiagnosticSpec(locationMarker: "3️⃣", message: "identifier can only start with a letter or underscore, not a number"),
DiagnosticSpec(message: "name can only start with a letter or underscore, not a number"),
]
)
}

func testNumberIdentifierErrors4b() {
AssertParse(
"""
typealias 1️⃣11.0 = Int
""",
diagnostics: [
DiagnosticSpec(message: "name can only start with a letter or underscore, not a number"),
]
)
}

func testNumberIdentifierErrors4c() {
AssertParse(
"""
typealias 1️⃣12typealias = Int
""",
diagnostics: [
DiagnosticSpec(message: "name can only start with a letter or underscore, not a number"),
]
)
}
Expand All @@ -112,8 +129,7 @@ final class NumberIdentifierErrorsTests: XCTestCase {
struct 1️⃣13 {}
""",
diagnostics: [
// TODO: Old parser expected error on line 1: struct name can only start with a letter or underscore, not a number
DiagnosticSpec(message: "expected name and member block in struct"),
DiagnosticSpec(message: "name can only start with a letter or underscore, not a number"),
]
)
}
Expand All @@ -124,8 +140,7 @@ final class NumberIdentifierErrorsTests: XCTestCase {
struct 1️⃣14.0 {}
""",
diagnostics: [
// TODO: Old parser expected error on line 1: struct name can only start with a letter or underscore, not a number
DiagnosticSpec(message: "expected name and member block in struct"),
DiagnosticSpec(message: "name can only start with a letter or underscore, not a number"),
]
)
}
Expand All @@ -136,9 +151,7 @@ final class NumberIdentifierErrorsTests: XCTestCase {
struct 1️⃣15struct {}
""",
diagnostics: [
// TODO: Old parser expected error on line 1: struct name can only start with a letter or underscore, not a number
// TODO: Old parser expected error on line 1: 's' is not a valid digit in integer literal
DiagnosticSpec(message: "identifier can only start with a letter or underscore, not a number"),
DiagnosticSpec(message: "name can only start with a letter or underscore, not a number"),
]
)
}
Expand All @@ -149,8 +162,7 @@ final class NumberIdentifierErrorsTests: XCTestCase {
enum 1️⃣16 {}
""",
diagnostics: [
// TODO: Old parser expected error on line 1: enum name can only start with a letter or underscore, not a number
DiagnosticSpec(message: "expected name and member block in enum"),
DiagnosticSpec(message: "name can only start with a letter or underscore, not a number"),
]
)
}
Expand All @@ -161,8 +173,7 @@ final class NumberIdentifierErrorsTests: XCTestCase {
enum 1️⃣17.0 {}
""",
diagnostics: [
// TODO: Old parser expected error on line 1: enum name can only start with a letter or underscore, not a number
DiagnosticSpec(message: "expected name and member block in enum"),
DiagnosticSpec(message: "name can only start with a letter or underscore, not a number"),
]
)
}
Expand All @@ -173,9 +184,7 @@ final class NumberIdentifierErrorsTests: XCTestCase {
enum 1️⃣18enum {}
""",
diagnostics: [
// TODO: Old parser expected error on line 1: enum name can only start with a letter or underscore, not a number
// TODO: Old parser expected error on line 1: 'n' is not a valid digit in floating point exponent
DiagnosticSpec(message: "identifier can only start with a letter or underscore, not a number"),
DiagnosticSpec(message: "name can only start with a letter or underscore, not a number"),
]
)
}
Expand All @@ -188,11 +197,8 @@ final class NumberIdentifierErrorsTests: XCTestCase {
}
""",
diagnostics: [
// TODO: Old parser expected error on line 1: class name can only start with a letter or underscore, not a number
DiagnosticSpec(locationMarker: "1️⃣", message: "expected name and member block in class"),
// TODO: Old parser expected error on line 2: function name can only start with a letter or underscore, not a number
DiagnosticSpec(locationMarker: "2️⃣", message: "expected name in function"),
DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code '20' before parameter clause"),
DiagnosticSpec(locationMarker: "1️⃣", message: "name can only start with a letter or underscore, not a number"),
DiagnosticSpec(locationMarker: "2️⃣", message: "name can only start with a letter or underscore, not a number"),
]
)
}
Expand All @@ -205,11 +211,8 @@ final class NumberIdentifierErrorsTests: XCTestCase {
}
""",
diagnostics: [
// TODO: Old parser expected error on line 1: class name can only start with a letter or underscore, not a number
DiagnosticSpec(locationMarker: "1️⃣", message: "expected name and member block in class"),
// TODO: Old parser expected error on line 2: function name can only start with a letter or underscore, not a number
DiagnosticSpec(locationMarker: "2️⃣", message: "expected name in function"),
DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code '22.0' before parameter clause"),
DiagnosticSpec(locationMarker: "1️⃣", message: "name can only start with a letter or underscore, not a number"),
DiagnosticSpec(locationMarker: "2️⃣", message: "name can only start with a letter or underscore, not a number"),
]
)
}
Expand All @@ -223,12 +226,8 @@ final class NumberIdentifierErrorsTests: XCTestCase {
}
""",
diagnostics: [
// TODO: Old parser expected error on line 1: class name can only start with a letter or underscore, not a number
// TODO: Old parser expected error on line 1: 'c' is not a valid digit in integer literal
DiagnosticSpec(locationMarker: "1️⃣", message: "identifier can only start with a letter or underscore, not a number"),
// TODO: Old parser expected error on line 2: function name can only start with a letter or underscore, not a number
// TODO: Old parser expected error on line 2: 'm' is not a valid digit in integer literal
DiagnosticSpec(locationMarker: "2️⃣", message: "identifier can only start with a letter or underscore, not a number"),
DiagnosticSpec(locationMarker: "1️⃣", message: "name can only start with a letter or underscore, not a number"),
DiagnosticSpec(locationMarker: "2️⃣", message: "name can only start with a letter or underscore, not a number"),
]
)
}
Expand Down