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

Add tests & fixes for Code Fixes #915

Merged
merged 29 commits into from
Apr 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
2b8f3dd
Sort Code Fix Tests alphabetically
Booksbaum Apr 7, 2022
47b2e4d
Fix: `GenerateAbstractClassStub` triggers twice
Booksbaum Apr 8, 2022
d1e77e6
Fix: `AddMissingRecKeyword` uses wrong function name in title
Booksbaum Apr 8, 2022
ddf44ec
Fix: `AddTypeToIndeterminateValue` doesn't trigger
Booksbaum Apr 8, 2022
0505dd6
Fix: `UseMutationWhenValueIsMutable` adds `<-` after `=` instead of r…
Booksbaum Apr 8, 2022
0f36396
Fix: `ConvertCSharpLambdaToFSharpLambda` joins multi-line body to sin…
Booksbaum Apr 8, 2022
842c24a
Rename `ColonInFieldType` to `ChangeEqualsInFieldTypeToColon`
Booksbaum Apr 8, 2022
678295e
Add test for `ConvertBangEqualsToInequality`
Booksbaum Apr 8, 2022
00098f2
Add tests for `ConvertInvalidRecordToAnonRecord`
Booksbaum Apr 8, 2022
a72ea78
Add tests for `ConvertDoubleEqualsToSingleEquals`
Booksbaum Apr 8, 2022
1a1eb45
Fix: Goto Declaration fails when declaration is in untitled file
Booksbaum Apr 8, 2022
6e75b7f
Fix: `AddMissingEqualsToTypeDefinition` doesn't trigger
Booksbaum Apr 9, 2022
4942871
Rename `NegationToSubtraction` to `ChangePrefixNegationToInfixSubtrac…
Booksbaum Apr 9, 2022
99591d5
Rename `NewWithDisposables` to `AddNewKeywordToDisposableConstructorI…
Booksbaum Apr 9, 2022
dd350c8
Fix: `ConvertPositionalDUToNamed` doesn't put space before wildcard
Booksbaum Apr 9, 2022
82fa65a
Rename `ParenthesizeEpression` to `WrapExpressionInParentheses`
Booksbaum Apr 9, 2022
0d8239e
Rename `RedundantQualifier` to `RemoveRedundantQualifier`
Booksbaum Apr 9, 2022
68dfb19
Rename `RefCellAccesToNot` to `ChangeRefCellDerefToNot`
Booksbaum Apr 9, 2022
8c13b8e
Add tests for `RemoveUnnecessaryReturnOrYield`
Booksbaum Apr 9, 2022
d82e22a
Change: Trigger `ChangeDerefBangToValue` on Diag `3370`
Booksbaum Apr 9, 2022
c6012ee
Rename `UseSafeCastInsteadOfUnsafe` to `ChangeDowncastToUpcast`
Booksbaum Apr 9, 2022
e510802
Rename test to match CodeFix
Booksbaum Apr 9, 2022
d9fdfe4
Fix: Incorrect sorted tests
Booksbaum Apr 9, 2022
972fe48
Fix: `ResolveNamespace` doesn't trigger when target is in last line
Booksbaum Apr 9, 2022
2b43d61
Fix: `ReplaceWithSuggestion` surrounds identifiers in double-backtick…
Booksbaum Apr 9, 2022
6061722
Fix: `RemoveUnusedOpens` doesn't trigger in first line
Booksbaum Apr 10, 2022
bacbb21
Rename `UnusedValue` to `RenameUnusedValue`
Booksbaum Apr 10, 2022
cc2a49f
Fix: Sort order of Tests
Booksbaum Apr 11, 2022
1db5841
Remove uncommented code
Booksbaum Apr 11, 2022
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
11 changes: 10 additions & 1 deletion src/FsAutoComplete.Core/ParseAndCheckResults.fs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ type ParseAndCheckResults
| None -> return Error "load directive not recognized"
}

member __.TryFindIdentifierDeclaration (pos: Position) (lineStr: LineStr) =
member x.TryFindIdentifierDeclaration (pos: Position) (lineStr: LineStr) =
match Lexer.findLongIdents (pos.Column, lineStr) with
| None -> async.Return(ResultOrString.Error "Could not find ident at this location")
| Some (col, identIsland) ->
Expand Down Expand Up @@ -151,6 +151,15 @@ type ParseAndCheckResults
return ResultOrString.Error(sprintf "Could not find declaration. %s" elaboration)
| FindDeclResult.DeclFound range when range.FileName.EndsWith(Range.rangeStartup.FileName) ->
return ResultOrString.Error "Could not find declaration"
| FindDeclResult.DeclFound range when range.FileName = UMX.untag x.FileName ->
// decl in same file
// necessary to get decl in untitled file (-> `File.Exists range.FileName` is false)
logger.info (
Log.setMessage "Got a declresult of {range} in same file"
>> Log.addContextDestructured "range" range
)

