diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md new file mode 100644 index 00000000000..9ecdf18ee3e --- /dev/null +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.400.md @@ -0,0 +1,3 @@ +### Fixed + +Various parenthesization API fixes. ([PR #16977](https://github.com/dotnet/fsharp/pull/16977)) diff --git a/src/Compiler/Service/SynExpr.fs b/src/Compiler/Service/SynExpr.fs index 2715d721fc9..a5904621ca6 100644 --- a/src/Compiler/Service/SynExpr.fs +++ b/src/Compiler/Service/SynExpr.fs @@ -554,6 +554,21 @@ module SynExpr = loop ValueNone startLine range.StartColumn + /// Matches constructs that are sensitive to + /// certain kinds of undentation in sequential expressions. + [] + let (|UndentationSensitive|_|) expr = + match expr with + | SynExpr.TryWith _ + | SynExpr.TryFinally _ + | SynExpr.For _ + | SynExpr.ForEach _ + | SynExpr.IfThenElse _ + | SynExpr.Match _ + | SynExpr.While _ + | SynExpr.Do _ -> ValueSome UndentationSensitive + | _ -> ValueNone + let rec shouldBeParenthesizedInContext (getSourceLineStr: int -> string) path expr : bool = let shouldBeParenthesizedInContext = shouldBeParenthesizedInContext getSourceLineStr let containsSensitiveIndentation = containsSensitiveIndentation getSourceLineStr @@ -732,6 +747,56 @@ module SynExpr = -> true + // There are certain constructs whose indentation in + // a sequential expression is valid when parenthesized + // (and separated from the following expression by a semicolon), + // but where the parsing of the outer expression would change if + // the parentheses were removed in place, e.g., + // + // [ + // (if p then q else r); // Cannot remove in place because of the indentation of y below. + // y + // ] + // + // or + // + // [ + // x; + // (if p then q else r); // Cannot remove in place because of the indentation of x above. + // z + // ] + // + // This analysis is imperfect in that it sometimes requires parens when they + // may not be required, but the only way to know for sure in such cases would be to walk up + // an unknown number of ancestral sequential expressions to check for problematic + // indentation (or to keep track of the offsides line throughout the AST traversal): + // + // [ + // x; // This line's indentation means we cannot remove below. + // (…); + // (…); + // (* 𝑛 more such lines. *) + // (…); + // (…); + // (if p then q else r); // Can no longer remove here because of the indentation of x above. + // z + // ] + | UndentationSensitive, + SyntaxNode.SynExpr(SynExpr.Sequential(expr1 = SynExpr.Paren(expr = Is expr))) :: SyntaxNode.SynExpr(SynExpr.Sequential( + expr1 = SynExpr.Paren(expr = other) | other)) :: _ + | UndentationSensitive, + SyntaxNode.SynExpr(SynExpr.Sequential(expr1 = SynExpr.Paren(expr = Is expr); expr2 = other)) :: SyntaxNode.SynExpr(SynExpr.Sequential _) :: _ + | UndentationSensitive, + SyntaxNode.SynExpr(SynExpr.Sequential(expr1 = SynExpr.Paren(expr = Is expr); expr2 = other)) :: SyntaxNode.SynExpr(SynExpr.ArrayOrListComputed _) :: _ + | UndentationSensitive, + SyntaxNode.SynExpr(SynExpr.Sequential(expr1 = SynExpr.Paren(expr = Is expr); expr2 = other)) :: SyntaxNode.SynExpr(SynExpr.ArrayOrList _) :: _ + | UndentationSensitive, + SyntaxNode.SynExpr(SynExpr.Sequential(expr1 = SynExpr.Paren(expr = Is expr); expr2 = other)) :: SyntaxNode.SynExpr(SynExpr.ComputationExpr _) :: _ when + expr.Range.StartLine <> other.Range.StartLine + && expr.Range.StartColumn <= other.Range.StartColumn + -> + true + // Check for nested matches, e.g., // // match … with … -> (…, match … with … -> … | … -> …) | … -> … @@ -990,6 +1055,8 @@ module SynExpr = -> true + | SynExpr.Sequential _, Dangling.Problematic(SynExpr.Sequential _) -> true + | SynExpr.Sequential(expr1 = SynExpr.Paren(expr = Is inner); expr2 = expr2), Dangling.Problematic _ when problematic inner.Range expr2.Range -> diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs index 7370b63b1b3..1994c5a7dac 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs @@ -901,6 +901,127 @@ in x """ printfn "1"; (printfn "2") """, """ printfn "1"; printfn "2" """ "let x = 3; (5) in x", "let x = 3; 5 in x" + """ + [ + () + (printfn "1"; ()) + () + ] + """, + """ + [ + () + (printfn "1"; ()) + () + ] + """ + + // Technically we could remove some parens in some of these, + // but additional exprs above or below can suddenly move the offsides line, + // and we don't want to do unbounded lookahead or lookbehind. + + " + let _ = + [ + (if p then q else r); + y + ] + ", + " + let _ = + [ + (if p then q else r); + y + ] + " + + " + let _ = + [ + (if p then q else r); + y + ] + ", + " + let _ = + [ + if p then q else r; + y + ] + " + + " + let _ = + [ + x; + (if p then q else r); + (if foo then bar else baz); + ] + ", + " + let _ = + [ + x; + (if p then q else r); + if foo then bar else baz; + ] + " + + " + let _ = + [ + (if p then q else r); + y; + (if foo then bar else baz); + ] + ", + " + let _ = + [ + (if p then q else r); + y; + if foo then bar else baz; + ] + " + + " + let _ = + [ + (if p then q else r); + (if foo then bar else baz); + z + ] + ", + " + let _ = + [ + if p then q else r; + (if foo then bar else baz); + z + ] + " + + " + let _ = + [ + x; + (if a then b else c); + (if p then q else r); + (if foo then bar else baz); + z + ] + ", + " + let _ = + [ + x; + (if a then b else c); + (if p then q else r); + (if foo then bar else baz); + z + ] + " + " let _ = let y = 100