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

Jump to bottom

Fixing hints for custom ops #15119

Merged
merged 1 commit into from
Apr 20, 2023
Merged

Fixing hints for custom ops #15119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 6 additions & 3 deletions vsintegration/src/FSharp.Editor/Hints/InlineParameterNameHints.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ type InlineParameterNameHints(parseResults: FSharpParseFileResults) =
>> Seq.map (fun location -> location.ArgumentRange)
>> Seq.contains range

let isCustomOperation (symbol: FSharpMemberOrFunctionOrValue) =
symbol.HasAttribute<CustomOperationAttribute>()

let getSourceTextAtRange (sourceText: SourceText) (range: range) =
(RoslynHelpers.FSharpRangeToTextSpan(sourceText, range) |> sourceText.GetSubText)
.ToString()
Expand All @@ -65,11 +68,9 @@ type InlineParameterNameHints(parseResults: FSharpParseFileResults) =
symbol.DeclaringEntity
|> Option.exists (fun entity -> entity.CompiledName <> "Operators")

let isNotCustomOperation = not <| symbol.HasAttribute<CustomOperationAttribute>()

(symbol.IsFunction && isNotBuiltInOperator) // arguably, hints for those would be rather useless
|| symbol.IsConstructor
|| (symbol.IsMethod && isNotCustomOperation)
|| symbol.IsMethod
else
false

Expand Down Expand Up @@ -100,8 +101,10 @@ type InlineParameterNameHints(parseResults: FSharpParseFileResults) =
|> Seq.filter (fun range -> argumentLocations |> (not << isNamedArgument range))

let argumentNames = Seq.map (getSourceTextAtRange sourceText) ranges
let skipped = if symbol |> isCustomOperation then 1 else 0

parameters
|> Seq.skip skipped
|> Seq.zip ranges // Seq.zip is important as List.zip requires equal lengths
|> Seq.where (snd >> parameterNameExists)
|> Seq.zip argumentNames
Expand Down
25 changes: 23 additions & 2 deletions vsintegration/tests/FSharp.Editor.Tests/Hints/InlineParameterNameHintTests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,17 +481,38 @@ let test sequences =
Assert.Equal(expected, actual)

[<Fact>]
let ``Hints are not shown when CustomOperation attribute is detected`` () =
let ``Hints are shown correctly for custom operations`` () =
let code =
"""
let q = query { for x in { 1 .. 10 } do select x }
"""

let document = getFsDocument code

let expected =
[
{
Content = "projection = "
Location = (1, 48)
}
]

let actual = getParameterNameHints document

Assert.Equal(expected, actual)

[<Fact>]
let ``Hints are not shown for custom operations with only 1 parameter`` () =
let code =
"""
let q = query { for _ in { 1 .. 10 } do count }
"""

let document = getFsDocument code

let actual = getParameterNameHints document

Assert.Empty actual
Assert.Empty(actual)

[<Fact>]
let ``Hints are not shown when parameter names coincide with variable names`` () =
Expand Down