Wrap arg in parens when needed when adding new keyword (#18179)

Author: brianrourkeboll

docs/release-notes/.VisualStudio/17.13.md

@@ -1,4 +1,5 @@
 ### Fixed
+* Wrap arg in parens when needed when adding `new` keyword. ([PR #18179](https://github.com/dotnet/fsharp/pull/18179))
 
 ### Added
 * Code fix for adding missing `seq`. ([PR #17772](https://github.com/dotnet/fsharp/pull/17772))

vsintegration/src/FSharp.Editor/CodeFixes/AddNewKeywordToDisposableConstructorInvocation.fs

@@ -5,6 +5,9 @@ namespace Microsoft.VisualStudio.FSharp.Editor
 open System.Composition
 open System.Collections.Immutable
 
+open FSharp.Compiler.Syntax
+open FSharp.Compiler.Text
+
 open Microsoft.CodeAnalysis.Text
 open Microsoft.CodeAnalysis.CodeFixes
 
@@ -24,11 +27,95 @@ type internal AddNewKeywordCodeFixProvider() =
 
     interface IFSharpCodeFixProvider with
         member _.GetCodeFixIfAppliesAsync context =
-            CancellableTask.singleton (
-                ValueSome
-                    {
-                        Name = CodeFix.AddNewKeyword
-                        Message = title
-                        Changes = [ TextChange(TextSpan(context.Span.Start, 0), "new ") ]
-                    }
-            )
+            cancellableTask {
+                let! sourceText = context.GetSourceTextAsync()
+                let! parseFileResults = context.Document.GetFSharpParseResultsAsync(nameof AddNewKeywordCodeFixProvider)
+
+                let getSourceLineStr line =
+                    sourceText.Lines[Line.toZ line].ToString()
+
+                let range =
+                    RoslynHelpers.TextSpanToFSharpRange(context.Document.FilePath, context.Span, sourceText)
+
+                // Constructor arg
+                // Qualified.Constructor arg
+                // Constructor<TypeArg> arg
+                // Qualified.Constructor<TypeArg> arg
+                let matchingApp path node =
+                    let (|TargetTy|_|) expr =
+                        match expr with
+                        | SynExpr.Ident id -> Some(SynType.LongIdent(SynLongIdent([ id ], [], [])))
+                        | SynExpr.LongIdent(longDotId = longDotId) -> Some(SynType.LongIdent longDotId)
+                        | SynExpr.TypeApp(SynExpr.Ident id, lessRange, typeArgs, commaRanges, greaterRange, _, range) ->
+                            Some(
+                                SynType.App(
+                                    SynType.LongIdent(SynLongIdent([ id ], [], [])),
+                                    Some lessRange,
+                                    typeArgs,
+                                    commaRanges,
+                                    greaterRange,
+                                    false,
+                                    range
+                                )
+                            )
+                        | SynExpr.TypeApp(SynExpr.LongIdent(longDotId = longDotId), lessRange, typeArgs, commaRanges, greaterRange, _, range) ->
+                            Some(
+                                SynType.App(SynType.LongIdent longDotId, Some lessRange, typeArgs, commaRanges, greaterRange, false, range)
+                            )
+                        | _ -> None
+
+                    match node with
+                    | SyntaxNode.SynExpr(SynExpr.App(funcExpr = TargetTy targetTy; argExpr = argExpr; range = m)) when
+                        m |> Range.equals range
+                        ->
+                        Some(targetTy, argExpr, path)
+                    | _ -> None
+
+                match (range.Start, parseFileResults.ParseTree) ||> ParsedInput.tryPick matchingApp with
+                | None -> return ValueNone
+                | Some(targetTy, argExpr, path) ->
+                    // Adding `new` may require additional parentheses: https://github.com/dotnet/fsharp/issues/15622
+                    let needsParens =
+                        let newExpr = SynExpr.New(false, targetTy, argExpr, range)
+
+                        argExpr
+                        |> SynExpr.shouldBeParenthesizedInContext getSourceLineStr (SyntaxNode.SynExpr newExpr :: path)
+
+                    let newText =
+                        let targetTyText =
+                            sourceText.ToString(RoslynHelpers.FSharpRangeToTextSpan(sourceText, targetTy.Range))
+
+                        // Constructor namedArg  → new Constructor(namedArg)
+                        // Constructor "literal" → new Constructor "literal"
+                        // Constructor ()        → new Constructor ()
+                        // Constructor()         → new Constructor()
+                        // Constructor           → new Constructor
+                        // ····indentedArg         ····(indentedArg)
+                        let textBetween =
+                            let range =
+                                Range.mkRange context.Document.FilePath targetTy.Range.End argExpr.Range.Start
+
+                            if needsParens && range.StartLine = range.EndLine then
+                                ""
+                            else
+                                sourceText.ToString(RoslynHelpers.FSharpRangeToTextSpan(sourceText, range))
+
+                        let argExprText =
+                            let originalArgText =
+                                sourceText.ToString(RoslynHelpers.FSharpRangeToTextSpan(sourceText, argExpr.Range))
+
+                            if needsParens then
+                                $"(%s{originalArgText})"
+                            else
+                                originalArgText
+
+                        $"new %s{targetTyText}%s{textBetween}%s{argExprText}"
+
+                    return
+                        ValueSome
+                            {
+                                Name = CodeFix.AddNewKeyword
+                                Message = title
+                                Changes = [ TextChange(context.Span, newText) ]
+                            }
+            }

vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddNewKeywordToDisposableConstructorInvocationTests.fs

@@ -29,3 +29,116 @@ let sr = new System.IO.StreamReader "test.txt"
     let actual = codeFix |> tryFix code Auto
 
     Assert.Equal(expected, actual)
+
+[<Fact>]
+let ``Fixes FS0760 — type app`` () =
+    let code =
+        """
+let _ = System.Threading.Tasks.Task<int>(fun _ -> 3)
+"""
+
+    let expected =
+        Some
+            {
+                Message = "Add 'new' keyword"
+                FixedCode =
+                    """
+let _ = new System.Threading.Tasks.Task<int>(fun _ -> 3)
+"""
+            }
+
+    let actual = codeFix |> tryFix code Auto
+
+    Assert.Equal(expected, actual)
+
+[<Fact>]
+let ``Fixes FS0760 — keeps space`` () =
+    let code =
+        """
+let stream = System.IO.MemoryStream ()
+"""
+
+    let expected =
+        Some
+            {
+                Message = "Add 'new' keyword"
+                FixedCode =
+                    """
+let stream = new System.IO.MemoryStream ()
+"""
+            }
+
+    let actual = codeFix |> tryFix code Auto
+
+    Assert.Equal(expected, actual)
+
+[<Fact>]
+let ``Fixes FS0760 — does not add space`` () =
+    let code =
+        """
+let stream = System.IO.MemoryStream()
+"""
+
+    let expected =
+        Some
+            {
+                Message = "Add 'new' keyword"
+                FixedCode =
+                    """
+let stream = new System.IO.MemoryStream()
+"""
+            }
+
+    let actual = codeFix |> tryFix code Auto
+
+    Assert.Equal(expected, actual)
+
+[<Fact>]
+let ``Fixes FS0760 — adds parentheses when needed`` () =
+    let code =
+        """
+let path = "test.txt"
+let sr = System.IO.StreamReader path
+"""
+
+    let expected =
+        Some
+            {
+                Message = "Add 'new' keyword"
+                FixedCode =
+                    """
+let path = "test.txt"
+let sr = new System.IO.StreamReader(path)
+"""
+            }
+
+    let actual = codeFix |> tryFix code Auto
+
+    Assert.Equal(expected, actual)
+
+[<Fact>]
+let ``Fixes FS0760 — adds parentheses when needed and keeps indentation`` () =
+    let code =
+        """
+let path = "test.txt"
+let sr =
+    System.IO.StreamReader
+        path
+"""
+
+    let expected =
+        Some
+            {
+                Message = "Add 'new' keyword"
+                FixedCode =
+                    """
+let path = "test.txt"
+let sr =
+    new System.IO.StreamReader
+        (path)
+"""
+            }
+
+    let actual = codeFix |> tryFix code Auto
+
+    Assert.Equal(expected, actual)