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

add code fix for wildcard _ in pattern match for fs43 on |-> #1157

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jkone27
Copy link
Contributor

@jkone27 jkone27 commented Sep 1, 2023

Suggest | _ -> for match mistaken with infix operator |->

add code fix for wildcard operator suggestion in pattern match for fs43, targets:
dotnet/fsharp#15748

as worked in amplifying fsharp session:
https://amplifying-fsharp.github.io/sessions/2023/09/01/

WHY

the operator |-> is not defined, so this is usually just a user forgetting to complete the match

HOW

🤖 Generated by Copilot at df83900

  • Add a new code fix for adding a missing wildcard operator to a match expression (link, link)
    • Define a new module FsAutoComplete.CodeFix.AddMissingWildcardOperator in src/FsAutoComplete/CodeFixes/AddMissingWildcardOperator.fs and its signature file in src/FsAutoComplete/CodeFixes/AddMissingWildcardOperator.fsi (link, link)
    • Export a title, a function to try to find the pattern that needs the fix, and a function to apply the fix using the CodeFix type and the SyntaxVisitorBase and SyntaxTraversal utilities (link, link)
  • Register the new code fix function in the language server implementations (link, link)
    • Add the function to the list of code fixes supported by the AdaptiveFSharpLspServer type in src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs (link)
    • Add the function to the list of code fixes supported by the FSharpLspServer type in src/FsAutoComplete/LspServers/FsAutoComplete.Lsp.fs (link)
  • Add a test case for the new code fix using the test helpers (link, link)
    • Define a new test module FsAutoComplete.Tests.CodeFixTests.AddMissingWildcardOperatorTests in test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddMissingWildcardOperatorTests.fs (link)
    • Check that the code fix can suggest a wildcard pattern for a missing match case in a simple example using the CodeFix and Diagnostics utilities (link)
    • Add the new test module to the list of test modules in the tests function in test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs (link)

Copy link
Member

@TheAngryByrd TheAngryByrd left a comment

Choose a reason for hiding this comment

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

@dawedawe mentioned some good fixes

jkone27 and others added 2 commits September 6, 2023 10:52
@baronfel
Copy link
Contributor

baronfel commented Sep 6, 2023

Needs a format but otherwise LGTM.

@jkone27
Copy link
Contributor Author

jkone27 commented Sep 13, 2023

fixed the suggestions, not clear how i run format, can it be added as a git hook at commit phase on master? i am not very familiar with fantomas, would be good if it could be hooked onto dotnet format or .editorconfig i guess most .net devs are familiar with those concepts. formatted the changed files individually with
dotnet fantomas filename.fs

@nojaf
Copy link
Contributor

nojaf commented Sep 14, 2023

Hi,

Running this target sets up the git hook:
https://github.com/fsharp/FsAutoComplete/blob/e3353360771969be51ab3c84a57a432cf41816cf/build/Program.fs#L107-L110

dotnet run --project ./build/build.fsproj -- -t EnsureRepoConfig

And I think it would be nice to see a message when
https://github.com/fsharp/FsAutoComplete/blob/e3353360771969be51ab3c84a57a432cf41816cf/Directory.Solution.targets#L10-L12

fails.

Something like Please run "dotnet fantomas src" to format your files.
I'm not sure how to do that one with msbuild trickery.

@edgarfgp
Copy link
Contributor

edgarfgp commented Sep 27, 2023

@jkone27 Could you have a look at the conflicts. Seems that this almost ready :)

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

Successfully merging this pull request may close these issues.

6 participants