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

Tracking issue for great error messages #40

Open
Nadrieril opened this issue Mar 30, 2019 · 8 comments
Open

Tracking issue for great error messages #40

Nadrieril opened this issue Mar 30, 2019 · 8 comments
Labels
help wanted Extra attention is needed tracking-issue Gather progress on a particular topic
Milestone

Comments

@Nadrieril
Copy link
Owner

No description provided.

@Nadrieril Nadrieril added the postponed This issue is low priority label Apr 14, 2019
@Nadrieril Nadrieril added this to the v1.0 milestone Apr 15, 2019
@Nadrieril Nadrieril removed the postponed This issue is low priority label Sep 4, 2019
@Nadrieril
Copy link
Owner Author

I'd like to follow the style of Rust error messages, because they rock.
Note to self: have a look at https://github.com/rust-lang/rust/blob/master/src/librustc_errors/lib.rs

@Nadrieril
Copy link
Owner Author

First step done ! #114

@Nadrieril Nadrieril changed the title Great type error messages Tracking issue for great type error messages Nov 11, 2019
@Nadrieril
Copy link
Owner Author

Next step: #116

@Nadrieril Nadrieril added the tracking-issue Gather progress on a particular topic label Nov 11, 2019
@Nadrieril
Copy link
Owner Author

Most of the groundwork is done, now there's mostly a need of writing nice error messages.

@Nadrieril Nadrieril changed the title Tracking issue for great type error messages Tracking issue for great error messages Mar 5, 2020
@Nadrieril
Copy link
Owner Author

The state of error messages can be seen in the .txt files in dhall/tests/.../failure/.... For example:

Type error: error: AssertMismatch
--> <current file>:1:0
|
1 | assert : 1 === 2
| ^^^^^^^^^^^^^^^^ AssertMismatch
|

This is not amazing yet, but at least it points to the correct location in the source file. Some errors fail to do even that, either because of some complicated Span story or because we don't even try (see e.g. import errors).
The goal would be better error messages like this one:
Type error: error: wrong type of function argument
--> <current file>:1:1
|
1 | (λ(_ : Natural) → _) True
| ^^^^^^^^^^^^^^^^^^ this expects an argument of type: Natural
| ^^^^ but this has type: Bool
|
= note: expected type `Natural`
found type `Bool`

@Nadrieril Nadrieril added the help wanted Extra attention is needed label Mar 5, 2020
@Nadrieril Nadrieril pinned this issue Mar 18, 2020
@rushsteve1
Copy link

Errors are not pretty printed in test outputs. It seems like it is attempting to use the Debug version instead. For example this is the output when I tried to create a test case for the issue in #173

---- test_rename_all_camel_case stdout ----
thread 'test_rename_all_camel_case' panicked at 'called `Result::unwrap()` on an `Err` value: Error(Dhall(Error { kind: Typecheck(TypeError { message: Custom("error: annot mismatch: { function : Text, mergeVariant : Optional Text, postVariant : Optional Text, preVariant : Optional Text } != { function : Text, `merge_variant` : Optional Text, `post_variant` : Optional Text, `pre_variant` : Optional Text }\n --> <current file>:2:1\n  |\n... |\n6 | |     }\n  | |______^ annot mismatch: { function : Text, mergeVariant : Optional Text, postVariant : Optional Text, preVariant : Optional Text } != { function : Text, `merge_variant` : Optional Text, `post_variant` : Optional Text, `pre_variant` : Optional Text }\n  |") }) }))', serde_dhall/tests/rename.rs:33:10

@Nadrieril
Copy link
Owner Author

You get this output because you used .unwrap() in your test case. .unwrap() will indeed use the Debug impl to display the values, which is not nice here. The way I would do it here is just define a parse function that extracts the error and manually prints is with its Display impl. Something like

fn parse<T: FromDhall + StaticType>(s: &str) -> T {
	match from_str(s).static_type_annotation().parse() {
		Ok(x) => x,
		Err(e) => panic!("{}", e),
	}
}

and then use parse::<Transform>(test_str). It's a bit annoying but the Debug impl is often useful for tricky cases so I want to keep it around.

Nadrieril added a commit that referenced this issue Nov 1, 2020
This is crude but helpful. See #40
@alefminus
Copy link

I created a relevant issue: #228

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed tracking-issue Gather progress on a particular topic
Projects
None yet
Development

No branches or pull requests

3 participants