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

[FEATURE] Improve error messages around type annotations #802

Open
7 tasks
shonfeder opened this issue May 4, 2021 · 1 comment
Open
7 tasks

[FEATURE] Improve error messages around type annotations #802

shonfeder opened this issue May 4, 2021 · 1 comment
Assignees
Labels
product-owner-triage This should be triaged by the product owner usability UX improvements

Comments

@shonfeder
Copy link
Contributor

shonfeder commented May 4, 2021

  • Missing annotations on VARIABLE's don't include line numbers:
PASS #1: TypeCheckerSnowcat                                       I@08:18:53.210
 > Running Snowcat .::.                                           I@08:18:53.210
Typing input error: Expected a type annotation for VARIABLE blockchain E@08:18:53.309
It took me 0 days  0 hours  0 min  0 sec                          I@08:18:53.310
Total time: 0.920 sec                                             I@08:18:53.310
EXITCODE: ERROR (255)
  • Detect missing ; at end of line: Confusing error message on missing semicolon in type #954
  • Detect incorrect tokens
    • Currently \* @type: [RM: String];= returns the confusing error that found =ing= when it exepted =], because it's not checking for the full token in the type, but just taking the Str prefix.
  • Type aliases that aren't actually defined should raise a specific error reporting this mismatch
  • TypeAlias declarations in invalid positions should raise an error (is that possible?)
  • Format type errors. They are currently hard to read for nontrivial types, e.g.,
Expected ((Set((Int -> [Commits: Set(Str), header: [NextVS: Set(Str), VS: Set(Str), hight: Int, lastCommit: Set(Str), time: Int]])), [Commits: Set(Str), header: [NextVS: Set(Str), VS: Set(Str), hight: Int, lastCommit: Set(Str), time: Int]]) => [Commits: Set(Str), header: [NextVS: Set(Str), VS: Set(Str), hight: Int, lastCommit: Set(Str), time: Int]]) in LightStoreUpdateBlocks. Found: ((Set((Int -> [Commits: Set(Str), header: [NextVS: Set(Str), VS: Set(Str), hight: Int, lastCommit: Set(Str), time: Int]])), [Commits: Set(Str), header: [NextVS: Set(Str), VS: Set(Str), hight: Int, lastCommit: Set(Str), time: Int]]) => (a438 -> [Commits: Set(Str), header: [NextVS: Set(Str), VS: Set(Str), hight: Int, lastCommit: Set(Str), time: Int]])) E@10:53:12.003
[Lightclient_003_draft.tla:213:1-216:49]: Error when computing the type of LightStoreUpdateBlocks E@10:53:12.015

We should instead format something like

In LightStoreUpdateBlocks:

Expected:

    (Set(Int -> [Commits: Set(Str), header: [NextVS: Set(Str), VS: Set(Str), hight: Int, lastCommit: Set(Str), time: Int]]), [Commits: Set(Str), header: [NextVS: Set(Str), VS: Set(Str), hight: Int, lastCommit: Set(Str), time: Int]]) 
    => [Commits: Set(Str), header: [NextVS: Set(Str), VS: Set(Str), hight: Int, lastCommit: Set(Str), time: Int]]
   
   
Found:

    (Set(Int -> [Commits: Set(Str), header: [NextVS: Set(Str), VS: Set(Str), hight: Int, lastCommit: Set(Str), time: Int]]), [Commits: Set(Str), header: [NextVS: Set(Str), VS: Set(Str), hight: Int, lastCommit: Set(Str), time: Int]]) 
    => (a438 -> [Commits: Set(Str), header: [NextVS: Set(Str), VS: Set(Str), hight: Int, lastCommit: Set(Str), time: Int]])

Achieved via indentation, sensible line breaks, and removing redundant parens.

  • Use typeAlias in scope instead of fully expanded type in type errors, but also report the full type of the alias at the end of the error. That would make errors like the previous much easier to read.
@shonfeder shonfeder added the bug label May 4, 2021
@shonfeder shonfeder self-assigned this May 4, 2021
@konnov
Copy link
Collaborator

konnov commented May 7, 2021

One issue is that the type checker is working at the level of EtcExpr, which lacks information about the original expressions. I think it makes sense to introduce another pass similar to ToEtcExpr that goes over TlaEx again and reports on errors by knowing the id of a problematic TlaEx.

@konnov konnov added the usability UX improvements label Nov 8, 2021
@shonfeder shonfeder removed the bug label Dec 13, 2021
@shonfeder shonfeder changed the title [BUG] Improve error messages around type annotations [FEATURE] Improve error messages around type annotations Dec 13, 2021
@shonfeder shonfeder added this to the Erorr Message Overhaul milestone Dec 13, 2021
@konnov konnov added the product-owner-triage This should be triaged by the product owner label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product-owner-triage This should be triaged by the product owner usability UX improvements
Projects
None yet
Development

No branches or pull requests

2 participants