Skip to content

Commit 9697305

Browse files
T-GroCopilot
andcommitted
Fix methods tagged as Member instead of Method in tooltips (#10540)
In NicePrint.fs, the SynMemberKind.Member case unconditionally used tagMember for all members. Now checks isNil argInfos to use tagMethod for methods vs tagMember for property-like members. In TypedTreeOps.fs fullDisplayTextOfValRefAsLayout, checks ValReprInfo.ArgInfos length to distinguish methods from property-like members (instance >1 arg group, static >0). Added 5 regression tests: instance method, method with params, static method, property-like member, and auto property. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ead77d1 commit 9697305

File tree

4 files changed

+69
-2
lines changed

4 files changed

+69
-2
lines changed

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
* Fix `YieldFromFinal`/`ReturnFromFinal` being incorrectly called in non-tail positions (`for`, `use`, `use!`, `try/with` handler). ([Issue #19402](https://github.com/dotnet/fsharp/issues/19402), [PR #19403](https://github.com/dotnet/fsharp/pull/19403))
1818
* Fixed how the source ranges of warn directives are reported (as trivia) in the parser output (by not reporting leading spaces). ([Issue #19405](https://github.com/dotnet/fsharp/issues/19405), [PR #19408]((https://github.com/dotnet/fsharp/pull/19408)))
1919
* Fix UoM value type `ToString()` returning garbage values when `--checknulls+` is enabled, caused by double address-taking in codegen. ([Issue #19435](https://github.com/dotnet/fsharp/issues/19435), [PR #19440](https://github.com/dotnet/fsharp/pull/19440))
20+
* Fix methods being tagged as `Member` instead of `Method` in tooltips. ([Issue #10540](https://github.com/dotnet/fsharp/issues/10540), [PR #19507](https://github.com/dotnet/fsharp/pull/19507))
2021

2122
### Added
2223

src/Compiler/Checking/NicePrint.fs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1383,7 +1383,8 @@ module PrintTastMemberOrVals =
13831383
let resL =
13841384
if short then tauL
13851385
else
1386-
let nameL = layoutMemberName denv vref niceMethodTypars argInfos tagMember vref.DisplayNameCoreMangled true
1386+
let tag = if isNil argInfos then tagMember else tagMethod
1387+
let nameL = layoutMemberName denv vref niceMethodTypars argInfos tag vref.DisplayNameCoreMangled true
13871388
let nameL = if short then nameL else mkInlineL denv vref.Deref nameL
13881389
stat --- ((nameL |> addColonL) ^^ tauL)
13891390
prettyTyparInst, resL

src/Compiler/TypedTree/TypedTreeOps.fs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3433,7 +3433,14 @@ let fullDisplayTextOfValRefAsLayout (vref: ValRef) =
34333433
| SynMemberKind.PropertyGetSet -> tagProperty vref.DisplayName
34343434
| SynMemberKind.ClassConstructor
34353435
| SynMemberKind.Constructor -> tagMethod vref.DisplayName
3436-
| SynMemberKind.Member -> tagMember vref.DisplayName
3436+
| SynMemberKind.Member ->
3437+
match vref.ValReprInfo with
3438+
| Some valReprInfo ->
3439+
let numArgGroups = valReprInfo.ArgInfos.Length
3440+
let isMethod = if memberInfo.MemberFlags.IsInstance then numArgGroups > 1 else numArgGroups > 0
3441+
if isMethod then tagMethod vref.DisplayName
3442+
else tagMember vref.DisplayName
3443+
| None -> tagMember vref.DisplayName
34373444
match fullNameOfParentOfValRefAsLayout vref with
34383445
| ValueNone -> wordL n
34393446
| ValueSome pathText ->

tests/FSharp.Compiler.Service.Tests/TooltipTests.fs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,17 @@ let assertAndGetSingleToolTipText items =
388388
let text,_xml,_remarks = assertAndExtractTooltip items
389389
text
390390

391+
let getMainDescriptionTags (ToolTipText(items)) =
392+
match items with
393+
| ToolTipElement.Group [ singleElement ] :: _ -> singleElement.MainDescription
394+
| _ -> failwith $"Expected single group in tooltip, got {items}"
395+
396+
let assertNameTagInTooltip expectedTag expectedName (tooltip: ToolTipText) =
397+
let tags = getMainDescriptionTags tooltip
398+
let found = tags |> Array.exists (fun t -> t.Tag = expectedTag && t.Text = expectedName)
399+
let desc = tags |> Array.map (fun t -> sprintf "(%A, %s)" t.Tag t.Text) |> String.concat ", "
400+
Assert.True(found, sprintf "Expected tag %A with text '%s' in tooltip, but found: %s" expectedTag expectedName desc)
401+
391402
let normalize (s: string) = s.Replace("\r\n", "\n").Replace("\n\n", "\n")
392403

393404
[<Fact>]
@@ -602,3 +613,50 @@ let normaliz{caret}e' x = x + 1
602613
"""
603614

604615
testXmlDocFallbackToSigFileWhileInImplFile sigSource implSource "Normalize with a prime"
616+
617+
// https://github.com/dotnet/fsharp/issues/10540
618+
[<Fact>]
619+
let ``Instance method should be tagged as Method in tooltip`` () =
620+
Checker.getTooltip """
621+
type T() =
622+
member x.Metho{caret}d() = ()
623+
"""
624+
|> assertNameTagInTooltip TextTag.Method "Method"
625+
626+
// https://github.com/dotnet/fsharp/issues/10540
627+
[<Fact>]
628+
let ``Instance method with parameters should be tagged as Method in tooltip`` () =
629+
Checker.getTooltip """
630+
type T() =
631+
member x.Ad{caret}d(a: int, b: int) = a + b
632+
"""
633+
|> assertNameTagInTooltip TextTag.Method "Add"
634+
635+
// https://github.com/dotnet/fsharp/issues/10540
636+
[<Fact>]
637+
let ``Static method should be tagged as Method in tooltip`` () =
638+
Checker.getTooltip """
639+
type T() =
640+
static member Creat{caret}e() = T()
641+
"""
642+
|> assertNameTagInTooltip TextTag.Method "Create"
643+
644+
// https://github.com/dotnet/fsharp/issues/10540
645+
[<Fact>]
646+
let ``Property-like member should be tagged as Property`` () =
647+
Checker.getTooltip """
648+
type T() =
649+
member x.Valu{caret}e = 42
650+
"""
651+
|> assertNameTagInTooltip TextTag.Property "Value"
652+
653+
// https://github.com/dotnet/fsharp/issues/10540
654+
[<Fact>]
655+
let ``Auto property should be tagged as Property`` () =
656+
Checker.getTooltip """
657+
namespace Foo
658+
659+
type Bar() =
660+
member val Fo{caret}o = "bla" with get, set
661+
"""
662+
|> assertNameTagInTooltip TextTag.Property "Foo"

0 commit comments

Comments
 (0)