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

(mostly) implement explicit type annotation codefix #807

Merged
merged 4 commits into from
Jun 25, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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
51 changes: 51 additions & 0 deletions src/FsAutoComplete.Core/FCSPatches.fs
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,57 @@ type FSharpParseFileResults with
None
| _ -> defaultTraverse expr })
)

member scope.IsTypeAnnotationGivenAtPositionPatched pos =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ported over from upstream

match scope.ParseTree with
| Some input ->
let result =
AstTraversal.Traverse(pos, input, { new AstTraversal.AstVisitorBase<_>() with
member _.VisitExpr(_path, _traverseSynExpr, defaultTraverse, expr) =
match expr with
| SynExpr.Typed (_expr, _typeExpr, range) when Pos.posEq range.Start pos ->
Some range
| _ -> defaultTraverse expr

override _.VisitSimplePats(pats) =
match pats with
| [] -> None
| _ ->
let exprFunc pat =
match pat with
| SynSimplePat.Typed (_pat, _targetExpr, range) when Pos.posEq range.Start pos ->
Some range
| _ ->
None

pats |> List.tryPick exprFunc

override _.VisitPat(defaultTraverse, pat) =
match pat with
| SynPat.Typed (_pat, _targetType, range) when Pos.posEq range.Start pos ->
Some range
| _ -> defaultTraverse pat })
result.IsSome
| None -> false

member scope.IsBindingALambdaAtPositionPatched pos =
match scope.ParseTree with
| Some input ->
let result =
AstTraversal.Traverse(pos, input, { new AstTraversal.AstVisitorBase<_>() with
member _.VisitExpr(_path, _traverseSynExpr, defaultTraverse, expr) =
defaultTraverse expr

override _.VisitBinding(defaultTraverse, binding) =
match binding with
| SynBinding.Binding(_, _, _, _, _, _, _, _, _, expr, range, _) when Pos.posEq range.Start pos ->
match expr with
| SynExpr.Lambda _ -> Some range
| _ -> None
| _ -> defaultTraverse binding })
result.IsSome
| None -> false

module SyntaxTreeOps =
open FSharp.Compiler.SyntaxTree
let rec synExprContainsError inpExpr =
Expand Down
2 changes: 1 addition & 1 deletion src/FsAutoComplete.Core/FileSystem.fs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type SourceTextExtensions =
static member GetText(t: ISourceText, m: FSharp.Compiler.Text.Range): Result<string, string> =
let allFileRange = Range.mkRange m.FileName Pos.pos0 (t.GetLastFilePosition())
if not (Range.rangeContainsRange allFileRange m)
then Error "%A{m} is outside of the bounds of the file"
then Error $"%A{m} is outside of the bounds of the file"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix for malformed format string

else
if m.StartLine = m.EndLine then // slice of a single line, just do that
let lineText = t.GetLineString (m.StartLine - 1)
Expand Down
9 changes: 9 additions & 0 deletions src/FsAutoComplete.Core/Utils.fs
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,15 @@ module Array =
| _ when n >= xs.Length || n < 0 -> xs, [||]
| _ -> xs.[0..n-1], xs.[n..]

let partitionResults (xs: _ []) =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

used in refactor of codefixes

let oks = ResizeArray(xs.Length)
let errors = ResizeArray(xs.Length)
for x in xs do
match x with
| Ok ok -> oks.Add ok
| Error err -> errors.Add err
oks.ToArray(), errors.ToArray()

module List =

///Returns the greatest of all elements in the list that is less than the threshold
Expand Down
17 changes: 7 additions & 10 deletions src/FsAutoComplete/CodeFixes.fs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ module Types =
type GetParseResultsForFile = string<LocalPath> -> FSharp.Compiler.Text.Pos -> Async<ResultOrString<ParseAndCheckResults * string * FSharp.Compiler.Text.ISourceText>>
type GetProjectOptionsForFile = string<LocalPath> -> ResultOrString<FSharp.Compiler.SourceCodeServices.FSharpProjectOptions>

[<RequireQualifiedAccess>]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally moving to RQA on DUs to prevent conflicts

type FixKind =
| Fix
| Refactor
Expand All @@ -37,7 +38,7 @@ module Types =
SourceDiagnostic: Diagnostic option
Kind: FixKind }

type CodeFix = CodeActionParams -> Async<Fix list>
type CodeFix = CodeActionParams -> Async<Result<Fix list, string>>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

