Skip to content

Improve diagnostics for invalid version tuples #1717

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

TTOzzi
Copy link
Member

@TTOzzi TTOzzi commented May 31, 2023

Resolve #1613

@TTOzzi TTOzzi requested a review from ahoppen as a code owner May 31, 2023 15:41
@TTOzzi TTOzzi force-pushed the remove-unexpected-node-between-argument-and-right-paren-in-attribute-syntax branch 2 times, most recently from 4a1a969 to ee1bd18 Compare June 3, 2023 17:01
@TTOzzi TTOzzi changed the title Remove the unexpected node between the argument and the right paren in AttributeSyntax Fix to produce a single diagnostic that says it's invalid when an unexpected node exists between the argument and the right paren in AttributeSyntax Jun 3, 2023
@TTOzzi TTOzzi force-pushed the remove-unexpected-node-between-argument-and-right-paren-in-attribute-syntax branch from ee1bd18 to 9ceda38 Compare June 3, 2023 17:10
@TTOzzi TTOzzi requested review from ahoppen and kimdv June 3, 2023 17:14
@@ -459,6 +459,29 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
)
return .visitChildren
}
if let unexpectedBetweenArgumentAndRightParen = node.unexpectedBetweenArgumentAndRightParen,
var missingArgumentNode = node.argument?.missingNodes.only
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missingNodes seems a little magical to me and I get the feeling that it’s not the right design because you need to patch it up further below. I think a better design would be to

  • Modify the parser and eat e.g. 0xff as an unexpected token when parsing the VersionTupleSyntax.
  • Generate the diagnostic in visit(_ node: VersionTupleSyntax)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha I'll try again. Thanks for the feedback! 🙇

@TTOzzi TTOzzi changed the title Fix to produce a single diagnostic that says it's invalid when an unexpected node exists between the argument and the right paren in AttributeSyntax Improve diagnostics for invalid version tuples Jun 18, 2023
@TTOzzi TTOzzi requested review from keith and DougGregor as code owners June 18, 2023 16:17
@keith
Copy link
Member

keith commented Jun 18, 2023

It looks like you have accidentally pulled in a few unrelated commits here

@TTOzzi
Copy link
Member Author

TTOzzi commented Jun 18, 2023

I apologize profusely 😓 It appears that there might be an issue with my branch.

@TTOzzi TTOzzi closed this Jun 18, 2023
@TTOzzi TTOzzi force-pushed the remove-unexpected-node-between-argument-and-right-paren-in-attribute-syntax branch from 2209cc8 to dd976be Compare June 18, 2023 16:23
@keith
Copy link
Member

keith commented Jun 18, 2023

No worries!

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.

10e10 is not emitted as an invalid version tuple
5 participants