return Ok(FindDeclarationResult.Range range)
| FindDeclResult.DeclFound range when System.IO.File.Exists range.FileName ->
let rangeStr = range.ToString()

Expand Down
117 changes: 117 additions & 0 deletions src/FsAutoComplete/CodeFixes.fs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,78 @@ module Types =
Command = None
Data = None }

module SourceText =
let inline private assertLineIndex lineIndex (sourceText: ISourceText) =
assert(0 <= lineIndex && lineIndex < sourceText.GetLineCount())
/// Note: this fails when `sourceText` is empty string (`""`)
/// -> No lines
/// Use `WithEmptyHandling.isFirstLine` to handle empty string
let isFirstLine lineIndex (sourceText: ISourceText) =
assertLineIndex lineIndex sourceText
lineIndex = 0
/// Note: this fails when `sourceText` is empty string (`""`)
/// -> No lines
/// Use `WithEmptyHandling.isLastLine` to handle empty string
let isLastLine lineIndex (sourceText: ISourceText) =
assertLineIndex lineIndex sourceText
lineIndex = sourceText.GetLineCount() - 1

/// SourceText treats empty string as no source:
/// ```fsharp
/// let text = SourceText.ofString ""
/// assert(text.ToString() = "")
/// assert(text.GetLastCharacterPosition() = (0, 0)) // Note: first line is `1`
/// assert(text.GetLineCount() = 0) // Note: `(SourceText.ofString "\n").GetLineCount()` is `2`
/// assert(text.GetLineString 0 ) // System.IndexOutOfRangeException: Index was outside the bounds of the array.
/// ```
/// -> Functions in here treat empty string as empty single line
///
/// Note: There's always at least empty single line
/// -> source MUST at least be empty (cannot not exist)
module WithEmptyHandling =
let getLineCount (sourceText: ISourceText) =
match sourceText.GetLineCount () with
| 0 -> 1
| c -> c
// or
// max 1 (sourceText.GetLineCount())

let inline private assertLineIndex lineIndex sourceText =
assert(0 <= lineIndex && lineIndex < getLineCount sourceText)

let getLineString lineIndex (sourceText: ISourceText) =
assertLineIndex lineIndex sourceText
if lineIndex = 0 && sourceText.GetLineCount() = 0 then
""
else
sourceText.GetLineString lineIndex

let isFirstLine lineIndex (sourceText: ISourceText) =
assertLineIndex lineIndex sourceText
// No need to check for inside `getLineCount`: there's always at least one line (might be empty)
lineIndex = 0

let isLastLine lineIndex (sourceText: ISourceText) =
assertLineIndex lineIndex sourceText
lineIndex = (getLineCount sourceText) - 1

/// Returns position after last character in specified line.
/// Same as line length.
///
/// Example:
/// ```fsharp
/// let text = SourceText.ofString "let a = 2\nlet foo = 42\na + foo\n"
///
/// assert(afterLastCharacterPosition 0 text = 9)
/// assert(afterLastCharacterPosition 1 text = 12)
/// assert(afterLastCharacterPosition 2 text = 7)
/// assert(afterLastCharacterPosition 2 text = 0)
/// ```
let afterLastCharacterPosition lineIndex (sourceText: ISourceText) =
assertLineIndex lineIndex sourceText
let line = sourceText |> getLineString lineIndex
line.Length

/// helpers for iterating along text lines
module Navigation =

Expand Down Expand Up @@ -144,6 +216,51 @@ module Navigation =
let walkForwardUntilCondition lines pos condition =
walkForwardUntilConditionWithTerminal lines pos condition (fun _ -> false)

/// Tries to detect the last cursor position in line before `currentLine` (0-based).
///
/// Returns `None` iff there's no prev line -> `currentLine` is first line
let tryEndOfPrevLine (lines: ISourceText) currentLine =
if SourceText.WithEmptyHandling.isFirstLine currentLine lines then
None
else
let prevLine = currentLine - 1
{ Line = prevLine; Character = lines |> SourceText.WithEmptyHandling.afterLastCharacterPosition prevLine }
|> Some
/// Tries to detect the first cursor position in line after `currentLine` (0-based).
///
/// Returns `None` iff there's no next line -> `currentLine` is last line
let tryStartOfNextLine (lines: ISourceText) currentLine =
if SourceText.WithEmptyHandling.isLastLine currentLine lines then
None
else
let nextLine = currentLine + 1
{ Line = nextLine; Character = 0 }
|> Some

