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

Improve error reporting: when type mismatch error involves a tuple, say it is a tuple #11234

Open
Tracked by #1103
cartermp opened this issue Mar 14, 2021 · 7 comments
Open
Tracked by #1103
Labels
Area-Diagnostics mistakes and possible improvements to diagnostics Feature Improvement
Milestone

Comments

@cartermp
Copy link
Contributor

Consider the following error message:

This expression was expected to have type
    'string'
but here has type
    ''a * 'b'

This can arise in code like this:

let g a b = "hello"

let f a b : string =
     g a, b

I got some feedback from a newcomer that this was confusing for several reasons:

  1. They weren't used to F# parameter applications yet and accidentally used a comma
  2. They didn't know what the * meant in the error message

The first confusion could potentially be made better by a code fixer.

The second one would need a better error message. I propose the following:

This expression was expected to have type:
    'string'
but here is a tuple of type:
    ''a * 'b'

This could still be confusing, but at least it's explicitly saying that it's a tuple type.

@cartermp
Copy link
Contributor Author

@isaacabraham wanna add this to your list?

@kerams
Copy link
Contributor

kerams commented Mar 14, 2021

A bit of a sidetrack, but what I find distracting at first glance are the apostrophes that enclose the type itself but also precede generic type parameters. Would it perhaps be possible to get rid of the former since the type is sitting on a line by itself?

@cartermp
Copy link
Contributor Author

It's possible, but also an annoyingly huge change to make. Many baseline files and strings of expected error messages where ' is used would have to get updated. What would you propose switching to?

@kerams
Copy link
Contributor

kerams commented Mar 14, 2021

Well, I was thinking in cases like this where the type signature is clearly separated from neighboring text, there'd be no need to visually escape it with extra characters.

@KathleenDollard
Copy link

@cartermp 's example is actually an even worse error message in my test:

Error FS0001 This expression was expected to have type
'string'
but here has type
'('a -> string) * 'b'

Adding the word tuple when there is a tuple will be a good change regardless, although I find the asterisk a speed bump in and of itself.

And I agree that removing the outer single quotes on the type would be super helpful. This example is a mess.

@dsyme
Copy link
Contributor

dsyme commented Nov 11, 2021

removing the outer single quotes on the type would be super helpful.

This is a systematic problem in our output of types, there are maybe 200+ error messages or more in FSComp.txt where we quote types with single quotes '...'. It's wrong and we should solve it.

I'd be happy to go through and modify all of them systematically if we have a policy. Using "...." probably won't make the error messages much better, but removing quotes altogether won't halpe except maybe on these types-on-a-separate-line cases.

@dsyme
Copy link
Contributor

dsyme commented Nov 11, 2021

Perhaps

Error FS0001 This expression was expected to have type
    string
but instead has tuple type with 2 elements

There is a core routine that displays the "minimal strings necessary to make it apparent that two types are different" (minimalStringsOfTwoTypes) and it's possible that this could be worked on and fix quite a range of messages. A change to that routine doing a careful case by case spec would be great, maybe moving to a two phase approach where we synthesize minimal information, then format.

Other examples might give

Error FS0001 This expression was expected to have type
    string
but instead has function type

Error FS0001 This expression was expected to have function type but instead has type
    string

and so on.

There is also a systematic addition of "context" for messages done by @forki which has greatly improved things

Note there's a risk that eliding information will mean people don't get the information they need to understand what's going on. But I think in the IDEs they get that information elsewhere nowadays. Everytime we've improved these core routines by eliding unnecessary information we've not regretted it

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 Feature Improvement
Projects
Status: New
Development

No branches or pull requests

5 participants