Skip to content

Commit

Permalink
Merge pull request #1888 from ahoppen/ahoppen/naming-inconsistencies
Browse files Browse the repository at this point in the history
Fix child name validation failures found by `testConsistentNamingOfChildren`
  • Loading branch information
ahoppen authored Jul 11, 2023
2 parents 6864c93 + 687bad2 commit 36ebbef
Show file tree
Hide file tree
Showing 40 changed files with 1,111 additions and 613 deletions.
9 changes: 6 additions & 3 deletions CodeGeneration/Sources/SyntaxSupport/AttributeNodes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,8 @@ public let ATTRIBUTE_NODES: [Node] = [
nameForDiagnostics: "declaration name",
children: [
Child(
name: "DeclBaseName",
name: "BaseName",
deprecatedName: "DeclBaseName",
kind: .token(choices: [
.token(tokenKind: "IdentifierToken"),
.token(tokenKind: "BinaryOperatorToken"),
Expand All @@ -314,7 +315,8 @@ public let ATTRIBUTE_NODES: [Node] = [
documentation: "The base name of the protocol's requirement."
),
Child(
name: "DeclNameArguments",
name: "Arguments",
deprecatedName: "DeclNameArguments",
kind: .node(kind: .declNameArguments),
nameForDiagnostics: "arguments",
documentation: "The argument labels of the protocol's requirement if it is a function requirement.",
Expand Down Expand Up @@ -505,7 +507,8 @@ public let ATTRIBUTE_NODES: [Node] = [
isOptional: true
),
Child(
name: "WhereClause",
name: "GenericWhereClause",
deprecatedName: "WhereClause",
kind: .node(kind: .genericWhereClause),
isOptional: true
),
Expand Down
15 changes: 10 additions & 5 deletions CodeGeneration/Sources/SyntaxSupport/DeclNodes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,8 @@ public let DECL_NODES: [Node] = [
nameForDiagnostics: "function signature",
children: [
Child(
name: "Input",
name: "ParameterClause",
deprecatedName: "Input",
kind: .node(kind: .parameterClause)
),
Child(
Expand All @@ -1096,7 +1097,8 @@ public let DECL_NODES: [Node] = [
isOptional: true
),
Child(
name: "Output",
name: "ReturnClause",
deprecatedName: "Output",
kind: .node(kind: .returnClause),
isOptional: true
),
Expand Down Expand Up @@ -1449,7 +1451,8 @@ public let DECL_NODES: [Node] = [
kind: .token(choices: [.token(tokenKind: "IdentifierToken")])
),
Child(
name: "GenericArguments",
name: "GenericArgumentClause",
deprecatedName: "GenericArguments",
kind: .node(kind: .genericArgumentClause),
isOptional: true
),
Expand Down Expand Up @@ -2214,11 +2217,13 @@ public let DECL_NODES: [Node] = [
isOptional: true
),
Child(
name: "Indices",
name: "ParameterClause",
deprecatedName: "Indices",
kind: .node(kind: .parameterClause)
),
Child(
name: "Result",
name: "ReturnClause",
deprecatedName: "Result",
kind: .node(kind: .returnClause)
),
Child(
Expand Down
15 changes: 10 additions & 5 deletions CodeGeneration/Sources/SyntaxSupport/ExprNodes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,8 @@ public let EXPR_NODES: [Node] = [
kind: .token(choices: [.token(tokenKind: "ColonToken")])
),
Child(
name: "VersionTuple",
name: "Version",
deprecatedName: "VersionTuple",
kind: .node(kind: .versionTuple)
),
]
Expand Down Expand Up @@ -536,14 +537,16 @@ public let EXPR_NODES: [Node] = [
isOptional: true
),
Child(
name: "Input",
name: "ParameterClause",
deprecatedName: "Input",
kind: .nodeChoices(choices: [
Child(
name: "SimpleInput",
kind: .node(kind: .closureParamList)
),
Child(
name: "Input",
name: "ParameterClause",
deprecatedName: "Input",
kind: .node(kind: .closureParameterClause)
),
]),
Expand All @@ -555,7 +558,8 @@ public let EXPR_NODES: [Node] = [
isOptional: true
),
Child(
name: "Output",
name: "ReturnClause",
deprecatedName: "Output",
kind: .node(kind: .returnClause),
isOptional: true
),
Expand Down Expand Up @@ -1154,7 +1158,8 @@ public let EXPR_NODES: [Node] = [
kind: .token(choices: [.token(tokenKind: "IdentifierToken")])
),
Child(
name: "GenericArguments",
name: "GenericArgumentClause",
deprecatedName: "GenericArguments",
kind: .node(kind: .genericArgumentClause),
isOptional: true
),
Expand Down
2 changes: 1 addition & 1 deletion CodeGeneration/Sources/SyntaxSupport/Traits.swift
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public let TRAITS: [Trait] = [
children: [
Child(name: "PoundToken", kind: .token(choices: [.token(tokenKind: "PoundToken")])),
Child(name: "Macro", kind: .token(choices: [.token(tokenKind: "IdentifierToken")])),
Child(name: "GenericArguments", kind: .node(kind: .genericArgumentClause), isOptional: true),
Child(name: "GenericArgumentClause", kind: .node(kind: .genericArgumentClause), isOptional: true),
Child(name: "LeftParen", kind: .token(choices: [.token(tokenKind: "LeftParenToken")]), isOptional: true),
Child(name: "ArgumentList", kind: .node(kind: .tupleExprElementList)),
Child(name: "RightParen", kind: .token(choices: [.token(tokenKind: "RightParenToken")]), isOptional: true),
Expand Down
3 changes: 2 additions & 1 deletion CodeGeneration/Sources/SyntaxSupport/TypeNodes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@ public let TYPE_NODES: [Node] = [
isOptional: true
),
Child(
name: "Output",
name: "ReturnClause",
deprecatedName: "Output",
kind: .node(kind: .returnClause)
),
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func syntaxNode(emitKind: SyntaxNodeKind) -> SourceFileSyntax {
}

let closureSignature = ClosureSignatureSyntax(
input: .input(
parameterClause: .parameterClause(
ClosureParameterClauseSyntax(
parameterList: ClosureParameterListSyntax {
ClosureParameterSyntax(firstName: .identifier("arena"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ let renamedChildrenBuilderCompatibilityFile = try! SourceFileSyntax(leadingTrivi
for layoutNode in SYNTAX_NODES.compactMap(\.layoutNode).filter({ $0.children.hasDeprecatedChild }) {
if let convenienceInit = try layoutNode.createConvenienceBuilderInitializer(useDeprecatedChildName: true) {
let deprecatedNames = layoutNode.children
.filter { !$0.isUnexpectedNodes }
.compactMap { $0.deprecatedName?.withFirstCharacterLowercased }
.filter { !$0.isUnexpectedNodes && $0.deprecatedName != nil }
.compactMap { $0.varName }
.joined(separator: ", ")

DeclSyntax(
Expand Down
115 changes: 39 additions & 76 deletions CodeGeneration/Tests/ValidateSyntaxNodes/ValidateSyntaxNodes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,22 @@ fileprivate extension Child {
}
}

fileprivate extension Array where Element: Hashable, Element: Comparable {
/// Returns the element that occurs most frequently in the array.
///
/// If there is a tie, returns the lexicographically first element.
///
/// Returns `nil` if the array is empty.
var mostCommon: Element? {
var elementCounts: [Element: Int] = [:]
for element in self {
elementCounts[element, default: 0] += 1
}
let maxCount = elementCounts.values.max()
return elementCounts.filter({ $0.value == maxCount }).map(\.key).sorted().first
}
}

class ValidateSyntaxNodes: XCTestCase {
/// All nodes with base kind e.g. `ExprSyntax` should end with `ExprSyntax`.
func testBaseKindSuffix() {
Expand Down Expand Up @@ -395,17 +411,18 @@ class ValidateSyntaxNodes: XCTestCase {
}
}

for (kind, children) in childrenByNodeKind where !kind.isBase && kind != .token {
for (kind, children) in childrenByNodeKind where !kind.isBase && kind != .token && kind != .stringLiteralExpr {
let childNames = children.map(\.child.name)
let firstChildName = childNames.first!
let mostCommonChildName = childNames.mostCommon!
let mostCommonChild = children.first(where: { $0.child.name == mostCommonChildName })!

for (node, child) in children.dropFirst() {
if child.name != firstChildName {
for (node, child) in children {
if child.name != mostCommonChildName {
failures.append(
ValidationFailure(
node: node.kind,
message:
"child '\(child.name)' is named inconsistently with '\(children.first!.node.kind.syntaxType).\(children.first!.child.name)', which has the same type ('\(kind.syntaxType)')"
"child '\(child.name)' is named inconsistently with '\(mostCommonChild.node.kind.syntaxType).\(mostCommonChildName)', which has the same type ('\(kind.syntaxType)')"
)
)
}
Expand All @@ -415,99 +432,45 @@ class ValidateSyntaxNodes: XCTestCase {
assertFailuresMatchXFails(
failures,
expectedFailures: [
ValidationFailure(
node: .differentiableAttributeArguments,
message: "child 'WhereClause' is named inconsistently with 'ActorDeclSyntax.GenericWhereClause', which has the same type ('GenericWhereClauseSyntax')"
),
ValidationFailure(
node: .subscriptDecl,
message: "child 'Indices' is named inconsistently with 'FunctionSignatureSyntax.Input', which has the same type ('ParameterClauseSyntax')"
),
// MARK: DeclNameArguments
// FIXME: IdentifierExprSyntax etc. should probably use DeclName as child instead of Name and Arguments
ValidationFailure(
node: .qualifiedDeclName,
message: "child 'Arguments' is named inconsistently with 'DeclNameSyntax.DeclNameArguments', which has the same type ('DeclNameArgumentsSyntax')"
message:
"child 'Arguments' is named inconsistently with 'IdentifierExprSyntax.DeclNameArguments', which has the same type ('DeclNameArgumentsSyntax')"
),
ValidationFailure(
node: .macroExpansionDecl,
node: .declName,
message:
"child 'GenericArguments' is named inconsistently with 'KeyPathPropertyComponentSyntax.GenericArgumentClause', which has the same type ('GenericArgumentClauseSyntax')"
"child 'Arguments' is named inconsistently with 'IdentifierExprSyntax.DeclNameArguments', which has the same type ('DeclNameArgumentsSyntax')"
),
// MARK: Alternate names for InitializerClauseSyntax
// The cases below don’t have intializers but just a syntactic element that happens to be spelled the same
ValidationFailure(
node: .macroExpansionExpr,
node: .enumCaseElement,
message:
"child 'GenericArguments' is named inconsistently with 'KeyPathPropertyComponentSyntax.GenericArgumentClause', which has the same type ('GenericArgumentClauseSyntax')"
"child 'RawValue' is named inconsistently with 'MatchingPatternConditionSyntax.Initializer', which has the same type ('InitializerClauseSyntax')"
),
ValidationFailure(
node: .enumCaseParameter,
message: "child 'DefaultArgument' is named inconsistently with 'EnumCaseElementSyntax.RawValue', which has the same type ('InitializerClauseSyntax')"
message:
"child 'DefaultArgument' is named inconsistently with 'MatchingPatternConditionSyntax.Initializer', which has the same type ('InitializerClauseSyntax')"
),
ValidationFailure(
node: .functionParameter,
message: "child 'DefaultArgument' is named inconsistently with 'EnumCaseElementSyntax.RawValue', which has the same type ('InitializerClauseSyntax')"
message:
"child 'DefaultArgument' is named inconsistently with 'MatchingPatternConditionSyntax.Initializer', which has the same type ('InitializerClauseSyntax')"
),
ValidationFailure(
node: .macroDecl,
message: "child 'Definition' is named inconsistently with 'EnumCaseElementSyntax.RawValue', which has the same type ('InitializerClauseSyntax')"
),
ValidationFailure(
node: .matchingPatternCondition,
message: "child 'Initializer' is named inconsistently with 'EnumCaseElementSyntax.RawValue', which has the same type ('InitializerClauseSyntax')"
),
ValidationFailure(
node: .optionalBindingCondition,
message: "child 'Initializer' is named inconsistently with 'EnumCaseElementSyntax.RawValue', which has the same type ('InitializerClauseSyntax')"
),
ValidationFailure(
node: .patternBinding,
message: "child 'Initializer' is named inconsistently with 'EnumCaseElementSyntax.RawValue', which has the same type ('InitializerClauseSyntax')"
),
ValidationFailure(
node: .tupleTypeElement,
message: "child 'Initializer' is named inconsistently with 'EnumCaseElementSyntax.RawValue', which has the same type ('InitializerClauseSyntax')"
message:
"child 'Definition' is named inconsistently with 'MatchingPatternConditionSyntax.Initializer', which has the same type ('InitializerClauseSyntax')"
),
// MARK: Miscellaneous
ValidationFailure(
node: .multipleTrailingClosureElement,
message: "child 'Closure' is named inconsistently with 'FunctionCallExprSyntax.TrailingClosure', which has the same type ('ClosureExprSyntax')"
),
ValidationFailure(
node: .subscriptDecl,
message: "child 'Result' is named inconsistently with 'ClosureSignatureSyntax.Output', which has the same type ('ReturnClauseSyntax')"
),
ValidationFailure(
node: .canImportVersionInfo,
message:
"child 'VersionTuple' is named inconsistently with 'AvailabilityVersionRestrictionSyntax.Version', which has the same type ('VersionTupleSyntax')"
),
ValidationFailure(
node: .exposeAttributeArguments,
message:
"child 'CxxName' is named inconsistently with 'ConventionAttributeArgumentsSyntax.CTypeString', which has the same type ('StringLiteralExprSyntax')"
),
ValidationFailure(
node: .opaqueReturnTypeOfAttributeArguments,
message:
"child 'MangledName' is named inconsistently with 'ConventionAttributeArgumentsSyntax.CTypeString', which has the same type ('StringLiteralExprSyntax')"
),
ValidationFailure(
node: .originallyDefinedInArguments,
message:
"child 'ModuleName' is named inconsistently with 'ConventionAttributeArgumentsSyntax.CTypeString', which has the same type ('StringLiteralExprSyntax')"
),
ValidationFailure(
node: .poundSourceLocationArgs,
message:
"child 'FileName' is named inconsistently with 'ConventionAttributeArgumentsSyntax.CTypeString', which has the same type ('StringLiteralExprSyntax')"
),
ValidationFailure(
node: .unavailableFromAsyncArguments,
message:
"child 'Message' is named inconsistently with 'ConventionAttributeArgumentsSyntax.CTypeString', which has the same type ('StringLiteralExprSyntax')"
),
ValidationFailure(
node: .underscorePrivateAttributeArguments,
message:
"child 'Filename' is named inconsistently with 'ConventionAttributeArgumentsSyntax.CTypeString', which has the same type ('StringLiteralExprSyntax')"
),
]
)
}
Expand Down
8 changes: 4 additions & 4 deletions Sources/SwiftParser/Attributes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ extension Parser {
kindSpecifierComma: kindSpecifierComma,
parameters: parameters,
parametersComma: parametersComma,
whereClause: whereClause,
genericWhereClause: whereClause,
arena: self.arena
)
}
Expand Down Expand Up @@ -683,8 +683,8 @@ extension Parser {
let (unexpectedBeforeColon, colon) = self.expect(.colon)
let (targetFunction, args) = self.parseDeclNameRef([.zeroArgCompoundNames, .keywordsUsingSpecialNames, .operators])
let declName = RawDeclNameSyntax(
declBaseName: targetFunction,
declNameArguments: args,
baseName: targetFunction,
arguments: args,
arena: self.arena
)
let comma = self.consume(if: .comma)
Expand Down Expand Up @@ -1031,7 +1031,7 @@ extension Parser {
.zeroArgCompoundNames, .keywordsUsingSpecialNames, .operators,
])
}
let method = RawDeclNameSyntax(declBaseName: base, declNameArguments: args, arena: self.arena)
let method = RawDeclNameSyntax(baseName: base, arguments: args, arena: self.arena)
return RawDynamicReplacementArgumentsSyntax(
unexpectedBeforeLabel,
forLabel: label,
Expand Down
Loading

0 comments on commit 36ebbef

Please sign in to comment.