Skip to content

Better error reporting for let bindings. - #17601

Merged
abonie merged 44 commits into
dotnet:mainfrom
edgarfgp:better-let-binding-error-range
Sep 5, 2024
Merged

Better error reporting for let bindings.#17601
abonie merged 44 commits into
dotnet:mainfrom
edgarfgp:better-let-binding-error-range

Conversation

@edgarfgp

@edgarfgpedgarfgp commented Aug 24, 2024

Copy link
Copy Markdown
Contributor

Description

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

Before

[<VolatileField>]^^^^^^^^^^^^^^^^^let mutablex=6^^^^^^^^^^^^^^

After

[<VolatileField>]let mutablex=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

@github-actions

github-actionsBot commented Aug 24, 2024

Copy link
Copy Markdown
Contributor

❗ Release notes required


✅ Found changes and release notes in following paths:

Change pathRelease notes pathDescription
src/Compilerdocs/release-notes/.FSharp.Compiler.Service/9.0.100.md

Comment threadsrc/Compiler/FSComp.txt Outdated
edgarfgpand others added 2 commits August 26, 2024 17:46
Co-authored-by: Brian Rourke Boll <brianrourkeboll@users.noreply.github.com>
Comment threadsrc/Compiler/FSComp.txt Outdated
@edgarfgpedgarfgp changed the title Use SynPat range for let binding errorsBetter error reporting for let bindings.Aug 26, 2024
@edgarfgp

edgarfgp commented Aug 30, 2024

Copy link
Copy Markdown
ContributorAuthor

@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 :)

Comment threadsrc/Compiler/Checking/Expressions/CheckExpressions.fs
Comment threadsrc/Compiler/Checking/Expressions/CheckExpressions.fs
Comment threadsrc/Compiler/Checking/Expressions/CheckExpressions.fs
@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 ownerAugust 30, 2024 20:41
Comment threadsrc/Compiler/FSComp.txt
Comment threadtests/fsharp/typecheck/sigs/neg45.bsl
@vzarytovskii

Copy link
Copy Markdown
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.

@psfinakipsfinaki left a comment

Copy link
Copy Markdown
Contributor

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 freeto 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

@edgarfgp@psfinaki@vzarytovskii@brianrourkeboll@abonie