/// Gets the range to delete the complete line `lineIndex` (0-based).
/// Deleting the line includes a linebreak if possible
/// -> range starts either at end of previous line (-> includes leading linebreak)
/// or start of next line (-> includes trailing linebreak)
///
/// Special case: there's just one line
/// -> delete text of (single) line
let rangeToDeleteFullLine lineIndex (lines: ISourceText) =
match tryEndOfPrevLine lines lineIndex with
| Some start ->
// delete leading linebreak
{ Start = start; End = { Line = lineIndex; Character = lines |> SourceText.WithEmptyHandling.afterLastCharacterPosition lineIndex } }
| None ->
match tryStartOfNextLine lines lineIndex with
| Some fin ->
// delete trailing linebreak
{ Start = { Line = lineIndex; Character = 0 }; End = fin }
| None ->
// single line
// -> just delete all text in line
{ Start = { Line = lineIndex; Character = 0 }; End = { Line = lineIndex; Character = lines |> SourceText.WithEmptyHandling.afterLastCharacterPosition lineIndex } }



module Run =
open Types

Expand Down
36 changes: 36 additions & 0 deletions src/FsAutoComplete/CodeFixes/AddMissingEqualsToTypeDefinition.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
module FsAutoComplete.CodeFix.AddMissingEqualsToTypeDefinition

open FsToolkit.ErrorHandling
open FsAutoComplete.CodeFix.Navigation
open FsAutoComplete.CodeFix.Types
open Ionide.LanguageServerProtocol.Types
open FsAutoComplete
open FsAutoComplete.LspHelpers

let title = "Add missing '=' to type definition"
/// a codefix that adds in missing '=' characters in type declarations
let fix (getFileLines: GetFileLines) =
Run.ifDiagnosticByCode
(Set.ofList [ "3360" ])
(fun diagnostic codeActionParams ->
asyncResult {
let fileName =
codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath

let! lines = getFileLines fileName
let! walkPos = dec lines diagnostic.Range.Start |> Result.ofOption (fun _ -> "No walk pos")
match walkBackUntilCondition lines walkPos (System.Char.IsWhiteSpace >> not) with
| Some firstNonWhitespaceChar ->
let! insertPos = inc lines firstNonWhitespaceChar |> Result.ofOption (fun _ -> "No insert pos")

return
[ { SourceDiagnostic = Some diagnostic
Title = title
File = codeActionParams.TextDocument
Edits =
[| { Range = { Start = insertPos; End = insertPos }
NewText = "= " } |]
Kind = FixKind.Fix } ]
| None -> return []
}
)
5 changes: 3 additions & 2 deletions src/FsAutoComplete/CodeFixes/AddMissingRecKeyword.fs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ open FsAutoComplete
open FsAutoComplete.LspHelpers
open FSharp.UMX

let title symbolName = $"Make '{symbolName}' recursive"
/// a codefix that adds the 'rec' modifier to a binding in a mutually-recursive loop
let fix (getFileLines: GetFileLines) (getLineText: GetLineText): CodeFix =
Run.ifDiagnosticByCode
Expand Down Expand Up @@ -60,10 +61,10 @@ let fix (getFileLines: GetFileLines) (getLineText: GetLineText): CodeFix =
let protocolRange =
fcsRangeToLsp (FSharp.Compiler.Text.Range.mkRange (UMX.untag fileName) fcsStartPos fcsEndPos)

let symbolName = getLineText lines protocolRange
let! symbolName = getLineText lines protocolRange

