Simplify error messages reported by the compiler #853

2021-12-20T20:18:53+09:00

ghallak commented

2021-12-20 20:18:53 +09:00

(Migrated from gitlab.com)

Fixes: #354

This PR changes most error messages to:

Remove formatting newlines (between sentences).
Remove trailing newlines (they will only show when print the error message using aeso_errors:pp but not in #err.msg).
Remove position info (e.g. at line x, col y) when it's already available from #err.pos.
Surround code with ` ` to make it distinguishable from the rest of the error message ("Unbound variable x", instead of "Unbound variable x").

Fixes: #354 This PR changes most error messages to: 1. Remove formatting newlines (between sentences). 2. Remove trailing newlines (they will only show when print the error message using `aeso_errors:pp` but not in `#err.msg`). 3. Remove position info (e.g. `at line x, col y`) when it's already available from `#err.pos`. 4. Surround code with \` \` to make it distinguishable from the rest of the error message ("Unbound variable `x`", instead of "Unbound variable x").

zxq9 commented

2021-12-28 02:22:38 +09:00

(Migrated from gitlab.com)

Created by: dincho

IMO the compiler lib should not do any formatting at all but provide a list of structured errors downstream, that is to the HTTP and CLI compilers.

As an example structure I've in mind would be: type, line, column, message. hint

*Created by: dincho* IMO the compiler lib should not do any formatting at all but provide a list of structured errors downstream, that is to the HTTP and CLI compilers. As an example structure I've in mind would be: type, line, column, message. hint

zxq9 commented

2021-12-28 02:34:42 +09:00

(Migrated from gitlab.com)

Created by: dincho

As an example of very hard to parse error message is:

➜  code_errors git:(master) aesophia_cli polymorphic_query_type.aes
Code generation error in 'polymorphic_query_type.aes' at line 3, col 5:
Invalid oracle type
  oracle('a, 'b)
The query type must not be polymorphic (contain type variables).

A lot of newlines.

Another example duplicating position information and non-sense newlines:

➜  contracts git:(master) aesophia_cli 05_greeter.aes 
Type error in '05_greeter.aes' at line 48, col 10:
The contract Greeter (at line 48, column 10) has no entrypoints. Since Sophia version 3.2, public
contract functions must be declared with the 'entrypoint' keyword instead of
'function'.

Type error in '05_greeter.aes' at line 53, col 5:
Use 'entrypoint' for declaration of blockheight (at line 53, column 5):
  entrypoint blockheight : (unit) => uint

*Created by: dincho* As an example of very hard to parse error message is: ``` ➜ code_errors git:(master) aesophia_cli polymorphic_query_type.aes Code generation error in 'polymorphic_query_type.aes' at line 3, col 5: Invalid oracle type oracle('a, 'b) The query type must not be polymorphic (contain type variables). ``` A lot of newlines. Another example duplicating position information and non-sense newlines: ``` ➜ contracts git:(master) aesophia_cli 05_greeter.aes Type error in '05_greeter.aes' at line 48, col 10: The contract Greeter (at line 48, column 10) has no entrypoints. Since Sophia version 3.2, public contract functions must be declared with the 'entrypoint' keyword instead of 'function'. Type error in '05_greeter.aes' at line 53, col 5: Use 'entrypoint' for declaration of blockheight (at line 53, column 5): entrypoint blockheight : (unit) => uint ```

zxq9 commented

2021-12-28 22:02:29 +09:00

(Migrated from gitlab.com)

Created by: hanssv

@dincho I think this is a good compromise, you can get a raw(ish) form of the error from the compilier, and you can ask the compiler to format it further. Or you can get the information in JSON. The reason for keeping it all in aesophia is to make updates/additions less cumbersome - otherwise you'd have to remember there is a new error when updating aesophia_cli/http, etc.

I don't see any major issues with the provided examples (they should no longer have duplicated position information!?) - separating different errors with a newline makes sense I think?

*Created by: hanssv* @dincho I think this is a good compromise, you can get a raw(ish) form of the error from the compilier, and you can ask the compiler to format it further. Or you can get the information in JSON. The reason for keeping it all in `aesophia` is to make updates/additions less cumbersome - otherwise you'd have to remember there is a new error when updating aesophia_cli/http, etc. I don't see any major issues with the provided examples (they should no longer have duplicated position information!?) - separating different errors with a newline makes sense I think?

zxq9 commented

2021-12-28 22:02:38 +09:00

(Migrated from gitlab.com)

Created by: hanssv

Review: Approved

*Created by: hanssv* **Review:** Approved

ghallak commented

2021-12-30 01:17:43 +09:00

(Migrated from gitlab.com)

@dincho Could you go over the changes to the files test/aeso_compiler_tests.erl and src/aeso_ast_infer_types.erl, to see if there are any error (or warning) messages that you think could be changed to be better?

@dincho Could you go over the changes to the files `test/aeso_compiler_tests.erl` and `src/aeso_ast_infer_types.erl`, to see if there are any error (or warning) messages that you think could be changed to be better?

zxq9 commented

2022-01-05 15:42:58 +09:00

(Migrated from gitlab.com)

Created by: dincho

separating different errors with a newline makes sense I think?

that's for sure, but I still see multiple newlines in a "single" error messages in the tests cc @ghallak

that's my main issue, for a practical example I'm building a sublime plugin that parses the CLI output, however using \n for error messages separator (which is fine, and is used in all the compilers I'm aware of), the usage of \n in the error message itself makes it un-parsable.

I've seen related issues for other languages reported elsewhere, and it seems to be e common (seen) practice in functional language compilers to use newlines in the error messages itself. Still, it does not makes much sense to me.

*Created by: dincho* > separating different errors with a newline makes sense I think? that's for sure, but I still see multiple newlines in a "single" error messages in the tests cc @ghallak that's my main issue, for a practical example I'm building a sublime plugin that parses the CLI output, however using \n for error messages separator (which is fine, and is used in all the compilers I'm aware of), the usage of \n in the error message itself makes it un-parsable. I've seen related issues for other languages reported elsewhere, and it seems to be e common (seen) practice in functional language compilers to use newlines in the error messages itself. Still, it does not makes much sense to me.

zxq9 commented

2022-01-05 16:47:37 +09:00

(Migrated from gitlab.com)

Created by: hanssv

The primary purpose of the errors output by aesophia_cli is to be read by a human rather than parsed by a tool! Perhaps we could add a switch (--raw_errors or something similar) to aesophia_cli?

And no, putting \n into a string doesn't make it unparseable but that discussion we've had before 😅

*Created by: hanssv* The primary purpose of the errors output by `aesophia_cli` is to be _read by a human_ rather than _parsed by a tool_! Perhaps we could add a switch (`--raw_errors` or something similar) to `aesophia_cli`? And no, putting `\n` into a string doesn't make it unparseable but that discussion we've had before 😅

zxq9 commented

2022-01-05 18:28:43 +09:00

(Migrated from gitlab.com)

Created by: dincho

The primary purpose of the errors output by aesophia_cli is to be read by a human rather than parsed by a tool! Perhaps we could add a switch (--raw_errors or something similar) to aesophia_cli?

And no, putting \n into a string doesn't make it unparseable but that discussion we've had before 😅

Humans are lazy and use tools .. 😅

*Created by: dincho* > The primary purpose of the errors output by `aesophia_cli` is to be _read by a human_ rather than _parsed by a tool_! Perhaps we could add a switch (`--raw_errors` or something similar) to `aesophia_cli`? > > And no, putting `\n` into a string doesn't make it unparseable but that discussion we've had before 😅 Humans are lazy and use tools .. 😅

zxq9 commented

2022-01-05 18:29:32 +09:00

(Migrated from gitlab.com)

Created by: dincho

And no, putting \n into a string doesn't make it unparseable but that discussion we've had before

Well, define that in regex, then we talk again :)

*Created by: dincho* > And no, putting `\n` into a string doesn't make it unparseable but that discussion we've had before Well, define that in regex, then we talk again :)

zxq9 commented

2022-01-05 18:41:37 +09:00

(Migrated from gitlab.com)

Created by: hanssv

Well... parsing != regex 😈

Normally you use /s or ?s as modifier to do multiline regex?

*Created by: hanssv* Well... `parsing != regex` 😈 Normally you use `/s` or `?s` as modifier to do multiline regex?

zxq9 commented

2022-01-05 19:22:14 +09:00

(Migrated from gitlab.com)

Created by: hanssv

Let's merge this, and I'll tackle @dincho's issue in a follow up PR (and aesophia_cli).

*Created by: hanssv* Let's merge this, and I'll tackle @dincho's issue in a follow up PR (and `aesophia_cli`).

zxq9 commented

2022-01-05 19:22:22 +09:00

(Migrated from gitlab.com)

Merged by: hanssv at 2022-01-05 10:22:22 UTC

*Merged by: hanssv at 2022-01-05 10:22:22 UTC*

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: QPQ-AG/sophia#853