making this AsyncResult makes authoring codefixes nicer, IMO. we do the unification and logging in the LSP endpoint itself now.


type CodeAction with
static member OfFix getFileVersion clientCapabilities (fix: Fix) =
Expand All @@ -62,9 +63,9 @@ module Types =
Kind =
Some
(match fixKind with
| Fix -> "quickfix"
| Refactor -> "refactor"
| Rewrite -> "refactor.rewrite")
| FixKind.Fix -> "quickfix"
| FixKind.Refactor -> "refactor"
| FixKind.Rewrite -> "refactor.rewrite")
Diagnostics = diagnostic |> Option.map Array.singleton
Edit = workspaceEdit
Command = None }
Expand Down Expand Up @@ -157,20 +158,16 @@ module Run =
open Types

let ifEnabled enabled codeFix: CodeFix =
fun codeActionParams -> if enabled () then codeFix codeActionParams else async.Return []
fun codeActionParams -> if enabled () then codeFix codeActionParams else AsyncResult.retn []

let private runDiagnostics pred handler: CodeFix =
let logger = LogProvider.getLoggerByName "CodeFixes"
fun codeActionParams ->
codeActionParams.Context.Diagnostics
|> Array.choose (fun d -> if pred d then Some d else None)
|> Array.toList
|> List.traverseAsyncResultM (fun d -> handler d codeActionParams)
|> AsyncResult.map List.concat
|> AsyncResult.foldResult id (fun errs ->
logger.warn (Log.setMessage "CodeFix returned an error: {error}" >> Log.addContextDestructured "error" errs)
[]
)


let ifDiagnosticByMessage (checkMessage: string) handler : CodeFix =
runDiagnostics (fun d -> d.Message.Contains checkMessage) handler
Expand Down
66 changes: 66 additions & 0 deletions src/FsAutoComplete/CodeFixes/AddExplicitTypeToParameter.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
module FsAutoComplete.CodeFix.AddExplicitTypeToParameter

open FsToolkit.ErrorHandling
open FsAutoComplete.CodeFix.Navigation
open FsAutoComplete.CodeFix.Types
open LanguageServerProtocol.Types
open FsAutoComplete
open FsAutoComplete.LspHelpers
open FSharp.Compiler.SourceCodeServices
open FsAutoComplete.FCSPatches

