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.9k stars 783 forks source link

`open type` does not bring enum cases into scope in patterns #17630

Open brianrourkeboll opened 2 months ago

brianrourkeboll commented 2 months ago

I meant to open this years ago, but I guess I never did.

Repro steps

type E = A = 1 | B = 2

open type E

let f x =
    match x with
    | A -> () // This binds a new value `A` instead of matching the enum case E.A, which is otherwise in scope unqualified.

Expected behavior

Enum cases brought into scope with an open type declaration (or an AutoOpen on the enum definition) should be in scope in pattern matching as unqualified literals, just like union cases or active pattern cases.

E.g., if you change the enum in the example above to a union type, A is brought into scope in something like this:

module M =
    type E = A | B

open type M.E

let f x =
    match x with
    | A -> ()
    | B -> ()

Actual behavior

Enum cases brought into scope with an open type declaration (or an AutoOpen on the enum definition) are not in scope in pattern matching as unqualified literals.

Known workarounds

N/A.

Related information

.NET 8, 9.

Note

I guess this probably can't be fixed, since there may now be existing code that relies on the current behavior — although it is quite possible that some of that existing code is wrong, because it was written with the understanding that open type would behave the same for enums as it does for unions, etc.

edgarfgp commented 2 months ago

open type should work the same for enum and unions. I propose we add a new warning(Informational) and adjust the current analysis to do the right thing.

abonie commented 1 month ago

Weirdly enough the open type with enums behaves like this only in pattern matches it seems.

Technically a breaking change so we should at least add a warning together with a fix.

edit: actually should add a warning regardless of the fix

auduchinok commented 1 month ago

Enum types are considered to have RequireQualifiedAccess, so I'd say it's a mistake that it's possible to open an enum type at all.

brianrourkeboll commented 1 month ago

Enum types are considered to have RequireQualifiedAccess

They are? I mean, I guess they were before open type, because they "required" qualified access by default (unlike F# unions), and there was no way to open them. But that is true of most uses of open type by definition: qualified access was required before, but open type enables unqualified access.

For what it's worth, using static on an enum in C# does bring the unqualified enum fields into scope both as expressions and patterns.

auduchinok commented 1 month ago

But that is true of most uses of open type by definition: qualified access was required before, but open type enables unqualified access.

IIRC open type was only designed to bring static members like methods and properties in scope. It should not affect things like union case or record fields when a type has RequireQualifiedAccess attribute.

module Module

[<RequireQualifiedAccess>]
type U =
    | A
    | B

    static member P = 1

open type U

P |> ignore

let a = A
Screenshot 2024-09-27 at 17 54 09

And without the attribute:

Screenshot 2024-09-27 at 17 54 17
brianrourkeboll commented 1 month ago

It should not affect things like union case or record fields when the type has RequireQualifiedAccess attribute.

Right. But enums don't have that attribute.

IIRC open type was only designed to bring static members like methods and properties in scope.

It was updated to apply to unions as well (except of course those with RequireQualifiedAccess) (https://github.com/fsharp/fslang-design/discussions/352#discussioncomment-231077):

Resolution: Union cases and record field labels should count as static content when processing open type and thus brought into scope..

@abelbraaksma did question the application to enums here: https://github.com/fsharp/fslang-design/discussions/352#discussioncomment-231082. But it doesn't look like anything was recorded in the RFC itself about enums specifically.

auduchinok commented 1 month ago

Right. But enums don't have that attribute.

They do, as per 8.9 in the language spec:

Each enum type declaration is implicitly annotated with the RequiresQualifiedAccess attribute and does not add the tags of the enumeration to the name environment.

brianrourkeboll commented 1 month ago

They do, as per 8.9 in the language spec:

Oh, interesting.

§ 8.9

Each enum type declaration is implicitly annotated with the RequiresQualifiedAccess attribute

The compiler doesn't actually emit the RequireQualifiedAccess attribute for F#-defined enums, though.

Does that really mean "Each enum type declaration [behaves as though it were] implicitly annotated with the RequiresQualifiedAccess attribute"? And were C#-defined enums meant to be included?

The funny thing is that I would seldom match on F#-defined enums; I would normally just use a union instead. It would normally be a C#-defined enum that I'd want to pattern-match on, anyway, and potentially open using open type. So it really depends how C#-defined enums are meant to be treated.

vzarytovskii commented 1 month ago

The compiler doesn't actually emit the RequireQualifiedAccess attribute for F#-defined enums, though.

Yeah, I think the wording is not accurate. I think it's just implicit behaviour for enums rather than implicitly emitting it