return
[ { Title = $"Make '{symbolName}' recursive"
[ { Title = title symbolName
File = codeActionParams.TextDocument
SourceDiagnostic = Some diagnostic
Edits =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
module FsAutoComplete.CodeFix.AddNewKeywordToDisposableConstructorInvocation

open FsToolkit.ErrorHandling
open FsAutoComplete.CodeFix
open FsAutoComplete.CodeFix.Types
open Ionide.LanguageServerProtocol.Types
open FsAutoComplete
open FsAutoComplete.LspHelpers

let title = "Add 'new'"
/// a codefix that suggests using the 'new' keyword on IDisposables
let fix (getRangeText: GetRangeText) =
Run.ifDiagnosticByCode
(Set.ofList [ "760" ])
(fun diagnostic codeActionParams ->
AsyncResult.retn [ { SourceDiagnostic = Some diagnostic
File = codeActionParams.TextDocument
Title = title
Edits =
[| { Range = { Start = diagnostic.Range.Start; End = diagnostic.Range.Start }
NewText = $"new " } |]
Kind = FixKind.Refactor } ]
)
13 changes: 7 additions & 6 deletions src/FsAutoComplete/CodeFixes/AddTypeToIndeterminateValue.fs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ open FsAutoComplete
open FsAutoComplete.LspHelpers
open FSharp.Compiler.EditorServices
open FSharp.Compiler.Symbols
open FSharp.UMX

let title = "Add explicit type annotation"
/// fix inderminate type errors by adding an explicit type to a value
let fix
(getParseResultsForFile: GetParseResultsForFile)
Expand All @@ -17,14 +19,13 @@ let fix
(Set.ofList ["72"; "3245"])
(fun diagnostic codeActionParams ->
asyncResult {
let filename = codeActionParams.TextDocument.GetFilePath ()
let typedFileName = filename |> Utils.normalizePath
let fileName = codeActionParams.TextDocument.GetFilePath () |> Utils.normalizePath
let fcsRange = protocolRangeToRange (codeActionParams.TextDocument.GetFilePath()) diagnostic.Range
let! (tyRes, line, lines) = getParseResultsForFile typedFileName fcsRange.Start
let! (tyRes, line, lines) = getParseResultsForFile fileName fcsRange.Start
let! (endColumn, identIslands) = Lexer.findLongIdents(fcsRange.Start.Column, line) |> Result.ofOption (fun _ -> "No long ident at position")
match tyRes.GetCheckResults.GetDeclarationLocation(fcsRange.Start.Line, endColumn, line, List.ofArray identIslands) with
| FindDeclResult.DeclFound declRange when declRange.FileName = filename ->
let! projectOptions = getProjectOptionsForFile typedFileName
| FindDeclResult.DeclFound declRange when declRange.FileName = UMX.untag fileName ->
let! projectOptions = getProjectOptionsForFile fileName
let protocolDeclRange = fcsRangeToLsp declRange
let! declText = lines.GetText declRange
let! declTextLine = lines.GetLine declRange.Start |> Result.ofOption (fun _ -> "No line found at pos")
Expand All @@ -48,7 +49,7 @@ let fix
else "(" + declText + ": " + typeString + ")", protocolDeclRange

return [{
Title = "Add explicit type annotation"
Title = title
File = codeActionParams.TextDocument
SourceDiagnostic = Some diagnostic
Kind = FixKind.Fix
Expand Down
44 changes: 0 additions & 44 deletions src/FsAutoComplete/CodeFixes/ChangeCSharpLambdaToFSharp.fs

This file was deleted.

60 changes: 60 additions & 0 deletions src/FsAutoComplete/CodeFixes/ChangeDerefBangToValue.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/// replace use of ! operator on ref cells with calls to .Value
module FsAutoComplete.CodeFix.ChangeDerefBangToValue

open FsToolkit.ErrorHandling
open FsAutoComplete.CodeFix.Types
open FsAutoComplete
open FsAutoComplete.LspHelpers
open FSharp.Compiler.Syntax
open FSharp.Compiler.Text.Range
open FSharp.UMX

/// adopted from `dotnet/fsharp` -> `FSharp.Compiler.CodeAnalysis.FSharpParseFileResults.TryRangeOfExpressionBeingDereferencedContainingPos`
let private tryGetRangeOfDeref input derefPos =
SyntaxTraversal.Traverse(derefPos, input, { new SyntaxVisitorBase<_>() with
member _.VisitExpr(_, _, defaultTraverse, expr) =
match expr with
| SynExpr.App(_, false, SynExpr.Ident funcIdent, expr, _) ->
if funcIdent.idText = "op_Dereference" && rangeContainsPos funcIdent.idRange derefPos then
Some (funcIdent.idRange, expr.Range)
else
None
| _ -> defaultTraverse expr })

let title = "Use `.Value` instead of dereference operator"
let fix
(getParseResultsForFile: GetParseResultsForFile)
(getLineText: GetLineText)
: CodeFix =
Run.ifDiagnosticByCode
(Set.ofList ["3370"])
(fun diagnostic codeActionParams -> asyncResult {
let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath

let derefOpRange = protocolRangeToRange (UMX.untag fileName) diagnostic.Range
let! parseResults, _, _ = getParseResultsForFile fileName derefOpRange.Start

let! (derefOpRange', exprRange) =
tryGetRangeOfDeref parseResults.GetParseResults.ParseTree derefOpRange.End
|> Result.ofOption (fun _ -> "No deref found at that pos")
assert(derefOpRange = derefOpRange')

return [
{
Title = title
File = codeActionParams.TextDocument
SourceDiagnostic = None
Kind = FixKind.Refactor
Edits = [|
// remove leading `!` (and whitespaces after `!`)
{ Range = { Start = fcsPosToLsp derefOpRange'.Start; End = fcsPosToLsp exprRange.Start }
NewText = "" }
// Append trailing `.Value`
{ Range =
let lspPos = fcsPosToLsp exprRange.End
{ Start = lspPos; End = lspPos }
NewText = ".Value" }
|]
}
]
})
Loading