Skip to content

Commit

Permalink
Non-functional code fix improvements (#15729)
Browse files Browse the repository at this point in the history
  • Loading branch information
psfinaki committed Aug 1, 2023
1 parent 6cb9792 commit 6dd6899
Show file tree
Hide file tree
Showing 28 changed files with 215 additions and 178 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@ type internal AddInstanceMemberParameterCodeFixProvider() =
Changes = [ TextChange(TextSpan(context.Span.Start, 0), "x.") ]
}

CancellableTask.singleton (Some codeFix)
CancellableTask.singleton (ValueSome codeFix)
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,18 @@ type internal AddMissingEqualsToTypeDefinitionCodeFixProvider() =

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

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

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

else
return
Some
ValueSome
{
Name = CodeFix.AddMissingEqualsToTypeDefinition
Message = title
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type internal AddMissingFunKeywordCodeFixProvider [<ImportingConstructor>] () =
let! textOfError = context.GetSquigglyTextAsync()

if textOfError <> "->" then
return None
return ValueNone
else
let! cancellationToken = CancellableTask.getCurrentCancellationToken ()
let document = context.Document
Expand All @@ -61,9 +61,10 @@ type internal AddMissingFunKeywordCodeFixProvider [<ImportingConstructor>] () =
strictIndentation,
cancellationToken
)
|> Option.bind (fun intendedArgLexerSymbol ->
RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, intendedArgLexerSymbol.Range))
|> Option.map (fun intendedArgSpan ->
|> ValueOption.ofOption
|> ValueOption.map (fun intendedArgLexerSymbol ->
RoslynHelpers.FSharpRangeToTextSpan(sourceText, intendedArgLexerSymbol.Range))
|> ValueOption.map (fun intendedArgSpan ->
{
Name = CodeFix.AddMissingFunKeyword
Message = title
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,10 @@ type internal AddMissingRecToMutuallyRecFunctionsCodeFixProvider [<ImportingCons
strictIndentation,
cancellationToken
)
|> Option.bind (fun funcLexerSymbol -> RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, funcLexerSymbol.Range))
|> Option.map (fun funcNameSpan -> sourceText.GetSubText(funcNameSpan).ToString())
|> Option.map (fun funcName ->
|> ValueOption.ofOption
|> ValueOption.map (fun funcLexerSymbol -> RoslynHelpers.FSharpRangeToTextSpan(sourceText, funcLexerSymbol.Range))
|> ValueOption.map (fun funcNameSpan -> sourceText.GetSubText(funcNameSpan).ToString())
|> ValueOption.map (fun funcName ->
{
Name = CodeFix.AddMissingRecToMutuallyRecFunctions
Message = String.Format(titleFormat, funcName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type internal AddNewKeywordCodeFixProvider() =
interface IFSharpCodeFixProvider with
member _.GetCodeFixIfAppliesAsync context =
CancellableTask.singleton (
Some
ValueSome
{
Name = CodeFix.AddNewKeyword
Message = title
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ type internal ChangePrefixNegationToInfixSubtractionCodeFixProvider() =
let pos = findNextNonWhitespacePos sourceText (context.Span.End + 1)

if sourceText[pos] <> '-' then
return None
return ValueNone
else
return
Some
ValueSome
{
Name = CodeFix.ChangePrefixNegationToInfixSubtraction
Message = title
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ type internal ChangeRefCellDerefToNotExpressionCodeFixProvider [<ImportingConstr

return
parseResults.TryRangeOfRefCellDereferenceContainingPos errorRange.Start
|> Option.bind (fun range -> RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, range))
|> Option.map (fun span ->
|> ValueOption.ofOption
|> ValueOption.map (fun range -> RoslynHelpers.FSharpRangeToTextSpan(sourceText, range))
|> ValueOption.map (fun span ->
{
Name = CodeFix.ChangeRefCellDerefToNotExpression
Message = title
Expand Down
4 changes: 2 additions & 2 deletions vsintegration/src/FSharp.Editor/CodeFixes/ChangeToUpcast.fs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ type internal ChangeToUpcastCodeFixProvider() =
Changes = changes
}

return Some codeFix
return ValueSome codeFix
else
return None
return ValueNone
}
38 changes: 35 additions & 3 deletions vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,40 @@ open Microsoft.CodeAnalysis.CodeFixes
open Microsoft.CodeAnalysis.CodeActions
open Microsoft.VisualStudio.FSharp.Editor.Telemetry

open FSharp.Compiler.Symbols
open FSharp.Compiler.Syntax

open CancellableTasks

module internal 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)

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))
|> ValueOption.ofOption
|> ValueOption.bind (fun symbolUse ->
match symbolUse.Symbol with
| :? FSharpMemberOrFunctionOrValue as func when func.IsValue -> ValueSome symbolUse.Symbol
| _ -> ValueNone)
}
else
CancellableTask.singleton ValueNone

[<RequireQualifiedAccess>]
module internal CodeFixHelpers =
let reportCodeFixTelemetry
Expand Down Expand Up @@ -86,8 +118,8 @@ module internal CodeFixExtensions =
member ctx.RegisterFsharpFix(codeFix: IFSharpCodeFixProvider) =
cancellableTask {
match! codeFix.GetCodeFixIfAppliesAsync ctx with
| Some codeFix -> ctx.RegisterFsharpFix(codeFix.Name, codeFix.Message, codeFix.Changes)
| None -> ()
| ValueSome codeFix -> ctx.RegisterFsharpFix(codeFix.Name, codeFix.Message, codeFix.Changes)
| ValueNone -> ()
}
|> CancellableTask.startAsTask ctx.CancellationToken

Expand Down Expand Up @@ -140,7 +172,7 @@ module IFSharpCodeFixProviderExtensions =
|> Seq.map (fun task -> task token)
|> Task.WhenAll

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,12 @@ type internal ConvertCSharpLambdaToFSharpLambdaCodeFixProvider [<ImportingConstr
static let title = SR.UseFSharpLambda()

let tryGetSpans (parseResults: FSharpParseFileResults) (range: range) sourceText =
let flatten3 options =
match options with
| Some (Some a, Some b, Some c) -> Some(a, b, c)
| _ -> None

parseResults.TryRangeOfParenEnclosingOpEqualsGreaterUsage range.Start
|> Option.map (fun (fullParenRange, lambdaArgRange, lambdaBodyRange) ->
RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, fullParenRange),
RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, lambdaArgRange),
RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, lambdaBodyRange))
|> flatten3
|> ValueOption.ofOption
|> ValueOption.map (fun (fullParenRange, lambdaArgRange, lambdaBodyRange) ->
RoslynHelpers.FSharpRangeToTextSpan(sourceText, fullParenRange),
RoslynHelpers.FSharpRangeToTextSpan(sourceText, lambdaArgRange),
RoslynHelpers.FSharpRangeToTextSpan(sourceText, lambdaBodyRange))

override _.FixableDiagnosticIds = ImmutableArray.Create "FS0039"

Expand All @@ -47,7 +42,7 @@ type internal ConvertCSharpLambdaToFSharpLambdaCodeFixProvider [<ImportingConstr

return
tryGetSpans parseResults errorRange sourceText
|> Option.map (fun (fullParenSpan, lambdaArgSpan, lambdaBodySpan) ->
|> ValueOption.map (fun (fullParenSpan, lambdaArgSpan, lambdaBodySpan) ->
let replacement =
let argText = sourceText.GetSubText(lambdaArgSpan).ToString()
let bodyText = sourceText.GetSubText(lambdaBodySpan).ToString()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ type internal ConvertToAnonymousRecordCodeFixProvider [<ImportingConstructor>] (

return
parseResults.TryRangeOfRecordExpressionContainingPos errorRange.Start
|> Option.bind (fun range -> RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, range))
|> Option.map (fun span ->
|> ValueOption.ofOption
|> ValueOption.map (fun range -> RoslynHelpers.FSharpRangeToTextSpan(sourceText, range))
|> ValueOption.map (fun span ->
{
Name = CodeFix.ConvertToAnonymousRecord
Message = title
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ type internal ConvertToNotEqualsEqualityExpressionCodeFixProvider() =
let! text = context.GetSquigglyTextAsync()

if text <> "!=" then
return None
return ValueNone
else
return
Some
ValueSome
{
Name = CodeFix.ConvertToNotEqualsEqualityExpression
Message = title
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ type internal ConvertToSingleEqualsEqualityExpressionCodeFixProvider() =
let! text = context.GetSquigglyTextAsync()

if text <> "==" then
return None
return ValueNone
else
return
Some
ValueSome
{
Name = CodeFix.ConvertToSingleEqualsEqualityExpression
Message = title
Expand Down
53 changes: 53 additions & 0 deletions vsintegration/src/FSharp.Editor/CodeFixes/DiscardUnusedValue.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// 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.Text
open Microsoft.CodeAnalysis.CodeFixes

open FSharp.Compiler.Symbols

open CancellableTasks

[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.RenameUnusedValue); Shared>]
type internal RenameUnusedValueWithUnderscoreCodeFixProvider [<ImportingConstructor>] () =

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! sourceText = context.GetSourceTextAsync()
let! symbol = UnusedCodeFixHelper.getUnusedSymbol context.Span context.Document sourceText CodeFix.RenameUnusedValue

return
symbol
|> ValueOption.filter (fun symbol ->
match symbol with
| :? FSharpMemberOrFunctionOrValue as x when x.IsConstructorThisValue -> false
| _ -> true)
|> ValueOption.map (fun symbol ->
{
Name = CodeFix.RenameUnusedValue
Message = getTitle symbol.DisplayName
Changes = [ TextChange(context.Span, "_") ]
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type internal RemoveDotFromIndexerAccessOptInCodeFixProvider() =
interface IFSharpCodeFixProvider with
member _.GetCodeFixIfAppliesAsync context =
CancellableTask.singleton (
Some
ValueSome
{
Name = CodeFix.RemoveIndexerDotBeforeBracket
Message = title
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ type FSharpCodeFix =
}

type IFSharpCodeFixProvider =
abstract member GetCodeFixIfAppliesAsync: context: CodeFixContext -> CancellableTask<FSharpCodeFix option>
abstract member GetCodeFixIfAppliesAsync: context: CodeFixContext -> CancellableTask<FSharpCodeFix voption>
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,17 @@ type internal MakeOuterBindingRecursiveCodeFixProvider [<ImportingConstructor>]
let! diagnosticRange = context.GetErrorRangeAsync()

if not <| parseResults.IsPosContainedInApplication diagnosticRange.Start then
return None
return ValueNone
else
return
parseResults.TryRangeOfNameOfNearestOuterBindingContainingPos diagnosticRange.Start
|> Option.bind (fun bindingRange -> RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, bindingRange))
|> Option.filter (fun bindingSpan ->
|> ValueOption.ofOption
|> ValueOption.map (fun bindingRange -> RoslynHelpers.FSharpRangeToTextSpan(sourceText, bindingRange))
|> ValueOption.filter (fun bindingSpan ->
sourceText
.GetSubText(bindingSpan)
.ContentEquals(sourceText.GetSubText context.Span))
|> Option.map (fun bindingSpan ->
|> ValueOption.map (fun bindingSpan ->
let title =
String.Format(SR.MakeOuterBindingRecursive(), sourceText.GetSubText(bindingSpan).ToString())

Expand Down
47 changes: 47 additions & 0 deletions vsintegration/src/FSharp.Editor/CodeFixes/PrefixUnusedValue.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// 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.Text
open Microsoft.CodeAnalysis.CodeFixes

open CancellableTasks

[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.PrefixUnusedValue); Shared>]
type internal PrefixUnusedValueWithUnderscoreCodeFixProvider [<ImportingConstructor>] () =

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! sourceText = context.GetSourceTextAsync()
let! symbol = UnusedCodeFixHelper.getUnusedSymbol context.Span context.Document sourceText CodeFix.PrefixUnusedValue

return
symbol
|> ValueOption.map (fun symbol ->
{
Name = CodeFix.PrefixUnusedValue
Message = getTitle symbol.DisplayName
Changes = [ TextChange(TextSpan(context.Span.Start, 0), "_") ]
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ type internal ProposeUppercaseLabelCodeFixProvider [<ImportingConstructor>] () =
// probably not the 100% robust way to do that
// but actually we could also just implement the code fix for this case as well
if errorText.StartsWith "exception " then
return None
return ValueNone
else
let upperCased = string (Char.ToUpper errorText[0]) + errorText.Substring(1)

let title =
CompilerDiagnostics.GetErrorMessage(FSharpDiagnosticKind.ReplaceWithSuggestion upperCased)

return
(Some
(ValueSome
{
Name = CodeFix.ProposeUppercaseLabel
Message = title
Expand Down
Loading

0 comments on commit 6dd6899

Please sign in to comment.