Improvements to integer literal utilities used in SourceKit-LSP #2626
Conversation
SourceKit-LSP's refactoring action has minor extensions for the integer literal utilities. Sink them down here for more general use: * IntegerLiteralExprSyntax.Radix becomes CaseIterable * IntegerLiteralExprSyntax.Radix gains a "literalPrefix" property to get the string needed to prefix a literal with this radix.
|
@swift-ci please test |
|
@swift-ci please test |
|
@swift-ci please test Windows |
1 similar comment
|
@swift-ci please test Windows |
| @@ -17,7 +17,7 @@ import SwiftSyntax | |||
| #endif | |||
|
|
|||
| extension IntegerLiteralExprSyntax { | |||
| public enum Radix { | |||
| public enum Radix: CaseIterable { | |||
There was a problem hiding this comment.
As for the use-case in SourceKit-LSP, I'd just do:
for radix: IntegerLiteralExprSyntax.Radix in [.binary, .octal, .hex, .decimal] { ... }This is clearer and more deterministic what it will do, compared to .allCases.
I personally avoid CaseIterable unless there's a strong reason to do. Users cannot control the order (maybe it might not matter in this specific use-case)
There was a problem hiding this comment.
Hmm, I'm doing this based on the discussion in swiftlang/sourcekit-lsp#1179 (comment). Code that is working with radix values via the APIs here can do so without having to reason about the particulars of the radix; it can handle all of them. That seems like it makes this a reasonable API to provide.
There was a problem hiding this comment.
Yes I know the discussion. But,
it can handle all of them.
All of current cases. Perhaps in the future, we might introduce a case that has "suffix". e.g. 0nA24nG_r62 (whatever it is). If we only handle known cases [.binary, .octal, .hex, .decimal], the code would probably still work correctly because we'd not change the syntax of the current radix cases. But using .allCases can cause misbehavior.
FWIW #2292, we have a history of removing CaseIterable. I want to be really cautious about re-adding it. IMO, adding CaseIterable as public API should have a super strong reasoning.
There was a problem hiding this comment.
I think it's unlikely to ever cause a problem, given that the literals have been stable for a decade, but neither is it more than an inconvenience if we don't have this. So I've removed it.
|
@swift-ci please test |
|
@swift-ci please test Windows |
SourceKit-LSP's refactoring action has minor extensions for the integer literal utilities. Sink them down here for more general use: