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

Correct inline parameter hints for named optional arguments #14498

Merged
merged 1 commit into from
Dec 21, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ namespace Microsoft.VisualStudio.FSharp.Editor.Hints

open Microsoft.VisualStudio.FSharp.Editor
open FSharp.Compiler.CodeAnalysis
open FSharp.Compiler.EditorServices
open FSharp.Compiler.Symbols
open FSharp.Compiler.Text
open Hints
Expand All @@ -30,7 +31,7 @@ module InlineParameterNameHints =
let private doesFieldNameExist (field: FSharpField) =
not field.IsNameGenerated

let private getTupleRanges
let private getArgumentLocations
(symbolUse: FSharpSymbolUse)
(parseResults: FSharpParseFileResults) =

Expand All @@ -40,9 +41,11 @@ module InlineParameterNameHints =

parseResults.FindParameterLocations position
|> Option.map (fun locations -> locations.ArgumentLocations)
|> Option.map (Seq.map (fun location -> location.ArgumentRange))
|> Option.defaultValue []
|> Seq.toList
|> Option.defaultValue [||]

let private getTupleRanges =
Seq.map (fun location -> location.ArgumentRange)
>> Seq.toList

let private getCurryRanges
(symbolUse: FSharpSymbolUse)
Expand All @@ -51,7 +54,15 @@ module InlineParameterNameHints =
parseResults.GetAllArgumentsForFunctionApplicationAtPosition symbolUse.Range.Start
|> Option.defaultValue []

let isMemberOrFunctionOrValueValidForHint (symbol: FSharpMemberOrFunctionOrValue) (symbolUse: FSharpSymbolUse) =
let private isNamedArgument range =
Seq.filter (fun location -> location.IsNamedArgument)
>> Seq.map (fun location -> location.ArgumentRange)
>> Seq.contains range

let isMemberOrFunctionOrValueValidForHint
(symbol: FSharpMemberOrFunctionOrValue)
(symbolUse: FSharpSymbolUse) =

if symbolUse.IsFromUse then
let isNotBuiltInOperator =
symbol.DeclaringEntity
Expand All @@ -73,10 +84,14 @@ module InlineParameterNameHints =
(symbolUse: FSharpSymbolUse) =

let parameters = symbol.CurriedParameterGroups |> Seq.concat
let argumentLocations = getArgumentLocations symbolUse parseResults

let tupleRanges = parseResults |> getTupleRanges symbolUse
let tupleRanges = argumentLocations |> getTupleRanges
let curryRanges = parseResults |> getCurryRanges symbolUse
let ranges = if tupleRanges |> (not << Seq.isEmpty) then tupleRanges else curryRanges

let ranges =
if tupleRanges |> (not << Seq.isEmpty) then tupleRanges else curryRanges
|> Seq.filter (fun range -> argumentLocations |> (not << isNamedArgument range))

parameters
|> Seq.zip ranges // Seq.zip is important as List.zip requires equal lengths
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -406,3 +406,45 @@ let x = "test".Split("").[0].Split("");
let actual = getParameterNameHints document

Assert.AreEqual(expected, actual)

[<Test>]
let ``Hints are not shown for optional parameters with specified names`` () =
let code =
"""
type MyType() =

member _.MyMethod(?beep: int, ?bap: int, ?boop: int) = ()

member this.Foo = this.MyMethod(3, boop = 4)
"""

let document = getFsDocument code

let expected =
[
{
Content = "beep = "
Location = (5, 37)
}
]

let actual = getParameterNameHints document

Assert.AreEqual(expected, actual)

[<Test>]
let ``Hints are not shown when all optional parameters are named`` () =
let code =
"""
type MyType() =

member _.MyMethod(?beep: int, ?bap : int, ?boop : int) = ()

member this.Foo = this.MyMethod(bap = 3, beep = 4)
"""

let document = getFsDocument code

let actual = getParameterNameHints document

Assert.IsEmpty(actual)