Remove unused record named pattern quick fix

[edgarfgp]

This PR adds a new quick to Remove unused record named pattern quick fix . This can be extended to work with named union types in a separate PR :)

[auduchinok]

@edgarfgp This is nice work!

The quick fix may remove extra nodes in some cases, and it would be better not to do it. I left a suggestion on how to better calculate the tree range to remove.

[auduchinok]

This approach doesn't look at what could be between the field patterns, and there are some important cases to consider when using it:

We probably don't want to remove the new line between the fields here:

match r with
| { F1 = 1; F2 = f2
    F3 = 3; F4 = 4 } -> ()

While here we would not want to keep an empty line:

match r with
| { F1 = 1
    F2 = f2
    F3 = 3 } -> ()

What I'd suggest trying to do here is this:

• get the field pattern node
• check if it's the last field pattern on the line, i.e. if fieldPat.EndLine <> nextPattern.StartLine
  • if it's the last one: try to go left from it and possibly include the previous new line token, like in the last example, using getFirstMatchingNodeBefore isInlineSpace fieldPat |> getThisOrPrevNewLine helpers
• try to move to the right using getLastMatchingNodeAfter isInlineSpace

Please check existing usages of these helpers.

[auduchinok]

Some more cases to add a test for:

match r with
| { F1 = 1; F2 = 2
    F3 = f3; F4 = 4 } -> ()

match r with
| { F1 = 1; F2 = 2
    F3 = f3 } -> ()

match r with
| { F1 = 1; F2 = f2 // comment
    F3 = 3 } -> ()

[auduchinok]

This changes the code meaning when there're other branches or union cases (included when they are added in the future). This is the case we've discussed previously. You should not remove the whole union case pattern when removing its last field.

In the following example the value of x is going to be different after this quick fix is applied:

type U =
    | A of i: int
    | B

let x =
    match B with
    | A(i = i) -> 1
    | B -> 2

[edgarfgp]

Ok I see. So in this case after applying the quick fix the result would be ?:

let x =
    match B with
    | A  _ -> 1
    | B -> 2

[auduchinok]

warning.Pat.IgnoreInnerParens()

Could you explain why this was needed, please?
UnusedValueWarning is currently only created with IReferencePattern, never with IParenPat.

resharper-fsharp/ReSharper.FSharp/src/FSharp.Psi.Daemon/src/Stages/FcsErrorsStageProcessBase.fs

Line 293 in c62d6a9

UnusedValueWarning(refPat) :> _

[auduchinok]

warning.Pat.IgnoreInnerParens().Parent

Please use FieldPatNavigator, not the lower level direct parent access.

[auduchinok]

What checking other patterns is needed for?

[auduchinok]

Please use FieldPatNavigator instead of accessing Parent directly.

[auduchinok]

Why IgnoreParentChameleonExpr is called here?
To my knowledge, there should be no cases where a fieldPat may be a direct child of a chameleon expression node.