From 0e24edf5d8a8cbffd1e5ccc8ba2690d23056690d Mon Sep 17 00:00:00 2001 From: Ross Kuehl <94796738+rosskuehl@users.noreply.github.com> Date: Thu, 29 Dec 2022 13:12:13 -0500 Subject: [PATCH 1/2] adjust symbol start to account for '?' at beginning of optional parameter declaration --- .../src/FSharp.Editor/Hints/InlineTypeHints.fs | 15 ++++++++++++++- .../Hints/InlineTypeHintTests.fs | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/vsintegration/src/FSharp.Editor/Hints/InlineTypeHints.fs b/vsintegration/src/FSharp.Editor/Hints/InlineTypeHints.fs index 402eddbaef0..ab40a8931be 100644 --- a/vsintegration/src/FSharp.Editor/Hints/InlineTypeHints.fs +++ b/vsintegration/src/FSharp.Editor/Hints/InlineTypeHints.fs @@ -6,6 +6,7 @@ open Microsoft.VisualStudio.FSharp.Editor open FSharp.Compiler.CodeAnalysis open FSharp.Compiler.Symbols open FSharp.Compiler.Text +open FSharp.Compiler.Text.Position open Hints module InlineTypeHints = @@ -44,8 +45,20 @@ module InlineTypeHints = (symbol: FSharpMemberOrFunctionOrValue) (symbolUse: FSharpSymbolUse) = + let isOptionalParameter = + symbolUse.IsFromDefinition + && symbol.FullType.IsAbbreviation + && symbol.FullType.TypeDefinition.DisplayName = "option" + + let adjustedRangeStart = + if isOptionalParameter then + // we need the position to start at the '?' symbol + mkPos symbolUse.Range.StartLine (symbolUse.Range.StartColumn - 1) + else + symbolUse.Range.Start + let isNotAnnotatedManually = - not (parseFileResults.IsTypeAnnotationGivenAtPosition symbolUse.Range.Start) + not (parseFileResults.IsTypeAnnotationGivenAtPosition adjustedRangeStart) let isNotAfterDot = symbolUse.IsFromDefinition diff --git a/vsintegration/tests/FSharp.Editor.Tests/Hints/InlineTypeHintTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Hints/InlineTypeHintTests.fs index 0faf11e554c..6df54bce6a7 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Hints/InlineTypeHintTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Hints/InlineTypeHintTests.fs @@ -247,3 +247,21 @@ type Number<'T when IAddition<'T>>(value: 'T) = let actual = getTypeHints document CollectionAssert.AreEquivalent(expected, actual) + + + [] + let ``Hints are not shown when type is specified`` () = + let code = + """ +type MyType() = + + member _.MyMethod(?beep: int, ?bap: int, ?boop: int) = () + + member this.Foo = this.MyMethod(bap = 3, boop = 4) +""" + + let document = getFsDocument code + + let result = getTypeHints document + + Assert.IsEmpty(result) \ No newline at end of file From a5f663e277b92fcaf58880e73c238031891b9db5 Mon Sep 17 00:00:00 2001 From: Ross Kuehl <94796738+rosskuehl@users.noreply.github.com> Date: Thu, 29 Dec 2022 22:18:32 -0500 Subject: [PATCH 2/2] small cleanup and refactor --- .../src/FSharp.Editor/Hints/HintService.fs | 64 +++++++++++-------- .../Hints/InlineParameterNameHints.fs | 12 ++-- 2 files changed, 42 insertions(+), 34 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Hints/HintService.fs b/vsintegration/src/FSharp.Editor/Hints/HintService.fs index da24834efcb..f196e3caac5 100644 --- a/vsintegration/src/FSharp.Editor/Hints/HintService.fs +++ b/vsintegration/src/FSharp.Editor/Hints/HintService.fs @@ -6,37 +6,48 @@ open Microsoft.CodeAnalysis open Microsoft.VisualStudio.FSharp.Editor open FSharp.Compiler.CodeAnalysis open FSharp.Compiler.Symbols -open FSharp.Compiler.Text open Hints module HintService = - let private getHintsForSymbol parseResults hintKinds (longIdEndLocations: Position list) (symbolUse: FSharpSymbolUse) = - match symbolUse.Symbol with - | :? FSharpMemberOrFunctionOrValue as symbol - when hintKinds |> Set.contains HintKind.TypeHint - && InlineTypeHints.isValidForHint parseResults symbol symbolUse -> - - InlineTypeHints.getHints symbol symbolUse, - longIdEndLocations - - | :? FSharpMemberOrFunctionOrValue as symbol - when hintKinds |> Set.contains HintKind.ParameterNameHint - && InlineParameterNameHints.isMemberOrFunctionOrValueValidForHint symbol symbolUse -> - InlineParameterNameHints.getHintsForMemberOrFunctionOrValue parseResults symbol symbolUse longIdEndLocations, - symbolUse.Range.End :: longIdEndLocations + type private NativeHintResolver = FSharpSymbolUse seq -> NativeHint seq + + let inline private getTypeHints parseResults symbol: NativeHintResolver = + Seq.filter (InlineTypeHints.isValidForHint parseResults symbol) + >> Seq.collect (InlineTypeHints.getHints symbol) - | :? FSharpUnionCase as symbol - when hintKinds |> Set.contains HintKind.ParameterNameHint - && InlineParameterNameHints.isUnionCaseValidForHint symbol symbolUse -> + let inline private getHintsForMemberOrFunctionOrValue parseResults symbol: NativeHintResolver = + Seq.filter (InlineParameterNameHints.isMemberOrFunctionOrValueValidForHint symbol) + >> Seq.collect (InlineParameterNameHints.getHintsForMemberOrFunctionOrValue parseResults symbol) - InlineParameterNameHints.getHintsForUnionCase parseResults symbol symbolUse, - longIdEndLocations + let inline private getHintsForUnionCase parseResults symbol: NativeHintResolver = + Seq.filter (InlineParameterNameHints.isUnionCaseValidForHint symbol) + >> Seq.collect (InlineParameterNameHints.getHintsForUnionCase parseResults symbol) - // we'll be adding other stuff gradually here - | _ -> - [], - longIdEndLocations + let private getHintResolvers parseResults hintKinds (symbol: FSharpSymbol): NativeHintResolver seq = + let rec resolve hintKinds resolvers = + match hintKinds with + | [] -> resolvers |> Seq.choose id + | hintKind :: hintKinds -> + match hintKind with + | HintKind.TypeHint -> + match symbol with + | :? FSharpMemberOrFunctionOrValue as symbol -> getTypeHints parseResults symbol |> Some + | _ -> None + | HintKind.ParameterNameHint -> + match symbol with + | :? FSharpMemberOrFunctionOrValue as symbol -> getHintsForMemberOrFunctionOrValue parseResults symbol |> Some + | :? FSharpUnionCase as symbol -> getHintsForUnionCase parseResults symbol |> Some + | _ -> None + // we'll be adding other stuff gradually here + :: resolvers |> resolve hintKinds + + in resolve hintKinds [] + + let private getHintsForSymbol parseResults hintKinds (symbol: FSharpSymbol, symbolUses: FSharpSymbolUse seq) = + symbol + |> getHintResolvers parseResults hintKinds + |> Seq.collect (fun resolve -> resolve symbolUses) let getHintsForDocument (document: Document) hintKinds userOpName cancellationToken = async { @@ -49,8 +60,7 @@ module HintService = return checkResults.GetAllUsesOfAllSymbolsInFile cancellationToken - |> Seq.mapFold (getHintsForSymbol parseResults hintKinds) [] - |> fst - |> Seq.concat + |> Seq.groupBy (fun symbolUse -> symbolUse.Symbol) + |> Seq.collect (getHintsForSymbol parseResults (hintKinds |> Set.toList)) |> Seq.toList } diff --git a/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs b/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs index d4278188329..e4bb923b9e9 100644 --- a/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs +++ b/vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs @@ -33,7 +33,6 @@ module InlineParameterNameHints = let private getArgumentLocations (symbolUse: FSharpSymbolUse) - (longIdEndLocations: Position list) (parseResults: FSharpParseFileResults) = let position = Position.mkPos @@ -41,9 +40,9 @@ module InlineParameterNameHints = (symbolUse.Range.End.Column + 1) parseResults.FindParameterLocations position - |> Option.filter (fun locations -> longIdEndLocations |> List.contains locations.LongIdEndLocation |> not) - |> Option.map (fun locations -> locations.ArgumentLocations) - |> Option.defaultValue [||] + |> Option.map (fun locations -> locations.ArgumentLocations |> Seq.filter (fun location -> Position.posGeq location.ArgumentRange.Start position)) + |> Option.filter (not << Seq.isEmpty) + |> Option.defaultValue Seq.empty let private getTupleRanges = Seq.map (fun location -> location.ArgumentRange) @@ -83,11 +82,10 @@ module InlineParameterNameHints = let getHintsForMemberOrFunctionOrValue (parseResults: FSharpParseFileResults) (symbol: FSharpMemberOrFunctionOrValue) - (symbolUse: FSharpSymbolUse) - (longIdEndLocations: Position list) = + (symbolUse: FSharpSymbolUse) = let parameters = symbol.CurriedParameterGroups |> Seq.concat - let argumentLocations = parseResults |> getArgumentLocations symbolUse longIdEndLocations + let argumentLocations = parseResults |> getArgumentLocations symbolUse let tupleRanges = argumentLocations |> getTupleRanges let curryRanges = parseResults |> getCurryRanges symbolUse