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 #15559

Closed
Martin521 opened this issue Jul 5, 2023 · 2 comments
Labels
Area-LangService-Diagnostics FCS code analysis, diagnostics, red squigglies Feature Request
Milestone

Comments

@Martin521
Copy link
Contributor

Martin521 commented Jul 5, 2023

I propose to 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.

The 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.

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.

Additional context

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.

Fun fact

The example code in this comment in the compiler code base confirms the typical confusion by assuming a union case deconstruction without the parentheses.

Related

#1103

EDIT March 2024:
Just in case somebody is looking into this again:
The F# spec actually defines the behavior for this ambiguity in §14.6.1.
And in hindsight, I should have created an analyzer instead of a warning that nobody will ever see.

@charlesroddie
Copy link
Contributor

let Case1 x = t // shouldn't this emit a warning?

You can warn on unused variables and get a warning for unused x.

This issue applies to complete active patterns too.

I think that matching with let symbol(x) = is bad though and it would be better to have a setting which warns on this and suggests replacing it with let x = match ...:

  • The overlap with function syntax is very strong and function syntax is bound to dominate, so most readers will be confused by the pattern syntax defining x instead of taking x as an input.
  • The only cases are complete active patterns (rare) and DUs with only one case (but the whole point of being discriminated is to allow for more than one case), so the value provided is very small.

@Martin521
Copy link
Contributor Author

Fixed in #16064

@github-project-automation github-project-automation bot moved this from Future to Done in F# Compiler and Tooling Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-LangService-Diagnostics FCS code analysis, diagnostics, red squigglies Feature Request
Projects
Archived in project
Development

No branches or pull requests

3 participants