From 197c45903bb66526f0bfdeb0666c72b4d2c3df77 Mon Sep 17 00:00:00 2001 From: Chet Husk Date: Sun, 17 Apr 2022 12:07:52 -0500 Subject: [PATCH] Don't trigger type inlay hints when the binding is already typed (#922) --- src/FsAutoComplete.Core/InlayHints.fs | 294 +++++++++++------- .../TestCases/InlayHints/Script.fsx | 3 + 2 files changed, 180 insertions(+), 117 deletions(-) diff --git a/src/FsAutoComplete.Core/InlayHints.fs b/src/FsAutoComplete.Core/InlayHints.fs index e638e9855..8448a0816 100644 --- a/src/FsAutoComplete.Core/InlayHints.fs +++ b/src/FsAutoComplete.Core/InlayHints.fs @@ -12,8 +12,14 @@ open System.Collections.Immutable open FSharp.Compiler.CodeAnalysis open System.Text -type HintKind = Parameter | Type -type Hint = { Text: string; Pos: Position; Kind: HintKind } +type HintKind = + | Parameter + | Type + +type Hint = + { Text: string + Pos: Position + Kind: HintKind } let private getArgumentsFor (state: FsAutoComplete.State, p: ParseAndCheckResults, identText: Range) = option { @@ -44,134 +50,188 @@ let private getArgumentsFor (state: FsAutoComplete.State, p: ParseAndCheckResult let private isSignatureFile (f: string) = System.IO.Path.GetExtension(UMX.untag f) = ".fsi" +type FSharp.Compiler.CodeAnalysis.FSharpParseFileResults with + // duplicates + extends the logic in FCS to match bindings of the form `let x: int = 12` + // so that they are considered logically the same as a 'typed' SynPat + member x.IsTypeAnnotationGivenAtPositionPatched pos = + let visitor: SyntaxVisitorBase = + { new SyntaxVisitorBase<_>() with + override _.VisitExpr(_path, _traverseSynExpr, defaultTraverse, expr) = + match expr with + | SynExpr.Typed (_expr, _typeExpr, range) when Position.posEq range.Start pos -> + Some range + | _ -> defaultTraverse expr + + override _.VisitSimplePats(_path, pats) = + match pats with + | [] -> None + | _ -> + let exprFunc pat = + match pat with + | SynSimplePat.Typed (_pat, _targetExpr, range) when Position.posEq range.Start pos -> + Some range + | _ -> + None + + pats |> List.tryPick exprFunc + + override _.VisitPat(_path, defaultTraverse, pat) = + match pat with + | SynPat.Typed (_pat, _targetType, range) when Position.posEq range.Start pos -> + Some range + | _ -> defaultTraverse pat + + override _.VisitBinding(_path, defaultTraverse, binding) = + match binding with + | SynBinding(headPat = SynPat.Named (range = patRange); returnInfo = Some (SynBindingReturnInfo(typeName = SynType.LongIdent (idents)))) -> Some patRange + | _ -> defaultTraverse binding + + } + let result = SyntaxTraversal.Traverse(pos, x.ParseTree, visitor) + result.IsSome + let getFirstPositionAfterParen (str: string) startPos = match str with | null -> -1 | str when startPos > str.Length -> -1 | str -> str.IndexOf('(') + 1 -let provideHints (text: NamedText, p: ParseAndCheckResults, range: Range) : Async = async { - let parseFileResults, checkFileResults = p.GetParseResults, p.GetCheckResults - let! cancellationToken = Async.CancellationToken - let symbolUses = - checkFileResults.GetAllUsesOfAllSymbolsInFile(cancellationToken) - |> Seq.filter (fun su -> Range.rangeContainsRange range su.Range) - |> Seq.toList - - let typeHints = ImmutableArray.CreateBuilder() - let parameterHints = ImmutableArray.CreateBuilder() - - let isValidForTypeHint (funcOrValue: FSharpMemberOrFunctionOrValue) (symbolUse: FSharpSymbolUse) = - let isLambdaIfFunction = - funcOrValue.IsFunction - && parseFileResults.IsBindingALambdaAtPosition symbolUse.Range.Start - - (funcOrValue.IsValue || isLambdaIfFunction) - && not (parseFileResults.IsTypeAnnotationGivenAtPosition symbolUse.Range.Start) - && symbolUse.IsFromDefinition - && not funcOrValue.IsMember - && not funcOrValue.IsMemberThisValue - && not funcOrValue.IsConstructorThisValue - && not (PrettyNaming.IsOperatorDisplayName funcOrValue.DisplayName) - - for symbolUse in symbolUses do - match symbolUse.Symbol with - | :? FSharpMemberOrFunctionOrValue as funcOrValue when isValidForTypeHint funcOrValue symbolUse -> - let layout = - ": " - + funcOrValue.ReturnParameter.Type.Format symbolUse.DisplayContext - - let hint = - { Text = layout - Pos = symbolUse.Range.End - Kind = Type } - - typeHints.Add(hint) - - | :? FSharpMemberOrFunctionOrValue as func when func.IsFunction && not symbolUse.IsFromDefinition -> - let appliedArgRangesOpt = - parseFileResults.GetAllArgumentsForFunctionApplicationAtPostion symbolUse.Range.Start - - match appliedArgRangesOpt with - | None -> () - | Some [] -> () - | Some appliedArgRanges -> - let parameters = func.CurriedParameterGroups |> Seq.concat - let appliedArgRanges = appliedArgRanges |> Array.ofList - let definitionArgs = parameters |> Array.ofSeq - // invariant - definitionArgs should be at least as long as applied args. - // if this is not the case (printfs?) we truncate to the lesser of the two - let minLength = min definitionArgs.Length appliedArgRanges.Length - if minLength = 0 then () - else - for idx = 0 to minLength - 1 do +let provideHints (text: NamedText, p: ParseAndCheckResults, range: Range) : Async = + async { + let parseFileResults, checkFileResults = p.GetParseResults, p.GetCheckResults + let! cancellationToken = Async.CancellationToken + + let symbolUses = + checkFileResults.GetAllUsesOfAllSymbolsInFile(cancellationToken) + |> Seq.filter (fun su -> Range.rangeContainsRange range su.Range) + |> Seq.toList + + let typeHints = ImmutableArray.CreateBuilder() + let parameterHints = ImmutableArray.CreateBuilder() + + let isValidForTypeHint (funcOrValue: FSharpMemberOrFunctionOrValue) (symbolUse: FSharpSymbolUse) = + let isLambdaIfFunction = + funcOrValue.IsFunction + && parseFileResults.IsBindingALambdaAtPosition symbolUse.Range.Start + + let isTypedPat (r: Range)= + parseFileResults.IsTypeAnnotationGivenAtPositionPatched r.Start + + (funcOrValue.IsValue || isLambdaIfFunction) + && not (isTypedPat symbolUse.Range) + && symbolUse.IsFromDefinition + && not funcOrValue.IsMember + && not funcOrValue.IsMemberThisValue + && not funcOrValue.IsConstructorThisValue + && not (PrettyNaming.IsOperatorDisplayName funcOrValue.DisplayName) + + + + for symbolUse in symbolUses do + match symbolUse.Symbol with + | :? FSharpMemberOrFunctionOrValue as funcOrValue when + isValidForTypeHint funcOrValue symbolUse + -> + let layout = + ": " + + funcOrValue.ReturnParameter.Type.Format symbolUse.DisplayContext + + let hint = + { Text = layout + Pos = symbolUse.Range.End + Kind = Type } + + typeHints.Add(hint) + + | :? FSharpMemberOrFunctionOrValue as func when func.IsFunction && not symbolUse.IsFromDefinition -> + let appliedArgRangesOpt = + parseFileResults.GetAllArgumentsForFunctionApplicationAtPostion symbolUse.Range.Start + + match appliedArgRangesOpt with + | None -> () + | Some [] -> () + | Some appliedArgRanges -> + let parameters = func.CurriedParameterGroups |> Seq.concat + let appliedArgRanges = appliedArgRanges |> Array.ofList + let definitionArgs = parameters |> Array.ofSeq + // invariant - definitionArgs should be at least as long as applied args. + // if this is not the case (printfs?) we truncate to the lesser of the two + let minLength = min definitionArgs.Length appliedArgRanges.Length + + if minLength = 0 then + () + else + for idx = 0 to minLength - 1 do + let appliedArgRange = appliedArgRanges.[idx] + let definitionArgName = definitionArgs.[idx].DisplayName + + if + not (String.IsNullOrWhiteSpace(definitionArgName)) + && definitionArgName <> "````" + then + let hint = + { Text = definitionArgName + " =" + Pos = appliedArgRange.Start + Kind = Parameter } + + parameterHints.Add(hint) + + | :? FSharpMemberOrFunctionOrValue as methodOrConstructor when methodOrConstructor.IsConstructor -> // TODO: support methods when this API comes into FCS + let endPosForMethod = symbolUse.Range.End + let line, _ = Position.toZ endPosForMethod + + let afterParenPosInLine = + getFirstPositionAfterParen (text.Lines.[line].ToString()) (endPosForMethod.Column) + + let tupledParamInfos = + parseFileResults.FindParameterLocations(Position.fromZ line afterParenPosInLine) + + let appliedArgRanges = + parseFileResults.GetAllArgumentsForFunctionApplicationAtPostion symbolUse.Range.Start + + match tupledParamInfos, appliedArgRanges with + | None, None -> () + + // Prefer looking at the "tupled" view if it exists, even if the other ranges exist. + // M(1, 2) can give results for both, but in that case we want the "tupled" view. + | Some tupledParamInfos, _ -> + let parameters = + methodOrConstructor.CurriedParameterGroups + |> Seq.concat + |> Array.ofSeq // TODO: need ArgumentLocations to be surfaced + + for idx = 0 to parameters.Length - 1 do + // let paramLocationInfo = tupledParamInfos. .ArgumentLocations.[idx] + // let paramName = parameters.[idx].DisplayName + // if not paramLocationInfo.IsNamedArgument && not (String.IsNullOrWhiteSpace(paramName)) then + // let hint = { Text = paramName + " ="; Pos = paramLocationInfo.ArgumentRange.Start; Kind = Parameter } + // parameterHints.Add(hint) + () + + // This will only happen for curried methods defined in F#. + | _, Some appliedArgRanges -> + let parameters = + methodOrConstructor.CurriedParameterGroups + |> Seq.concat + + let appliedArgRanges = appliedArgRanges |> Array.ofList + let definitionArgs = parameters |> Array.ofSeq + + for idx = 0 to appliedArgRanges.Length - 1 do let appliedArgRange = appliedArgRanges.[idx] let definitionArgName = definitionArgs.[idx].DisplayName - if not (String.IsNullOrWhiteSpace(definitionArgName)) && definitionArgName <> "````" then + if not (String.IsNullOrWhiteSpace(definitionArgName)) then let hint = { Text = definitionArgName + " =" Pos = appliedArgRange.Start Kind = Parameter } parameterHints.Add(hint) + | _ -> () - | :? FSharpMemberOrFunctionOrValue as methodOrConstructor when methodOrConstructor.IsConstructor -> // TODO: support methods when this API comes into FCS - let endPosForMethod = symbolUse.Range.End - let line, _ = Position.toZ endPosForMethod - - let afterParenPosInLine = - getFirstPositionAfterParen (text.Lines.[line].ToString()) (endPosForMethod.Column) - - let tupledParamInfos = - parseFileResults.FindParameterLocations(Position.fromZ line afterParenPosInLine) - - let appliedArgRanges = - parseFileResults.GetAllArgumentsForFunctionApplicationAtPostion symbolUse.Range.Start - - match tupledParamInfos, appliedArgRanges with - | None, None -> () - - // Prefer looking at the "tupled" view if it exists, even if the other ranges exist. - // M(1, 2) can give results for both, but in that case we want the "tupled" view. - | Some tupledParamInfos, _ -> - let parameters = - methodOrConstructor.CurriedParameterGroups - |> Seq.concat - |> Array.ofSeq // TODO: need ArgumentLocations to be surfaced - - for idx = 0 to parameters.Length - 1 do - // let paramLocationInfo = tupledParamInfos. .ArgumentLocations.[idx] - // let paramName = parameters.[idx].DisplayName - // if not paramLocationInfo.IsNamedArgument && not (String.IsNullOrWhiteSpace(paramName)) then - // let hint = { Text = paramName + " ="; Pos = paramLocationInfo.ArgumentRange.Start; Kind = Parameter } - // parameterHints.Add(hint) - () - - // This will only happen for curried methods defined in F#. - | _, Some appliedArgRanges -> - let parameters = - methodOrConstructor.CurriedParameterGroups - |> Seq.concat - - let appliedArgRanges = appliedArgRanges |> Array.ofList - let definitionArgs = parameters |> Array.ofSeq - - for idx = 0 to appliedArgRanges.Length - 1 do - let appliedArgRange = appliedArgRanges.[idx] - let definitionArgName = definitionArgs.[idx].DisplayName - - if not (String.IsNullOrWhiteSpace(definitionArgName)) then - let hint = - { Text = definitionArgName + " =" - Pos = appliedArgRange.Start - Kind = Parameter } - - parameterHints.Add(hint) - | _ -> () - - let typeHints = typeHints.ToImmutableArray() - let parameterHints = parameterHints.ToImmutableArray() - - return typeHints.AddRange(parameterHints).ToArray() -} + let typeHints = typeHints.ToImmutableArray() + let parameterHints = parameterHints.ToImmutableArray() + + return typeHints.AddRange(parameterHints).ToArray() + } diff --git a/test/FsAutoComplete.Tests.Lsp/TestCases/InlayHints/Script.fsx b/test/FsAutoComplete.Tests.Lsp/TestCases/InlayHints/Script.fsx index d96bf4e27..3f6d54611 100644 --- a/test/FsAutoComplete.Tests.Lsp/TestCases/InlayHints/Script.fsx +++ b/test/FsAutoComplete.Tests.Lsp/TestCases/InlayHints/Script.fsx @@ -14,3 +14,6 @@ System.Environment.GetEnvironmentVariable "Blah" // shows that sprintf-like functions with only one 'parameter' (in this case the format) // are don't throw anymore let s = sprintf "thing %s" "blah" + +// shows that explicit type bindings don't results in a type hint +let s': string = ""