Skip to content

Commit

Permalink
Add ChangeRefCellDerefToNotExpression code fixer (dotnet#10469)
Browse files Browse the repository at this point in the history
* Add ChangeRefCellDerefToNotExpression code fixer

* Area
  • Loading branch information
cartermp authored and nosami committed Feb 22, 2021
1 parent dd50231 commit 38b2f23
Show file tree
Hide file tree
Showing 20 changed files with 201 additions and 1 deletion.
18 changes: 18 additions & 0 deletions src/fsharp/service/ServiceUntypedParse.fs
Expand Up @@ -100,6 +100,24 @@ type FSharpParseFileResults(errors: FSharpErrorInfo[], input: ParsedInput option

member scope.ParseTree = input

member scope.TryRangeOfRefCellDereferenceContainingPos expressionPos =
match input with
| Some input ->
let res =
AstTraversal.Traverse(expressionPos, input, { new AstTraversal.AstVisitorBase<_>() with
member _.VisitExpr(_, _, defaultTraverse, expr) =
match expr with
| SynExpr.App(_, false, SynExpr.Ident funcIdent, expr, _) ->
if funcIdent.idText = "op_Dereference" && rangeContainsPos expr.Range expressionPos then
Some funcIdent.idRange
else
None
| _ -> defaultTraverse expr
})
res
| None ->
None

member scope.FindNoteworthyParamInfoLocations pos =
match input with
| Some input -> FSharpNoteworthyParamInfoLocations.Find(pos, input)
Expand Down
7 changes: 7 additions & 0 deletions src/fsharp/service/ServiceUntypedParse.fsi
Expand Up @@ -19,6 +19,13 @@ type public FSharpParseFileResults =
/// The syntax tree resulting from the parse
member ParseTree : ParsedInput option

/// <summary>
/// Given the position of an expression, attempts to find the range of the
/// '!' in a derefence operation of that expression, like:
/// '!expr', '!(expr)', etc.
/// </summary>
member TryRangeOfRefCellDereferenceContainingPos: expressionPos: pos -> Option<range>

/// Notable parse info for ParameterInfo at a given location
member FindNoteworthyParamInfoLocations : pos:pos -> FSharpNoteworthyParamInfoLocations option

Expand Down
Expand Up @@ -22749,6 +22749,7 @@ FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: Boolean get_ParseHadE
FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: FSharp.Compiler.SourceCodeServices.FSharpErrorInfo[] Errors
FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: FSharp.Compiler.SourceCodeServices.FSharpErrorInfo[] get_Errors()
FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: FSharp.Compiler.SourceCodeServices.FSharpNavigationItems GetNavigationItems()
FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Range+range] TryRangeOfRefCellDereferenceContainingPos(pos)
FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Range+range] ValidateBreakpointLocation(pos)
FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.SourceCodeServices.FSharpNoteworthyParamInfoLocations] FindNoteworthyParamInfoLocations(pos)
FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.SyntaxTree+ParsedInput] ParseTree
Expand Down
53 changes: 53 additions & 0 deletions tests/service/ServiceUntypedParseTests.fs
Expand Up @@ -305,3 +305,56 @@ type T =
new (x:int) = ()
"""
getTypeMemberRange source |> shouldEqual [ (3, 4), (3, 20) ]


[<Test>]
let ``TryRangeOfRefCellDereferenceContainingPos - simple``() =
let source = """
let x = false
let y = !x
"""
let parseFileResults, _ = getParseAndCheckResults source
let res = parseFileResults.TryRangeOfRefCellDereferenceContainingPos (mkPos 3 9)
match res with
| Some res ->
res
|> tups
|> fst
|> shouldEqual (3, 8)
| None ->
Assert.Fail("No deref operator found in source.")

[<Test>]
let ``TryRangeOfRefCellDereferenceContainingPos - parens``() =
let source = """
let x = false
let y = !(x)
"""
let parseFileResults, _ = getParseAndCheckResults source
let res = parseFileResults.TryRangeOfRefCellDereferenceContainingPos (mkPos 3 10)
match res with
| Some res ->
res
|> tups
|> fst
|> shouldEqual (3, 8)
| None ->
Assert.Fail("No deref operator found in source.")


[<Test>]
let ``TryRangeOfRefCellDereferenceContainingPos - binary expr``() =
let source = """
let x = false
let y = !(x = false)
"""
let parseFileResults, _ = getParseAndCheckResults source
let res = parseFileResults.TryRangeOfRefCellDereferenceContainingPos (mkPos 3 10)
match res with
| Some res ->
res
|> tups
|> fst
|> shouldEqual (3, 8)
| None ->
Assert.Fail("No deref operator found in source.")
@@ -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.Threading.Tasks

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

[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = "ChangeRefCellDerefToNotExpression"); Shared>]
type internal FSharpChangeRefCellDerefToNotExpressionCodeFixProvider
[<ImportingConstructor>]
(
checkerProvider: FSharpCheckerProvider,
projectInfoManager: FSharpProjectOptionsManager
) =
inherit CodeFixProvider()

static let userOpName = "FSharpChangeRefCellDerefToNotExpressionCodeFix"
let fixableDiagnosticIds = set ["FS0001"]

override __.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds

override this.RegisterCodeFixesAsync context : Task =
asyncMaybe {
let document = context.Document
let! parsingOptions, _ = projectInfoManager.TryGetOptionsForEditingDocumentOrProject(document, context.CancellationToken, userOpName)
let! sourceText = context.Document.GetTextAsync(context.CancellationToken)
let! parseResults = checkerProvider.Checker.ParseFile(document.FilePath, sourceText.ToFSharpSourceText(), parsingOptions, userOpName) |> liftAsync

let errorRange = RoslynHelpers.TextSpanToFSharpRange(document.FilePath, context.Span, sourceText)
let! derefRange = parseResults.TryRangeOfRefCellDereferenceContainingPos errorRange.Start
let! derefSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, derefRange)

let title = SR.UseNotForNegation()

let diagnostics =
context.Diagnostics
|> Seq.filter (fun x -> fixableDiagnosticIds |> Set.contains x.Id)
|> Seq.toImmutableArray

let codeFix =
CodeFixHelpers.createTextChangeCodeFix(
title,
context,
(fun () -> asyncMaybe.Return [| TextChange(derefSpan, "not ") |]))

context.RegisterCodeFix(codeFix, diagnostics)
}
|> Async.Ignore
|> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)
1 change: 1 addition & 0 deletions vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj
Expand Up @@ -180,6 +180,7 @@
<Compile Include="Commands\FsiCommandService.fs" />
<Compile Include="Commands\XmlDocCommandService.fs" />
<Compile Include="CodeFix\CodeFixHelpers.fs" />
<Compile Include="CodeFix\ChangeRefCellDerefToNotExpression.fs" />
<Compile Include="CodeFix\WrapExpressionInParentheses.fs" />
<Compile Include="CodeFix\ChangePrefixNegationToInfixSubtraction.fs" />
<Compile Include="CodeFix\AddNewKeywordToDisposableConstructorInvocation.fs" />
Expand Down
5 changes: 4 additions & 1 deletion vsintegration/src/FSharp.Editor/FSharp.Editor.resx
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema
Expand Down Expand Up @@ -219,6 +219,9 @@
<data name="FSharpDisposableTopLevelValuesClassificationType" xml:space="preserve">
<value>F# Dispostable Values (top-level)</value>
</data>
<data name="UseNotForNegation" xml:space="preserve">
<value>Use 'not' to negate expression</value>
</data>
<data name="WrapExpressionInParentheses" xml:space="preserve">
<value>Wrap expression in parentheses</value>
</data>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf
Expand Up @@ -177,6 +177,11 @@
<target state="translated">Formátování</target>
<note />
</trans-unit>
<trans-unit id="UseNotForNegation">
<source>Use 'not' to negate expression</source>
<target state="new">Use 'not' to negate expression</target>
<note />
</trans-unit>
<trans-unit id="WrapExpressionInParentheses">
<source>Wrap expression in parentheses</source>
<target state="new">Wrap expression in parentheses</target>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf
Expand Up @@ -177,6 +177,11 @@
<target state="translated">Formatierung</target>
<note />
</trans-unit>
<trans-unit id="UseNotForNegation">
<source>Use 'not' to negate expression</source>
<target state="new">Use 'not' to negate expression</target>
<note />
</trans-unit>
<trans-unit id="WrapExpressionInParentheses">
<source>Wrap expression in parentheses</source>
<target state="new">Wrap expression in parentheses</target>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf
Expand Up @@ -177,6 +177,11 @@
<target state="translated">Formato</target>
<note />
</trans-unit>
<trans-unit id="UseNotForNegation">
<source>Use 'not' to negate expression</source>
<target state="new">Use 'not' to negate expression</target>
<note />
</trans-unit>
<trans-unit id="WrapExpressionInParentheses">
<source>Wrap expression in parentheses</source>
<target state="new">Wrap expression in parentheses</target>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf
Expand Up @@ -177,6 +177,11 @@
<target state="translated">Mise en forme</target>
<note />
</trans-unit>
<trans-unit id="UseNotForNegation">
<source>Use 'not' to negate expression</source>
<target state="new">Use 'not' to negate expression</target>
<note />
</trans-unit>
<trans-unit id="WrapExpressionInParentheses">
<source>Wrap expression in parentheses</source>
<target state="new">Wrap expression in parentheses</target>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf
Expand Up @@ -177,6 +177,11 @@
<target state="translated">Formattazione</target>
<note />
</trans-unit>
<trans-unit id="UseNotForNegation">
<source>Use 'not' to negate expression</source>
<target state="new">Use 'not' to negate expression</target>
<note />
</trans-unit>
<trans-unit id="WrapExpressionInParentheses">
<source>Wrap expression in parentheses</source>
<target state="new">Wrap expression in parentheses</target>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf
Expand Up @@ -177,6 +177,11 @@
<target state="translated">書式設定</target>
<note />
</trans-unit>
<trans-unit id="UseNotForNegation">
<source>Use 'not' to negate expression</source>
<target state="new">Use 'not' to negate expression</target>
<note />
</trans-unit>
<trans-unit id="WrapExpressionInParentheses">
<source>Wrap expression in parentheses</source>
<target state="new">Wrap expression in parentheses</target>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf
Expand Up @@ -177,6 +177,11 @@
<target state="translated">서식</target>
<note />
</trans-unit>
<trans-unit id="UseNotForNegation">
<source>Use 'not' to negate expression</source>
<target state="new">Use 'not' to negate expression</target>
<note />
</trans-unit>
<trans-unit id="WrapExpressionInParentheses">
<source>Wrap expression in parentheses</source>
<target state="new">Wrap expression in parentheses</target>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf
Expand Up @@ -177,6 +177,11 @@
<target state="translated">Formatowanie</target>
<note />
</trans-unit>
<trans-unit id="UseNotForNegation">
<source>Use 'not' to negate expression</source>
<target state="new">Use 'not' to negate expression</target>
<note />
</trans-unit>
<trans-unit id="WrapExpressionInParentheses">
<source>Wrap expression in parentheses</source>
<target state="new">Wrap expression in parentheses</target>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pt-BR.xlf
Expand Up @@ -177,6 +177,11 @@
<target state="translated">Formatação</target>
<note />
</trans-unit>
<trans-unit id="UseNotForNegation">
<source>Use 'not' to negate expression</source>
<target state="new">Use 'not' to negate expression</target>
<note />
</trans-unit>
<trans-unit id="WrapExpressionInParentheses">
<source>Wrap expression in parentheses</source>
<target state="new">Wrap expression in parentheses</target>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf
Expand Up @@ -177,6 +177,11 @@
<target state="translated">Форматирование</target>
<note />
</trans-unit>
<trans-unit id="UseNotForNegation">
<source>Use 'not' to negate expression</source>
<target state="new">Use 'not' to negate expression</target>
<note />
</trans-unit>
<trans-unit id="WrapExpressionInParentheses">
<source>Wrap expression in parentheses</source>
<target state="new">Wrap expression in parentheses</target>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf
Expand Up @@ -177,6 +177,11 @@
<target state="translated">Biçimlendirme</target>
<note />
</trans-unit>
<trans-unit id="UseNotForNegation">
<source>Use 'not' to negate expression</source>
<target state="new">Use 'not' to negate expression</target>
<note />
</trans-unit>
<trans-unit id="WrapExpressionInParentheses">
<source>Wrap expression in parentheses</source>
<target state="new">Wrap expression in parentheses</target>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hans.xlf
Expand Up @@ -177,6 +177,11 @@
<target state="translated">正在格式化</target>
<note />
</trans-unit>
<trans-unit id="UseNotForNegation">
<source>Use 'not' to negate expression</source>
<target state="new">Use 'not' to negate expression</target>
<note />
</trans-unit>
<trans-unit id="WrapExpressionInParentheses">
<source>Wrap expression in parentheses</source>
<target state="new">Wrap expression in parentheses</target>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hant.xlf
Expand Up @@ -177,6 +177,11 @@
<target state="translated">格式化</target>
<note />
</trans-unit>
<trans-unit id="UseNotForNegation">
<source>Use 'not' to negate expression</source>
<target state="new">Use 'not' to negate expression</target>
<note />
</trans-unit>
<trans-unit id="WrapExpressionInParentheses">
<source>Wrap expression in parentheses</source>
<target state="new">Wrap expression in parentheses</target>
Expand Down

0 comments on commit 38b2f23

Please sign in to comment.