Ensure there are no optional syntax collection in the syntax tree #2043
Conversation
ahoppen
force-pushed
the
ahoppen/no-optional-collections
branch
from
b3f3692
to
47d09ff
Compare
ahoppen
force-pushed
the
ahoppen/no-optional-collections
branch
from
47d09ff
to
b379078
Compare
ahoppen
force-pushed
the
ahoppen/no-optional-collections
branch
2 times, most recently
from
fedb281
to
c1aa2e9
Compare
There was a problem hiding this comment.
whose parameter I also changed from defaulting to nil to not having a default value because most of the time you want to specify minor and patch components of a version
I don't know how true that actually is, major is fine for most availability cases for example (though maybe .0 is common to add)
| @@ -58,7 +58,7 @@ extension AccessorDeclListSyntax: SyntaxParseable { | |||
| return parser.parseAccessorList() ?? RawAccessorDeclListSyntax(elements: [], arena: parser.arena) | |||
| } makeMissing: { remainingTokens, arena in | |||
| return RawAccessorDeclSyntax( | |||
| attributes: nil, | |||
| attributes: RawAttributeListSyntax(elements: [], arena: arena), | |||
There was a problem hiding this comment.
Did you make this not emptyCollection because it's an entry function?
There was a problem hiding this comment.
Because that would require access to the parser, which would be an overlapping access to parser. So to actually use emptyCollection, we would need to pass the parser into makeMissing and I didn’t want to change that since creation of an empty collection is sufficiently simple here and shouldn’t happen in normal code.
This hoists the ability to create syntax collections from `SwiftSyntaxBuilder` to the `SwiftSyntax` module. Impact on API compatibility: Initializing an `ExprListSyntax` or `UnexpectedNodesSyntax` now requires the nodes to be `ExprSyntax` and `Syntax` opposed to `ExprSyntaxProtocol` or `SyntaxProtocol`, respectively. `ExprListSyntax` is used as the child of a `SequenceExprSyntax`. I think this is OK because - Users don’t genrally construct `UnexpectedNodesSyntax` - To construct a `SequenceExprSyntax`, you can use the result builder on `SequenceExprSyntax`, which takes `ExprSyntaxProtocol`
All syntax collections should be non-optional, which makes it easier to work with them. Also, the semantics of what an empty collection vs. a `nil` collection have not been clear.
ahoppen
force-pushed
the
ahoppen/no-optional-collections
branch
from
c1aa2e9
to
b6f4595
Compare
|
swiftlang/swift-format#585 @swift-ci Please test Windows |
All syntax collections should be non-optional, which makes it easier to work with them. Also, the semantics of what an empty collection vs. a
nilcollection have not been clear.The children that were modified were:
additionalTrailingClosuresmodifiersattributesVersionTupleSyntax.components(whose parameter I also changed from defaulting tonilto not having a default value because most of the time you want to specify minor and patch components of a version)ClosureCaptureClauseSyntax.items(also doesn’t have a default value anymore because it doesn’t make sense to define an empty capture clause)CatchClauseSyntax.catchItemsDoStmt.catchClausesThis has API compatibility impact because we don’t have an API-compatible way to change the optionality of variables with deprecation warnings. That’s why I want to get these changes in before the 509 release.
To make the construction of syntax collections easier, I hoisted the conformance of syntax collections to
ExpressibleByArrayLiteralfromSwiftSyntaxBuilderto theSwiftSyntaxmodule. ThisFixes #2040