Skip to content

Improving FS0010 code fixes #15763

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

Merged
merged 2 commits into from
Aug 11, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ namespace Microsoft.VisualStudio.FSharp.Editor

open System.Composition
open System.Collections.Immutable
open System.Threading.Tasks

open Microsoft.CodeAnalysis.Text
open Microsoft.CodeAnalysis.CodeFixes
Expand All @@ -18,33 +19,33 @@ type internal AddMissingEqualsToTypeDefinitionCodeFixProvider() =

override _.FixableDiagnosticIds = ImmutableArray.Create "FS0010"

override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix this
override this.RegisterCodeFixesAsync context =
// This is a performance shortcut.
// Since FS0010 fires all too often, we're just stopping any processing if it's a different error message.
// The code fix logic itself still has this logic and implements it more reliably.
if
context.Diagnostics
|> Seq.exists (fun d -> d.Descriptor.MessageFormat.ToString().Contains "=")
then
context.RegisterFsharpFix this
else
Task.CompletedTask

interface IFSharpCodeFixProvider with
member _.GetCodeFixIfAppliesAsync context =
cancellableTask {
let message =
context.Diagnostics
|> Seq.exactlyOne
|> fun d -> d.Descriptor.MessageFormat.ToString()
let! range = context.GetErrorRangeAsync()
let! parseResults = context.Document.GetFSharpParseResultsAsync(nameof AddMissingEqualsToTypeDefinitionCodeFixProvider)

// this should eliminate 99.9% of germs
if not <| message.Contains "=" then
if not <| parseResults.IsPositionWithinTypeDefinition range.Start then
return ValueNone
else

let! range = context.GetErrorRangeAsync()
let! parseResults = context.Document.GetFSharpParseResultsAsync(nameof AddMissingEqualsToTypeDefinitionCodeFixProvider)

if not <| parseResults.IsPositionWithinTypeDefinition range.Start then
return ValueNone

else
return
ValueSome
{
Name = CodeFix.AddMissingEqualsToTypeDefinition
Message = title
Changes = [ TextChange(TextSpan(context.Span.Start, 0), "= ") ]
}
else
return
ValueSome
{
Name = CodeFix.AddMissingEqualsToTypeDefinition
Message = title
Changes = [ TextChange(TextSpan(context.Span.Start, 0), "= ") ]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ namespace Microsoft.VisualStudio.FSharp.Editor
open System
open System.Composition
open System.Collections.Immutable
open System.Threading.Tasks

open Microsoft.CodeAnalysis.Text
open Microsoft.CodeAnalysis.CodeFixes
Expand All @@ -28,7 +29,17 @@ type internal AddMissingFunKeywordCodeFixProvider [<ImportingConstructor>] () =

override _.FixableDiagnosticIds = ImmutableArray.Create "FS0010"

override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix this
override this.RegisterCodeFixesAsync context =
// This is a performance shortcut.
// Since FS0010 fires all too often, we're just stopping any processing if it's a different error message.
// The code fix logic itself still has this logic and implements it more reliably.
if
context.Diagnostics
|> Seq.exists (fun d -> d.Descriptor.MessageFormat.ToString().Contains "->")
then
context.RegisterFsharpFix this
else
Task.CompletedTask

interface IFSharpCodeFixProvider with
member _.GetCodeFixIfAppliesAsync context =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type internal ChangeEqualsInFieldTypeToColonCodeFixProvider() =
// This is a performance shortcut.
// Since FS0010 fires all too often, we're just stopping any processing if it's a different error message.
// The code fix logic itself still has this logic and implements it more reliably.
if context.Diagnostics[ 0 ].GetMessage() = errorMessage then
if context.Diagnostics |> Seq.exists (fun d -> d.GetMessage() = errorMessage) then
context.RegisterFsharpFix this
else
Task.CompletedTask
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ type Name = Name of string
[<Theory>]
[<InlineData "type X = open">]
[<InlineData "=">]
[<InlineData "let f x =
match x with
| _ ->
let _ = [
x with
]
">]
let ``Doesn't fix FS0010 for random unexpected symbols`` code =
let expected = None

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,18 @@ let gettingEven numbers =

Assert.Equal(expected, actual)

[<Fact>]
let ``Doesn't fix FS0010 for random unexpected symbols`` () =
let code =
"""
[<Theory>]
[<InlineData("""
=
"""

""")>]
[<InlineData "let f x =
match x with
| _ ->
let _ = [
x with
]
">]
let ``Doesn't fix FS0010 for random unexpected symbols`` code =
let expected = None

let actual = codeFix |> tryFix code Auto
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ type Band = { Name open string }
[<InlineData("""
type Band = {| Name open string |}
""")>]
[<InlineData "let f x =
match x with
| _ ->
let _ = [
x with
]
">]
let ``Doesn't fix FS0010 for random unexpected symbols`` code =
let expected = None

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,43 +36,54 @@ let getDocument code mode =
| WithOption option -> RoslynTestHelpers.GetFsDocument(code, option)
| Manual _ -> RoslynTestHelpers.GetFsDocument code

let getRelevantDiagnostic (document: Document) =
let getRelevantDiagnostics (document: Document) =
cancellableTask {
let! _, checkFileResults = document.GetFSharpParseAndCheckResultsAsync "test"

return checkFileResults.Diagnostics |> Seq.head
return checkFileResults.Diagnostics
}
|> CancellableTask.startWithoutCancellation
|> fun task -> task.Result

let createTestCodeFixContext (code: string) document (mode: Mode) =
let createTestCodeFixContext (code: string) document (mode: Mode) diagnosticIds =
cancellableTask {
let! cancellationToken = CancellableTask.getCancellationToken ()

let sourceText = SourceText.From code

let! diagnostic =
let diagnostics =
match mode with
| Auto -> getRelevantDiagnostic document
| WithOption _ -> getRelevantDiagnostic document
| Auto ->
getRelevantDiagnostics document
|> Array.filter (fun d -> diagnosticIds |> Seq.contains d.ErrorNumberText)
| WithOption _ -> getRelevantDiagnostics document
| Manual (squiggly, number) ->
let spanStart = code.IndexOf squiggly
let span = TextSpan(spanStart, squiggly.Length)
let range = RoslynHelpers.TextSpanToFSharpRange(document.FilePath, span, sourceText)
CancellableTask.singleton (FSharpDiagnostic.Create(FSharpDiagnosticSeverity.Warning, "test", number, range))

let location =
RoslynHelpers.RangeToLocation(diagnostic.Range, sourceText, document.FilePath)
[|
FSharpDiagnostic.Create(FSharpDiagnosticSeverity.Warning, "test", number, range)
|]

let range = diagnostics[0].Range
let location = RoslynHelpers.RangeToLocation(range, sourceText, document.FilePath)
let textSpan = RoslynHelpers.FSharpRangeToTextSpan(sourceText, range)

let roslynDiagnostic = RoslynHelpers.ConvertError(diagnostic, location)
let roslynDiagnostics =
diagnostics
|> Array.map (fun diagnostic -> RoslynHelpers.ConvertError(diagnostic, location))
|> ImmutableArray.ToImmutableArray

return CodeFixContext(document, roslynDiagnostic, mockAction, cancellationToken)
return CodeFixContext(document, textSpan, roslynDiagnostics, mockAction, cancellationToken)
}

let tryFix (code: string) mode (fixProvider: IFSharpCodeFixProvider) =
let tryFix (code: string) mode (fixProvider: 'T when 'T :> IFSharpCodeFixProvider and 'T :> CodeFixProvider) =
cancellableTask {
let sourceText = SourceText.From code
let document = getDocument code mode

let! context = createTestCodeFixContext code document mode
let! context = createTestCodeFixContext code document mode fixProvider.FixableDiagnosticIds

let! result = fixProvider.GetCodeFixIfAppliesAsync context

Expand All @@ -85,5 +96,5 @@ let tryFix (code: string) mode (fixProvider: IFSharpCodeFixProvider) =
FixedCode = (sourceText.WithChanges codeFix.Changes).ToString()
}))
}
|> CancellableTask.start CancellationToken.None
|> CancellableTask.startWithoutCancellation
|> fun task -> task.Result