From 8c9cdf27b5303db4fbb007c743ed12e5f8d8c690 Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 12 Jul 2023 17:00:38 +0200 Subject: [PATCH 01/11] Fixing code fixes, part 5 --- ...eywordToDisposableConstructorInvocation.fs | 34 ++-- .../FSharp.Editor/CodeFixes/CodeFixHelpers.fs | 91 +++++++++- .../CodeFixes/DiscardUnusedValue.fs | 51 ++++++ .../CodeFixes/FixIndexerAccess.fs | 75 ++------ .../CodeFixes/FixIndexerAccessLegacy.fs | 52 ++++++ .../CodeFixes/PrefixUnusedValue.fs | 45 +++++ ...SuperflousCaptureForUnionCaseWithNoData.fs | 64 ------- ...uperfluousCaptureForUnionCaseWithNoData.fs | 58 +++++++ .../CodeFixes/RemoveUnusedBinding.fs | 105 +++++++----- .../CodeFixes/RenameUnusedValue.fs | 161 ------------------ .../Common/FSharpCodeAnalysisExtensions.fs | 34 +++- .../src/FSharp.Editor/FSharp.Editor.fsproj | 6 +- .../AddInstanceMemberParameterTests.fs | 3 +- .../AddMissingEqualsToTypeDefinitionTests.fs | 7 +- .../CodeFixes/AddMissingFunKeywordTests.fs | 5 +- ...ddMissingRecToMutuallyRecFunctionsTests.fs | 3 +- ...dToDisposableConstructorInvocationTests.fs | 31 ++++ ...gePrefixNegationToInfixSubtractionTests.fs | 5 +- .../ChangeRefCellDerefToNotExpressionTests.fs | 5 +- .../CodeFixes/ChangeToUpcastTests.fs | 7 +- .../CodeFixes/CodeFixTestFramework.fs | 37 ++-- .../ConvertCSharpLambdaToFSharpLambdaTests.fs | 6 +- .../ConvertToAnonymousRecordTests.fs | 5 +- ...nvertToNotEqualsEqualityExpressionTests.fs | 5 +- ...rtToSingleEqualsEqualityExpressionTests.fs | 5 +- .../CodeFixes/DiscardUnusedValueTests.fs | 73 ++++++++ .../CodeFixes/FixIndexerAccessTests.fs | 35 ++++ .../MakeOuterBindingRecursiveTests.fs | 5 +- .../CodeFixes/PrefixUnusedValueTests.fs | 81 +++++++++ .../CodeFixes/ProposeUppercaseLabelTests.fs | 5 +- .../CodeFixes/RemoveReturnOrYieldTests.fs | 10 +- ...luousCaptureForUnionCaseWithNoDataTests.fs | 141 +++++++++++++++ .../CodeFixes/RemoveUnusedBindingTests.fs | 97 +++++++++++ .../UseTripleQuotedInterpolationTests.fs | 3 +- .../WrapExpressionInParenthesesTests.fs | 3 +- .../FSharp.Editor.Tests.fsproj | 6 + .../Helpers/RoslynHelpers.fs | 15 +- 37 files changed, 962 insertions(+), 412 deletions(-) create mode 100644 vsintegration/src/FSharp.Editor/CodeFixes/DiscardUnusedValue.fs create mode 100644 vsintegration/src/FSharp.Editor/CodeFixes/FixIndexerAccessLegacy.fs create mode 100644 vsintegration/src/FSharp.Editor/CodeFixes/PrefixUnusedValue.fs delete mode 100644 vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperflousCaptureForUnionCaseWithNoData.fs create mode 100644 vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperfluousCaptureForUnionCaseWithNoData.fs delete mode 100644 vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs create mode 100644 vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddNewKeywordToDisposableConstructorInvocationTests.fs create mode 100644 vsintegration/tests/FSharp.Editor.Tests/CodeFixes/DiscardUnusedValueTests.fs create mode 100644 vsintegration/tests/FSharp.Editor.Tests/CodeFixes/FixIndexerAccessTests.fs create mode 100644 vsintegration/tests/FSharp.Editor.Tests/CodeFixes/PrefixUnusedValueTests.fs create mode 100644 vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveSuperfluousCaptureForUnionCaseWithNoDataTests.fs create mode 100644 vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnusedBindingTests.fs diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/AddNewKeywordToDisposableConstructorInvocation.fs b/vsintegration/src/FSharp.Editor/CodeFixes/AddNewKeywordToDisposableConstructorInvocation.fs index 5dcc030fcf8..c211e9e4453 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/AddNewKeywordToDisposableConstructorInvocation.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/AddNewKeywordToDisposableConstructorInvocation.fs @@ -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 + [] type internal AddNewKeywordCodeFixProvider() = inherit CodeFixProvider() static let title = SR.AddNewKeyword() - override _.FixableDiagnosticIds = ImmutableArray.Create "FS0760" - member this.GetChanges(_document: Document, diagnostics: ImmutableArray, _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 ") ] + } + ) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs index 3b96ca8696f..7b94b9388cc 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs @@ -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 @@ -13,11 +15,14 @@ open Microsoft.CodeAnalysis.CodeFixes open Microsoft.CodeAnalysis.CodeActions open Microsoft.VisualStudio.FSharp.Editor.Telemetry +open FSharp.Compiler.Syntax +open FSharp.Compiler.Symbols + open CancellableTasks [] module internal CodeFixHelpers = - let private reportCodeFixTelemetry + let reportCodeFixTelemetry (diagnostics: ImmutableArray) (doc: Document) (staticName: string) @@ -70,6 +75,38 @@ module internal CodeFixHelpers = name ) + let getUnusedSymbol textSpan (document: Document) codeFixName = + cancellableTask { + let! token = CancellableTask.getCurrentCancellationToken () + let! sourceText = document.GetTextAsync token + + let ident = sourceText.ToString textSpan + + // Prefixing operators and backticked identifiers does not make sense. + // We have to use the additional check for backtickes + if PrettyNaming.IsIdentifierName ident then + let! lexerSymbol = + document.TryFindFSharpLexerSymbolAsync(textSpan.Start, SymbolLookupKind.Greedy, false, false, CodeFix.RenameUnusedValue) + + let range = + RoslynHelpers.TextSpanToFSharpRange(document.FilePath, textSpan, sourceText) + + let lineText = (sourceText.Lines.GetLineFromPosition textSpan.Start).ToString() + + let! _, checkResults = document.GetFSharpParseAndCheckResultsAsync codeFixName + + return + lexerSymbol + |> Option.bind (fun symbol -> + checkResults.GetSymbolUseAtLocation(range.StartLine, range.EndColumn, lineText, symbol.FullIsland)) + |> Option.bind (fun symbolUse -> + match symbolUse.Symbol with + | :? FSharpMemberOrFunctionOrValue as func when func.IsValue -> Some symbolUse.Symbol + | _ -> None) + else + return None + } + [] module internal CodeFixExtensions = type CodeFixContext with @@ -106,3 +143,55 @@ 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 +[] +module IFSharpCodeFixProviderExtensions = + type IFSharpCodeFixProvider with + + member provider.RegisterFsharpFixAll() = + FixAllProvider.Create(fun fixAllCtx doc allDiagnostics -> + cancellableTask { + let sw = Stopwatch.StartNew() + + let! token = CancellableTask.getCurrentCancellationToken () + let! sourceText = doc.GetTextAsync token + + // this is not used anywhere, it's just needed to create the context + let action = + Action>(fun _ _ -> ()) + + let! codeFixOpts = + allDiagnostics + // The distiction is to avoid collisions of compiler and analyzer diags + // See: https://github.com/dotnet/fsharp/issues/15620 + |> Seq.distinctBy (fun d -> d.Id, d.Location) + |> Seq.map (fun diag -> CodeFixContext(doc, diag, 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 + } + |> CancellableTask.start fixAllCtx.CancellationToken) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/DiscardUnusedValue.fs b/vsintegration/src/FSharp.Editor/CodeFixes/DiscardUnusedValue.fs new file mode 100644 index 00000000000..65731a1e94c --- /dev/null +++ b/vsintegration/src/FSharp.Editor/CodeFixes/DiscardUnusedValue.fs @@ -0,0 +1,51 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +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 + +open FSharp.Compiler.Symbols + +open CancellableTasks + +[] +type internal RenameUnusedValueWithUnderscoreCodeFixProvider [] () = + inherit CodeFixProvider() + + static let getTitle (symbolName: string) = + String.Format(SR.RenameValueToUnderscore(), symbolName) + + override _.FixableDiagnosticIds = ImmutableArray.Create "FS1182" + + override this.RegisterCodeFixesAsync context = + if context.Document.Project.IsFSharpCodeFixesUnusedDeclarationsEnabled then + context.RegisterFsharpFix this + else + Task.CompletedTask + + override this.GetFixAllProvider() = this.RegisterFsharpFixAll() + + interface IFSharpCodeFixProvider with + member _.GetCodeFixIfAppliesAsync context = + cancellableTask { + let! symbol = CodeFixHelpers.getUnusedSymbol context.Span context.Document CodeFix.RenameUnusedValue + + return + symbol + |> Option.filter (fun symbol -> + match symbol with + | :? FSharpMemberOrFunctionOrValue as x when x.IsConstructorThisValue -> false + | _ -> true) + |> Option.map (fun symbol -> + { + Name = CodeFix.RenameUnusedValue + Message = getTitle symbol.DisplayName + Changes = [ TextChange(context.Span, "_") ] + }) + } diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/FixIndexerAccess.fs b/vsintegration/src/FSharp.Editor/CodeFixes/FixIndexerAccess.fs index 400982edd73..df631077e4b 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/FixIndexerAccess.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/FixIndexerAccess.fs @@ -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 - -[] -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 [] -type internal FsharpFixRemoveDotFromIndexerAccessOptIn() as this = +type internal RemoveDotFromIndexerAccessOptInCodeFixProvider() = inherit CodeFixProvider() static let title = CompilerDiagnostics.GetErrorMessage FSharpDiagnosticKind.RemoveIndexerDot - member this.GetChanges(_document: Document, diagnostics: ImmutableArray, _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, "") ] + } + ) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/FixIndexerAccessLegacy.fs b/vsintegration/src/FSharp.Editor/CodeFixes/FixIndexerAccessLegacy.fs new file mode 100644 index 00000000000..9b37c396473 --- /dev/null +++ b/vsintegration/src/FSharp.Editor/CodeFixes/FixIndexerAccessLegacy.fs @@ -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 + +[] +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) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/PrefixUnusedValue.fs b/vsintegration/src/FSharp.Editor/CodeFixes/PrefixUnusedValue.fs new file mode 100644 index 00000000000..0a913e7fa81 --- /dev/null +++ b/vsintegration/src/FSharp.Editor/CodeFixes/PrefixUnusedValue.fs @@ -0,0 +1,45 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +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 + +open CancellableTasks + +[] +type internal PrefixUnusedValueWithUnderscoreCodeFixProvider [] () = + inherit CodeFixProvider() + + static let getTitle (symbolName: string) = + String.Format(SR.PrefixValueNameWithUnderscore(), symbolName) + + override _.FixableDiagnosticIds = ImmutableArray.Create "FS1182" + + override this.RegisterCodeFixesAsync context = + if context.Document.Project.IsFSharpCodeFixesUnusedDeclarationsEnabled then + context.RegisterFsharpFix this + else + Task.CompletedTask + + override this.GetFixAllProvider() = this.RegisterFsharpFixAll() + + interface IFSharpCodeFixProvider with + member _.GetCodeFixIfAppliesAsync context = + cancellableTask { + let! symbol = CodeFixHelpers.getUnusedSymbol context.Span context.Document CodeFix.PrefixUnusedValue + + return + symbol + |> Option.map (fun symbol -> + { + Name = CodeFix.PrefixUnusedValue + Message = getTitle symbol.DisplayName + Changes = [ TextChange(TextSpan(context.Span.Start, 0), "_") ] + }) + } diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperflousCaptureForUnionCaseWithNoData.fs b/vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperflousCaptureForUnionCaseWithNoData.fs deleted file mode 100644 index 349f4f502fa..00000000000 --- a/vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperflousCaptureForUnionCaseWithNoData.fs +++ /dev/null @@ -1,64 +0,0 @@ -// 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.Threading -open System.Threading.Tasks -open System.Collections.Immutable - -open Microsoft.CodeAnalysis -open Microsoft.CodeAnalysis.Text -open Microsoft.CodeAnalysis.CodeFixes - -open FSharp.Compiler.EditorServices - -[] -type internal RemoveSuperflousCaptureForUnionCaseWithNoDataCodeFixProvider [] () = - - inherit CodeFixProvider() - - static let title = SR.RemoveUnusedBinding() - override _.FixableDiagnosticIds = ImmutableArray.Create("FS0725", "FS3548") - - member this.GetChanges(document: Document, diagnostics: ImmutableArray, ct: CancellationToken) = - backgroundTask { - - let! sourceText = document.GetTextAsync(ct) - let! _, checkResults = document.GetFSharpParseAndCheckResultsAsync(CodeFix.RemoveSuperfluousCapture) - - 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) - - let unionCaseItem = - classifications - |> Array.tryFind (fun c -> c.Type = SemanticClassificationType.UnionCase) - - 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 - - let reminderSpan = - new TextSpan(textSpan.Start + typeInfoLength, textSpan.Length - typeInfoLength) - - yield TextChange(reminderSpan, "") - } - - return changes - } - - 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) - } - - override this.GetFixAllProvider() = - CodeFixHelpers.createFixAllProvider CodeFix.RemoveSuperfluousCapture this.GetChanges diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperfluousCaptureForUnionCaseWithNoData.fs b/vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperfluousCaptureForUnionCaseWithNoData.fs new file mode 100644 index 00000000000..7896d19b582 --- /dev/null +++ b/vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperfluousCaptureForUnionCaseWithNoData.fs @@ -0,0 +1,58 @@ +// 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.EditorServices + +open CancellableTasks + +[] +type internal RemoveSuperfluousCaptureForUnionCaseWithNoDataCodeFixProvider [] () = + inherit CodeFixProvider() + + static let title = SR.RemoveUnusedBinding() + + override _.FixableDiagnosticIds = ImmutableArray.Create("FS0725", "FS3548") // cannot happen at once + + override this.RegisterCodeFixesAsync ctx = + if ctx.Document.Project.IsFSharpCodeFixesUnusedDeclarationsEnabled then + ctx.RegisterFsharpFix this + else + Task.CompletedTask + + override this.GetFixAllProvider() = this.RegisterFsharpFixAll() + + interface IFSharpCodeFixProvider with + member _.GetCodeFixIfAppliesAsync context = + cancellableTask { + let! sourceText = context.GetSourceTextAsync() + let! _, checkResults = context.Document.GetFSharpParseAndCheckResultsAsync CodeFix.RemoveSuperfluousCapture + + let m = + RoslynHelpers.TextSpanToFSharpRange(context.Document.FilePath, context.Span, sourceText) + + let classifications = checkResults.GetSemanticClassification(Some m) + + 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 + + let reminderSpan = + TextSpan(context.Span.Start + typeInfoLength, context.Span.Length - typeInfoLength) + + { + Name = CodeFix.RemoveSuperfluousCapture + Message = title + Changes = [ TextChange(reminderSpan, "") ] + }) + } diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/RemoveUnusedBinding.fs b/vsintegration/src/FSharp.Editor/CodeFixes/RemoveUnusedBinding.fs index eb55bd3f0dd..10b4a44106a 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/RemoveUnusedBinding.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/RemoveUnusedBinding.fs @@ -4,68 +4,91 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System 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 + [] type internal RemoveUnusedBindingCodeFixProvider [] () = inherit CodeFixProvider() static let title = SR.RemoveUnusedBinding() - override _.FixableDiagnosticIds = ImmutableArray.Create("FS1182") - member this.GetChanges(document: Document, diagnostics: ImmutableArray, ct: CancellationToken) = - backgroundTask { + override _.FixableDiagnosticIds = ImmutableArray.Create "FS1182" + + override this.RegisterCodeFixesAsync context = + if context.Document.Project.IsFSharpCodeFixesUnusedDeclarationsEnabled then + context.RegisterFsharpFix this + else + Task.CompletedTask + + override this.GetFixAllProvider() = this.RegisterFsharpFixAll() + + interface IFSharpCodeFixProvider with + member _.GetCodeFixIfAppliesAsync context = + cancellableTask { + let! token = CancellableTask.getCurrentCancellationToken () + + let! sourceText = context.Document.GetTextAsync token + let! parseResults = context.Document.GetFSharpParseResultsAsync(nameof RemoveUnusedBindingCodeFixProvider) + + let change = + let bindingRangeOpt = + RoslynHelpers.TextSpanToFSharpRange(context.Document.FilePath, context.Span, sourceText) + |> fun r -> parseResults.TryRangeOfBindingWithHeadPatternWithPos(r.Start) + + match bindingRangeOpt with + | Some (Expression range) -> + let span = RoslynHelpers.FSharpRangeToTextSpan(sourceText, range) + + let keywordEndColumn = + let rec loop ch pos = + if not (Char.IsWhiteSpace(ch)) then + pos + else + loop sourceText.[pos - 1] (pos - 1) + + loop sourceText.[span.Start - 1] (span.Start - 1) - let! sourceText = document.GetTextAsync(ct) - let! parseResults = document.GetFSharpParseResultsAsync(nameof (RemoveUnusedBindingCodeFixProvider)) + let keywordStartColumn = keywordEndColumn - 2 // removes 'let' or 'use' + let fullSpan = TextSpan(keywordStartColumn, span.End - keywordStartColumn) - let changes = - seq { - for d in diagnostics do - let textSpan = d.Location.SourceSpan + Some(TextChange(fullSpan, "")) - let symbolRange = - RoslynHelpers.TextSpanToFSharpRange(document.FilePath, textSpan, sourceText) + | Some (SelfId range) -> + let span = RoslynHelpers.FSharpRangeToTextSpan(sourceText, range) - let spanOfBindingOpt = - parseResults.TryRangeOfBindingWithHeadPatternWithPos(symbolRange.Start) - |> Option.bind (fun rangeOfBinding -> RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, rangeOfBinding)) + let rec findAs index (str: SourceText) = + if str[index] <> ' ' then index else findAs (index - 1) str - match spanOfBindingOpt with - | Some spanOfBinding -> - let keywordEndColumn = - let rec loop ch pos = - if not (Char.IsWhiteSpace(ch)) then - pos - else - loop sourceText.[pos - 1] (pos - 1) + let rec findEqual index (str: SourceText) = + if str[index] <> ' ' then + index + else + findEqual (index + 1) str - loop sourceText.[spanOfBinding.Start - 1] (spanOfBinding.Start - 1) + let asStart = findAs (span.Start - 1) sourceText - 1 + let equalStart = findEqual span.End sourceText - // This is safe, since we could never have gotten here unless there was a `let` or `use` - let keywordStartColumn = keywordEndColumn - 2 - let fullSpan = TextSpan(keywordStartColumn, spanOfBinding.End - keywordStartColumn) + let fullSpan = TextSpan(asStart, equalStart - asStart) - yield TextChange(fullSpan, "") - | None -> () - } + Some(TextChange(fullSpan, "")) - return changes - } + | Some Member -> None - 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.RemoveUnusedBinding, title, changes) - } + | None -> None - override this.GetFixAllProvider() = - CodeFixHelpers.createFixAllProvider CodeFix.RemoveUnusedBinding this.GetChanges + return + change + |> Option.map (fun change -> + { + Name = CodeFix.RemoveUnusedBinding + Message = title + Changes = [ change ] + }) + } diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs b/vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs deleted file mode 100644 index b3afac2ae0f..00000000000 --- a/vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs +++ /dev/null @@ -1,161 +0,0 @@ -// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. - -namespace Microsoft.VisualStudio.FSharp.Editor - -open System -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.Symbols -open FSharp.Compiler.Syntax - -module UnusedCodeFixHelper = - let getUnusedSymbol (sourceText: SourceText) (textSpan: TextSpan) (document: Document) = - - let ident = sourceText.ToString(textSpan) - - // Prefixing operators and backticked identifiers does not make sense. - // We have to use the additional check for backtickes - if PrettyNaming.IsIdentifierName ident then - asyncMaybe { - let! lexerSymbol = - document.TryFindFSharpLexerSymbolAsync(textSpan.Start, SymbolLookupKind.Greedy, false, false, CodeFix.RenameUnusedValue) - - let m = RoslynHelpers.TextSpanToFSharpRange(document.FilePath, textSpan, sourceText) - - let lineText = (sourceText.Lines.GetLineFromPosition textSpan.Start).ToString() - - let! _, checkResults = - document.GetFSharpParseAndCheckResultsAsync(CodeFix.RenameUnusedValue) - |> liftAsync - - return! checkResults.GetSymbolUseAtLocation(m.StartLine, m.EndColumn, lineText, lexerSymbol.FullIsland) - - } - else - async { return None } - -[] -type internal PrefixUnusedValueWithUnderscoreCodeFixProvider [] () = - - inherit CodeFixProvider() - - static let title (symbolName: string) = - String.Format(SR.PrefixValueNameWithUnderscore(), symbolName) - - override _.FixableDiagnosticIds = ImmutableArray.Create("FS1182") - - member this.GetChanges(document: Document, diagnostics: ImmutableArray, ct: CancellationToken) = - backgroundTask { - let! sourceText = document.GetTextAsync(ct) - - let! changes = - seq { - for d in diagnostics do - let textSpan = d.Location.SourceSpan - - yield - async { - let! symbolUse = UnusedCodeFixHelper.getUnusedSymbol sourceText textSpan document - - return - seq { - match symbolUse with - | None -> () - | Some symbolUse -> - match symbolUse.Symbol with - | :? FSharpMemberOrFunctionOrValue -> yield TextChange(TextSpan(textSpan.Start, 0), "_") - | _ -> () - } - } - } - |> Async.Parallel - - return (changes |> Seq.concat) - } - - override this.RegisterCodeFixesAsync ctx : Task = - backgroundTask { - if ctx.Document.Project.IsFSharpCodeFixesUnusedDeclarationsEnabled then - let! sourceText = ctx.Document.GetTextAsync(ctx.CancellationToken) - let! unusedSymbol = UnusedCodeFixHelper.getUnusedSymbol sourceText ctx.Span ctx.Document - - match unusedSymbol with - | None -> () - | Some symbolUse -> - match symbolUse.Symbol with - | :? FSharpMemberOrFunctionOrValue -> - let prefixTitle = title symbolUse.Symbol.DisplayName - let! changes = this.GetChanges(ctx.Document, ctx.Diagnostics, ctx.CancellationToken) - ctx.RegisterFsharpFix(CodeFix.PrefixUnusedValue, prefixTitle, changes) - | _ -> () - } - - override this.GetFixAllProvider() = - CodeFixHelpers.createFixAllProvider CodeFix.PrefixUnusedValue this.GetChanges - -[] -type internal FSharpRenameUnusedValueWithUnderscoreCodeFixProvider [] () = - - inherit CodeFixProvider() - - static let title (symbolName: string) = - String.Format(SR.RenameValueToUnderscore(), symbolName) - - override _.FixableDiagnosticIds = ImmutableArray.Create("FS1182") - - member this.GetChanges(document: Document, diagnostics: ImmutableArray, ct: CancellationToken) = - backgroundTask { - let! sourceText = document.GetTextAsync(ct) - - let! changes = - seq { - for d in diagnostics do - let textSpan = d.Location.SourceSpan - - yield - async { - let! symbolUse = UnusedCodeFixHelper.getUnusedSymbol sourceText textSpan document - - return - seq { - match symbolUse with - | None -> () - | Some symbolUse -> - match symbolUse.Symbol with - | :? FSharpMemberOrFunctionOrValue as func when func.IsValue -> yield TextChange(textSpan, "_") - | _ -> () - } - } - } - |> Async.Parallel - - return (changes |> Seq.concat) - } - - override this.RegisterCodeFixesAsync ctx : Task = - backgroundTask { - if ctx.Document.Project.IsFSharpCodeFixesUnusedDeclarationsEnabled then - let! sourceText = ctx.Document.GetTextAsync(ctx.CancellationToken) - let! unusedSymbol = UnusedCodeFixHelper.getUnusedSymbol sourceText ctx.Span ctx.Document - - match unusedSymbol with - | None -> () - | Some symbolUse -> - match symbolUse.Symbol with - | :? FSharpMemberOrFunctionOrValue as func when func.IsValue -> - let prefixTitle = title symbolUse.Symbol.DisplayName - - let! changes = this.GetChanges(ctx.Document, ctx.Diagnostics, ctx.CancellationToken) - ctx.RegisterFsharpFix(CodeFix.RenameUnusedValue, prefixTitle, changes) - | _ -> () - } - - override this.GetFixAllProvider() = - CodeFixHelpers.createFixAllProvider CodeFix.RenameUnusedValue this.GetChanges diff --git a/vsintegration/src/FSharp.Editor/Common/FSharpCodeAnalysisExtensions.fs b/vsintegration/src/FSharp.Editor/Common/FSharpCodeAnalysisExtensions.fs index 51fc47ec72b..5003cee72b0 100644 --- a/vsintegration/src/FSharp.Editor/Common/FSharpCodeAnalysisExtensions.fs +++ b/vsintegration/src/FSharp.Editor/Common/FSharpCodeAnalysisExtensions.fs @@ -5,6 +5,11 @@ open FSharp.Compiler.CodeAnalysis open FSharp.Compiler.Syntax open FSharp.Compiler.Text +type UnusedBinding = + | Expression of range + | SelfId of range + | Member + type FSharpParseFileResults with member this.TryRangeOfBindingWithHeadPatternWithPos pos = @@ -20,18 +25,43 @@ type FSharpParseFileResults with match binding with | SynBinding (kind = SynBindingKind.Normal; headPat = pat) as binding -> if Position.posEq binding.RangeOfHeadPattern.Start pos then - Some binding.RangeOfBindingWithRhs + Some(Expression binding.RangeOfBindingWithRhs) else // Check if it's an operator match pat with | SynPat.LongIdent(longDotId = LongIdentWithDots ([ id ], _)) when id.idText.StartsWith("op_") -> if Position.posEq id.idRange.Start pos then - Some binding.RangeOfBindingWithRhs + Some(Expression binding.RangeOfBindingWithRhs) else defaultTraverse binding | _ -> defaultTraverse binding | _ -> defaultTraverse binding + + override _.VisitComponentInfo(path, _) = + path + |> List.collect (fun node -> + match node with + | SyntaxNode.SynModule (SynModuleDecl.Types (types, _)) -> types + | _ -> []) + |> Seq.choose (fun node -> + match node with + | SynTypeDefn(implicitConstructor = Some (SynMemberDefn.ImplicitCtor(selfIdentifier = Some ident))) when + ident.idRange.Start = pos + -> + Some(SelfId ident.idRange) + | SynTypeDefn(typeRepr = SynTypeDefnRepr.ObjectModel (members = defs)) -> + defs + |> List.choose (fun node -> + match node with + | SynMemberDefn.Member (SynBinding(headPat = SynPat.LongIdent (longDotId = id)), _) when + id.Range.Start = pos + -> + Some Member + | _ -> None) + |> List.tryExactlyOne + | _ -> None) + |> Seq.tryExactlyOne } ) diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj index d50ba5d9b6a..970287c0510 100644 --- a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj +++ b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj @@ -125,13 +125,15 @@ - + - + + + diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddInstanceMemberParameterTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddInstanceMemberParameterTests.fs index b47462f7be6..b72f44179f3 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddInstanceMemberParameterTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddInstanceMemberParameterTests.fs @@ -8,7 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = AddInstanceMemberParameterCodeFixProvider() -let private diagnostic = 0673 // This instance member needs a parameter to represent the object being invoked... [] let ``Fixes FS0673`` () = @@ -29,6 +28,6 @@ type UsefulTestHarness() = """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingEqualsToTypeDefinitionTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingEqualsToTypeDefinitionTests.fs index 4b02e503e54..a2ead170a1b 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingEqualsToTypeDefinitionTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingEqualsToTypeDefinitionTests.fs @@ -8,7 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = AddMissingEqualsToTypeDefinitionCodeFixProvider() -let private diagnostic = 0010 // Unexpected symbol... [] let ``Fixes FS0010 for missing equals in type def - simple types`` () = @@ -27,7 +26,7 @@ type Song = { Artist : string; Title : int } """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) @@ -48,7 +47,7 @@ type Name = Name of string """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) @@ -58,6 +57,6 @@ type Name = Name of string let ``Doesn't fix FS0010 for random unexpected symbols`` code = let expected = None - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingFunKeywordTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingFunKeywordTests.fs index 44f02147908..a52daf0f546 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingFunKeywordTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingFunKeywordTests.fs @@ -8,7 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = AddMissingFunKeywordCodeFixProvider() -let private diagnostic = 0010 // Unexpected symbol... [] let ``Fixes FS0010 for missing fun keyword`` () = @@ -31,7 +30,7 @@ let gettingEven numbers = """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) @@ -44,6 +43,6 @@ let ``Doesn't fix FS0010 for random unexpected symbols`` () = let expected = None - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs index a3d1bc3d526..2b4fc9d09e1 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs @@ -8,7 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = AddMissingRecToMutuallyRecFunctionsCodeFixProvider() -let private diagnostic = 0576 // The declaration form 'let ... and ...' for non-recursive bindings is not used in F# code... // TODO: write some negative test cases here @@ -45,6 +44,6 @@ and isOdd n = """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code (Manual("let", 0576)) Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddNewKeywordToDisposableConstructorInvocationTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddNewKeywordToDisposableConstructorInvocationTests.fs new file mode 100644 index 00000000000..2709d139387 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddNewKeywordToDisposableConstructorInvocationTests.fs @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.AddNewKeywordToDisposableConstructorInvocationTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = AddNewKeywordCodeFixProvider() + +[] +let ``Fixes FS0760`` () = + let code = + """ +let sr = System.IO.StreamReader "test.txt" +""" + + let expected = + Some + { + Message = "Add 'new' keyword" + FixedCode = + """ +let sr = new System.IO.StreamReader "test.txt" +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangePrefixNegationToInfixSubtractionTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangePrefixNegationToInfixSubtractionTests.fs index 766edf5f58c..5245d8a3ae0 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangePrefixNegationToInfixSubtractionTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangePrefixNegationToInfixSubtractionTests.fs @@ -8,7 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = ChangePrefixNegationToInfixSubtractionCodeFixProvider() -let private diagnostic = 0003 // The value is not a function and cannot be applied [] [] @@ -33,7 +32,7 @@ let f (numbers: 'a array) = """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) @@ -46,6 +45,6 @@ let x = 1 (+) 2 let expected = None - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeRefCellDerefToNotExpressionTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeRefCellDerefToNotExpressionTests.fs index bb2d9473626..828f33f7097 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeRefCellDerefToNotExpressionTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeRefCellDerefToNotExpressionTests.fs @@ -8,7 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = ChangeRefCellDerefToNotExpressionCodeFixProvider() -let private diagnostic = 0001 // Type mismatch... [] let ``Fixes FS0001 for invalid negation syntax`` () = @@ -27,7 +26,7 @@ let myNot (value: bool) = not value """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) @@ -40,6 +39,6 @@ let one, two = 1, 2, 3 let expected = None - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeToUpcastTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeToUpcastTests.fs index e2a1522e84f..a22f534cf87 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeToUpcastTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeToUpcastTests.fs @@ -8,7 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = ChangeToUpcastCodeFixProvider() -let private diagnostic = 3198 // The conversion is an upcast, not a downcast... // Test cases are taken from the original PR: // https://github.com/dotnet/fsharp/pull/10463 @@ -36,7 +35,7 @@ let Thing : IFoo = Foo() :> IFoo """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code (Manual("Foo() :?> IFoo", 3198)) Assert.Equal(expected, actual) @@ -63,7 +62,7 @@ let Thing : IFoo = upcast Foo() """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code (Manual("downcast Foo()", 3198)) Assert.Equal(expected, actual) @@ -80,6 +79,6 @@ let Thing : IdowncastFoo = Foo() :?> IdowncastFoo let expected = None - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code (Manual("Foo() :?> IdowncastFoo", 3198)) Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs index 4fb17657a19..0d18e991ff4 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs @@ -17,25 +17,43 @@ open FSharp.Editor.Tests.Helpers type TestCodeFix = { Message: string; FixedCode: string } +type Mode = + | Auto + | WithOption of CustomProjectOption: string + | Manual of Squiggly: string * Number: int + let mockAction = Action>(fun _ _ -> ()) -let getRelevantDiagnostic (document: Document) errorNumber = +let getDocument code mode = + match mode with + | Auto -> RoslynTestHelpers.GetFsDocument code + | WithOption option -> RoslynTestHelpers.GetFsDocument(code, option) + | Manual _ -> RoslynTestHelpers.GetFsDocument code + +let getRelevantDiagnostic (document: Document) = cancellableTask { let! _, checkFileResults = document.GetFSharpParseAndCheckResultsAsync "test" - return - checkFileResults.Diagnostics - |> Seq.where (fun d -> d.ErrorNumber = errorNumber) - |> Seq.head + return checkFileResults.Diagnostics |> Seq.head } -let createTestCodeFixContext (code: string) (document: Document) (diagnostic: FSharpDiagnostic) = +let createTestCodeFixContext (code: string) document (mode: Mode) = cancellableTask { let! cancellationToken = CancellableTask.getCurrentCancellationToken () let sourceText = SourceText.From code + let! diagnostic = + match mode with + | Auto -> getRelevantDiagnostic document + | WithOption _ -> getRelevantDiagnostic 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) @@ -44,13 +62,12 @@ let createTestCodeFixContext (code: string) (document: Document) (diagnostic: FS return CodeFixContext(document, roslynDiagnostic, mockAction, cancellationToken) } -let tryFix (code: string) diagnostic (fixProvider: IFSharpCodeFixProvider) = +let tryFix (code: string) mode (fixProvider: IFSharpCodeFixProvider) = cancellableTask { let sourceText = SourceText.From code - let document = RoslynTestHelpers.GetFsDocument code + let document = getDocument code mode - let! diagnostic = getRelevantDiagnostic document diagnostic - let! context = createTestCodeFixContext code document diagnostic + let! context = createTestCodeFixContext code document mode let! result = fixProvider.GetCodeFixIfAppliesAsync context diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertCSharpLambdaToFSharpLambdaTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertCSharpLambdaToFSharpLambdaTests.fs index 3c66ceb5370..b1c15cc35b3 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertCSharpLambdaToFSharpLambdaTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertCSharpLambdaToFSharpLambdaTests.fs @@ -9,8 +9,6 @@ open CodeFixTestFramework let private codeFix = ConvertCSharpLambdaToFSharpLambdaCodeFixProvider() -let private diagnostic = 0039 // Something is not defined - [] let ``Fixes FS0039 for lambdas`` () = let code = @@ -28,7 +26,7 @@ let incAll = List.map (fun n -> n + 1) """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) @@ -41,6 +39,6 @@ let f = g let expected = None - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToAnonymousRecordTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToAnonymousRecordTests.fs index 55af0a5e040..198399e6d2b 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToAnonymousRecordTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToAnonymousRecordTests.fs @@ -8,7 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = ConvertToAnonymousRecordCodeFixProvider() -let private diagnostic = 0039 // ... is not defined... [] let ``Fixes FS0039 for records`` () = @@ -27,7 +26,7 @@ let band = {| Name = "The Velvet Underground" |} """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) @@ -40,6 +39,6 @@ let x = someUndefinedFunction 42 let expected = None - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToNotEqualsEqualityExpressionTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToNotEqualsEqualityExpressionTests.fs index 3b621cfa863..2aea6ddafd5 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToNotEqualsEqualityExpressionTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToNotEqualsEqualityExpressionTests.fs @@ -8,7 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = ConvertToNotEqualsEqualityExpressionCodeFixProvider() -let private diagnostic = 0043 // The type doesn't support the value... [] let ``Fixes FS0043 for C# inequality operator`` () = @@ -27,7 +26,7 @@ let areNotEqual (x: int) (y: int) = x <> y """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) @@ -42,6 +41,6 @@ let x : RecordType = null let expected = None - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToSingleEqualsEqualityExpressionTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToSingleEqualsEqualityExpressionTests.fs index 94a7d4119a4..3ead2412220 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToSingleEqualsEqualityExpressionTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToSingleEqualsEqualityExpressionTests.fs @@ -8,7 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = ConvertToSingleEqualsEqualityExpressionCodeFixProvider() -let private diagnostic = 0043 // The type doesn't support the value... [] let ``Fixes FS0043 for C# equality operator`` () = @@ -27,7 +26,7 @@ let areEqual (x: int) (y: int) = x = y """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) @@ -42,6 +41,6 @@ let x : RecordType = null let expected = None - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/DiscardUnusedValueTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/DiscardUnusedValueTests.fs new file mode 100644 index 00000000000..56ea9d0e601 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/DiscardUnusedValueTests.fs @@ -0,0 +1,73 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.DiscardUnusedValueTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = RenameUnusedValueWithUnderscoreCodeFixProvider() + +[] +let ``Fixes FS1182 - let bindings in classes`` () = + let code = + """ +type T() = + let blah = 42 +""" + + let expected = + Some + { + Message = "Rename 'blah' to '_'" + FixedCode = + """ +type T() = + let _ = 42 +""" + } + + let actual = codeFix |> tryFix code (WithOption "--warnon:1182") + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS1182 - let bindings within let bindings`` () = + let code = + """ +let f() = + let blah = "test" + 42 +""" + + let expected = + Some + { + Message = "Rename 'blah' to '_'" + FixedCode = + """ +let f() = + let _ = "test" + 42 +""" + } + + let actual = codeFix |> tryFix code (WithOption "--warnon:1182") + + Assert.Equal(expected, actual) + +[] +let ``Doesn't fix FS1182 - class identifiers`` () = + let code = + """ +type T() as this = class end +""" + + let expected = None + + let actual = codeFix |> tryFix code (WithOption "--warnon:1182") + + Assert.Equal(expected, actual) + +// TODO: add tests for scenarios with signature files diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/FixIndexerAccessTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/FixIndexerAccessTests.fs new file mode 100644 index 00000000000..eefecea132f --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/FixIndexerAccessTests.fs @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.FixIndexerAccessTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = RemoveDotFromIndexerAccessOptInCodeFixProvider() + +[] +let ``Fixes FS3366`` () = + let code = + """ +let list = [ 42 ] + +let _ = list.[0] +""" + + let expected = + Some + { + Message = "The syntax 'arr.[idx]' is now revised to 'arr[idx]'. Please update your code." + FixedCode = + """ +let list = [ 42 ] + +let _ = list[0] +""" + } + + let actual = codeFix |> tryFix code (WithOption "--warnon:3366") + + Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/MakeOuterBindingRecursiveTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/MakeOuterBindingRecursiveTests.fs index 4f438e38f1c..613cec7eb02 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/MakeOuterBindingRecursiveTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/MakeOuterBindingRecursiveTests.fs @@ -8,7 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = MakeOuterBindingRecursiveCodeFixProvider() -let private diagnostic = 0039 // Something is not defined... [] let ``Fixes FS0039 for recursive functions`` () = @@ -33,7 +32,7 @@ let rec factorial n = """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) @@ -46,6 +45,6 @@ let f = g let expected = None - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/PrefixUnusedValueTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/PrefixUnusedValueTests.fs new file mode 100644 index 00000000000..a0350bf5d75 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/PrefixUnusedValueTests.fs @@ -0,0 +1,81 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.PrefixUnusedValueTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = PrefixUnusedValueWithUnderscoreCodeFixProvider() + +[] +let ``Fixes FS1182 - let bindings in classes`` () = + let code = + """ +type T() = + let blah = 42 +""" + + let expected = + Some + { + Message = "Prefix 'blah' with underscore" + FixedCode = + """ +type T() = + let _blah = 42 +""" + } + + let actual = codeFix |> tryFix code (WithOption "--warnon:1182") + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS1182 - let bindings within let bindings`` () = + let code = + """ +let f() = + let blah = "test" + 42 +""" + + let expected = + Some + { + Message = "Prefix 'blah' with underscore" + FixedCode = + """ +let f() = + let _blah = "test" + 42 +""" + } + + let actual = codeFix |> tryFix code (WithOption "--warnon:1182") + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS1182 - class identifiers`` () = + let code = + """ +type T() as this = class end +""" + + let expected = + Some + { + Message = "Prefix 'this' with underscore" + FixedCode = + """ +type T() as _this = class end +""" + } + + let actual = codeFix |> tryFix code (WithOption "--warnon:1182") + + Assert.Equal(expected, actual) + +// TODO: add tests for scenarios with signature files diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ProposeUppercaseLabelTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ProposeUppercaseLabelTests.fs index 14550624c9f..c03b49d7a32 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ProposeUppercaseLabelTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ProposeUppercaseLabelTests.fs @@ -8,7 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = ProposeUppercaseLabelCodeFixProvider() -let private diagnostic = 0053 // ... must be uppercase identifiers ... [] let ``Fixes FS0053 for discriminated unions`` () = @@ -27,7 +26,7 @@ type MyNumber = Number of int """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) @@ -48,6 +47,6 @@ exception LowException of string """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveReturnOrYieldTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveReturnOrYieldTests.fs index a4eaf01e268..23d5908d557 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveReturnOrYieldTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveReturnOrYieldTests.fs @@ -8,8 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = RemoveReturnOrYieldCodeFixProvider() -let private yieldDiagnostic = 0747 // This construct may only be used within list, array and sequence expressions... -let private returnDiagnostic = 0748 // This construct may only be used with computation expressions... // TODO: write some negative tests here @@ -32,7 +30,7 @@ let answer question = """ } - let actual = codeFix |> tryFix code yieldDiagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) @@ -55,7 +53,7 @@ let answer question = """ } - let actual = codeFix |> tryFix code yieldDiagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) @@ -78,7 +76,7 @@ let answer question = """ } - let actual = codeFix |> tryFix code returnDiagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) @@ -101,6 +99,6 @@ let answer question = """ } - let actual = codeFix |> tryFix code returnDiagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveSuperfluousCaptureForUnionCaseWithNoDataTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveSuperfluousCaptureForUnionCaseWithNoDataTests.fs new file mode 100644 index 00000000000..6ea555d1aac --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveSuperfluousCaptureForUnionCaseWithNoDataTests.fs @@ -0,0 +1,141 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.RemoveSuperflousCaptureForUnionCaseWithNoDataTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = + RemoveSuperfluousCaptureForUnionCaseWithNoDataCodeFixProvider() + +[] +[] +[] +[] +let ``Fixes FS3548 - DUs`` caseValue = + let code = + $""" +type Type = | A | B of int + +let f x = + match x with + | A {caseValue} -> 42 + | B number -> number +""" + + let expected = + Some + { + Message = "Remove unused binding" + FixedCode = + """ +type Type = | A | B of int + +let f x = + match x with + | A -> 42 + | B number -> number +""" + } + + let actual = codeFix |> tryFix code (WithOption "--langversion:preview") + + Assert.Equal(expected, actual) + +[] +[] +[] +[] +let ``Fixes FS3548 - marker types`` caseValue = + let code = + $""" +type Type = Type + +let f x = + match x with + | Type {caseValue} -> () +""" + + let expected = + Some + { + Message = "Remove unused binding" + FixedCode = + """ +type Type = Type + +let f x = + match x with + | Type -> () +""" + } + + let actual = codeFix |> tryFix code (WithOption "--langversion:preview") + + Assert.Equal(expected, actual) + +[] +[] +[] +let ``Fixes FS0725 - DUs`` caseValue = + let code = + $""" +type Type = | A | B of int + +let f x = + match x with + | A {caseValue} -> 42 + | B number -> number +""" + + let expected = + Some + { + Message = "Remove unused binding" + FixedCode = + """ +type Type = | A | B of int + +let f x = + match x with + | A -> 42 + | B number -> number +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +[] +[] +let ``Fixes FS0725 - marker types`` caseValue = + let code = + $""" +type T = T + +let f x = + match x with + | T {caseValue} -> () +""" + + let expected = + Some + { + Message = "Remove unused binding" + FixedCode = + """ +type T = T + +let f x = + match x with + | T -> () +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnusedBindingTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnusedBindingTests.fs new file mode 100644 index 00000000000..745107f4d48 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveUnusedBindingTests.fs @@ -0,0 +1,97 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.RemoveUnusedBindingTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = RemoveUnusedBindingCodeFixProvider() + +[] +let ``Fixes FS1182 - let bindings in classes`` () = + let code = + """ +type T() = + let blah = "test" +""" + + let expected = + Some + { + Message = "Remove unused binding" + FixedCode = + """ +type T() = + +""" + } + + let actual = codeFix |> tryFix code (WithOption "--warnon:1182") + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS1182 - let bindings within let bindings`` () = + let code = + """ +let f() = + let blah = "test" + 42 +""" + + let expected = + Some + { + Message = "Remove unused binding" + FixedCode = + """ +let f() = + + 42 +""" + } + + let actual = codeFix |> tryFix code (WithOption "--warnon:1182") + + Assert.Equal(expected, actual) + +[] +[] +[] +[] +[] +let ``Fixes FS1182 - class identifiers`` preSpaces postSpaces = + let code = + $""" +type C() as{preSpaces}this{postSpaces}= class end +""" + + let expected = + Some + { + Message = "Remove unused binding" + FixedCode = + """ +type C() = class end +""" + } + + let actual = codeFix |> tryFix code (Manual("this", 1182)) + + Assert.Equal(expected, actual) + +[] +let ``Doesn't fix FS1182 for member identifiers`` () = + let code = + $""" +type T() = + member x.FortyTwo = 42 +""" + + let expected = None + + let actual = codeFix |> tryFix code (Manual("x", 1182)) + + Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/UseTripleQuotedInterpolationTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/UseTripleQuotedInterpolationTests.fs index c7ba9ba34c5..0f539abd137 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/UseTripleQuotedInterpolationTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/UseTripleQuotedInterpolationTests.fs @@ -8,7 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = UseTripleQuotedInterpolationCodeFixProvider() -let private diagnostic = 3373 // ... invalid interpolated string ... [] let ``Fixes FS3373`` () = @@ -29,6 +28,6 @@ let createMsg x = $"Review in {x} {pluralize x "day"}" + "\r\n" } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/WrapExpressionInParenthesesTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/WrapExpressionInParenthesesTests.fs index bd22ca56966..0b4ee54f99e 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/WrapExpressionInParenthesesTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/WrapExpressionInParenthesesTests.fs @@ -8,7 +8,6 @@ open Xunit open CodeFixTestFramework let private codeFix = WrapExpressionInParenthesesCodeFixProvider() -let private diagnostic = 0597 // ... arguments involving function or method applications should be parenthesized // Test case is taken from the original PR: // https://github.com/dotnet/fsharp/pull/10460 @@ -34,6 +33,6 @@ printfn "Hello %d" (rng.Next(5)) """ } - let actual = codeFix |> tryFix code diagnostic + let actual = codeFix |> tryFix code Auto Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj index 5127add6eb7..5558e1f05d0 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -49,6 +49,12 @@ + + + + + + diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index 7d92ec6715c..36e70eae0d9 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -338,11 +338,20 @@ type RoslynTestHelpers private () = solution, checker - static member GetFsDocument code = - // without this lib some symbols are not loaded + static member GetFsDocument(code, ?customProjectOption: string) = + let customProjectOptions = + customProjectOption + |> Option.map (fun o -> [| o |]) + |> Option.defaultValue (Array.empty) + let options = { RoslynTestHelpers.DefaultProjectOptions with - OtherOptions = [| "--targetprofile:netcore" |] + OtherOptions = + [| + "--targetprofile:netcore" // without this lib some symbols are not loaded + "--nowarn:3384" // The .NET SDK for this script could not be determined + |] + |> Array.append customProjectOptions } RoslynTestHelpers.CreateSolution(code, options = options) From 7f7e24629e5955c7668872931291875933badebe Mon Sep 17 00:00:00 2001 From: Petr Date: Sat, 22 Jul 2023 12:07:21 +0200 Subject: [PATCH 02/11] Up --- .../FSharp.Editor/CodeFixes/CodeFixHelpers.fs | 35 ----- .../CodeFixes/DiscardUnusedValue.fs | 51 -------- .../CodeFixes/PrefixUnusedValue.fs | 45 ------- ...uperflousCaptureForUnionCaseWithNoData.fs} | 0 .../CodeFixes/RenameUnusedValue.fs | 121 ++++++++++++++++++ .../src/FSharp.Editor/FSharp.Editor.fsproj | 5 +- 6 files changed, 123 insertions(+), 134 deletions(-) delete mode 100644 vsintegration/src/FSharp.Editor/CodeFixes/DiscardUnusedValue.fs delete mode 100644 vsintegration/src/FSharp.Editor/CodeFixes/PrefixUnusedValue.fs rename vsintegration/src/FSharp.Editor/CodeFixes/{RemoveSuperfluousCaptureForUnionCaseWithNoData.fs => RemoveSuperflousCaptureForUnionCaseWithNoData.fs} (100%) create mode 100644 vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs index 7b94b9388cc..a7c5265daa2 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs @@ -15,9 +15,6 @@ open Microsoft.CodeAnalysis.CodeFixes open Microsoft.CodeAnalysis.CodeActions open Microsoft.VisualStudio.FSharp.Editor.Telemetry -open FSharp.Compiler.Syntax -open FSharp.Compiler.Symbols - open CancellableTasks [] @@ -75,38 +72,6 @@ module internal CodeFixHelpers = name ) - let getUnusedSymbol textSpan (document: Document) codeFixName = - cancellableTask { - let! token = CancellableTask.getCurrentCancellationToken () - let! sourceText = document.GetTextAsync token - - let ident = sourceText.ToString textSpan - - // Prefixing operators and backticked identifiers does not make sense. - // We have to use the additional check for backtickes - if PrettyNaming.IsIdentifierName ident then - let! lexerSymbol = - document.TryFindFSharpLexerSymbolAsync(textSpan.Start, SymbolLookupKind.Greedy, false, false, CodeFix.RenameUnusedValue) - - let range = - RoslynHelpers.TextSpanToFSharpRange(document.FilePath, textSpan, sourceText) - - let lineText = (sourceText.Lines.GetLineFromPosition textSpan.Start).ToString() - - let! _, checkResults = document.GetFSharpParseAndCheckResultsAsync codeFixName - - return - lexerSymbol - |> Option.bind (fun symbol -> - checkResults.GetSymbolUseAtLocation(range.StartLine, range.EndColumn, lineText, symbol.FullIsland)) - |> Option.bind (fun symbolUse -> - match symbolUse.Symbol with - | :? FSharpMemberOrFunctionOrValue as func when func.IsValue -> Some symbolUse.Symbol - | _ -> None) - else - return None - } - [] module internal CodeFixExtensions = type CodeFixContext with diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/DiscardUnusedValue.fs b/vsintegration/src/FSharp.Editor/CodeFixes/DiscardUnusedValue.fs deleted file mode 100644 index 65731a1e94c..00000000000 --- a/vsintegration/src/FSharp.Editor/CodeFixes/DiscardUnusedValue.fs +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. - -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 - -open FSharp.Compiler.Symbols - -open CancellableTasks - -[] -type internal RenameUnusedValueWithUnderscoreCodeFixProvider [] () = - inherit CodeFixProvider() - - static let getTitle (symbolName: string) = - String.Format(SR.RenameValueToUnderscore(), symbolName) - - override _.FixableDiagnosticIds = ImmutableArray.Create "FS1182" - - override this.RegisterCodeFixesAsync context = - if context.Document.Project.IsFSharpCodeFixesUnusedDeclarationsEnabled then - context.RegisterFsharpFix this - else - Task.CompletedTask - - override this.GetFixAllProvider() = this.RegisterFsharpFixAll() - - interface IFSharpCodeFixProvider with - member _.GetCodeFixIfAppliesAsync context = - cancellableTask { - let! symbol = CodeFixHelpers.getUnusedSymbol context.Span context.Document CodeFix.RenameUnusedValue - - return - symbol - |> Option.filter (fun symbol -> - match symbol with - | :? FSharpMemberOrFunctionOrValue as x when x.IsConstructorThisValue -> false - | _ -> true) - |> Option.map (fun symbol -> - { - Name = CodeFix.RenameUnusedValue - Message = getTitle symbol.DisplayName - Changes = [ TextChange(context.Span, "_") ] - }) - } diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/PrefixUnusedValue.fs b/vsintegration/src/FSharp.Editor/CodeFixes/PrefixUnusedValue.fs deleted file mode 100644 index 0a913e7fa81..00000000000 --- a/vsintegration/src/FSharp.Editor/CodeFixes/PrefixUnusedValue.fs +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. - -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 - -open CancellableTasks - -[] -type internal PrefixUnusedValueWithUnderscoreCodeFixProvider [] () = - inherit CodeFixProvider() - - static let getTitle (symbolName: string) = - String.Format(SR.PrefixValueNameWithUnderscore(), symbolName) - - override _.FixableDiagnosticIds = ImmutableArray.Create "FS1182" - - override this.RegisterCodeFixesAsync context = - if context.Document.Project.IsFSharpCodeFixesUnusedDeclarationsEnabled then - context.RegisterFsharpFix this - else - Task.CompletedTask - - override this.GetFixAllProvider() = this.RegisterFsharpFixAll() - - interface IFSharpCodeFixProvider with - member _.GetCodeFixIfAppliesAsync context = - cancellableTask { - let! symbol = CodeFixHelpers.getUnusedSymbol context.Span context.Document CodeFix.PrefixUnusedValue - - return - symbol - |> Option.map (fun symbol -> - { - Name = CodeFix.PrefixUnusedValue - Message = getTitle symbol.DisplayName - Changes = [ TextChange(TextSpan(context.Span.Start, 0), "_") ] - }) - } diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperfluousCaptureForUnionCaseWithNoData.fs b/vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperflousCaptureForUnionCaseWithNoData.fs similarity index 100% rename from vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperfluousCaptureForUnionCaseWithNoData.fs rename to vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperflousCaptureForUnionCaseWithNoData.fs diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs b/vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs new file mode 100644 index 00000000000..e54268886c5 --- /dev/null +++ b/vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs @@ -0,0 +1,121 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace Microsoft.VisualStudio.FSharp.Editor + +open System +open System.Composition +open System.Threading.Tasks +open System.Collections.Immutable + +open Microsoft.CodeAnalysis +open Microsoft.CodeAnalysis.Text +open Microsoft.CodeAnalysis.CodeFixes + +open FSharp.Compiler.Symbols +open FSharp.Compiler.Syntax + +open CancellableTasks + +module UnusedCodeFixHelper = + let getUnusedSymbol textSpan (document: Document) codeFixName = + cancellableTask { + let! token = CancellableTask.getCurrentCancellationToken () + let! sourceText = document.GetTextAsync token + + let ident = sourceText.ToString textSpan + + // Prefixing operators and backticked identifiers does not make sense. + // We have to use the additional check for backtickes + if PrettyNaming.IsIdentifierName ident then + let! lexerSymbol = + document.TryFindFSharpLexerSymbolAsync(textSpan.Start, SymbolLookupKind.Greedy, false, false, CodeFix.RenameUnusedValue) + + let m = RoslynHelpers.TextSpanToFSharpRange(document.FilePath, textSpan, sourceText) + + let lineText = (sourceText.Lines.GetLineFromPosition textSpan.Start).ToString() + + let! _, checkResults = + document.GetFSharpParseAndCheckResultsAsync codeFixName + + return + lexerSymbol + |> Option.bind (fun symbol -> + checkResults.GetSymbolUseAtLocation(m.StartLine, m.EndColumn, lineText, symbol.FullIsland)) + |> Option.bind (fun symbolUse -> + match symbolUse.Symbol with + | :? FSharpMemberOrFunctionOrValue as func when func.IsValue -> Some symbolUse.Symbol + | _ -> None) + else + return None + } + +[] +type internal PrefixUnusedValueWithUnderscoreCodeFixProvider [] () = + + inherit CodeFixProvider() + + static let getTitle (symbolName: string) = + String.Format(SR.PrefixValueNameWithUnderscore(), symbolName) + + override _.FixableDiagnosticIds = ImmutableArray.Create "FS1182" + + override this.RegisterCodeFixesAsync context = + if context.Document.Project.IsFSharpCodeFixesUnusedDeclarationsEnabled then + context.RegisterFsharpFix this + else + Task.CompletedTask + + override this.GetFixAllProvider() = this.RegisterFsharpFixAll() + + interface IFSharpCodeFixProvider with + member _.GetCodeFixIfAppliesAsync context = + cancellableTask { + let! symbol = UnusedCodeFixHelper.getUnusedSymbol context.Span context.Document CodeFix.PrefixUnusedValue + + return + symbol + |> Option.map (fun symbol -> + { + Name = CodeFix.PrefixUnusedValue + Message = getTitle symbol.DisplayName + Changes = [ TextChange(TextSpan(context.Span.Start, 0), "_") ] + }) + } + + +[] +type internal RenameUnusedValueWithUnderscoreCodeFixProvider [] () = + + inherit CodeFixProvider() + + static let getTitle (symbolName: string) = + String.Format(SR.RenameValueToUnderscore(), symbolName) + + override _.FixableDiagnosticIds = ImmutableArray.Create "FS1182" + + override this.RegisterCodeFixesAsync context = + if context.Document.Project.IsFSharpCodeFixesUnusedDeclarationsEnabled then + context.RegisterFsharpFix this + else + Task.CompletedTask + + override this.GetFixAllProvider() = this.RegisterFsharpFixAll() + + interface IFSharpCodeFixProvider with + member _.GetCodeFixIfAppliesAsync context = + cancellableTask { + let! symbol = UnusedCodeFixHelper.getUnusedSymbol context.Span context.Document CodeFix.RenameUnusedValue + + return + symbol + |> Option.filter (fun symbol -> + match symbol with + | :? FSharpMemberOrFunctionOrValue as x when x.IsConstructorThisValue -> false + | _ -> true) + |> Option.map (fun symbol -> + { + Name = CodeFix.RenameUnusedValue + Message = getTitle symbol.DisplayName + Changes = [ TextChange(context.Span, "_") ] + }) + } diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj index 970287c0510..b3514c959ad 100644 --- a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj +++ b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj @@ -125,10 +125,9 @@ - + - - + From 542575127d74f542986fbdcfa7867e7103bd7cc4 Mon Sep 17 00:00:00 2001 From: Petr Date: Sat, 22 Jul 2023 12:24:17 +0200 Subject: [PATCH 03/11] fantomas --- .../RemoveSuperflousCaptureForUnionCaseWithNoData.fs | 2 +- .../src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperflousCaptureForUnionCaseWithNoData.fs b/vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperflousCaptureForUnionCaseWithNoData.fs index 7896d19b582..5176b03620b 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperflousCaptureForUnionCaseWithNoData.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/RemoveSuperflousCaptureForUnionCaseWithNoData.fs @@ -3,8 +3,8 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System.Composition -open System.Collections.Immutable open System.Threading.Tasks +open System.Collections.Immutable open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.CodeFixes diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs b/vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs index e54268886c5..b06b8010921 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs @@ -34,13 +34,11 @@ module UnusedCodeFixHelper = let lineText = (sourceText.Lines.GetLineFromPosition textSpan.Start).ToString() - let! _, checkResults = - document.GetFSharpParseAndCheckResultsAsync codeFixName + let! _, checkResults = document.GetFSharpParseAndCheckResultsAsync codeFixName return lexerSymbol - |> Option.bind (fun symbol -> - checkResults.GetSymbolUseAtLocation(m.StartLine, m.EndColumn, lineText, symbol.FullIsland)) + |> Option.bind (fun symbol -> checkResults.GetSymbolUseAtLocation(m.StartLine, m.EndColumn, lineText, symbol.FullIsland)) |> Option.bind (fun symbolUse -> match symbolUse.Symbol with | :? FSharpMemberOrFunctionOrValue as func when func.IsValue -> Some symbolUse.Symbol @@ -82,7 +80,6 @@ type internal PrefixUnusedValueWithUnderscoreCodeFixProvider [] type internal RenameUnusedValueWithUnderscoreCodeFixProvider [] () = From de13de5f0930bed4b6a2b23e72f18b7591e25edb Mon Sep 17 00:00:00 2001 From: Petr Date: Sat, 22 Jul 2023 13:19:07 +0200 Subject: [PATCH 04/11] Update CodeFixHelpers.fs --- vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs index a7c5265daa2..c819a3d2262 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs @@ -110,7 +110,8 @@ module internal CodeFixExtensions = } // This cannot be an extension on the code fix context -// because the underlying GetFixAllProvider method doesn't take the context in +// because the underlying GetFixAllProvider method doesn't take the context in. +#nowarn "3511" // state machine not statically compilable [] module IFSharpCodeFixProviderExtensions = type IFSharpCodeFixProvider with From 0bfa7949e3acc8c42c487b682316d2563c015073 Mon Sep 17 00:00:00 2001 From: Petr Date: Sat, 22 Jul 2023 13:46:19 +0200 Subject: [PATCH 05/11] fantomas --- vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs | 1 + 1 file changed, 1 insertion(+) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs index c819a3d2262..f730efb73b5 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs @@ -112,6 +112,7 @@ module internal CodeFixExtensions = // 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 + [] module IFSharpCodeFixProviderExtensions = type IFSharpCodeFixProvider with From 2112aeb379876858ff8168ec5b7f3952bde4ce87 Mon Sep 17 00:00:00 2001 From: Petr Date: Mon, 24 Jul 2023 13:05:59 +0200 Subject: [PATCH 06/11] Update RenameUnusedValue.fs --- .../CodeFixes/RenameUnusedValue.fs | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs b/vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs index b06b8010921..169ce549748 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/RenameUnusedValue.fs @@ -16,17 +16,15 @@ open FSharp.Compiler.Syntax open CancellableTasks -module UnusedCodeFixHelper = - let getUnusedSymbol textSpan (document: Document) codeFixName = - cancellableTask { - let! token = CancellableTask.getCurrentCancellationToken () - let! sourceText = document.GetTextAsync token - - let ident = sourceText.ToString textSpan - - // Prefixing operators and backticked identifiers does not make sense. - // We have to use the additional check for backtickes - if PrettyNaming.IsIdentifierName ident then +[] +module private UnusedCodeFixHelper = + let getUnusedSymbol textSpan (document: Document) (sourceText: SourceText) codeFixName = + let ident = sourceText.ToString textSpan + + // Prefixing operators and backticked identifiers does not make sense. + // We have to use the additional check for backticks + if PrettyNaming.IsIdentifierName ident then + cancellableTask { let! lexerSymbol = document.TryFindFSharpLexerSymbolAsync(textSpan.Start, SymbolLookupKind.Greedy, false, false, CodeFix.RenameUnusedValue) @@ -43,9 +41,9 @@ module UnusedCodeFixHelper = match symbolUse.Symbol with | :? FSharpMemberOrFunctionOrValue as func when func.IsValue -> Some symbolUse.Symbol | _ -> None) - else - return None - } + } + else + CancellableTask.singleton None [] type internal PrefixUnusedValueWithUnderscoreCodeFixProvider [] () = @@ -68,7 +66,8 @@ type internal PrefixUnusedValueWithUnderscoreCodeFixProvider [ Date: Wed, 26 Jul 2023 11:34:02 +0200 Subject: [PATCH 07/11] action --- .../src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs index f730efb73b5..4ea17d67c7d 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs @@ -117,6 +117,10 @@ module internal CodeFixExtensions = module IFSharpCodeFixProviderExtensions = type IFSharpCodeFixProvider with + // this is not used anywhere, it's just needed to create the context + static member private Action = + Action>(fun _ _ -> ()) + member provider.RegisterFsharpFixAll() = FixAllProvider.Create(fun fixAllCtx doc allDiagnostics -> cancellableTask { @@ -125,16 +129,12 @@ module IFSharpCodeFixProviderExtensions = let! token = CancellableTask.getCurrentCancellationToken () let! sourceText = doc.GetTextAsync token - // this is not used anywhere, it's just needed to create the context - let action = - Action>(fun _ _ -> ()) - let! codeFixOpts = allDiagnostics // The distiction is to avoid collisions of compiler and analyzer diags // See: https://github.com/dotnet/fsharp/issues/15620 |> Seq.distinctBy (fun d -> d.Id, d.Location) - |> Seq.map (fun diag -> CodeFixContext(doc, diag, action, token)) + |> 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 From 05e136cf2081cf9d1ec4225ab8944b9508727762 Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 26 Jul 2023 12:04:19 +0200 Subject: [PATCH 08/11] lambda --- .../FSharp.Editor/CodeFixes/CodeFixHelpers.fs | 79 ++++++++++--------- 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs index 4ea17d67c7d..5b41544db36 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs @@ -121,44 +121,47 @@ module IFSharpCodeFixProviderExtensions = static member private Action = Action>(fun _ _ -> ()) - member provider.RegisterFsharpFixAll() = - FixAllProvider.Create(fun fixAllCtx doc allDiagnostics -> - cancellableTask { - let sw = Stopwatch.StartNew() - - let! token = CancellableTask.getCurrentCancellationToken () - let! sourceText = doc.GetTextAsync token + member private this.FixAllAsync (fixAllCtx: FixAllContext) (doc: Document) (allDiagnostics: ImmutableArray) = + cancellableTask { + let sw = Stopwatch.StartNew() - let! codeFixOpts = - allDiagnostics - // The distiction is to avoid collisions of compiler and analyzer diags - // See: https://github.com/dotnet/fsharp/issues/15620 - |> 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 |] + 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 + |> Seq.distinctBy (fun d -> d.Id, d.Location) + |> Seq.map (fun diag -> CodeFixContext(doc, diag, IFSharpCodeFixProvider.Action, token)) + |> Seq.map (fun context -> this.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 + } - return updatedDoc - } + member provider.RegisterFsharpFixAll() = + FixAllProvider.Create(fun fixAllCtx doc allDiagnostics -> + provider.FixAllAsync fixAllCtx doc allDiagnostics |> CancellableTask.start fixAllCtx.CancellationToken) From 29d2212682722b55d10f0cb02dd7a84fd79e8caf Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 26 Jul 2023 12:04:49 +0200 Subject: [PATCH 09/11] up --- vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs index 5b41544db36..63d5b913f38 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs @@ -121,7 +121,7 @@ module IFSharpCodeFixProviderExtensions = static member private Action = Action>(fun _ _ -> ()) - member private this.FixAllAsync (fixAllCtx: FixAllContext) (doc: Document) (allDiagnostics: ImmutableArray) = + member private provider.FixAllAsync (fixAllCtx: FixAllContext) (doc: Document) (allDiagnostics: ImmutableArray) = cancellableTask { let sw = Stopwatch.StartNew() @@ -134,7 +134,7 @@ module IFSharpCodeFixProviderExtensions = // See: https://github.com/dotnet/fsharp/issues/15620 |> Seq.distinctBy (fun d -> d.Id, d.Location) |> Seq.map (fun diag -> CodeFixContext(doc, diag, IFSharpCodeFixProvider.Action, token)) - |> Seq.map (fun context -> this.GetCodeFixIfAppliesAsync context) + |> Seq.map (fun context -> provider.GetCodeFixIfAppliesAsync context) |> Seq.map (fun task -> task token) |> Task.WhenAll From 4fb5c788facfc18ac461ffd9d1713e09404c1aff Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 26 Jul 2023 13:07:42 +0200 Subject: [PATCH 10/11] Update CodeFixHelpers.fs --- vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs index 63d5b913f38..5eca331b694 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs @@ -130,8 +130,10 @@ module IFSharpCodeFixProviderExtensions = let! codeFixOpts = allDiagnostics - // The distiction is to avoid collisions of compiler and analyzer diags + // 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) From 13c05fb71f5340d3297d742012ee4cfa8b9bd59e Mon Sep 17 00:00:00 2001 From: Petr Date: Thu, 27 Jul 2023 15:09:21 +0200 Subject: [PATCH 11/11] ftms --- vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs index 5eca331b694..7c18ae07a86 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs @@ -121,7 +121,7 @@ module IFSharpCodeFixProviderExtensions = static member private Action = Action>(fun _ _ -> ()) - member private provider.FixAllAsync (fixAllCtx: FixAllContext) (doc: Document) (allDiagnostics: ImmutableArray) = + member private provider.FixAllAsync (fixAllCtx: FixAllContext) (doc: Document) (allDiagnostics: ImmutableArray) = cancellableTask { let sw = Stopwatch.StartNew() @@ -164,6 +164,6 @@ module IFSharpCodeFixProviderExtensions = } member provider.RegisterFsharpFixAll() = - FixAllProvider.Create(fun fixAllCtx doc allDiagnostics -> + FixAllProvider.Create(fun fixAllCtx doc allDiagnostics -> provider.FixAllAsync fixAllCtx doc allDiagnostics |> CancellableTask.start fixAllCtx.CancellationToken)