Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Non-functional code fix improvements #15729

Merged
merged 2 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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