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

Fixing code fixes, part 5 #15663

Merged
merged 11 commits into from
Jul 31, 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 @@ -3,36 +3,32 @@
namespace Microsoft.VisualStudio.FSharp.Editor

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

open Microsoft.CodeAnalysis
open Microsoft.CodeAnalysis.Text
open Microsoft.CodeAnalysis.CodeFixes

open CancellableTasks

[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.AddNewKeyword); Shared>]
type internal AddNewKeywordCodeFixProvider() =
inherit CodeFixProvider()

static let title = SR.AddNewKeyword()
override _.FixableDiagnosticIds = ImmutableArray.Create "FS0760"

member this.GetChanges(_document: Document, diagnostics: ImmutableArray<Diagnostic>, _ct: CancellationToken) =
backgroundTask {

let changes =
diagnostics
|> Seq.map (fun d -> TextChange(TextSpan(d.Location.SourceSpan.Start, 0), "new "))
override _.FixableDiagnosticIds = ImmutableArray.Create "FS0760"

return changes
}
override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix this

override this.RegisterCodeFixesAsync ctx : Task =
backgroundTask {
let! changes = this.GetChanges(ctx.Document, ctx.Diagnostics, ctx.CancellationToken)
ctx.RegisterFsharpFix(CodeFix.AddNewKeyword, title, changes)
}
override this.GetFixAllProvider() = this.RegisterFsharpFixAll()

override this.GetFixAllProvider() =
CodeFixHelpers.createFixAllProvider CodeFix.AddNewKeyword this.GetChanges
interface IFSharpCodeFixProvider with
member _.GetCodeFixIfAppliesAsync context =
CancellableTask.singleton (
Some
{
Name = CodeFix.AddNewKeyword
Message = title
Changes = [ TextChange(TextSpan(context.Span.Start, 0), "new ") ]
}
)
63 changes: 62 additions & 1 deletion vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

namespace Microsoft.VisualStudio.FSharp.Editor

open System
open System.Threading
open System.Threading.Tasks
open System.Collections.Immutable
open System.Diagnostics

Expand All @@ -17,7 +19,7 @@ open CancellableTasks

[<RequireQualifiedAccess>]
module internal CodeFixHelpers =
let private reportCodeFixTelemetry
let reportCodeFixTelemetry
(diagnostics: ImmutableArray<Diagnostic>)
(doc: Document)
(staticName: string)
Expand Down Expand Up @@ -106,3 +108,62 @@ module internal CodeFixExtensions =
let! sourceText = ctx.GetSourceTextAsync()
return RoslynHelpers.TextSpanToFSharpRange(ctx.Document.FilePath, ctx.Span, sourceText)
}

// This cannot be an extension on the code fix context
// because the underlying GetFixAllProvider method doesn't take the context in.
#nowarn "3511" // state machine not statically compilable

[<AutoOpen>]
module IFSharpCodeFixProviderExtensions =
type IFSharpCodeFixProvider with

// this is not used anywhere, it's just needed to create the context
static member private Action =
Action<CodeActions.CodeAction, ImmutableArray<Diagnostic>>(fun _ _ -> ())

member private provider.FixAllAsync (fixAllCtx: FixAllContext) (doc: Document) (allDiagnostics: ImmutableArray<Diagnostic>) =
cancellableTask {
let sw = Stopwatch.StartNew()

let! token = CancellableTask.getCurrentCancellationToken ()
let! sourceText = doc.GetTextAsync token

let! codeFixOpts =
allDiagnostics
// The distiction is to avoid collisions of compiler and analyzer diags.
// See: https://github.com/dotnet/fsharp/issues/15620
// TODO: this crops the diags on a very high level,
// a proper fix is needed.
|> Seq.distinctBy (fun d -> d.Id, d.Location)
|> Seq.map (fun diag -> CodeFixContext(doc, diag, IFSharpCodeFixProvider.Action, token))
|> Seq.map (fun context -> provider.GetCodeFixIfAppliesAsync context)
|> Seq.map (fun task -> task token)
|> Task.WhenAll

let codeFixes = codeFixOpts |> Seq.choose id
let changes = codeFixes |> Seq.collect (fun codeFix -> codeFix.Changes)
let updatedDoc = doc.WithText(sourceText.WithChanges changes)

let name =
codeFixes
|> Seq.tryHead
|> Option.map (fun fix -> fix.Name)
// Now, I cannot see this happening.
// How could a bulk code fix get activated for zero changes?
// But since that's for telemetry purposes,
// let's be on the safe side.
|> Option.defaultValue "UnknownCodeFix"

CodeFixHelpers.reportCodeFixTelemetry
allDiagnostics
updatedDoc
name
[| "scope", fixAllCtx.Scope.ToString(); "elapsedMs", sw.ElapsedMilliseconds |]

return updatedDoc
}

