dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.93k stars 788 forks source link

Operators starting with '>' don't get IDE features #3462

Open cartermp opened 7 years ago

cartermp commented 7 years ago

Current status

It seems that symbols for operators are sometimes picked up for document highlight and inline rename, but it's inconsistent. Given the following code in a .NET Core project:

module Operators =
    let (>>=) a f = Result.bind f a
    let (<*>) x f = f x
    let (<!>) = Result.map
    let (<+>) x f = f x
    let (<?>) x f = f x

open Operators

[<EntryPoint>]
let main argv =
    (float <!> (Ok 12)) |> ignore

    printfn "Hello World from F#!"
    0 // return an integer exit code

There three notable issues. Some of these are driven by the same root cause as #3873.

  1. Symbols for operators except for >>= are picked up. For >>=, there is nothing - no QuickInfo, no document highlight, etc.
  2. When you try to rename <!> at the call site, it will escape the symbol with double backticks.

Note that colorization of >>= and any operator that starts with > is similarly affected, as per https://github.com/dotnet/fsharp/issues/10272

Update 2

Modules are now found at the open site by Inline Rename and Find all References due to #3803, thanks @vasily-kirichenko

Update 1

Using 15.4.1, and given the following code:

module Foo =
    let (>.>) x f = f x

open Foo

[<EntryPoint>]
let main argv =
    12. >.> sqrt |> ignore
    0 // return an integer exit code

Click on Foo at the declaration site:

Epected --> The Foo at open Foo is highlighted Actual --> It's not.

Notice that Find Refs and Inline Rename also fail to pick up the symbol.

Click on >.> anywhere it's used.

Expected --> Highlights the operator everywhere it exists Actual --> No highlight

Find Refs and Inline Rename also do not pick up this symbol.

Old Issue

VS 2017 15.3 + 8/16/2017 nightly build.

Run Find all References or Rename on ThisIsAProperty.

// Learn more about F# at http://fsharp.org
// See the 'F# Tutorial' project for more help.

open TestCS

type C() =
    member val ThisIsAProperty = 12 with get, set

[<EntryPoint>]
let main argv = 
    let c = C()
    c.ThisIsAProperty <- 0
    0 // return an integer exit code

no-refs-proprety

rename-no-worky

I don't think that this the same as #3033, because rename and FAR work for the class definition itself.

The above also fails on modules and customer operators.

cartermp commented 7 years ago

Dunno if this is relevant, but in debugging, I noticed that this line gets hit four times on ThisIsAProperty has a match, when document highlight hits it twice. I'll probably debug this tomorrow when I'm not dead tired

isaacabraham commented 7 years ago

I'm not sure if this is the same issue. I've just upgraded to latest nightly and VS2017 etc.. In a standalone script file, if I try to F2 rename, the dialog pops up etc. but as soon as I start typing, the refactoring disappears and the original symbol is untouched.

cartermp commented 7 years ago

@isaacabraham Can you be specific about which constructs this fails on? This works fine in scripts for functions, F# types, F# class names, etc. But fails for members, modules, and operators.

cartermp commented 7 years ago

In a hive of VS, Find all References works for properties, but outside of that hive, it doesn't. Rename fails, though.

isaacabraham commented 7 years ago

It was for record field.

abelbraaksma commented 7 years ago

@isaacabraham, @cartermp, I just tried to repro Isaac's findings (also VS2017 15.3.1.0 P1, nightly of yesterday) and for a record field it pops up with the "apply where" box and I can start typing. However, after hitting the apply-button, some fields are renamed and some aren't.

It looks like for record types it only replaced anything in a full record constructor, not references with dot-notation nor patterns. I'll try to create a small repro (unless already known).

Screenshot is just after I changed ImplicitContext to ImplicitContext2, you can see that the main record definition was changed, but right below that where I use it, it did not update. In all it replaced 25 items across multiple projects, missing 11 occurrences.

image

Summary: the reported bug here also, at least in part, applies to record fields. But I have never seen this feature successful in its fullness so I (unfortunately) stopped using it, so I am uncertain I am reporting something new here.

cartermp commented 7 years ago

Thanks @abelbraaksma, that is also what I see. I'll open a separate issue there.

@isaacabraham is this similar to what you saw as well?

cartermp commented 7 years ago

3493 is tracking the rename/record label issue

vasily-kirichenko commented 7 years ago

Find all refs on ThisIsAProperty works in 15.4:

image

Rename does not work.

cartermp commented 7 years ago

Renamed issue

vasily-kirichenko commented 7 years ago

Properties renaming fixed in https://github.com/Microsoft/visualfsharp/pull/3762

