Skip to content

Commit

Permalink
Handle some simple cases of dotnet#16999
Browse files Browse the repository at this point in the history
  • Loading branch information
@brianrourkeboll
brianrourkeboll committed Apr 9, 2024
1 parent 005cc16 commit d31886c
Show file tree
Hide file tree
Showing 2 changed files with 240 additions and 0 deletions.
46 changes: 46 additions & 0 deletions src/Compiler/Service/SynPat.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,52 @@ module SynPat =
MemberKind = SynMemberKind.PropertyGetSet | SynMemberKind.PropertyGet | SynMemberKind.PropertySet
}))) :: _ -> true

// Parens must be kept when there is a multiline expression
// to the right whose offsides line would be shifted if the
// parentheses were removed from a leading pattern on the same line, e.g.,
//
// match maybe with
// | Some(x) -> let y = x * 2
// let z = 99
// x + y + z
// | None -> 3
//
// or
//
// let (x) = printfn "…"
// printfn "…"
| _ when
// This is arbitrary and will result in some false positives.
let maxBacktracking = 10

let rec wouldMoveRhsOffsides n pat path =
if n = maxBacktracking then
true
else
// This does not thoroughly search the trailing
// expression — nor does it go up the expression
// tree and search farther rightward, or look at record bindings,
// etc., etc., etc. — and will result in some false negatives.
match path with
// Expand the range to that of the outer pattern, since
// the parens may extend beyond the inner pat
| SyntaxNode.SynPat outer :: path when n = 1 -> wouldMoveRhsOffsides (n + 1) outer path
| SyntaxNode.SynPat _ :: path -> wouldMoveRhsOffsides (n + 1) pat path

| SyntaxNode.SynExpr(SynExpr.Lambda(body = rhs)) :: _
| SyntaxNode.SynExpr(SynExpr.LetOrUse(body = rhs)) :: _
| SyntaxNode.SynExpr(SynExpr.LetOrUseBang(body = rhs)) :: _
| SyntaxNode.SynBinding(SynBinding(expr = rhs)) :: _
| SyntaxNode.SynMatchClause(SynMatchClause(resultExpr = rhs)) :: _ ->
let rhsRange = rhs.Range
rhsRange.StartLine <> rhsRange.EndLine && pat.Range.EndLine = rhsRange.StartLine

| _ -> false

wouldMoveRhsOffsides 1 pat path
->
true

// () is parsed as this.
| SynPat.Const(SynConst.Unit, _), _ -> true

Expand Down
194 changes: 194 additions & 0 deletions vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2194,6 +2194,93 @@ let _ = (2 + 2) { return 5 }
[<Theory; MemberData(nameof infixOperatorsWithLeadingAndTrailingChars)>]
let ``Infix operators with leading and trailing chars`` expr expected = expectFix expr expected

let failing =
memberData {
// See https://github.com/dotnet/fsharp/issues/16999
"""
(x) < (printfn $"{y}"
y)
""",
"""
(x) < (printfn $"{y}"
y)
"""

// See https://github.com/dotnet/fsharp/issues/16999
"""
id (x) < (printfn $"{y}"
y)
""",
"""
id (x) < (printfn $"{y}"
y)
"""

// See https://github.com/dotnet/fsharp/issues/16999
"""
id (id (id (x))) < (printfn $"{y}"
y)
""",
"""
id (id (id (x))) < (printfn $"{y}"
y)
"""

// See https://github.com/dotnet/fsharp/issues/16999
"""
(x) <> z && x < (printfn $"{y}"
y)
""",
"""
(x) <> z && x < (printfn $"{y}"
y)
"""

// See https://github.com/dotnet/fsharp/issues/16999
"""
(x) < match y with
| Some y -> let y = y
y
| y)
""",
"""
(x) < match y with
| Some y -> let y = y
y
| y)
"""

// See https://github.com/dotnet/fsharp/issues/16999
"""
printfn "1"; printfn ("2"); (id <| match y with Some y -> let y = y
y
| None -> 3)
""",
"""
printfn "1"; printfn ("2"); (id <| match y with Some y -> let y = y
y
| None -> 3)
"""

// See https://github.com/dotnet/fsharp/issues/16999
"""
printfn ("1"
); printfn "2"; (id <| match y with Some y -> let y = y
y
| None -> 3)
""",
"""
printfn ("1"
); printfn "2"; (id <| match y with Some y -> let y = y
y
| None -> 3)
"""
}

[<Theory; MemberData(nameof failing)>]
let ``Failing tests`` expr expected =
Assert.ThrowsAsync<UnexpectedCodeFixException>(fun () -> expectFix expr expected)

module Patterns =
type SynPat =
| Const of string
Expand Down Expand Up @@ -2880,6 +2967,113 @@ module Patterns =
| _ -> ()
"

"
match maybe with
| Some(x) -> let y = x * 2
let z = 99
x + y + z
| None -> 3
",
"
match maybe with
| Some(x) -> let y = x * 2
let z = 99
x + y + z
| None -> 3
"

"
match maybe with
| Some(x) -> id <| (let y = x * 2
let z = 99
x + y + z)
| None -> 3
",
"
match maybe with
| Some(x) -> id <| (let y = x * 2
let z = 99
x + y + z)
| None -> 3
"

"
match maybe with
| Some(
x
) -> let y = x * 2
let z = 99
x + y + z
| None -> 3
",
"
match maybe with
| Some(
x
) -> let y = x * 2
let z = 99
x + y + z
| None -> 3
"

"
match q with
| { A = Some(
x
) } -> let y = x * 2
let z = 99
x + y + z
| { A = None } -> 3
",
"
match q with
| { A = Some(
x
) } -> let y = x * 2
let z = 99
x + y + z
| { A = None } -> 3
"

// This removal is somewhat ugly, albeit valid.
// Maybe we can make it nicer someday.
"
match q with
| { A = Some (
x
)
} -> let y = x * 2
let z = 99
x + y + z
| { A = None } -> 3
",
"
match q with
| { A = Some
x
} -> let y = x * 2
let z = 99
x + y + z
| { A = None } -> 3
"

"
match q with
| { A = Some (x)
} -> let y = x * 2
let z = 99
x + y + z
| { A = None } -> 3
",
"
match q with
| { A = Some x
} -> let y = x * 2
let z = 99
x + y + z
| { A = None } -> 3
"

"
type T () =
member this.Item
Expand Down

0 comments on commit d31886c

Please sign in to comment.