SwiftSyntax support for module selectors #3091
Conversation
This capability is currently unused, but it’ll matter soon.
And make sure it doesn’t break Objective-C selector lexing.
Treating introducers as keywords is now always conditional on whether `shouldParsePatternBinding(introducer:)` returns `true`. That method has also been modified to correctly handle an edge case with wildcard patterns.
Changes the syntax tree to represent module selectors:
• A `ModuleSelectorSyntax` node represents a module selector abstractly.
• The following nodes now have an optional `moduleSelector` child:
• `DeclReferenceExprSyntax`
• `IdentifierTypeSyntax`
• `MacroExpansionExprSyntax`
• `MemberTypeSyntax`
• BasicFormat knows the preferred format for module selectors.
Other components, particularly the parser, were also updated to continue building, though without any changes in behavior. Parser implementation will come in a future commit.
Changes it to share code with `parseTypeIdentifier()` and clean up the member type parsing a little. Also tweaks call sites of `parseTypeIdentifier()`.
|
🥳 |
| /// | ||
| /// - Precondition: `node` must, at minimum, have a descendant with an unexpected nodes child; it therefore cannot be | ||
| /// a token or an empty collection. | ||
| func attach<Node: RawSyntaxNodeProtocol>(_ moduleSelector: RawModuleSelectorSyntax?, to node: Node) -> Node { |
There was a problem hiding this comment.
Retroactively rewriting a node to insert new information is a pretty unconventional way for SwiftParser to do things, so I thought I should justify this design.
Because a module selector is a prefix on a different syntax and requires two tokens, it's difficult to peek past one and make decisions based on the tokens that follow it. Instead, I found it easier to parse them in relatively high-level productions; that gets the tokens out of the way, but it means they're often parsed a significant distance from the node that will become their parent. I tried two other approaches to coping with this problem:
-
Adding new
ModuleSelectorExprSyntaxandModuleSelectorTypeSyntaxnodes. The issue was that I want to make sure we could parse invalid module selectors—even in declaration syntax, patterns, etc.—into unexpected nodes, and that didn't offer a good way to do so. (I also found that the module selectors on member lookups wouldn't be able to be handled in this way; I didn't really like that.) -
Passing the
ModuleSelectorSyntaxnode down as a parameter and threading it through to wherever it was needed. This required me to add a lot of parameters and manually insert unexpected module selectors into a lot of nodes, but that's not actually what scuttled the idea—it's that it dramatically increased stack usage. At one point I had to reduce the maximum recursion in development builds to 10, which isn't even enough to handle the swift-syntax repo itself.
Retroactively attaching module selectors in this fashion allows more of the parser to ignore the fact that an invalid module selector might have been parsed earlier on, while also avoiding the stack usage problems I mentioned.
There was a problem hiding this comment.
Could you explain why we want
I want to make sure we could parse invalid module selectors—even in declaration syntax, patterns, etc.
What makes module selectors different from other invalid things? E.g.:
let Foo.Bar = 12
There was a problem hiding this comment.
A couple reasons:
-
The difference between a name and a name reference is a bit subtle and hasn't previously been grammatically important; I can imagine that developers might be confused about where they are allowed to use module selectors.
-
If a developer writes
Foo::Barin a place where module selectors are not valid, we can be almost certain that they want to keepBar(because we know thatFoowas supposed to be a module name), but the naïve recovery behavior will treatFooas the name and::Baras unexpected syntax. Tailoring the recovery gives us a tree that better reflects the user's likely intent. (Whereas withlet Foo.Barthe developer might have wantedlet Bar, orlet Foo, orlet Foo = .Bar, or any number of other possibilities; it's harder to be certain of their intent.)
This commit ports over tests from the compiler’s (future) `test/NameLookup/module_selector.swift` file and makes sure the correct uses parse as expected. It also tests that ill-formed module selectors (ones with a missing or non-identifier module name) are diagnosed correctly. This commit doesn’t fully handle recovery from module selectors inserted at invalid locations; the test cases that require recovery are XFAILed. TODO: • Add assertions about syntax tree nodes for valid parses
Specifically, from module selectors at incorrect locations. This is done through a couple of mechanisms: • The various `expect(…)` methods consume a module selector as unexpected syntax. • Various identifier-parsing productions now pre-parse an invalid module selector and convert it to unexpected syntax. In some cases this involves adjusting matching `can`/`at` methods to consume otherwise-invalid module selectors. • The previously-introduced `attach(_:to:)` mechanism is now used in more places. This makes all test cases inherited from the Swift tests pass, except for the `import` syntax which I’m a little iffy on.
This PR adds module selectors to SwiftSyntax and SwiftParser (draft of matching compiler feature in swiftlang/swift#34556). This feature was pitched ages ago; a proper proposal is on my todo list.
Note: This PR is still a work in progress—in particular, I need to go back and improve the tests, both by expanding their coverage and by adding assertions about the resulting syntax trees—but I'd appreciate feedback on the design while I'm working on that.