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

Type checker errors should be reported when syntax errors are present #17787

Open
auduchinok opened this issue Sep 25, 2024 · 2 comments · May be fixed by #18311
Open

Type checker errors should be reported when syntax errors are present #17787

auduchinok opened this issue Sep 25, 2024 · 2 comments · May be fixed by #18311
Labels
Area-Diagnostics mistakes and possible improvements to diagnostics Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Milestone

Comments

@auduchinok
Copy link
Member

auduchinok commented Sep 25, 2024

Consider this case:

do
    let a = b.ToString()
Screenshot 2024-09-25 at 11 37 57

The are two errors:

  • syntax error: missing in part of the let expression
  • unresolved b name

The second error is not reported. This has been like that for long time where we tried to minimize cascading type checker errors, so some of them are hidden when there are errors in preceding phases. However, with the significantly improved parser recovery it should be much better to report these errors now. Having any in expression helps in this case:

do
    let a = b.ToString()
    ()
Screenshot 2024-09-25 at 11 38 03

But we want a better error reporting while editing too 🙂

The same applies to other case where the parser recovery has been introduced, for example tuple expressions. When there are no syntax errors, type checker errors about the unresolved name and a missing overload are reported:

Math.Max(a)
Screenshot 2024-09-25 at 11 41 06

When typing the second argument (and adding a syntax error in the tuple), the resolved names are reported, but the type checker errors are hidden (even though the parser recovery now allows the subsequent analysis):

Math.Max(a, )
Screenshot 2024-09-25 at 11 41 44

When the syntax error is fixed, everything is reported, even a warning about the unused expression result:

Math.Max(a, 1)
Screenshot 2024-09-25 at 11 43 20

Typing subsequent arguments makes errors blink, which is not great:

Screenshot 2024-09-25 at 11 45 06
@abonie
Copy link
Member

abonie commented Sep 30, 2024

@auduchinok Do you know if it ever worked? Either way we should do it

@abonie abonie added Area-Diagnostics mistakes and possible improvements to diagnostics Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code. and removed Needs-Triage labels Sep 30, 2024
@auduchinok
Copy link
Member Author

@auduchinok Do you know if it ever worked? Either way we should do it

No. Previously the parser would fail to a point where the analysis is not possible at all. Now the analysis is being done because of the improved parser, but type checker errors are hidden because of the syntax errors (that have been recovered). I suggest we should not hide these errors anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Diagnostics mistakes and possible improvements to diagnostics Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Projects
Status: New
Development

Successfully merging a pull request may close this issue.

2 participants