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

Record names are not suggested in the pattern matching #15889

Closed
psfinaki opened this issue Aug 28, 2023 · 7 comments · Fixed by #15903
Closed

Record names are not suggested in the pattern matching #15889

psfinaki opened this issue Aug 28, 2023 · 7 comments · Fixed by #15903
Labels
Area-LangService-AutoComplete autocomplete/intellisense Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Milestone

Comments

@psfinaki
Copy link
Member

This code:

type SomeType = { TheField : string }

let f x =
    match x with
    | { TheField = "A" } -> true
    | {  = "B" } -> true
    | _ -> false

... doesn't suggest TheField for the pattern matching rule at any time.

bug2.mp4

I am quite sure this is a bug and a regression. This is implied by this bug and by common sense.

@kerams
Copy link
Contributor

kerams commented Aug 29, 2023

I'll fix this.

@nojaf
Copy link
Contributor

nojaf commented Aug 29, 2023

I noticed the AST is also rather lacking https://fsprojects.github.io/fantomas-tools/#/ast?data=N4KABGBEAmCmBmBLAdrAzpAXFSAacUiaAYmolmPAIYA2as%2BEkAxgPZwWQAuAngA6wwAZVYBbWABV%2BggLxhgYCQAtYxRLBrQw2NFwBOKAOZgAvgB09Z5Bas1YXSmAAeYGTeQQIoql2ZLnYADuiFxK7p5gAD7yiipqGlpyZpAAgsmmYAC0AHxg%2BgCusOGe0QquYMkAQukmWbkFRZYeJWAA%2BnWUtPR4BJCwTnxUyNAUDSAmQA

I don't think there is any proper recovery for the { = "B" } pattern.

| Record of fieldPats: ((LongIdent * Ident) * range * SynPat) list * range: range

That most likely also needs addressing.

@kerams
Copy link
Contributor

kerams commented Aug 29, 2023

I'll fix this.

Actually, I don't think I broke it in my completions PRs. Are we positive this worked in 17.7 or earlier? There's nevertheless certainly room for improvement in terms of filtering, adding field names and suggesting pattern names to the right of =.

@psfinaki
Copy link
Member Author

Well the demo is on VS GA, 17.7.2.
I mean, I don't know if this got broken recently but I would be very surprised if it turns out that this never worked / is by design. :)

@kerams
Copy link
Contributor

kerams commented Aug 29, 2023

I wouldn't - specialized completions for patterns were non-existent just a couple of weeks ago. We only have a completion context for record expressions, which results in field names being inserted into the completions list.

@0101
Copy link
Contributor

0101 commented Aug 29, 2023

It's the same in 17.6.6.

@psfinaki
Copy link
Member Author

Hmhm alright, I'll remove the regression tag then for now.
Nevertheless - it would be definitely a useful addition.

@0101 0101 added Area-LangService-AutoComplete autocomplete/intellisense Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code. and removed Needs-Triage labels Sep 4, 2023
@github-project-automation github-project-automation bot moved this from Not Planned to Done in F# Compiler and Tooling Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-LangService-AutoComplete autocomplete/intellisense Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants