Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/release-notes/.VisualStudio/17.13.md
Original file line number Diff line number Diff line change
@@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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) ]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Loading