member provider.RegisterFsharpFixAll() =
FixAllProvider.Create(fun fixAllCtx doc allDiagnostics ->
provider.FixAllAsync fixAllCtx doc allDiagnostics
|> CancellableTask.start fixAllCtx.CancellationToken)
75 changes: 16 additions & 59 deletions vsintegration/src/FSharp.Editor/CodeFixes/FixIndexerAccess.fs
Original file line number Diff line number Diff line change
Expand Up @@ -4,77 +4,34 @@ namespace Microsoft.VisualStudio.FSharp.Editor

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

open Microsoft.CodeAnalysis
open Microsoft.CodeAnalysis.Text
open Microsoft.CodeAnalysis.CodeFixes
open FSharp.Compiler.Diagnostics

[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.FixIndexerAccess); Shared>]
type internal LegacyFixAddDotToIndexerAccessCodeFixProvider() =
inherit CodeFixProvider()

static let title = CompilerDiagnostics.GetErrorMessage FSharpDiagnosticKind.AddIndexerDot

override _.FixableDiagnosticIds = ImmutableArray.Create("FS3217")

override _.RegisterCodeFixesAsync context : Task =
async {
let! sourceText = context.Document.GetTextAsync() |> Async.AwaitTask

context.Diagnostics
|> Seq.iter (fun diagnostic ->

let span, replacement =
try
let mutable span = context.Span

let notStartOfBracket (span: TextSpan) =
let t = sourceText.GetSubText(TextSpan(span.Start, span.Length + 1))
t.[t.Length - 1] <> '['

// skip all braces and blanks until we find [
while span.End < sourceText.Length && notStartOfBracket span do
span <- TextSpan(span.Start, span.Length + 1)

span, sourceText.GetSubText(span).ToString()
with _ ->
context.Span, sourceText.GetSubText(context.Span).ToString()
open FSharp.Compiler.Diagnostics

do
context.RegisterFsharpFix(
CodeFix.FixIndexerAccess,
title,
[| TextChange(span, replacement.TrimEnd() + ".") |],
ImmutableArray.Create(diagnostic)
))
}
|> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)
open CancellableTasks

[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.RemoveIndexerDotBeforeBracket); Shared>]
type internal FsharpFixRemoveDotFromIndexerAccessOptIn() as this =
type internal RemoveDotFromIndexerAccessOptInCodeFixProvider() =
inherit CodeFixProvider()

static let title =
CompilerDiagnostics.GetErrorMessage FSharpDiagnosticKind.RemoveIndexerDot

member this.GetChanges(_document: Document, diagnostics: ImmutableArray<Diagnostic>, _ct: CancellationToken) =
backgroundTask {
let changes =
diagnostics |> Seq.map (fun x -> TextChange(x.Location.SourceSpan, ""))

return changes
}
override _.FixableDiagnosticIds = ImmutableArray.Create "FS3366"

override _.FixableDiagnosticIds = ImmutableArray.Create("FS3366")
override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix this

override _.RegisterCodeFixesAsync ctx : Task =
backgroundTask {
let! changes = this.GetChanges(ctx.Document, ctx.Diagnostics, ctx.CancellationToken)
ctx.RegisterFsharpFix(CodeFix.RemoveIndexerDotBeforeBracket, title, changes)
}
override this.GetFixAllProvider() = this.RegisterFsharpFixAll()

override this.GetFixAllProvider() =
CodeFixHelpers.createFixAllProvider CodeFix.RemoveIndexerDotBeforeBracket this.GetChanges
interface IFSharpCodeFixProvider with
member _.GetCodeFixIfAppliesAsync context =
CancellableTask.singleton (
Some
{
Name = CodeFix.RemoveIndexerDotBeforeBracket
Message = title
Changes = [ TextChange(context.Span, "") ]
}
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information.

namespace Microsoft.VisualStudio.FSharp.Editor

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

open Microsoft.CodeAnalysis.Text
open Microsoft.CodeAnalysis.CodeFixes
open FSharp.Compiler.Diagnostics

[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.FixIndexerAccess); Shared>]
type internal LegacyFixAddDotToIndexerAccessCodeFixProvider() =
inherit CodeFixProvider()

static let title = CompilerDiagnostics.GetErrorMessage FSharpDiagnosticKind.AddIndexerDot

override _.FixableDiagnosticIds = ImmutableArray.Create("FS3217")

