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

Tooltip shows wrong name resolution on uncallable static member due to name collision #8875

Open
smoothdeveloper opened this issue Apr 3, 2020 · 4 comments
Labels
Area-LangService-ToolTips tooltips/hover/quickinfo Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Milestone

Comments

@smoothdeveloper
Copy link
Contributor

module Dom =
  
  type WindowDelimiter =
    | WindowDelimiter of int
    static member FromCs (a: obj) = WindowDelimiter 1
  
  type Tree =
    | WindowDelimiter of WindowDelimiter
    static member FromCs (a: obj) = WindowDelimiter(WindowDelimiter.WindowDelimiter(1))

let a = Dom.WindowDelimiter.FromCs null

Expected behavior

The last line should be OK, even the tooling shows the method in auto-complete, the signature is on the type tooltip, and the member definition highlights when the carret is on the invocation.

Actual behavior

The field , constructor or member 'FromCs' is not defined

Known workarounds

@gusty pointed that SRTP trait call enables calling the member.

let inline fromCs< ^t when  ^t : (static member FromCs : obj ->  ^t)> (x:obj) =
    (^t : (static member FromCs : obj -> 't) (x) )

let a = fromCs<Dom.WindowDelimiter> null

Another work around involve putting RequireQualifiedAccess which is not always possible / wanted.

Related information

Visual Studio 16.5.2

#6805 (comment) => a case where SRTP is useful to work around a compiler limitation.

@charlesroddie
Copy link
Contributor

Simpler reproduction:

module Dom =
    type WindowDelimiter =
        static member FromCs = ()
    type Tree =
        | WindowDelimiter
let a = Dom.WindowDelimiter.FromCs

@cartermp
Copy link
Contributor

cartermp commented Apr 4, 2020

This feels like a bug on two fronts:

  • FCS is not giving you the correct "view" of the world that compilation does
  • The code should probably just compile

@charlesroddie Yours looks slightly different. Since Dom.WindowDelimiter could refer to either construct, the compiler picks the last one that it saw. So in this case it's binding to something that doesn't define FromCs. Annoying, but consistent with shadowing and other "pick the last one we saw" behavior elsewhere.

@charlesroddie
Copy link
Contributor

Since Dom.WindowDelimiter could refer to either construct, the compiler picks the last one that it saw.

Doesn't the fact that my code give the same behaviour show that its the same issue? It eliminates various things that could be causing the issue (multiple potential conflicts in the use of the name WindowDelimiter, and the existence of Tree.FromCs) and shows that they didn't affect the result, leaving you with the issue you identified.

I'm happy to agree with you that my reduction shouldn't compile for this reason, and that the fix should be to adjust intellisense rather than compilation. But that should apply equally to the code in the OP.

@dsyme
Copy link
Contributor

dsyme commented Sep 1, 2020

This is currently by design, and an RFC will be needed to change things, so I'm tracking this with this language suggestion fsharp/fslang-suggestions#907

Essentially in Dom.WindowDelimiter the union case is always preferred, and the last union case among many of the same name.

The type should be used to qualify the name in these cases, with or without RequireQUalifiedAccess

The fact that the IDE is showing the other resolution is a bug

@dsyme dsyme added the Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code. label Sep 1, 2020
@dsyme dsyme changed the title static member on DU not callable due to name collision Tooltip shows wrong name resolution on uncallable static member due to name collision Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-LangService-ToolTips tooltips/hover/quickinfo Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Projects
Status: New
Development

No branches or pull requests

4 participants