-
Notifications
You must be signed in to change notification settings - Fork 441
Improve diagnostic in testMissingArgumentToAttribute
#738
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
a4b1728
to
42fbea4
Compare
42fbea4
to
9379ce4
Compare
b866222
to
24c4339
Compare
…ArgumentToAttribute`
24c4339
to
ad35386
Compare
@swift-ci Please test |
public let attributeName: TokenSyntax | ||
|
||
public var message: String { | ||
return "Expected argument for '@\(attributeName)' attribute" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
argument(s)
IMO, same with the fix-it message.
@@ -149,7 +149,8 @@ public enum RawTokenKind: Equatable, Hashable { | |||
case ${token.swift_kind()} | |||
% end | |||
|
|||
var defaultText: SyntaxText? { | |||
@_spi(RawSyntax) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to mention this in the other PR, but since this seems to extract this part... IMO we shouldn't be using RawSyntax
in the diagnostic generator. It should be a general client, which I wouldn't expect to need it. In saying that, RawTokenKind
seems somewhat useful to me, so perhaps it shouldn't be RawSyntax
?
let newKind = TokenKind.fromRaw(kind: rawKind, text: rawKind.defaultText.map(String.init) ?? "<#\(rawKind.nameForDiagnostics)#>") | ||
presentToken = TokenSyntax(newKind, presence: .present) | ||
} | ||
return Syntax(Format().format(syntax: presentToken)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about Format().format
as the other PR, ie. it would be nice if this was an extension on the token instead. Maybe we can change that when fixing up Doug's comment in the original Format
extraction PR though, so fine for it to be a follow up.
import SwiftBasicFormat | ||
|
||
/// Walks a tree and checks whether the tree contained any present tokens. | ||
class PresentNodeChecker: SyntaxAnyVisitor { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should really introduce a .stop
sooner rather than later 😅
@swift-ci Please test macOS |
This allows us to add fix-its that insert missing tokens.
Fixes #821