let fix (getParseResultsForFile: GetParseResultsForFile): CodeFix =
fun codeActionParams ->
asyncResult {
let filePath = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath
let fcsStartPos = protocolPosToPos codeActionParams.Range.Start
let! (parseAndCheck, lineStr, sourceText) = getParseResultsForFile filePath fcsStartPos
let parseFileResults = parseAndCheck.GetParseResults
let! (rightCol, idents) =
Lexer.findLongIdents(fcsStartPos.Column, lineStr)
|> Result.ofOption (fun _ -> $"Couldn't find long ident at %A{fcsStartPos} in file %s{codeActionParams.TextDocument.GetFilePath()}")
let! symbolUse =
parseAndCheck.GetCheckResults.GetSymbolUseAtLocation(fcsStartPos.Line, rightCol, lineStr, List.ofArray idents)
|> Result.ofOption (fun _ -> $"Couldn't find symbolUse at %A{(fcsStartPos.Line, rightCol)} in file %s{codeActionParams.TextDocument.GetFilePath()}")

let isValidParameterWithoutTypeAnnotation (funcOrValue: FSharpMemberOrFunctionOrValue) (symbolUse: FSharpSymbolUse) =
// TODO: remove patched functions and uncomment this boolean check after FCS 40 update
let isLambdaIfFunction =
// funcOrValue.IsFunction &&
parseFileResults.IsBindingALambdaAtPositionPatched symbolUse.RangeAlternate.Start

(funcOrValue.IsValue || isLambdaIfFunction) &&
parseFileResults.IsPositionContainedInACurriedParameter symbolUse.RangeAlternate.Start &&
not (parseFileResults.IsTypeAnnotationGivenAtPositionPatched symbolUse.RangeAlternate.Start) &&
not funcOrValue.IsMember &&
not funcOrValue.IsMemberThisValue &&
not funcOrValue.IsConstructorThisValue &&
not (PrettyNaming.IsOperatorName funcOrValue.DisplayName)

match symbolUse.Symbol with
| :? FSharpMemberOrFunctionOrValue as v when isValidParameterWithoutTypeAnnotation v symbolUse ->
let typeString = v.FullType.Format symbolUse.DisplayContext
let title = "Add explicit type annotation"
let fcsSymbolRange = symbolUse.RangeAlternate
let protocolSymbolRange = fcsRangeToLsp fcsSymbolRange
let! symbolText = sourceText.GetText(fcsSymbolRange)

let alreadyWrappedInParens =
let hasLeftParen = Navigation.walkBackUntilConditionWithTerminal sourceText protocolSymbolRange.Start (fun c -> c = '(') System.Char.IsWhiteSpace
let hasRightParen = Navigation.walkForwardUntilConditionWithTerminal sourceText protocolSymbolRange.End (fun c -> c = ')') System.Char.IsWhiteSpace
hasLeftParen.IsSome && hasRightParen.IsSome

let changedText, changedRange =
if alreadyWrappedInParens
then ": " + typeString, { Start = protocolSymbolRange.End; End = protocolSymbolRange.End }
else "(" + symbolText + ": " + typeString + ")", protocolSymbolRange
return [ {
Edits = [| { Range = changedRange; NewText = changedText } |]
File = codeActionParams.TextDocument
Title = title
SourceDiagnostic = None
Kind = FixKind.Refactor
} ]
| _ ->
return []
}
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/AddMissingFunKeyword.fs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ let fix (getFileLines: GetFileLines) (getLineText: GetLineText): CodeFix =
Edits =
[| { Range = symbolStartRange
NewText = "fun " } |]
Kind = Fix } ]
Kind = FixKind.Fix } ]
| None -> return []
}
)
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/AddMissingRecKeyword.fs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ let fix (getFileLines: GetFileLines) (getLineText: GetLineText): CodeFix =
Edits =
[| { Range = { Start = endOfError; End = endOfError }
NewText = " rec" } |]
Kind = Fix } ]
Kind = FixKind.Fix } ]
| None -> return []
}
)
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ let fix
Title = "Add explicit type annotation"
File = codeActionParams.TextDocument
SourceDiagnostic = Some diagnostic
Kind = Fix
Kind = FixKind.Fix
Edits = [| { Range = changedRange
NewText = changedText } |] }]
| _ -> return []
Expand Down
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/ChangeCSharpLambdaToFSharp.fs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ let fix (getParseResultsForFile: GetParseResultsForFile) (getLineText: GetLineTe
Edits =
[| { Range = replacementRange
NewText = replacementText } |]
Kind = Refactor } ]
Kind = FixKind.Refactor } ]
| None -> return []
}
)
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix =
{ Start = equalsPos
End = (inc lines equalsPos) }
NewText = "<-" } |]
Kind = Refactor } ]
Kind = FixKind.Refactor } ]
| None -> return []
| _ -> return []
}
Expand Down
1 change: 0 additions & 1 deletion src/FsAutoComplete/CodeFixes/ChangeTypeOfNameToNameOf.fs
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,3 @@ let fix (getParseResultsForFile: GetParseResultsForFile): CodeFix =
SourceDiagnostic = None
Kind = FixKind.Refactor }]
}
|> AsyncResult.foldResult id (fun _ -> [])
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/ColonInFieldType.fs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ let fix: CodeFix =
Edits =
[| { Range = diagnostic.Range
NewText = ":" } |]
Kind = Fix } ]
Kind = FixKind.Fix } ]
else
AsyncResult.retn [])
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ let fix (getRangeText: GetRangeText): CodeFix =
return [{ Title = "Use <> for inequality check"
File = codeActionParams.TextDocument
SourceDiagnostic = Some diag
Kind = Fix
Kind = FixKind.Fix
Edits = [| { Range = diag.Range
NewText = "<>" } |] }]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix =
NewText = "|" }
{ Range = endInsertRange
NewText = "|" } |]
Kind = Refactor } ]
Kind = FixKind.Refactor } ]
| None -> return []
}
)
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/DoubleEqualsToSingleEquals.fs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ let fix (getRangeText: GetRangeText) : CodeFix =
Edits =
[| { Range = diagnostic.Range
NewText = "=" } |]
Kind = Fix } ]
Kind = FixKind.Fix } ]
| _ -> return []
}
)
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/ExternalSystemDiagnostics.fs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ let private mapExternalDiagnostic diagnosticType =
File = codeActionParams.TextDocument
Title = $"Fix issue"
Edits = fixes |> List.toArray
Kind = Fix } ]
Kind = FixKind.Fix } ]