cartermp commented 7 years ago

Module issue fixed in #3803

cartermp commented 6 years ago

Updated with a more comprehensive set of problems with .NET Core-based projects and operators. Thanks to @vasily-kirichenko all other issues previously mentioned seem to no longer be a problem, and this is really now just about IDE feature inconsistencies with operators.

cartermp commented 6 years ago

Another case in 15.6 Preview 3:

let (>.) x f = f x
let (|.) x f = f x

12.0 >. //<<-- You get completion here
13.0 |. //<<-- No completion here

In 15.6 Preview 3, operators were added as something which should not trigger completion. This is because you can end an operator with a ., as above. However, the first case (>.) gets not symbol uses and thus cannot be classified as an operator, and you still get completion after typing .. In the latter case, everything works as expected.

cartermp commented 6 years ago

Updated some of the issues above. The list is down from 7 to 3.

cartermp commented 6 years ago

Down to 2 issues.

abelbraaksma commented 4 years ago

Updated some of the issues above. The list is down from 7 to 3. Down to 2 issues.

@cartermp, sounds good, but can you clarify? I see a few issues mentioned at the top, but don't see how it is a list of 7 issues, nor which one are still open. Maybe make it a checkmark-list?

Also, you just linked my issue, which is indeed related, but I don't see it explicitly specified here, so for completeness sake & to summarize:

cartermp commented 4 years ago

It's the same issue. None of the features are distinctly different from one another from the perspective of this problem.

abelbraaksma commented 4 years ago

Ah, you mean something like "these operators are not reported as operators, or wrongly reported" by the compiler so the tooling cannot do its trick in highlighting/showing tooltips/go-to definition/rename?

cartermp commented 4 years ago

Yes, that's correct. The IDE won't pick them up as symbols like the other operators.

abelbraaksma commented 6 months ago

@vzarytovskii, you linked that other issue recently, but I think this is resolved now. Here's an exercise in obfuscated code in VSCode:

image

And here's Visual Studio 2022, latest:

image

Also, hovering over any of these (left-angle or right-angle) shows the tooltip, in which you can click to "go to definition". And typing the dot at the end does NOT show a dropdown list of functions anymore (which is good).

It must have been fixed quite recently, as I'm quite certain that I kept seeing this issue until not so long ago.

image

@cartermp, you created this issue 7 years ago, do you agree to close as resolved? While the only coloring issue is still with (*), the style guide requires to write it as ( * ) in which case all's good anyway. Not really worth spending too much time on.

brianrourkeboll commented 6 months ago

@abelbraaksma Things like this still reproduce for me in both VS and Ionide.

let (..>=) = (>=)
let (>..=>) = (>=)
let (>>>>) = (>)
let (.>>>>) = (>)
let (>>>>.) = (>)
let (<<<<) = (<)
let (.<<<<) = (<)
let (<<<<.) = (<)

let _ = 2 ..>= 2
let _ = 2 >..=> 2
let _ = 2 >>>> 2
let _ = 2 .>>>> 2
let _ = 2 >>>>. 2
let _ = 2 <<<< 2
let _ = 2 .<<<< 2
let _ = 2 <<<<. 2

https://github.com/dotnet/fsharp/assets/14795984/ff89ec3a-fc56-452a-969a-2310b2c780e2

It seems there are likely multiple different bugs involved here, since the coloring behavior does not correlate exactly with the tooltip (and go-to-definition, rename, etc.) behavior.

abelbraaksma commented 6 months ago

@brianrourkeboll do I see this correctly: only when there's a leading dot and a following angle bracket, that there's still a coloring issue and/or tooltip issue?

It looks to me that anything else is good now, right?

brianrourkeboll commented 6 months ago

only when there's a leading dot and a following angle bracket, that there's still a coloring issue and/or tooltip issue?

The color is different when there is a leading > (with no leading .s).

It looks to me that anything else is good now, right?

I am not sure. I actually started looking at this a while ago, and I feel like there were still some more scenarios that could lead to these inconsistencies, but I don't remember.

I just played around for another minute now, and it seems that more than one trailing . also breaks things, regardless of what the first character is:

let (+..) = (+)

let _ = 2 +.. 2

There may be more such combinations.

Edit: :> and :?> also don't get tooltips. Should they?

abelbraaksma commented 6 months ago

Thanks. The color difference is very subtle in your video (bold vs not bold, it seems), I think that's what you mean?

Let's leave the issue open for the remaining cases.

brianrourkeboll commented 6 months ago

The color difference is very subtle in your video (bold vs not bold, it seems), I think that's what you mean?

Yes, it's much more noticeable in other themes:

image