Skip to content

Commit c6a00ff

Browse files
T-GroCopilot
andcommitted
Fix methods tagged as Member instead of Method in tooltips (#10540)
Rebase onto main after TypedTreeOps split. Apply the change to TypedTreeOps.FreeVars.fs (previously TypedTreeOps.fs). - NicePrint.fs: check isNil argInfos to use tagMethod for methods vs tagMember - TypedTreeOps.FreeVars.fs: check ValReprInfo.ArgInfos to distinguish methods - 8 regression tests: methods, properties, indexers, explicit getters - Release notes added Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9567ee7 commit c6a00ff

4 files changed

Lines changed: 108 additions & 2 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
* 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)))
2020
* 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))
2121
* Fix completion inconsistently showing some obsolete members (fields and events) while hiding others (methods and properties). All obsolete members are now consistently hidden by default. ([Issue #13512](https://github.com/dotnet/fsharp/issues/13512), [PR #19506](https://github.com/dotnet/fsharp/pull/19506))
22+
* 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))
2223

2324
### Added
2425

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.FreeVars.fs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1449,7 +1449,14 @@ module internal MemberRepresentation =
14491449
| SynMemberKind.PropertyGetSet -> tagProperty vref.DisplayName
14501450
| SynMemberKind.ClassConstructor
14511451
| SynMemberKind.Constructor -> tagMethod vref.DisplayName
1452-
| SynMemberKind.Member -> tagMember vref.DisplayName
1452+
| SynMemberKind.Member ->
1453+
match vref.ValReprInfo with
1454+
| Some valReprInfo ->
1455+
let numArgGroups = valReprInfo.ArgInfos.Length
1456+
let isMethod = if memberInfo.MemberFlags.IsInstance then numArgGroups > 1 else numArgGroups > 0
1457+
if isMethod then tagMethod vref.DisplayName
1458+
else tagMember vref.DisplayName
1459+
| None -> tagMember vref.DisplayName
14531460

14541461
match fullNameOfParentOfValRefAsLayout vref with
14551462
| ValueNone -> wordL n

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

Lines changed: 97 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,89 @@ 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"
663+
664+
// https://github.com/dotnet/fsharp/issues/10540
665+
[<Fact>]
666+
let ``Indexer should be tagged as Property`` () =
667+
Checker.getTooltip """
668+
type T() =
669+
member x.Ite{caret}m with get(i: int) = i
670+
"""
671+
|> assertNameTagInTooltip TextTag.Property "Item"
672+
673+
// https://github.com/dotnet/fsharp/issues/10540
674+
[<Fact>]
675+
let ``Indexer with getter and setter should be tagged as Property`` () =
676+
Checker.getTooltip """
677+
type T() =
678+
let mutable data = [| 0; 1; 2 |]
679+
member x.Ite{caret}m
680+
with get(i: int) = data.[i]
681+
and set (i: int) (v: int) = data.[i] <- v
682+
"""
683+
|> assertNameTagInTooltip TextTag.Property "Item"
684+
685+
// https://github.com/dotnet/fsharp/issues/10540
686+
[<Fact>]
687+
let ``Property with explicit getter should be tagged as Property`` () =
688+
Checker.getTooltip """
689+
type T() =
690+
member x.Valu{caret}e with get() = 42
691+
"""
692+
|> assertNameTagInTooltip TextTag.Property "Value"
693+
694+
// https://github.com/dotnet/fsharp/issues/10540
695+
[<Fact>]
696+
let ``Static property should be tagged as Property`` () =
697+
Checker.getTooltip """
698+
type T() =
699+
static member Defaul{caret}t = T()
700+
"""
701+
|> assertNameTagInTooltip TextTag.Property "Default"

0 commit comments

Comments
 (0)