Skip to content
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

Quick fixes for record positional record literals #51446

Closed
2 tasks done
Tracked by #49959
CoderDake opened this issue Feb 17, 2023 · 5 comments
Closed
2 tasks done
Tracked by #49959

Quick fixes for record positional record literals #51446

CoderDake opened this issue Feb 17, 2023 · 5 comments
Assignees
Labels
analyzer-quick-fix analyzer-ux analyzer-warning Issues with the analyzer's Warning codes area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on

Comments

@CoderDake
Copy link

CoderDake commented Feb 17, 2023

Tasks

  • Implement new diagnostic RECORD_LITERAL_ONE_POSITIONAL_NO_TRAILING_COMMA b0078ad
  • Add a quick fix for RECORD_LITERAL_ONE_POSITIONAL_NO_TRAILING_COMMA 43fbb1d

Context

While testing records I found that the following gives an error:

(int,) intRecord1 = (1);

The error is:

A value of type 'int' can't be assigned to a variable of type '(int)'.
Try changing the type of the variable, or casting the right-hand type to '(int)'.

After doing some reserearch into the spec I found out that records that look like this must have a trailing comma to show that they are a record. This was not immediately apparent when reading the errors or looking at the quick fixes though.

So the proper way to write this would have been:

(int,) intRecord1 = (1,);

It would be helpful if the error mentioned that records must end in a comma, or perhaps there was a quick fix that offered to add the trailing comma.

@CoderDake
Copy link
Author

@bwilkerson I heard you might be interested in this

@bwilkerson
Copy link
Member

Yes, I'm definitely interested. @keertip @pq might also be interested.

I think the right solution here is to define a new diagnostic for this case (because that's how we can have a better message here). We might even be able to re-use ParserErrorCode.RECORD_LITERAL_ONE_POSITIONAL_NO_TRAILING_COMMA, though we don't normally produce parse errors outside the parser, so that might be too odd.

Then we can create a fix that will add the comma. (That fix could be applied to a couple of other diagnostics as well.)

@bwilkerson bwilkerson added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-ux analyzer-quick-fix P2 A bug or feature request we're likely to work on labels Feb 17, 2023
@bwilkerson
Copy link
Member

I gave this a P2, but I'd really like to see it ship with the feature because I think it will help keep users on the rails.

@pq
Copy link
Member

pq commented Feb 21, 2023

I agree. This feels like a significant foot-gun. I'l take a look.

@pq pq self-assigned this Feb 21, 2023
copybara-service bot pushed a commit that referenced this issue Feb 22, 2023
See: #51446

Change-Id: I4ebb0a8978bd98fc658737f69b790c2039c17d52
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284501
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
@pq pq added the analyzer-warning Issues with the analyzer's Warning codes label Feb 22, 2023
copybara-service bot pushed a commit that referenced this issue Feb 22, 2023
See: #51446

Change-Id: I372f05c6e87a5ee4b55edcb05cf05c035107f250
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284549
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
@pq
Copy link
Member

pq commented Feb 22, 2023

Completed w/ 43fbb1d.

Thanks again for the report @CoderDake!

@pq pq closed this as completed Feb 22, 2023
@pq pq changed the title Quick fixes for record positional record types Quick fixes for record positional record literals Feb 23, 2023
copybara-service bot pushed a commit that referenced this issue Feb 23, 2023
Follow-up from #51446

Change-Id: I22de580e9a163dd371825a494ce92e2332edbb79
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/285020
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-quick-fix analyzer-ux analyzer-warning Issues with the analyzer's Warning codes area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

3 participants