Skip to content

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

Merged
merged 1 commit into from
Sep 27, 2022

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Sep 8, 2022

This allows us to add fix-its that insert missing tokens.

Fixes #821

@ahoppen ahoppen force-pushed the ahoppen/missing-attr-diag branch from a4b1728 to 42fbea4 Compare September 22, 2022 10:17
@ahoppen ahoppen force-pushed the ahoppen/missing-attr-diag branch from 42fbea4 to 9379ce4 Compare September 26, 2022 13:28
@ahoppen ahoppen requested a review from bnbarham September 26, 2022 13:28
@ahoppen ahoppen marked this pull request as ready for review September 26, 2022 13:34
@ahoppen ahoppen force-pushed the ahoppen/missing-attr-diag branch 3 times, most recently from b866222 to 24c4339 Compare September 26, 2022 16:34
@ahoppen ahoppen force-pushed the ahoppen/missing-attr-diag branch from 24c4339 to ad35386 Compare September 26, 2022 16:39
@ahoppen
Copy link
Member Author

ahoppen commented Sep 26, 2022

@swift-ci Please test

public let attributeName: TokenSyntax

public var message: String {
return "Expected argument for '@\(attributeName)' attribute"
Copy link
Contributor

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)
Copy link
Contributor

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))
Copy link
Contributor

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 {
Copy link
Contributor

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 😅

@ahoppen
Copy link
Member Author

ahoppen commented Sep 26, 2022

swiftlang/swift#61298

@swift-ci Please test macOS

@ahoppen ahoppen merged commit 9348629 into swiftlang:main Sep 27, 2022
@ahoppen ahoppen deleted the ahoppen/missing-attr-diag branch September 27, 2022 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unterminated @_dynamicReplacement eats func keyword
2 participants