Skip to content

Conversation

@nojaf
Copy link
Contributor

@nojaf nojaf commented Jun 16, 2023

Fixes #15423

@nojaf nojaf requested a review from a team as a code owner June 16, 2023 15:10
@vzarytovskii
Copy link
Member

I don't think that's right.
Private setter, public getter scenario is very common one. Or am I missing something (looking from my phone)?

@T-Gro
Copy link
Member

T-Gro commented Jun 16, 2023

Please see the comment here #15423 (comment) .

The error is only in case of mixing "outer" and "inner" declaration of visibility for a property.
Getter and setter can have different accessibilities just fine.

@KevinRansom

This comment was marked as duplicate.

KevinRansom

This comment was marked as duplicate.

Copy link
Contributor

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nojaf - I believe this is not the correct fix.

type Foo() =
    member f.X with internal get (key1, key2) = true and private set (key1, key2) value = ()
    member internal f.Y with get (key1, key2) = true and private set (key1, key2) value = ()

If you do not specify the visibility of the property F# allows you specify individual visibilities for the getter and setter, as shown in the X property of the repro.

The reason F# needs this behaviour is for C# interop which also allows, distinct visibilities for the getter or setter,
E.g.

        public string ShowSomething { set; private get; } = "Some text value";

The Y property has an error because if you specify the visibility of the property, then you are not allowed to specify the visibility of the getters and setters.

I think improving the error message would have been better. Something like, "When the visibility for a property is specified, setting the visibility of the set or get method is not allowed."

@nojaf
Copy link
Contributor Author

nojaf commented Jun 19, 2023

Thanks everyone, I was indeed confused about all of this.
I've updated the error message as @KevinRansom suggested.

@nojaf nojaf requested a review from KevinRansom June 19, 2023 12:38
@nojaf nojaf changed the title Raise error when the accessibility of the getter and setter don't match. Update error message of parsMultipleAccessibilitiesForGetSet Jun 19, 2023
@KevinRansom KevinRansom enabled auto-merge (squash) June 19, 2023 19:16
auto-merge was automatically disabled June 20, 2023 11:26

Head branch was pushed to by a user without write access

@0101 0101 merged commit a22ab7c into dotnet:main Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Multiple accessibilities given for property getter or setter is not raised for property

5 participants