Add diagnostic for misplaced specifiers #947
Conversation
|
@swift-ci Please test |
| // We can only stick one specifier on this type. Let's pick the first one | ||
| if specifier == nil, let misplacedSpecifier = misplacedSpecifiers.first { | ||
| specifier = missingToken(misplacedSpecifier.tokenKind, text: misplacedSpecifier.tokenText) | ||
| } | ||
|
|
||
| if self.at(any: [.atSign, .inoutKeyword]) { | ||
| return (specifier, self.parseTypeAttributeListPresent()) |
There was a problem hiding this comment.
I can't add a comment where I want it - but TypeSpecifier could be used in the at below and in parseTypeAttributeListPresent (we change the name to modifiers there which is also interesting).
| let specifier = self.consume(ifAny: [.inoutKeyword], contextualKeywords: ["__shared", "__owned"]) | ||
| public mutating func parseTypeAttributeList(misplacedSpecifiers: [RawTokenSyntax] = []) -> (RawTokenSyntax?, RawAttributeListSyntax?) { | ||
| var specifier: RawTokenSyntax? = self.consume(ifAnyIn: TypeSpecifier.self) | ||
| // We can only stick one specifier on this type. Let's pick the first one |
There was a problem hiding this comment.
I feel the same way about this PR as I do about mine. I'm not sure it's worth keeping around all the state in order to allow adding the "missing" specifier here.
Also note that specifiers are parsed into RawAttributeListSyntax below, without being marked as unexpected and without the eventual fix-it. The current diagnostic (in the actual compiler) isn't great about this either, we end up with a '<specifier>' may only be used on parameters diagnostic and only recover from a single specifier after the attribute list and after that lose the type.
Whatever we end up doing here, it would be nice to be consistent across All the Things (where "things" here are attributes/modifiers/specifiers). Ie. should we ever expect a single, or should we always allow a list? Do we diagnose moving them to their correct position, or just add them (parsed correctly) as unexpected nodes?
There are current clients (eg. SwiftLint) that check eg. the async keyword in a function declaration to determine if a function is async. Should that check pass if the async is in any position, or just the correct?
| DiagnosticSpec(message: "'inout inout' before a parameter name is not allowed", fixIts: ["move 'inout inout' in front of type"]), | ||
| ], fixedSource: "func f2_43591(b: inout Int) {}" |
There was a problem hiding this comment.
That's somewhat confusing 😅. What happens when there's already an inout in this case? Does it say "remove redundant inout inout"?
| @@ -1387,7 +1392,7 @@ extension Parser { | |||
|
|
|||
| let type: RawTypeSyntax? | |||
| if shouldParseType { | |||
| type = self.parseType() | |||
| type = self.parseType(misplacedSpecifiers: misplacedSpecifiers) | |||
There was a problem hiding this comment.
This name is somewhat confusing since function types can also have specifiers, but this is for the type specifiers specifically.
No description provided.