| _ -> AsyncResult.retn []
)
Expand Down
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/GenerateAbstractClassStub.fs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ let fix (getParseResultsForFile: GetParseResultsForFile)
Edits =
[| { Range = fcsPosToProtocolRange position
NewText = replaced } |]
Kind = Fix } ]
Kind = FixKind.Fix } ]
| _ -> return []
}
)
4 changes: 1 addition & 3 deletions src/FsAutoComplete/CodeFixes/GenerateInterfaceStub.fs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ let fix (getParseResultsForFile: GetParseResultsForFile)
Edits =
[| { Range = fcsPosToProtocolRange position
NewText = replaced } |]
Kind = Fix } ]
Kind = FixKind.Fix } ]
| _ -> return []

}
|> AsyncResult.foldResult id (fun _ -> [])
3 changes: 1 addition & 2 deletions src/FsAutoComplete/CodeFixes/GenerateRecordStub.fs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ let fix (getParseResultsForFile: GetParseResultsForFile)
Edits =
[| { Range = fcsPosToProtocolRange position
NewText = replaced } |]
Kind = Fix } ]
Kind = FixKind.Fix } ]
| _ -> return []
}
|> AsyncResult.foldResult id (fun _ -> [])
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/GenerateUnionCases.fs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ let fix (getFileLines: GetFileLines)
File = codeActionParams.TextDocument
Title = "Generate union pattern match cases"
Edits = [| { Range = range; NewText = replaced } |]
Kind = Fix } ]
Kind = FixKind.Fix } ]

| _ -> return []
}
Expand Down
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/MakeDeclarationMutable.fs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ let fix (getParseResultsForFile: GetParseResultsForFile)
{ Start = lspRange.Start
End = lspRange.Start }
NewText = "mutable " } |]
Kind = Refactor } ]
Kind = FixKind.Refactor } ]
| _ -> return []
| None -> return []
}
Expand Down
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/MakeOuterBindingRecursive.fs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ let fix (getParseResultsForFile: GetParseResultsForFile) (getLineText: GetLineTe
[ { Title = "Make outer binding recursive"
File = codeActionParams.TextDocument
SourceDiagnostic = Some diagnostic
Kind = Fix
Kind = FixKind.Fix
Edits =
[| { Range =
{ Start = lspOuterBindingRange.Start
Expand Down
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/MissingEquals.fs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ let fix (getFileLines: GetFileLines) =
Edits =
[| { Range = { Start = insertPos; End = insertPos }
NewText = " =" } |]
Kind = Fix } ]
Kind = FixKind.Fix } ]
| None -> return []
else
return []
Expand Down
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/NegationToSubtraction.fs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ let fix (getFileLines: GetFileLines): CodeFix =
Edits =
[| { Range = { Start = dash; End = inc lines dash }
NewText = "- " } |]
Kind = Fix } ]
Kind = FixKind.Fix } ]
| None -> return []
}
)
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/NewWithDisposables.fs
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ let fix (getRangeText: GetRangeText) =
Edits =
[| { Range = diagnostic.Range
NewText = $"new %s{errorText}" } |]
Kind = Refactor } ]
Kind = FixKind.Refactor } ]
| Error _ -> AsyncResult.retn [])
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/ParenthesizeExpression.fs
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ let fix (getRangeText: GetRangeText): CodeFix =
Edits =
[| { Range = diagnostic.Range
NewText = $"(%s{erroringExpression})" } |]
Kind = Fix } ]
Kind = FixKind.Fix } ]
| Error _ -> AsyncResult.retn [])
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/RedundantQualifier.fs
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ let fix =
File = codeActionParams.TextDocument
Title = "Remove redundant qualifier"
SourceDiagnostic = Some diagnostic
Kind = Refactor } ])
Kind = FixKind.Refactor } ])
2 changes: 1 addition & 1 deletion src/FsAutoComplete/CodeFixes/RefCellAccessToNot.fs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ let fix (getParseResultsForFile: GetParseResultsForFile): CodeFix =
Edits =
[| { Range = fcsRangeToLsp derefRange
NewText = "not " } |]
Kind = Fix } ]
Kind = FixKind.Fix } ]
| None -> return []
}
)
Loading