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

Qualify types as necessary in hints in warnings/errors #1647

Open
hdgarrood opened this issue Nov 20, 2015 · 7 comments

Comments

@hdgarrood
Copy link
Member

commented Nov 20, 2015

For example, I have a module which has a toplevel function which returns a Text.Parsing.Parsers.Pos.Position which isn't annotated with a type. The hint that appears with the warning says the inferred type was forall t11. t11 -> Position, however, if I write that verbatim, I get a type error "Unknown type Position".

Could we make it so that it if the type isn't already in scope, it gets qualified? So it could print the inferred type as Text.Parsing.Parsers.Pos.Position in this case.

@garyb

This comment has been minimized.

Copy link
Member

commented Nov 20, 2015

Yes! Also, if something is imported from a qualified module we should include the qualification in the error: currently in SlamData we've had trouble errors like "Could not unify type Query with Query", as there are two query algebras imported qualified from different modules.

This isn't super easy to do right now though, we'll have to introduce something so that after desugaring the identifier contains the pre-desugared value too.

@garyb garyb self-assigned this Nov 20, 2015

@paf31

This comment has been minimized.

Copy link
Member

commented Nov 20, 2015

Right now, we don't pretty-print module names, but another option would be to strip module names before putting a type/expression into an ErrorMessage value, whenever a name is in scope, and to pretty-print module names if they appear in a Qualified something.

We could keep the pre-desugared thing around, like for qualified imports, but it's probably fine to just show either no module (default if in scope) or the whole module (if not), surely?

@garyb

This comment has been minimized.

Copy link
Member

commented Nov 20, 2015

That works too, but since everything gets fully qualified in name desugaring it's still a bit of work to figure out whether it's in scope or not (it means checking things that arrive via re-export too, not just considering the list of imports directly).

It'd be nice to show the user the code they actually wrote in error messages too, where possible! 😄

@paf31 paf31 added this to the 0.9.0 milestone Nov 24, 2015

@paf31

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

@gary I've put this in 0.9, feel free to move it if you think it belongs elsewhere, I'm just doing some bookkeeping.

@paf31 paf31 closed this Nov 24, 2015

@paf31 paf31 reopened this Nov 24, 2015

@garyb

This comment has been minimized.

Copy link
Member

commented Nov 24, 2015

0.9 sounds good, now the end of 0.8 is in sight I'd like to get it wrapped up and out the door 😄

@kritzcreek

This comment has been minimized.

Copy link
Member

commented Jan 25, 2018

I'd like to solve this by using a pretty printer that supports annotations, and annotate all names with their respective qualification. That way editors or other interactive environments could show the module of the given identifier on hover and we can decide to print the qualification in the error message at the last step Doc -> Text.

@gabejohnson

This comment has been minimized.

Copy link

commented Aug 19, 2019

This bit me today. We have several modules in an application at work that use the same type name but are referenced qualified outside of the module. I changed the qualifier in one place but not another. The error was of little help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.