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: Emit warning when forgotten parentheses turn DU deconstruction into function definition #1282

Closed
5 tasks done
Martin521 opened this issue Jul 4, 2023 · 5 comments

Comments

@Martin521
Copy link

I propose we emit a warning when forgotten parentheses turn a DU deconstruction into a function definition that shadows the DU case.

This would remove a common case of puzzlement for developers.

type T = Case1 of int
let t = Case1 42
let Case1(x) = t  // shouldn't this emit a warning?
let t2 = Case1 "42"  // no error
let xx = x  // The value or constructor 'x' is not defined.

In line 3 of the above example, a DU deconstruction was intended, but the compiler silently interprets the line as a function definition. You need to put the left-hand side pattern into parentheses to avoid this. Since the new function shadows the DU constructor, there can be weird follow-up problems that can make the situation difficult to understand.

Pros and Cons

The advantage of making this adjustment to F# is the prevention of developer frustration and time loss. The above mistake is easily made, as patterns usually don't need to be enclosed in parentheses. And the mistake is not that easily found, as illustrated above.

The disadvantages of making this adjustment to F# are

  • There is a theoretical possibilty that somebody intentionally wants to shadow a DU constructor by a function. I cannot imagine a good use case for it. But even if somebody wants to do it, a warning does not hurt, and it can be suppressed for the file or the compilation.
  • Another (tiny) bit of complexity needs to be added to the compiler.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): S

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.

Please tick all that apply:

  • This is not a breaking change to the F# language design
  • I would be willing to help implement and/or test this

For Readers

If you would like to see this issue implemented, please click the 👍 emoji on this issue. These counts are used to generally order the suggestions by engagement.

@Martin521
Copy link
Author

Actually, according to both the the language guide (here and here) and the outdated FSharpSpec-4.1 (page 303), let Case1(x) = t is both a valid function binding and a valid value binding (i.e. in this case DU deconstruction). It seems the compiler silently gives precedence to the function binding.

@Happypig375
Copy link
Contributor

Not a language change, but rather a compiler change. Post here instead

@Martin521
Copy link
Author

Thank you @Happypig375. I agree it is not a language change. I was adviced, however, to bring it here.

image

@vzarytovskii
Copy link

That's kinda a new warning which can be a breaking change, and a change to the specification.

@Martin521
Copy link
Author

I have created a compiler issue here.

Not sure if I should close this here.

@vzarytovskii vzarytovskii closed this as not planned Won't fix, can't repro, duplicate, stale Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants