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

Better error reporting for let bindings. #17601

Merged
merged 44 commits into from
Sep 5, 2024

Conversation

edgarfgp
Copy link
Contributor

@edgarfgp edgarfgp commented Aug 24, 2024

Description

  • Updates TcNormalizedBinding to use the SynPat range to show more accurate error ranges.

Before

[<VolatileField>]
^^^^^^^^^^^^^^^^^
let mutable x = 6
^^^^^^^^^^^^^^

After

[<VolatileField>]
let mutable x = 6
            ^
  • Adds a new compiler error for Multi-case partial active patterns

Before

We just showed an unclear error message that did not make it clear that multi-case partial active patterns are not supported.

let (|A|B|_|) = None  // FS0827: This is not a valid name for an active pattern

After

let (|A|B|_|) = None  // FS3872: Multi-case partial active patterns are not supported. Consider using a single-case partial active pattern or a full active pattern.
  • Updates FS0827 to include the active pattern's name

Before

FS0827: This is not a valid name for an active pattern

After

FS0827: (|A|B|)' is not a valid method name. Use a 'let' binding instead.

Checklist

  • Test cases added
  • Release notes entry updated

Copy link
Contributor

github-actions bot commented Aug 24, 2024

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/9.0.100.md

@edgarfgp edgarfgp force-pushed the better-let-binding-error-range branch from 9ad6df3 to 7eb8ad2 Compare August 26, 2024 06:16
@edgarfgp edgarfgp force-pushed the better-let-binding-error-range branch from 7eb8ad2 to 49c59fb Compare August 26, 2024 07:48
@edgarfgp edgarfgp force-pushed the better-let-binding-error-range branch from d785305 to 14b9b61 Compare August 26, 2024 14:39
edgarfgp and others added 2 commits August 26, 2024 17:46
Co-authored-by: Brian Rourke Boll <brianrourkeboll@users.noreply.github.com>
@edgarfgp edgarfgp changed the title Use SynPat range for let binding errors Better error reporting for let bindings. Aug 26, 2024
@edgarfgp
Copy link
Contributor Author

edgarfgp commented Aug 30, 2024

@psfinaki Thanks a lot for the help. You are a Legend. Will be updating the PR description soon to cover and explain all the goodness here :)

@edgarfgp edgarfgp marked this pull request as ready for review August 30, 2024 20:41
@edgarfgp edgarfgp requested a review from a team as a code owner August 30, 2024 20:41
@vzarytovskii
Copy link
Member

@psfinaki @T-Gro @abonie please review, we will have to either merge it soon, or wait until after 9.0.100 release, since SDK freeze is soon (in couple of weeks), and we will need to merge it and wait for translations.

Copy link
Member

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I this this is in the good shape, thanks Edgar!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants