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.

Sign up for GitHub

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

Fix tooltip errorhandling #1195

Merged
merged 2 commits into from
Nov 16, 2023
Merged

Fix tooltip errorhandling #1195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
683625a
Results of Amplifying F# Sesssion
pblasucci Nov 6, 2023
9d0b967
Fantomas
pblasucci Nov 6, 2023
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
8 changes: 3 additions & 5 deletions src/FsAutoComplete.Core/Commands.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,9 +534,7 @@ module Commands =
| Error e -> return CoreResponse.ErrorRes e
}

let typesig (tyRes: ParseAndCheckResults) (pos: Position) lineStr =
tyRes.TryGetToolTip pos lineStr
|> Result.bimap CoreResponse.Res CoreResponse.ErrorRes
let typesig (tyRes: ParseAndCheckResults) (pos: Position) lineStr = tyRes.TryGetToolTip pos lineStr

// Calculates pipeline hints for now as in fSharp/pipelineHint with a bit of formatting on the hints
let inlineValues (contents: IFSACSourceText) (tyRes: ParseAndCheckResults) : Async<(pos * String)[]> =
Expand All @@ -546,7 +544,7 @@ module Commands =
option {
let! lineStr = contents.GetLine pos

let! tip = tyRes.TryGetToolTip pos lineStr |> Option.ofResult
let! tip = tyRes.TryGetToolTip pos lineStr

return TipFormatter.extractGenericParameters tip
}
Expand Down Expand Up @@ -618,7 +616,7 @@ module Commands =
option {
let! lineStr = contents.GetLine pos

let! tip = tyRes.TryGetToolTip pos lineStr |> Option.ofResult
let! tip = tyRes.TryGetToolTip pos lineStr

return TipFormatter.extractGenericParameters tip
}
Expand Down
53 changes: 35 additions & 18 deletions src/FsAutoComplete.Core/ParseAndCheckResults.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,9 @@ type ParseAndCheckResults

member __.TryGetToolTip (pos: Position) (lineStr: LineStr) =
match Lexer.findLongIdents (pos.Column, lineStr) with
| None -> ResultOrString.Error "Cannot find ident for tooltip"
| None ->
logger.info (Log.setMessageI $"Cannot find ident for tooltip: {pos.Column:column} in {lineStr:lineString}")
None
| Some(col, identIsland) ->
let identIsland = Array.toList identIsland
// TODO: Display other tooltip types, for example for strings or comments where appropriate
Expand All @@ -330,27 +332,30 @@ type ParseAndCheckResults
match identIsland with
| [ ident ] ->
match KeywordList.keywordTooltips.TryGetValue ident with
| true, tip -> Ok tip
| _ -> ResultOrString.Error "No tooltip information"
| _ -> ResultOrString.Error "No tooltip information"
| _ -> Ok(tip)

member x.TryGetToolTipEnhanced
(pos: Position)
(lineStr: LineStr)
: Result<option<TryGetToolTipEnhancedResult>, string> =
| true, tip -> Some tip
| _ ->
logger.info (Log.setMessageI $"Cannot find ident for tooltip: {pos.Column:column} in {lineStr:lineString}")
None
| _ ->
logger.info (Log.setMessageI $"Cannot find ident for tooltip: {pos.Column:column} in {lineStr:lineString}")
None
| _ -> Some tip

member x.TryGetToolTipEnhanced (pos: Position) (lineStr: LineStr) : option<TryGetToolTipEnhancedResult> =
let (|EmptyTooltip|_|) (ToolTipText elems) =
match elems with
| [] -> Some()
| elems when elems |> List.forall ((=) ToolTipElement.None) -> Some()
| _ -> None

match Completion.atPos (pos, x.GetParseResults.ParseTree) with
| Completion.Context.StringLiteral -> Ok None
| Completion.Context.StringLiteral -> None
| Completion.Context.SynType
| Completion.Context.Unknown ->
match Lexer.findLongIdents (pos.Column, lineStr) with
| None -> Error "Cannot find ident for tooltip"
| None ->
logger.info (Log.setMessageI $"Cannot find ident for tooltip: {pos.Column:column} in {lineStr:lineString}")
None
| Some(col, identIsland) ->
let identIsland = Array.toList identIsland
// TODO: Display other tooltip types, for example for strings or comments where appropriate
Expand All @@ -371,12 +376,20 @@ type ParseAndCheckResults
Footer = ""
SymbolInfo = TryGetToolTipEnhancedResult.Keyword ident }
|> Some
|> Ok
| _ -> Error "No tooltip information"
| _ -> Error "No tooltip information"
| _ ->
logger.info (
Log.setMessageI $"Cannot find ident for tooltip: {pos.Column:column} in {lineStr:lineString}"
)

None
| _ ->
logger.info (Log.setMessageI $"Cannot find ident for tooltip: {pos.Column:column} in {lineStr:lineString}")
None
| _ ->
match symbol with
| None -> Error "No tooltip information"
| None ->
logger.info (Log.setMessageI $"Cannot find ident for tooltip: {pos.Column:column} in {lineStr:lineString}")
None
| Some symbol ->

// Retrieve the FSharpSymbol instance so we can find the XmlDocSig
Expand All @@ -386,7 +399,12 @@ type ParseAndCheckResults
let resolvedType = symbol.Symbol.GetAbbreviatedParent()

match SignatureFormatter.getTooltipDetailsFromSymbolUse symbol with
| None -> Error "No tooltip information"
| None ->
logger.info (
Log.setMessageI $"Cannot find tooltip for {symbol:symbol} ({pos.Column:column} in {lineStr:lineString})"
)

None
| Some(signature, footer) ->
{ ToolTipText = tip
Signature = signature
Expand All @@ -396,7 +414,6 @@ type ParseAndCheckResults
{| XmlDocSig = resolvedType.XmlDocSig
Assembly = symbol.Symbol.Assembly.SimpleName |} }
|> Some
|> Ok

member __.TryGetFormattedDocumentation (pos: Position) (lineStr: LineStr) =
match Lexer.findLongIdents (pos.Column, lineStr) with
Expand Down
4 changes: 2 additions & 2 deletions src/FsAutoComplete.Core/ParseAndCheckResults.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ type ParseAndCheckResults =
member TryFindIdentifierDeclaration: pos: Position -> lineStr: LineStr -> Async<Result<FindDeclarationResult, string>>

member TryFindTypeDeclaration: pos: Position -> lineStr: LineStr -> Async<Result<FindDeclarationResult, string>>
member TryGetToolTip: pos: Position -> lineStr: LineStr -> Result<ToolTipText, string>
member TryGetToolTip: pos: Position -> lineStr: LineStr -> ToolTipText option

member TryGetToolTipEnhanced: pos: Position -> lineStr: LineStr -> Result<TryGetToolTipEnhancedResult option, string>
member TryGetToolTipEnhanced: pos: Position -> lineStr: LineStr -> TryGetToolTipEnhancedResult option

member TryGetFormattedDocumentation:
pos: Position ->
Expand Down
52 changes: 26 additions & 26 deletions src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ type AdaptiveFSharpLspServer
return None // An empty file has empty completions. Otherwise we would error down there
else

let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.lineLookupErr

