Skip to content

Commit 1d36fc2

Browse files
authored
Merge pull request #1857 from ahoppen/ahoppen/naming-inconsistencies
Fix child name validation failures found by `testMultipleKeywordChoicesNaming`
2 parents 69c0be3 + c88c3a5 commit 1d36fc2

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+1588
-712
lines changed

CodeGeneration/Sources/SyntaxSupport/AttributeNodes.swift

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,8 @@ public let ATTRIBUTE_NODES: [Node] = [
358358
isOptional: true
359359
),
360360
Child(
361-
name: "AccessorKind",
361+
name: "AccessorSpecifier",
362+
deprecatedName: "AccessorKind",
362363
kind: .token(choices: [.keyword(text: "get"), .keyword(text: "set")]),
363364
documentation: "The accessor name.",
364365
isOptional: true
@@ -369,7 +370,8 @@ public let ATTRIBUTE_NODES: [Node] = [
369370
isOptional: true
370371
),
371372
Child(
372-
name: "DiffParams",
373+
name: "Parameters",
374+
deprecatedName: "DiffParams",
373375
kind: .node(kind: .differentiabilityParamsClause),
374376
isOptional: true
375377
),
@@ -477,23 +479,27 @@ public let ATTRIBUTE_NODES: [Node] = [
477479
"The arguments for the `@differentiable` attribute: an optional differentiability kind, an optional differentiability parameter clause, and an optional 'where' clause.",
478480
children: [
479481
Child(
480-
name: "DiffKind",
482+
name: "KindSpecifier",
483+
deprecatedName: "DiffKind",
481484
kind: .token(choices: [.keyword(text: "_forward"), .keyword(text: "reverse"), .keyword(text: "_linear")]),
482485
isOptional: true
483486
),
484487
Child(
485-
name: "DiffKindComma",
488+
name: "KindSpecifierComma",
489+
deprecatedName: "DiffKindComma",
486490
kind: .token(choices: [.token(tokenKind: "CommaToken")]),
487491
documentation: "The comma following the differentiability kind, if it exists.",
488492
isOptional: true
489493
),
490494
Child(
491-
name: "DiffParams",
495+
name: "Parameters",
496+
deprecatedName: "DiffParams",
492497
kind: .node(kind: .differentiabilityParamsClause),
493498
isOptional: true
494499
),
495500
Child(
496-
name: "DiffParamsComma",
501+
name: "ParametersComma",
502+
deprecatedName: "DiffParamsComma",
497503
kind: .token(choices: [.token(tokenKind: "CommaToken")]),
498504
documentation: "The comma following the differentiability parameters clause, if it exists.",
499505
isOptional: true

CodeGeneration/Sources/SyntaxSupport/DeclNodes.swift

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ public let DECL_NODES: [Node] = [
154154
isOptional: true
155155
),
156156
Child(
157-
name: "AccessorKind",
157+
name: "AccessorSpecifier",
158+
deprecatedName: "AccessorKind",
158159
kind: .token(choices: [
159160
.keyword(text: "get"),
160161
.keyword(text: "set"),
@@ -1213,7 +1214,8 @@ public let DECL_NODES: [Node] = [
12131214
documentation: "The `import` keyword for this declaration."
12141215
),
12151216
Child(
1216-
name: "ImportKind",
1217+
name: "ImportKindSpecifier",
1218+
deprecatedName: "ImportKind",
12171219
kind: .token(choices: [
12181220
.keyword(text: "typealias"),
12191221
.keyword(text: "struct"),
@@ -1570,7 +1572,8 @@ public let DECL_NODES: [Node] = [
15701572
],
15711573
children: [
15721574
Child(
1573-
name: "Fixity",
1575+
name: "FixitySpecifier",
1576+
deprecatedName: "Fixity",
15741577
kind: .token(choices: [.keyword(text: "prefix"), .keyword(text: "postfix"), .keyword(text: "infix")]),
15751578
nameForDiagnostics: "fixity",
15761579
documentation: "The fixity applied to the 'operator' declaration."
@@ -1785,7 +1788,8 @@ public let DECL_NODES: [Node] = [
17851788
kind: .token(choices: [.token(tokenKind: "ColonToken")])
17861789
),
17871790
Child(
1788-
name: "Flag",
1791+
name: "Value",
1792+
deprecatedName: "Flag",
17891793
kind: .token(choices: [.keyword(text: "true"), .keyword(text: "false")]),
17901794
documentation:
17911795
"When true, an operator in the corresponding precedence group uses the same grouping rules during optional chaining as the assignment operators from the standard library. Otherwise, operators in the precedence group follows the same optional chaining rules as operators that don't perform assignment."
@@ -1919,7 +1923,7 @@ public let DECL_NODES: [Node] = [
19191923
documentation: "Specify the new precedence group's relation to existing precedence groups.",
19201924
children: [
19211925
Child(
1922-
name: "HigherThanOrLowerThanKeyword",
1926+
name: "HigherThanOrLowerThanLabel",
19231927
deprecatedName: "HigherThanOrLowerThan",
19241928
kind: .token(choices: [.keyword(text: "higherThan"), .keyword(text: "lowerThan")]),
19251929
documentation: "The relation to specified other precedence groups.",
@@ -2350,7 +2354,8 @@ public let DECL_NODES: [Node] = [
23502354
isOptional: true
23512355
),
23522356
Child(
2353-
name: "BindingKeyword",
2357+
name: "BindingSpecifier",
2358+
deprecatedName: "BindingKeyword",
23542359
kind: .token(choices: [.keyword(text: "let"), .keyword(text: "var"), .keyword(text: "inout")])
23552360
),
23562361
Child(

CodeGeneration/Sources/SyntaxSupport/ExprNodes.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,8 @@ public let EXPR_NODES: [Node] = [
168168
nameForDiagnostics: "bool literal",
169169
children: [
170170
Child(
171-
name: "BooleanLiteral",
171+
name: "Literal",
172+
deprecatedName: "BooleanLiteral",
172173
kind: .token(choices: [.keyword(text: "true"), .keyword(text: "false")])
173174
)
174175
]
@@ -1221,7 +1222,8 @@ public let EXPR_NODES: [Node] = [
12211222
nameForDiagnostics: "'consume' expression",
12221223
children: [
12231224
Child(
1224-
name: "MoveKeyword",
1225+
name: "ConsumeKeyword",
1226+
deprecatedName: "MoveKeyword",
12251227
kind: .token(choices: [.keyword(text: "_move"), .keyword(text: "consume")])
12261228
),
12271229
Child(

CodeGeneration/Sources/SyntaxSupport/PatternNodes.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,8 @@ public let PATTERN_NODES: [Node] = [
146146
nameForDiagnostics: "value binding pattern",
147147
children: [
148148
Child(
149-
name: "BindingKeyword",
149+
name: "BindingSpecifier",
150+
deprecatedName: "BindingKeyword",
150151
kind: .token(choices: [.keyword(text: "let"), .keyword(text: "var"), .keyword(text: "inout")])
151152
),
152153
Child(

CodeGeneration/Sources/SyntaxSupport/StmtNodes.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,8 @@ public let STMT_NODES: [Node] = [
437437
nameForDiagnostics: "optional binding",
438438
children: [
439439
Child(
440-
name: "BindingKeyword",
440+
name: "BindingSpecifier",
441+
deprecatedName: "BindingKeyword",
441442
kind: .token(choices: [.keyword(text: "let"), .keyword(text: "var"), .keyword(text: "inout")])
442443
),
443444
Child(

CodeGeneration/Sources/SyntaxSupport/TypeNodes.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,8 @@ public let TYPE_NODES: [Node] = [
324324
kind: .token(choices: [.token(tokenKind: "PeriodToken")])
325325
),
326326
Child(
327-
name: "TypeOrProtocol",
327+
name: "MetatypeSpecifier",
328+
deprecatedName: "TypeOrProtocol",
328329
kind: .token(choices: [.keyword(text: "Type"), .keyword(text: "Protocol")])
329330
),
330331
]

CodeGeneration/Tests/ValidateSyntaxNodes/ValidateSyntaxNodes.swift

Lines changed: 66 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,20 @@ fileprivate extension Child {
7474
func hasSameType(as other: Child) -> Bool {
7575
return name == other.name && kind.hasSameType(as: other.kind) && isOptional == other.isOptional
7676
}
77+
78+
func isFollowedByColonToken(in node: LayoutNode) -> Bool {
79+
guard let childIndex = node.children.firstIndex(where: { $0.name == self.name }) else {
80+
preconditionFailure("\(self.name) is not a child of \(node.kind.syntaxType)")
81+
}
82+
guard childIndex + 2 < node.children.count else {
83+
return false
84+
}
85+
if case .token(choices: [.token(tokenKind: "ColonToken")], _, _) = node.children[childIndex + 2].kind {
86+
return true
87+
} else {
88+
return false
89+
}
90+
}
7791
}
7892

7993
class ValidateSyntaxNodes: XCTestCase {
@@ -102,13 +116,13 @@ class ValidateSyntaxNodes: XCTestCase {
102116
/// Implementation detail of `testSingleTokenChoiceChildNaming`, validating a single child.
103117
///
104118
/// - Returns: A failure message if validation failed, otherwise `nil`
105-
private func validateSingleTokenChoiceChild(child: Child, childIndex: Int, in node: LayoutNode) -> String? {
119+
private func validateSingleTokenChoiceChild(child: Child, in node: LayoutNode) -> String? {
106120
guard case .token(choices: let tokenChoices, _, _) = child.kind, let choice = tokenChoices.only else {
107121
return nil
108122
}
109123
switch choice {
110124
case .keyword(text: let keyword):
111-
if childIndex + 2 < node.children.count, case .token(choices: [.token(tokenKind: "ColonToken")], _, _) = node.children[childIndex + 2].kind {
125+
if child.isFollowedByColonToken(in: node) {
112126
if child.name != "\(keyword.withFirstCharacterUppercased)Label" {
113127
return
114128
"child '\(child.name)' has a single keyword as its only token choice and is followed by a colon. It should thus be named '\(keyword.withFirstCharacterUppercased)Label'"
@@ -148,8 +162,8 @@ class ValidateSyntaxNodes: XCTestCase {
148162
func testSingleTokenChoiceChildNaming() {
149163
var failures: [ValidationFailure] = []
150164
for node in SYNTAX_NODES.compactMap(\.layoutNode) {
151-
for (childIndex, child) in node.children.enumerated() {
152-
if let failureMessage = validateSingleTokenChoiceChild(child: child, childIndex: childIndex, in: node) {
165+
for child in node.children {
166+
if let failureMessage = validateSingleTokenChoiceChild(child: child, in: node) {
153167
failures.append(ValidationFailure(node: node.kind, message: failureMessage))
154168
}
155169
}
@@ -189,11 +203,11 @@ class ValidateSyntaxNodes: XCTestCase {
189203
// If there are two tokens of the same kind in a node, we can't follow the naming rule without conflict
190204
ValidationFailure(
191205
node: .differentiableAttributeArguments,
192-
message: "child 'DiffKindComma' has a comma keyword as its only token choice and should thus be named 'Comma' or 'TrailingComma'"
206+
message: "child 'KindSpecifierComma' has a comma keyword as its only token choice and should thus be named 'Comma' or 'TrailingComma'"
193207
),
194208
ValidationFailure(
195209
node: .differentiableAttributeArguments,
196-
message: "child 'DiffParamsComma' has a comma keyword as its only token choice and should thus be named 'Comma' or 'TrailingComma'"
210+
message: "child 'ParametersComma' has a comma keyword as its only token choice and should thus be named 'Comma' or 'TrailingComma'"
197211
),
198212
ValidationFailure(
199213
node: .poundSourceLocationArgs,
@@ -292,79 +306,75 @@ class ValidateSyntaxNodes: XCTestCase {
292306
var failures: [ValidationFailure] = []
293307
for node in SYNTAX_NODES.compactMap(\.layoutNode) {
294308
for child in node.children {
295-
if case .token(choices: let tokenChoices, _, _) = child.kind,
296-
tokenChoices.count > 1, // single token choices are handled by `validateSingleTokenChoiceChildNaming`
297-
tokenChoices.allSatisfy({ $0.isKeyword }),
298-
!child.name.hasSuffix("Keyword")
299-
{
300-
failures.append(
301-
ValidationFailure(
302-
node: node.kind,
303-
message: "child '\(child.name)' only has keywords as its token choices and should thus and with 'Keyword'"
304-
)
305-
)
309+
guard case .token(choices: let tokenChoices, _, _) = child.kind,
310+
tokenChoices.count > 1,
311+
tokenChoices.allSatisfy({ $0.isKeyword })
312+
else {
313+
// Not a child with only keyword choices
314+
// Single token choices are handled by `validateSingleTokenChoiceChildNaming`
315+
continue
316+
}
317+
var failureMessage: String?
318+
if child.isFollowedByColonToken(in: node) {
319+
if !child.name.hasSuffix("Label") {
320+
failureMessage = "child '\(child.name)' only has keywords as its token choices, is followed by a colon and should thus end with 'Label'"
321+
}
322+
} else {
323+
if !child.name.hasSuffix("Specifier") {
324+
failureMessage = "child '\(child.name)' only has keywords as its token choices and should thus end with 'Specifier'"
325+
}
326+
}
327+
if let failureMessage {
328+
failures.append(ValidationFailure(node: node.kind, message: failureMessage))
306329
}
307330
}
308331
}
309332

310333
assertFailuresMatchXFails(
311334
failures,
312335
expectedFailures: [
313-
ValidationFailure(node: .accessorDecl, message: "child 'AccessorKind' only has keywords as its token choices and should thus and with 'Keyword'"),
314-
ValidationFailure(node: .attributedType, message: "child 'Specifier' only has keywords as its token choices and should thus and with 'Keyword'"),
336+
// MARK: Only one non-deprecated keyword
315337
ValidationFailure(
316-
node: .availabilityLabeledArgument,
317-
message: "child 'Label' only has keywords as its token choices and should thus and with 'Keyword'"
338+
node: .discardStmt,
339+
message: "child 'DiscardKeyword' only has keywords as its token choices and should thus end with 'Specifier'"
340+
// DiscardKeyword can be 'discard' or '_forget' and '_forget' is deprecated
318341
),
319342
ValidationFailure(
320-
node: .booleanLiteralExpr,
321-
message: "child 'BooleanLiteral' only has keywords as its token choices and should thus and with 'Keyword'"
322-
),
323-
ValidationFailure(node: .canImportVersionInfo, message: "child 'Label' only has keywords as its token choices and should thus and with 'Keyword'"),
324-
ValidationFailure(
325-
node: .closureCaptureItemSpecifier,
326-
message: "child 'Specifier' only has keywords as its token choices and should thus and with 'Keyword'"
327-
),
328-
ValidationFailure(
329-
node: .closureCaptureItemSpecifier,
330-
message: "child 'Detail' only has keywords as its token choices and should thus and with 'Keyword'"
343+
node: .moveExpr,
344+
message: "child 'ConsumeKeyword' only has keywords as its token choices and should thus end with 'Specifier'"
345+
// ConsumeKeyword can be 'consume' or '_move' and '_move' is deprecated
331346
),
347+
348+
// MARK: Conceptually a value, not a specifier
332349
ValidationFailure(
333-
node: .constrainedSugarType,
334-
message: "child 'SomeOrAnySpecifier' only has keywords as its token choices and should thus and with 'Keyword'"
335-
),
336-
ValidationFailure(node: .declModifier, message: "child 'Name' only has keywords as its token choices and should thus and with 'Keyword'"),
337-
ValidationFailure(
338-
node: .derivativeRegistrationAttributeArguments,
339-
message: "child 'AccessorKind' only has keywords as its token choices and should thus and with 'Keyword'"
350+
node: .booleanLiteralExpr,
351+
message: "child 'Literal' only has keywords as its token choices and should thus end with 'Specifier'"
352+
// TrueOrFalseKeyword would be a stupid name here
340353
),
341354
ValidationFailure(
342-
node: .differentiableAttributeArguments,
343-
message: "child 'DiffKind' only has keywords as its token choices and should thus and with 'Keyword'"
355+
node: .precedenceGroupAssignment,
356+
message: "child 'Value' only has keywords as its token choices and should thus end with 'Specifier'"
344357
),
345358
ValidationFailure(
346-
node: .documentationAttributeArgument,
347-
message: "child 'Label' only has keywords as its token choices and should thus and with 'Keyword'"
359+
node: .precedenceGroupAssociativity,
360+
message: "child 'Value' only has keywords as its token choices and should thus end with 'Specifier'"
348361
),
362+
363+
// MARK: Miscellaneous
364+
// 'weak' or 'unowned' are already the specifier, this is the detail in parens
349365
ValidationFailure(
350-
node: .functionEffectSpecifiers,
351-
message: "child 'AsyncSpecifier' only has keywords as its token choices and should thus and with 'Keyword'"
366+
node: .closureCaptureItemSpecifier,
367+
message: "child 'Detail' only has keywords as its token choices and should thus end with 'Specifier'"
352368
),
369+
// This really is the modifier name and not a specifier
353370
ValidationFailure(
354-
node: .functionEffectSpecifiers,
355-
message: "child 'ThrowsSpecifier' only has keywords as its token choices and should thus and with 'Keyword'"
371+
node: .declModifier,
372+
message: "child 'Name' only has keywords as its token choices and should thus end with 'Specifier'"
356373
),
357-
ValidationFailure(node: .importDecl, message: "child 'ImportKind' only has keywords as its token choices and should thus and with 'Keyword'"),
374+
// Conceptually, this isn't a specifier, it's more like a type inheritance
358375
ValidationFailure(
359376
node: .layoutRequirement,
360-
message: "child 'LayoutConstraint' only has keywords as its token choices and should thus and with 'Keyword'"
361-
),
362-
ValidationFailure(node: .metatypeType, message: "child 'TypeOrProtocol' only has keywords as its token choices and should thus and with 'Keyword'"),
363-
ValidationFailure(node: .operatorDecl, message: "child 'Fixity' only has keywords as its token choices and should thus and with 'Keyword'"),
364-
ValidationFailure(node: .precedenceGroupAssignment, message: "child 'Flag' only has keywords as its token choices and should thus and with 'Keyword'"),
365-
ValidationFailure(
366-
node: .precedenceGroupAssociativity,
367-
message: "child 'Value' only has keywords as its token choices and should thus and with 'Keyword'"
377+
message: "child 'LayoutConstraint' only has keywords as its token choices and should thus end with 'Specifier'"
368378
),
369379
]
370380
)

Sources/SwiftIDEUtils/generated/SyntaxClassification.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ extension SyntaxClassification {
9191
return (.operatorIdentifier, false)
9292
case \PrecedenceGroupAssociativitySyntax.associativityLabel:
9393
return (.keyword, false)
94-
case \PrecedenceGroupRelationSyntax.higherThanOrLowerThanKeyword:
94+
case \PrecedenceGroupRelationSyntax.higherThanOrLowerThanLabel:
9595
return (.keyword, false)
9696
case \SimpleTypeIdentifierSyntax.name:
9797
return (.typeIdentifier, false)

Sources/SwiftOperators/OperatorTable+Semantics.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ extension PrecedenceGroup {
2323
switch attr {
2424
// Relation (lowerThan, higherThan)
2525
case .precedenceGroupRelation(let relation):
26-
let isLowerThan = relation.higherThanOrLowerThanKeyword.text == "lowerThan"
26+
let isLowerThan = relation.higherThanOrLowerThanLabel.text == "lowerThan"
2727
for otherGroup in relation.otherNames {
2828
let otherGroupName = otherGroup.name.text
2929
let relationKind: PrecedenceRelation.Kind =
@@ -40,7 +40,7 @@ extension PrecedenceGroup {
4040

4141
// Assignment
4242
case .precedenceGroupAssignment(let assignment):
43-
self.assignment = assignment.flag.text == "true"
43+
self.assignment = assignment.value.text == "true"
4444

4545
// Associativity
4646
case .precedenceGroupAssociativity(let associativity):
@@ -68,7 +68,7 @@ extension Operator {
6868
/// TODO: This ignores all semantic errors.
6969
init(from syntax: OperatorDeclSyntax) {
7070
self.syntax = syntax
71-
kind = OperatorKind(rawValue: syntax.fixity.text) ?? .infix
71+
kind = OperatorKind(rawValue: syntax.fixitySpecifier.text) ?? .infix
7272

7373
name = syntax.identifier.text
7474

0 commit comments

Comments
 (0)