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: Match expressions with DUs missing parentheses around their tupled arguments #1511

Open
Tracked by #1103
rmunn opened this issue Sep 1, 2016 · 2 comments
Labels
Feature Improvement Theme-Simple-F# A cross-community initiative called "Simple F#", keeping people in the sweet spot of the language.
Milestone

Comments

@rmunn
Copy link

rmunn commented Sep 1, 2016

What

Another suggestion for #1103. At 27:36 of this F# tutorial video, the presenter deliberately types the following incorrectly-typed code:

type Exit =
    | PassableExit of description : string * roomId : string
    | LockedExit of key : string * description : string * exitOnceUnlocked : Exit

let getExit direction exits =
    match (direction exits) with
    | PassableExit _, roomId -> Success roomId
    | LockedExit _, _, _ -> Failure "That exit is locked"

At this point, he gets a type error on the LockedExit match case in getExit:

Type mismatch. Expecting a
    'Exit * 'a'    
but given a
    'Exit * 'a * 'b'    
The tuples have differing lengths of 2 and 3

In the video, he goes on to explain that his error was actually on the previous line: he should have written PassableExit (_, roomId) -> Success roomId (and likewise, should have put parentheses around _, _, _ in the LockedExit line as well). And he does explain that when you're using discriminated unions, you should put parentheses around a DU case's parameters. But it struck me that this is another case where the compiler ought to be able to detect the mistake and nudge the user towards correcting it.

Why

A novice user would look at that error message and be totally confused. "Where's it getting a tuple? I didn't pass it a tuple! I passed it a LockedExit value, and said I didn't care about any of the data that LockedExit carries with it. So where's the tuple?" Then he might look at Exit * 'a * 'b and think "Oh, I see. LockedExit is of type exit, and then the 'a and 'b are two of its three parameters... but where's the third parameter? I gave it three parameters, why does F# only see two parameters?" He'd be on the road to understanding the error at this point, but he probably wouldn't realize that at all, as he'd be too stuck on the "mystery of the missing parameter that's RIGHT THERE in my code" to take a step back and figure out that the operator precedence of tuple creation is lower than function application and so his line looks like (LockedExit _), _, _ to the compiler.

How

I would like to see one of two things:

  1. A compiler warning on PassableExit _, roomId -> ... that says

PassableExit takes a 2-tuple of (description : string) * (roomId : string), but you have passed it a single wildcard _. Your match case is currently equivalent to (PassableExit _), roomId Did you mean to write PassableExit (_, roomId) instead?

This compiler warning could be disabled either with a #nowarn directive, or by putting explicit parentheses around (PassableExit _) if that's what the coder actually meant to do.

Or: 2) Similar to 1, but instead of a warning, that's an error message that only displays when there's a type error in the match expression. If the entire match expression compiles and its types can all resolve correctly, then the coder probably knew what he was doing and doesn't need a warning. But if there's a type error involving tuples of different sizes in the match expression, then put the type error on the first tuple of the match expression and make it say "Did you mean to write (DU with correct parentheses) instead?"

After some thought, my preference is for 2. If the coder has managed to make this particular mistake, it's very unlikely that his code will compile as-is -- because the expression he's matching on (direction exits in this sample code) probably produces a single DU and he's matching it against a tuple. So getExit will type it as 'a -> (some tuple) instead of 'a -> Exit. And so even if his tuples are all the same arity, he'll get a type mismatch at the point where he calls getExit. Therefore, if his code does compile without type errors, it's very likely that he knew exactly what he was doing and didn't need the warning.

This should be pretty straightforward to detect. If

  1. there's a type error involving tuples of different sizes in the match expression, and
  2. the tuples all happen to have a DU case as their first item, and
  3. each tuple's arity exactly matches the arity of its DU case,

then it's very likely that this is a novice "forgot the parentheses around the DU case's parameters" error, and the customized "Did you mean to write (DU with correct parentheses) instead?" error should be given. There's also a possibility that the match case typechecks, because all the DU cases were of the same arity and therefore so were the incorrectly-tupled cases. If that happens, then the type error will show up elsewhere (in this sample, it would be at the point where the getExit function gets called, because the direction parameter passed in to getExit won't have the type the coder thinks it has, and so when he passes it a function he'll cause a type error). I don't know if it's possible to detect this error, and give a customized error message, in that scenario (where the match expression manages to compile and the first type error happens far away). But in most scenarios, the type error should show up in the match expression. And if the match expression does have a type error, and matches all three prongs of the test (tuples of different sizes in the cases, DU as first item of the tuples, each tuple's arity matches DU case's arity) then it's virtually certain that this is what's going on, and the user would benefit from a customized error message.

@cartermp cartermp added this to the Unknown milestone Aug 25, 2018
@dsyme dsyme added the Theme-Simple-F# A cross-community initiative called "Simple F#", keeping people in the sweet spot of the language. label Sep 16, 2021
@KathleenDollard
Copy link

I played with this example, and there are a few ways to get to a confusing spot. Here's the code of the issue updated:

type Exit =
    | PassableExit of description : string * roomId : string
    | LockedExit of key : string * description : string * exitOnceUnlocked : Exit

let getExit direction exits =
    match (direction exits) with
    | PassableExit _, roomId -> Ok roomId
    | LockedExit _, _, _ -> Error "That exit is locked"

Is there a legal and sane interpretation of the code above with (PassableExit _), roomId ? IOW, when should that be valid code?

@baronfel
Copy link
Member

if direction was a tuple-returning function then the syntax (PassableExit _), roomId would be perfectly fine: it would imply that the first element of the tuple was a PassableExit but not extract any of the fields, and bind the second element of the tuple to roomId for the match case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Improvement Theme-Simple-F# A cross-community initiative called "Simple F#", keeping people in the sweet spot of the language.
Projects
Archived in project
Development

No branches or pull requests

7 participants