if lineStr.StartsWith "#" then
let completionList =
Expand Down Expand Up @@ -568,7 +568,12 @@ type AdaptiveFSharpLspServer
asyncResult {

let! volatileFile = state.GetOpenFileOrRead filePath
let! lineStr = volatileFile.Source |> tryGetLineStr pos

let! lineStr =
volatileFile.Source
|> tryGetLineStr pos
|> Result.mapError ErrorMsgUtils.formatLineLookErr
//TODO ⮝⮝⮝ good candidate for better error model -- review!

// TextDocumentCompletion will sometimes come in before TextDocumentDidChange
// This will require the trigger character to be at the place VSCode says it is
Expand Down Expand Up @@ -871,11 +876,11 @@ type AdaptiveFSharpLspServer

let (filePath, pos) = getFilePathAndPosition p
let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.lineLookupErr
and! tyRes = state.GetOpenFileTypeCheckResultsCached filePath |> AsyncResult.ofStringErr

match tyRes.TryGetToolTipEnhanced pos lineStr with
| Ok(Some tooltipResult) ->
| Some tooltipResult ->
logger.info (
Log.setMessage "TryGetToolTipEnhanced : {params}"
>> Log.addContextDestructured "params" tooltipResult
Expand Down Expand Up @@ -936,13 +941,8 @@ type AdaptiveFSharpLspServer

| TipFormatter.TipFormatterResult.None -> return None

| Ok(None) ->
| None -> return None

return! LspResult.internalError $"No TryGetToolTipEnhanced results for {filePath}"
| Error e ->
trace.RecordError(e, "TextDocumentHover.Error") |> ignore<Activity>
logger.error (Log.setMessage "Failed with {error}" >> Log.addContext "error" e)
return! LspResult.internalError e
with e ->
trace |> Tracing.recordException e

Expand All @@ -964,7 +964,7 @@ type AdaptiveFSharpLspServer

let (filePath, pos) = getFilePathAndPosition p
let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.lineLookupErr
let! tyRes = state.GetOpenFileTypeCheckResults filePath |> AsyncResult.ofStringErr

let! (_, _, range) =
Expand All @@ -987,7 +987,7 @@ type AdaptiveFSharpLspServer

let (filePath, pos) = getFilePathAndPosition p
let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.lineLookupErr
and! tyRes = state.GetOpenFileTypeCheckResults filePath |> AsyncResult.ofStringErr

// validate name and surround with backticks if necessary
Expand Down Expand Up @@ -1061,7 +1061,7 @@ type AdaptiveFSharpLspServer
let (filePath, pos) = getFilePathAndPosition p
let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr

let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.lineLookupErr
and! tyRes = state.GetOpenFileTypeCheckResults filePath |> AsyncResult.ofStringErr
let! decl = tyRes.TryFindDeclaration pos lineStr |> AsyncResult.ofStringErr
return decl |> findDeclToLspLocation |> GotoResult.Single |> Some
Expand Down Expand Up @@ -1091,7 +1091,7 @@ type AdaptiveFSharpLspServer
let (filePath, pos) = getFilePathAndPosition p

let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.lineLookupErr
and! tyRes = state.GetOpenFileTypeCheckResults filePath |> AsyncResult.ofStringErr
let! decl = tyRes.TryFindTypeDeclaration pos lineStr |> AsyncResult.ofStringErr
return decl |> findDeclToLspLocation |> GotoResult.Single |> Some
Expand Down Expand Up @@ -1120,7 +1120,7 @@ type AdaptiveFSharpLspServer

let (filePath, pos) = getFilePathAndPosition p
let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr
let! lineStr = tryGetLineStr pos volatileFile.Source |> Result.ofStringErr
let! lineStr = tryGetLineStr pos volatileFile.Source |> Result.lineLookupErr
and! tyRes = state.GetOpenFileTypeCheckResults filePath |> AsyncResult.ofStringErr

let! usages =
Expand Down Expand Up @@ -1156,7 +1156,7 @@ type AdaptiveFSharpLspServer

let (filePath, pos) = getFilePathAndPosition p
let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr
let! lineStr = tryGetLineStr pos volatileFile.Source |> Result.ofStringErr
let! lineStr = tryGetLineStr pos volatileFile.Source |> Result.lineLookupErr
and! tyRes = state.GetOpenFileTypeCheckResults filePath |> AsyncResult.ofStringErr

match
Expand Down Expand Up @@ -1198,7 +1198,7 @@ type AdaptiveFSharpLspServer

let (filePath, pos) = getFilePathAndPosition p
let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr
let! lineStr = tryGetLineStr pos volatileFile.Source |> Result.ofStringErr
let! lineStr = tryGetLineStr pos volatileFile.Source |> Result.lineLookupErr
and! tyRes = state.GetOpenFileTypeCheckResults filePath |> AsyncResult.ofStringErr

logger.info (
Expand Down Expand Up @@ -1557,7 +1557,7 @@ type AdaptiveFSharpLspServer
)

let! (sourceText: IFSACSourceText) = state.GetOpenFileSource filePath |> AsyncResult.ofStringErr
let! lineStr = sourceText |> tryGetLineStr pos |> Result.ofStringErr
let! lineStr = sourceText |> tryGetLineStr pos |> Result.lineLookupErr

let typ = data.[1]
let! r = Async.Catch(f arg pos tyRes sourceText lineStr typ filePath)
Expand Down Expand Up @@ -2068,7 +2068,7 @@ type AdaptiveFSharpLspServer
let filePath = Path.FileUriToLocalPath p.Item.Uri |> Utils.normalizePath
let pos = protocolPosToPos p.Item.SelectionRange.Start
let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr
let! lineStr = tryGetLineStr pos volatileFile.Source |> Result.ofStringErr
let! lineStr = tryGetLineStr pos volatileFile.Source |> Result.lineLookupErr
// Incoming file may not be "Opened" so we need to force a typecheck
let! tyRes = state.GetTypeCheckResultsForFile filePath |> AsyncResult.ofStringErr

Expand Down Expand Up @@ -2155,7 +2155,7 @@ type AdaptiveFSharpLspServer
|> getFilePathAndPosition

let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr
let! lineStr = tryGetLineStr pos volatileFile.Source |> Result.ofStringErr
let! lineStr = tryGetLineStr pos volatileFile.Source |> Result.lineLookupErr
and! tyRes = state.GetOpenFileTypeCheckResults filePath |> AsyncResult.ofStringErr

let! decl = tyRes.TryFindDeclaration pos lineStr |> AsyncResult.ofStringErr
Expand Down Expand Up @@ -2237,9 +2237,9 @@ type AdaptiveFSharpLspServer
let (filePath, pos) = getFilePathAndPosition p
let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr

let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.lineLookupErr
and! tyRes = state.GetOpenFileTypeCheckResults filePath |> AsyncResult.ofStringErr
let! tip = Commands.typesig tyRes pos lineStr |> Result.ofCoreResponse
let tip = Commands.typesig tyRes pos lineStr

return
tip
Expand Down Expand Up @@ -2272,7 +2272,7 @@ type AdaptiveFSharpLspServer

let filePath = p.TextDocument.GetFilePath() |> Utils.normalizePath
let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.lineLookupErr

and! tyRes = state.GetOpenFileTypeCheckResults filePath |> AsyncResult.ofStringErr
let! (typ, parms, generics) = tyRes.TryGetSignatureData pos lineStr |> Result.ofStringErr
Expand Down Expand Up @@ -2308,7 +2308,7 @@ type AdaptiveFSharpLspServer
let (filePath, pos) = getFilePathAndPosition p
let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr

let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.lineLookupErr
and! tyRes = state.GetOpenFileTypeCheckResults filePath |> AsyncResult.ofStringErr

match!
Expand Down Expand Up @@ -2656,7 +2656,7 @@ type AdaptiveFSharpLspServer

let (filePath, pos) = getFilePathAndPosition p
let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.lineLookupErr
and! tyRes = state.GetOpenFileTypeCheckResults filePath |> AsyncResult.ofStringErr

match! Commands.Help tyRes pos lineStr |> Result.ofCoreResponse with
Expand Down Expand Up @@ -2687,7 +2687,7 @@ type AdaptiveFSharpLspServer

let (filePath, pos) = getFilePathAndPosition p
let! volatileFile = state.GetOpenFileOrRead filePath |> AsyncResult.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.ofStringErr
let! lineStr = volatileFile.Source |> tryGetLineStr pos |> Result.lineLookupErr
and! tyRes = state.GetOpenFileTypeCheckResults filePath |> AsyncResult.ofStringErr
lastFSharpDocumentationTypeCheck <- Some tyRes

Expand Down
7 changes: 6 additions & 1 deletion src/FsAutoComplete/LspServers/AdaptiveServerState.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1634,7 +1634,12 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac
let tryGetParseResultsForFile filePath pos =
asyncResult {
let! (file) = forceFindOpenFileOrRead filePath
let! lineStr = file.Source |> tryGetLineStr pos

let! lineStr =
file.Source
|> tryGetLineStr pos
|> Result.mapError ErrorMsgUtils.formatLineLookErr
//TODO ⮝⮝⮝ good candidate for better error model -- review!
and! tyRes = forceGetOpenFileTypeCheckResults filePath
return tyRes, lineStr, file.Source
}
Expand Down
22 changes: 21 additions & 1 deletion src/FsAutoComplete/LspServers/Common.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,27 @@ open FSharp.UMX
open CliWrap


module ErrorMsgUtils =
let formatLineLookErr
(x:
{| FileName: string<LocalPath>
Position: FcsPos |})
=
let position = fcsPosToLsp x.Position
$"No line in {x.FileName} at position {position}"


module Result =
let ofStringErr r = r |> Result.mapError JsonRpc.Error.InternalErrorMessage

let lineLookupErr
(r:
Result<'T, {| FileName: string<LocalPath>
Position: FcsPos |}>)
=
r
|> Result.mapError (ErrorMsgUtils.formatLineLookErr >> JsonRpc.Error.InternalErrorMessage)

let ofCoreResponse (r: CoreResponse<'a>) =
match r with
| CoreResponse.Res a -> Ok(Some a)
Expand Down Expand Up @@ -187,7 +205,9 @@ module Helpers =

let tryGetLineStr pos (text: IFSACSourceText) =
text.GetLine(pos)
|> Result.ofOption (fun () -> $"No line in {text.FileName} at position {pos}")
|> Result.ofOption (fun () ->
{| FileName = text.FileName
Position = pos |})


let fullPathNormalized = Path.GetFullPath >> Utils.normalizePath >> UMX.untag
Expand Down
Loading
Loading