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

RemoveUnusedBinding offers to remove unused parameters but doesn't remove them #11741

Open
Tracked by #15408
baronfel opened this issue Jun 28, 2021 · 2 comments
Open
Tracked by #15408
Labels
Area-LangService-CodeFixes Code fixes associated with diagnostics Bug good first issue
Milestone

Comments

@baronfel
Copy link
Member

baronfel commented Jun 28, 2021

Is your feature request related to a problem? Please describe.

I think this reported issue/feature gap in FSAC is reproducible in the VS tooling as well.

For the code:

let inc i =
    2

with --warnon:1182 enabled, there will be a diagnostic issued for the parameter i, but the current codefix will not handle this scenario.

Describe the solution you'd like

I think a small change to the TryRangeOfBindingWithHeadPatternWithPos member could be made very similarly to what I've done in this PR, and the range that is derived from this member could be tagged as coming from a pattern or a binding.

Then, later in the codefix that result would be inspected to walk the text in slightly different ways for patterns and bindings, so that both kinds of triggering entities for this diagnostic could be handled.

Describe alternatives you've considered

Not doing this work, so that only nested bindings are removable.

@dsyme dsyme added the Area-LangService-CodeFixes Code fixes associated with diagnostics label Apr 20, 2022
@vzarytovskii vzarytovskii moved this to Not Planned in F# Compiler and Tooling Jun 17, 2022
@vzarytovskii vzarytovskii added this to the Backlog milestone Oct 19, 2022
@psfinaki psfinaki changed the title Enhancement to RemoveUnusedBinding codefix to remove unused parameter patterns as well RemoveUnusedBinding offers to remove unused parameters but doesn't remove them Jun 15, 2023
@psfinaki psfinaki added the Bug label Jun 15, 2023
@psfinaki
Copy link
Member

Hah this has somehow transformed into a bug as of now - as the fix is suggested but not applied.

bug.mp4

@psfinaki
Copy link
Member

There are a few extra things to consider here:

  1. Spacing
  2. Parameters can have explicit types specifies - they should be removed as well
  3. Removing a parameter in one-parameter function shouldn't turn the function into a value (let f x = 42 -> let f() = 42)

@psfinaki psfinaki self-assigned this Jul 31, 2023
@psfinaki psfinaki modified the milestones: Backlog, August-2023 Jul 31, 2023
@psfinaki psfinaki modified the milestones: August-2023, Backlog Sep 7, 2023
@psfinaki psfinaki removed their assignment Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-LangService-CodeFixes Code fixes associated with diagnostics Bug good first issue
Projects
Status: New
Development

No branches or pull requests

4 participants