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

Do not remove property setters #664

Closed
aspnetde opened this issue Jan 31, 2020 · 3 comments · Fixed by #732
Closed

Do not remove property setters #664

aspnetde opened this issue Jan 31, 2020 · 3 comments · Fixed by #732

Comments

@aspnetde
Copy link

When formatting

type IPlatformViewFactory =
    interface end

type public PlatformViewFactory =
    [<DefaultValue>]
    static val mutable private instance : IPlatformViewFactory

    static member Instance
        with set (value) = PlatformViewFactory.instance <- value
        and get() = PlatformViewFactory.instance

... this will result in

type IPlatformViewFactory =
    interface
    end

type public PlatformViewFactory =
    [<DefaultValue>]
    static val mutable private instance: IPlatformViewFactory

    static member Instance
        with set (value) = PlatformViewFactory.instance <- value
    static member Instance = PlatformViewFactory.instance

(Repro)

Is there a reason why the getter of Instance is being replaced?

@nojaf
Copy link
Contributor

nojaf commented Jan 31, 2020

At first glance, I would say that this is a bug.
I would expect that same behaviour as in this test.
Should debug this to see what is going on...

@auduchinok
Copy link
Contributor

FCS reports properties with multiple accessors as separate type members. This is something we've had to workaround too.

Look at how they're processed:
https://github.com/dotnet/fsharp/blob/e43e7d74b05160d97db35c7aa4dcddd052bce457/src/fsharp/pars.fsy#L1648
and where multiple accessors are reported:
https://github.com/dotnet/fsharp/blob/e43e7d74b05160d97db35c7aa4dcddd052bce457/src/fsharp/pars.fsy#L1615

These property members have wrong ranges and it's something that can be checked to recognize them, like we're doing here:
https://github.com/JetBrains/fsharp-support/blob/6de2627fc1dc58ad235fcd79ef79758d0b53ecfa/ReSharper.FSharp/src/FSharp.Psi.Features/src/Parsing/FSharpImplTreeBuilder.fs#L300-L303

@knocte
Copy link
Contributor

knocte commented Mar 26, 2020

I've found getters being removed as well.

jindraivanek added a commit that referenced this issue Mar 26, 2020
* update test

* Property setter can be before getter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants