Add a function to check if a name can be used as an identifier in a given context #2434
Conversation
|
@swift-ci Please test |
f24b22a to
b81c8bc
Compare
|
@swift-ci Please test |
| /// - `test` is a valid identifier for member access because `myStruct.test` is valid | ||
| /// - `class` is a valid identifier for member access becuase `myStruct.class` is valid, even though `class` | ||
| /// needs to be wrapped in backticks when used to declare a variable. | ||
| /// - `self` is not a valid identifiers for member access because `myStruct.self` does not access a member named |
There was a problem hiding this comment.
| /// - `self` is not a valid identifiers for member access because `myStruct.self` does not access a member named | |
| /// - `self` is not a valid identifier for member access because `myStruct.self` does not access a member named |
| XCTAssertTrue(isValidIdentifier("`class`", for: .memberAccess)) | ||
| XCTAssertFalse(isValidIdentifier("self", for: .memberAccess)) | ||
| XCTAssertTrue(isValidIdentifier("`self`", for: .memberAccess)) | ||
| XCTAssertTrue(isValidIdentifier("`let`", for: .memberAccess)) |
There was a problem hiding this comment.
Changed the tests to add an assertValidIdentifier that makes it easier to check if a name is a valid identifier in the different contexts.
There was a problem hiding this comment.
Ah, nice. If you're going that far you could have the init just take a map rather than each case, but it's just a test so I what you have is also fine 👍
b81c8bc to
23637ba
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
| private func isValidVariableName(_ name: String) -> Bool { | ||
| var parser = Parser("var \(name)") | ||
| let decl = DeclSyntax.parse(from: &parser) | ||
| guard parser.at(.endOfFile) else { |
There was a problem hiding this comment.
This should also check the current lexeme's leadingTriviaByteLength == 0.
| // We parsed the name as a keyword, eg. `self`, so not a valid identifier. | ||
| return false | ||
| } | ||
| guard identifier.leadingTrivia.isEmpty && identifier.trailingTrivia.isEmpty else { |
There was a problem hiding this comment.
variable.bindingSpecifier.trailingTriviaLength == 1 too. Consider name = " foo" case.
There was a problem hiding this comment.
Or maybe identifier.rawText.count == name.utf8.count so we can remove all the trivia check including .endOfFile check above.
edit: wait, we're already checking identifier.tokenKind == .identifier(name). Is this trivia check actually needed?
There was a problem hiding this comment.
Yes, you’re right. The trivia checks aren’t actually needed. Nice 👍🏽
Which also means that we don’t need the check that there are only 2/3 tokens in the parsed tree.
| guard let identifier = variable.bindings.first?.pattern.as(IdentifierPatternSyntax.self)?.identifier else { | ||
| return false | ||
| } | ||
| guard identifier.tokenKind == .identifier(name) else { |
There was a problem hiding this comment.
| guard identifier.tokenKind == .identifier(name) else { | |
| guard identifier.rawTokenKind == .identfiier && identifier.rawTokenText.count == name.utf8.count else { |
should give us a tiny performance increase.
23637ba to
8e22883
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
8e22883 to
8df8315
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
fd86785 to
8df8315
Compare
…iven context rdar://120721971
8df8315 to
38b7b38
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
rdar://120721971