Skip to content

Patterns: Wrong diagnostic generated when case clause has const constructor call #51139

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

Open
keertip opened this issue Jan 26, 2023 · 4 comments
Labels
legacy-area-front-end Legacy: Use area-dart-model instead.

Comments

@keertip
Copy link
Contributor

keertip commented Jan 26, 2023

Looking at adding a fix for the case clause with const constructor calls

before patterns

case SomeClass(1, 2): 

now with patterns has to changed to

case const SomeClass(1, 2):

But the analyzer does not procedure any diagnostic associated with patterns. Instead see

error: The getter name is not specified explicitly, and the pattern is not a variable. (missing_object_pattern_getter_name at ...)

This is not helpful and also does not allow for associating the needed fix in this case.

@keertip keertip added the legacy-area-front-end Legacy: Use area-dart-model instead. label Jan 26, 2023
@keertip
Copy link
Contributor Author

keertip commented Jan 26, 2023

/cc @scheglov , @bwilkerson

@scheglov
Copy link
Contributor

I think object pattern does not allow positional fields, only named. So, I would expect that the parser should report a syntax error here (and still produce a positional field). This syntactic error could be then used to attach a quick fix to it.

The shared analysis logic then probably should not require the getter name, because we already know that this code is not syntactically valid.

@bwilkerson
Copy link
Member

@jensjoha It looks like the parser is interpreting SomeClass(1, 2) as an object pattern rather than a constructor invocation. How difficult would it be to recover by noticing that none of the pattern fields have names, and instead treat this as a constructor invocation with a missing const keyword? Having a single diagnostic would be a better UX and would make it easier for people migrating their code.

If we can't produce a single diagnostic, then we'll have to attach the fix to the repeated diagnostic and only provide a fix for the first one (and then only if none of the fields have a name).

@johnniwinther
Copy link
Member

The parser is currently stateful so that it generates different results depending on whether patterns are enabled. It is there a non-trivial task to recognize old syntax during parsing.

cc @stereotype441

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-front-end Legacy: Use area-dart-model instead.
Projects
None yet
Development

No branches or pull requests

4 participants