Add infrastructure for automated validation of syntax node structure #1748
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci Please test |
kimdv
approved these changes
Jun 9, 2023
ahoppen
force-pushed
the
ahoppen/syntax-node-validation
branch
from
f613daf to
a09e768
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
ahoppen
force-pushed
the
ahoppen/syntax-node-validation
branch
from
a09e768 to
b0d3092
Compare
The goal here is to generalize the API inconsistency reported by @AnthonyLatsis and automatically detect them. There are valid reasons for nodes or children to violate these base rules. My idea is that we note these exceptions in expected failures, ideally with a sort explanation that motivates the deviation from standard naming rules. We aren’t running these tests at the moment yet. I’d like to merge them as a starting point for API improvement ideas at the moment. The current rules are - All nodes with base kind e.g. `ExprSyntax` should end with `ExprSyntax`. - If a child has a single keyword as its choice, it should be named `*Keyword` (e.g. `ImportKeyword`) - If a child has a single non-keyword token kind, name it the same as the the token choice (e.g. `LeftParen`) - If a token only has keyword token choices, its name should end with `Keyword`. - Children that have the same type also have the same child name. - Every node that can conform to a trait does so
ahoppen
force-pushed
the
ahoppen/syntax-node-validation
branch
from
b0d3092 to
02a0c5d
Compare
|
@swift-ci Please test |
|
Nice, I'd say the rules generally make sense. And we can always have expected failures for when we really want to differ for some reason. Thanks Alex! |
bnbarham
approved these changes
Jun 17, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal here is to generalize the API inconsistency reported by @AnthonyLatsis and automatically detect them.
There are valid reasons for nodes or children to violate these base rules. My idea is that we note these exceptions in expected failures, ideally with a sort explanation that motivates the deviation from standard naming rules.
We aren’t running these tests at the moment yet. I’d like to merge them as a starting point for API improvement ideas at the moment.
The current rules are
ExprSyntaxshould end withExprSyntax.*Keyword(e.g.ImportKeyword) Inconsistent name ofImportDeclSyntaxintroducer keyword #1723LeftParen)KeywordPrecedenceGroupRelationSyntax.higherThanOrLowerThanshould be namedhigherThanOrLowerThanKeyword#1747FunctionTypeSyntax.argumentsshould be namedparameters#1720, Inconsistent naming scheme for the elements of tuple-like syntax nodes #1713