From 8805750344529935b95b89dae0edc9ac5bfa7b3d Mon Sep 17 00:00:00 2001 From: Brian Rourke Boll Date: Tue, 23 Jan 2024 19:10:12 -0500 Subject: [PATCH 1/4] =?UTF-8?q?Keep=20parens=20for=20problematic=20exprs?= =?UTF-8?q?=20in=20`$"{(=E2=80=A6):f}"`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Compiler/Service/SynExpr.fs | 12 +++++++++++- .../RemoveUnnecessaryParenthesesTests.fs | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/Compiler/Service/SynExpr.fs b/src/Compiler/Service/SynExpr.fs index 0df9a1a1c17..37a0ca47f1e 100644 --- a/src/Compiler/Service/SynExpr.fs +++ b/src/Compiler/Service/SynExpr.fs @@ -423,7 +423,9 @@ module SynExpr = | SynExpr.MatchLambda(matchClauses = Last(SynMatchClause(resultExpr = expr))) | SynExpr.MatchBang(clauses = Last(SynMatchClause(resultExpr = expr))) | SynExpr.TryWith(withCases = Last(SynMatchClause(resultExpr = expr))) - | SynExpr.TryFinally(finallyExpr = expr) -> loop expr + | SynExpr.TryFinally(finallyExpr = expr) + | SynExpr.Do(expr = expr) + | SynExpr.DoBang(expr = expr) -> loop expr | _ -> ValueNone loop @@ -810,6 +812,14 @@ module SynExpr = -> true + | SynExpr.InterpolatedString(contents = contents), (SynExpr.Tuple(isStruct = false) | Dangling.Problematic _) when + contents + |> List.exists (function + | SynInterpolatedStringPart.FillExpr(qualifiers = Some _) -> true + | _ -> false) + -> + true + | SynExpr.Record(copyInfo = Some(SynExpr.Paren(expr = Is inner), _)), Dangling.Problematic _ | SynExpr.AnonRecd(copyInfo = Some(SynExpr.Paren(expr = Is inner), _)), Dangling.Problematic _ -> true diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs index 69d6456e378..91ee98119d5 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs @@ -800,6 +800,23 @@ in x "$\"{(id 3)}\"", "$\"{id 3}\"" "$\"{(x)}\"", "$\"{x}\"" + "$\"{(if true then 1 else 0):N0}\"", "$\"{(if true then 1 else 0):N0}\"" + "$\"{(if true then 1 else 0),-3}\"", "$\"{(if true then 1 else 0),-3}\"" + "$\"{(match () with () -> 1):N0}\"", "$\"{(match () with () -> 1):N0}\"" + "$\"{(match () with () -> 1),-3}\"", "$\"{(match () with () -> 1),-3}\"" + "$\"{(try () with _ -> 1):N0}\"", "$\"{(try () with _ -> 1):N0}\"" + "$\"{(try () with _ -> 1),-3}\"", "$\"{(try () with _ -> 1),-3}\"" + "$\"{(try 1 finally ()):N0}\"", "$\"{(try 1 finally ()):N0}\"" + "$\"{(try 1 finally ()),-3}\"", "$\"{(try 1 finally ()),-3}\"" + "$\"{(let x = 3 in x):N0}\"", "$\"{(let x = 3 in x):N0}\"" + "$\"{(let x = 3 in x),-3}\"", "$\"{(let x = 3 in x),-3}\"" + "$\"{(do (); 3):N0}\"", "$\"{(do (); 3):N0}\"" + "$\"{(do (); 3),-3}\"", "$\"{(do (); 3),-3}\"" + "$\"{(x <- 3):N0}\"", "$\"{(x <- 3):N0}\"" + "$\"{(x <- 3),-3}\"", "$\"{(x <- 3),-3}\"" + "$\"{(1, 2):N0}\"", "$\"{(1, 2):N0}\"" + "$\"{(1, 2),-3}\"", "$\"{(1, 2),-3}\"" + """ $"{(3 + LanguagePrimitives.GenericZero):N0}" """, From f8734fb9b075a5f34e6b0718f72bf5779e1df863 Mon Sep 17 00:00:00 2001 From: Brian Rourke Boll Date: Tue, 23 Jan 2024 19:16:25 -0500 Subject: [PATCH 2/4] Update release notes --- docs/release-notes/.FSharp.Compiler.Service/8.0.300.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md index 1c70a67edc6..4e3a58526e5 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md @@ -2,6 +2,7 @@ * Code generated files with > 64K methods and generated symbols crash when loaded. Use infered sequence points for debugging. ([Issue #16399](https://github.com/dotnet/fsharp/issues/16399), [#PR 16514](https://github.com/dotnet/fsharp/pull/16514)) * `nameof Module` expressions and patterns are processed to link files in `--test:GraphBasedChecking`. ([PR #16550](https://github.com/dotnet/fsharp/pull/16550)) +* Keep parens for problematic exprs (`if`, `match`, etc.) in `$"{(…):N0}"`, `$"{(…),-3}"`, etc. ([PR #16578](https://github.com/dotnet/fsharp/pull/16578)) ### Added From 447b200a14ad906ba1684ed63721c09e5e0783c1 Mon Sep 17 00:00:00 2001 From: Brian Rourke Boll Date: Tue, 23 Jan 2024 19:20:05 -0500 Subject: [PATCH 3/4] Better --- src/Compiler/Service/SynExpr.fs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Compiler/Service/SynExpr.fs b/src/Compiler/Service/SynExpr.fs index 37a0ca47f1e..41dbd4d4d18 100644 --- a/src/Compiler/Service/SynExpr.fs +++ b/src/Compiler/Service/SynExpr.fs @@ -812,13 +812,11 @@ module SynExpr = -> true - | SynExpr.InterpolatedString(contents = contents), (SynExpr.Tuple(isStruct = false) | Dangling.Problematic _) when + | SynExpr.InterpolatedString(contents = contents), (SynExpr.Tuple(isStruct = false) | Dangling.Problematic _) -> contents |> List.exists (function | SynInterpolatedStringPart.FillExpr(qualifiers = Some _) -> true | _ -> false) - -> - true | SynExpr.Record(copyInfo = Some(SynExpr.Paren(expr = Is inner), _)), Dangling.Problematic _ | SynExpr.AnonRecd(copyInfo = Some(SynExpr.Paren(expr = Is inner), _)), Dangling.Problematic _ -> true From 40abfb8f726e983db4d0254f6a02032b857a5833 Mon Sep 17 00:00:00 2001 From: Brian Rourke Boll Date: Tue, 23 Jan 2024 19:22:46 -0500 Subject: [PATCH 4/4] Sanity --- .../CodeFixes/RemoveUnnecessaryParenthesesTests.fs | 1 + 1 file changed, 1 insertion(+) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs index 91ee98119d5..cd3bbdd2bfc 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnnecessaryParenthesesTests.fs @@ -800,6 +800,7 @@ in x "$\"{(id 3)}\"", "$\"{id 3}\"" "$\"{(x)}\"", "$\"{x}\"" + "$\"{(if true then 1 else 0)}\"", "$\"{if true then 1 else 0}\"" "$\"{(if true then 1 else 0):N0}\"", "$\"{(if true then 1 else 0):N0}\"" "$\"{(if true then 1 else 0),-3}\"", "$\"{(if true then 1 else 0),-3}\"" "$\"{(match () with () -> 1):N0}\"", "$\"{(match () with () -> 1):N0}\""