Allow open type on RequireQualifiedAccess DUs/records

NinoFloris commented 2 years ago

I propose we allow DUs/records annotated with RequireQualifiedAccess to have their symbols brought into scope with open type. Modules annotated with RQA (RequireQualifiedAccess) would still stay exempt from this relaxation.

This would need some discussion as to what open type would bring into scope for DUs and records, just the cases/labels? Or potentially less desirable overall - but more consistent with current open type semantics - also their static members?

The existing way of approaching this problem in F# is:

Today you choose to accept the symbol pollution or you apply RQA to forever disallow anybody bringing these symbols into scope again. Neither are 'always' the best way to go.

Pros and Cons

The advantages of making this adjustment to F# are:

Additional, easily auditable, flexibility when using types annotated with RQA.
Increased orthogonality for open type. I tried using it on a DU somewhat expecting/hoping it would already work today.

The disadvantages of making this adjustment to F# are:

Increased complexity if we end up with inconsistent semantics for open type, pending discussion.
Some people may want to prevent even open type from bringing RQA'ed symbols into scope, please add a comment if so.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): S

Affidavit (please submit!)

Please tick this by placing a cross in the box:

[x] This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
[x] I have searched both open and closed suggestions on this site and believe this is not a duplicate
[x] This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.

Please tick all that apply:

[x] This is not a breaking change to the F# language design
[x] I or my company would be willing to help implement and/or test this

For Readers

If you would like to see this issue implemented, please click the :+1: emoji on this issue. These counts are used to generally order the suggestions by engagement.

cartermp commented 2 years ago

Or potentially less desirable overall - but more consistent with current open type semantics - also their static members?

If we were to do this then I think we'd bring the labels into scope, like today without RQA.

I think this is probable also in the XS category, since this was explicitly designed for and would probably just involve removing an if check in a few places, like so: https://github.com/dotnet/fsharp/blob/main/src/fsharp/NameResolution.fs#L1200-L1224

cartermp commented 2 years ago

I think I'm in favor of this. I guess it's just a question of which should "win", but I believe it makes more sense to let open type win because it's an opt-in mechanism and so it doesn't really violate the intent of the attribute.

LyndonGingerich commented 2 years ago

The precision control would be very handy, but it may not offset the added conceptual complexity. It would seemingly make most sense either to have RQA by default and be able to turn it off (for example, with open type) or to have RQA disabled by default with the ability to enable it (for example, with the tag, as in the current implementation). Having RQA disabled by default, with the option of adding the attribute, and then later the choice of opening the type to disable it again, seems overcomplicated.

realparadyne commented 2 years ago

If it were the same person making those choices all within the same project, it might be overcomplicated. But if a library author makes the RQA choice because it makes sense for 'most users' and then someone else (with no control over that library) could really benefit from overriding it for a special case then it seems straightforward instead to have 'open RQAType' as a way to force it despite the RQA.

cartermp commented 2 years ago

@realparadyne Yes, this is largely why I'm in favor of this change. And TBH I don't think this is terribly complicated either. I can think of many ways you can utilize existing, single features that are much more complicated than this.

auduchinok commented 2 years ago

If approved, let's make this behaviour different from open type where static members are imported, to reduce complexity? It could be something like this:

[<RequireQualifiedAccess>]
type Record =
    { Field: int }

    static member Prop = 1

open Record      // imports `Field`
open type Record // imports `Prop`

cartermp commented 2 years ago

@auduchinok I think that would be even more confusing, since open type already exposes DU cases without RQA

//[<RequireQualifiedAccess>]
type DU =
    | ACase of int

    static member Prop = 1

open type DU // imports `Prop`

ACase |> ignore
Prop |> ignore

Requiring something different from open type when RQA is active would be too difficult to keep track of IMO. I'd much rather just bring this into open type.

l3m commented 2 years ago

I feel it might increase the cognitive load when reading real-world code. Right now, RQA is simple and consistent: you need to use the type name before the DU-case.

Too many exceptions and special cases add up. To me, this seems like something that people would be dissuaded from using in coding conventions.

dsyme commented 2 years ago

Having RQA disabled by default, with the option of adding the attribute, and then later the choice of opening the type to disable it again, seems overcomplicated.

I agree with this. RQA means what it says, opting in to it then suppressing it seems too layered.

I will close this as a result

fsharp / fslang-suggestions