override _.RegisterCodeFixesAsync context : Task =
async {
let! sourceText = context.Document.GetTextAsync() |> Async.AwaitTask

context.Diagnostics
|> Seq.iter (fun diagnostic ->

let span, replacement =
try
let mutable span = context.Span

let notStartOfBracket (span: TextSpan) =
let t = sourceText.GetSubText(TextSpan(span.Start, span.Length + 1))
t.[t.Length - 1] <> '['

// skip all braces and blanks until we find [
while span.End < sourceText.Length && notStartOfBracket span do
span <- TextSpan(span.Start, span.Length + 1)

span, sourceText.GetSubText(span).ToString()
with _ ->
context.Span, sourceText.GetSubText(context.Span).ToString()

do
context.RegisterFsharpFix(
CodeFix.FixIndexerAccess,
title,
[| TextChange(span, replacement.TrimEnd() + ".") |],
ImmutableArray.Create(diagnostic)
))
}
|> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)
Original file line number Diff line number Diff line change
Expand Up @@ -3,62 +3,56 @@
namespace Microsoft.VisualStudio.FSharp.Editor

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

open Microsoft.CodeAnalysis
open Microsoft.CodeAnalysis.Text
open Microsoft.CodeAnalysis.CodeFixes

open FSharp.Compiler.EditorServices

[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.RemoveSuperfluousCapture); Shared>]
type internal RemoveSuperflousCaptureForUnionCaseWithNoDataCodeFixProvider [<ImportingConstructor>] () =
open CancellableTasks

[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.RemoveSuperfluousCapture); Shared>]
type internal RemoveSuperfluousCaptureForUnionCaseWithNoDataCodeFixProvider [<ImportingConstructor>] () =
inherit CodeFixProvider()

static let title = SR.RemoveUnusedBinding()
override _.FixableDiagnosticIds = ImmutableArray.Create("FS0725", "FS3548")

member this.GetChanges(document: Document, diagnostics: ImmutableArray<Diagnostic>, ct: CancellationToken) =
backgroundTask {

let! sourceText = document.GetTextAsync(ct)
let! _, checkResults = document.GetFSharpParseAndCheckResultsAsync(CodeFix.RemoveSuperfluousCapture)
override _.FixableDiagnosticIds = ImmutableArray.Create("FS0725", "FS3548") // cannot happen at once

let changes =
seq {
for d in diagnostics do
let textSpan = d.Location.SourceSpan
let m = RoslynHelpers.TextSpanToFSharpRange(document.FilePath, textSpan, sourceText)
let classifications = checkResults.GetSemanticClassification(Some m)
override this.RegisterCodeFixesAsync ctx =
if ctx.Document.Project.IsFSharpCodeFixesUnusedDeclarationsEnabled then
ctx.RegisterFsharpFix this
else
Task.CompletedTask

let unionCaseItem =
classifications
|> Array.tryFind (fun c -> c.Type = SemanticClassificationType.UnionCase)
override this.GetFixAllProvider() = this.RegisterFsharpFixAll()

match unionCaseItem with
| None -> ()
| Some unionCaseItem ->
// The error/warning captures entire pattern match, like "Ns.Type.DuName bindingName". We want to keep type info when suggesting a replacement, and only remove "bindingName".
let typeInfoLength = unionCaseItem.Range.EndColumn - m.StartColumn
interface IFSharpCodeFixProvider with
member _.GetCodeFixIfAppliesAsync context =
cancellableTask {
let! sourceText = context.GetSourceTextAsync()
let! _, checkResults = context.Document.GetFSharpParseAndCheckResultsAsync CodeFix.RemoveSuperfluousCapture

let reminderSpan =
new TextSpan(textSpan.Start + typeInfoLength, textSpan.Length - typeInfoLength)
let m =
RoslynHelpers.TextSpanToFSharpRange(context.Document.FilePath, context.Span, sourceText)

yield TextChange(reminderSpan, "")
}
let classifications = checkResults.GetSemanticClassification(Some m)

return changes
}
return
classifications
|> Array.tryFind (fun c -> c.Type = SemanticClassificationType.UnionCase)
|> Option.map (fun unionCaseItem ->
// The error/warning captures entire pattern match, like "Ns.Type.DuName bindingName". We want to keep type info when suggesting a replacement, and only remove "bindingName".
let typeInfoLength = unionCaseItem.Range.EndColumn - m.StartColumn

override this.RegisterCodeFixesAsync ctx : Task =
backgroundTask {
if ctx.Document.Project.IsFSharpCodeFixesUnusedDeclarationsEnabled then
let! changes = this.GetChanges(ctx.Document, ctx.Diagnostics, ctx.CancellationToken)
ctx.RegisterFsharpFix(CodeFix.RemoveSuperfluousCapture, title, changes)
}
let reminderSpan =
TextSpan(context.Span.Start + typeInfoLength, context.Span.Length - typeInfoLength)

override this.GetFixAllProvider() =
CodeFixHelpers.createFixAllProvider CodeFix.RemoveSuperfluousCapture this.GetChanges
{
Name = CodeFix.RemoveSuperfluousCapture
Message = title
Changes = [ TextChange(reminderSpan, "") ]
})
}
Loading