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

Access controls for discriminated unions #810

Closed
5 tasks done
voronoipotato opened this issue Nov 26, 2019 · 27 comments
Closed
5 tasks done

Access controls for discriminated unions #810

voronoipotato opened this issue Nov 26, 2019 · 27 comments

Comments

@voronoipotato
Copy link

voronoipotato commented Nov 26, 2019

I propose a way to make the DU constructor private without making the cases private (which prevents matching). This would allow us to make a tryCreate function to create the DU. Constraining types beyond primitives is often useful, and is the heart of why properties are so popular in OO. I don't think it's realistic (or in scope) to have refinement types in F#, so it would be nice to have one way to constrain types of DU. Even though technically everything happens in run time, because the constructor is scoped to the module it's meaningfully possible to know if the constructor is always used correctly. This should work for many case DU as well as single case DU.

type NegativeNumber =
    | NegativeNumber of int
    private new()
//not sure if a module with the same name as the type 
//has access to private, but if so a member could be used here instead
module NegativeNumber =
    let tryCreate i = if i < 0 then Some (NegativeNumber i) else None

We have no direct way to constrain discriminated unions beyond primitives. This is a problem when you want to ensure some fact is invariant in runtime. If you try to make the cases private, you can construct them with an associated "tryCreate" function but you can't match on them. This creates a scenario where you must use ActivePatterns for every DU that you want to be more constrained than an entire primitive and someone might fail to do so.

Pros and Cons

The advantages are the ability to talk about the domain in clear terms, and to transfer over some of the value that properties provide to OO in an immutable and idiomatic F# context.

The disadvantages is that you technically can accomplish this in active patterns but I would argue that it's a good deal more boilerplate.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): S
(I'm a poor estimator but that's my best guess)
Related suggestions:

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • 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:

  • This is not a breaking change to the F# language design
  • I or my company would be willing to help implement and/or test this
@cartermp
Copy link
Member

An alternate title (not that this should have the title changed), would be "enforced usage of tryCreate functions". I think the idea that we want to disallow construction but enforce pattern matching is in line with idiomatic F# programming so this seems reasonable to me.

@NinoFloris
Copy link

NinoFloris commented Nov 26, 2019

Today the pattern usually looks like the following fssnip http://fssnip.net/ma, but the boilerplate active pattern really shouldn't be necessarry. Right now F# — like haskell — strikes the wrong balance by making you choose for a boilerplate trick (many people wouldn't know that this is possible) or invalid states while it should be very simple and idiomatic.

fssnip

module Email =
    type EmailAddress =
        private
        | ValidEmail of string
        | InvalidEmail of string
    
    let ofString = function
        | "validEmail" -> ValidEmail "validEmail"
        | invalid -> InvalidEmail invalid 
    
    let (|ValidEmail|InvalidEmail|) = function
        | ValidEmail email -> ValidEmail email
        | InvalidEmail email -> InvalidEmail email

open Email

let invalid = Email.ofString "invalid"
let valid = Email.ofString "validEmail"

match invalid with
| InvalidEmail invalid -> printfn "invalid was InvalidEmail %s" invalid
| ValidEmail valid -> printfn "invalid was ValidEmail %s" valid

match valid with
| InvalidEmail invalid -> printfn "valid was InvalidEmail %s" invalid
| ValidEmail valid -> printfn "valid was ValidEmail %s" valid

@charlesroddie
Copy link

charlesroddie commented Nov 26, 2019

This proposal has to do with privacy modifiers on DU constructors. Use of this to enforce constraints is only one application. Regrding the implementation, DUs have multiple constructors so making all cases private is too stark. I therefore propose:

Privacy modifers on DU case constructors

type Shape =
    | Point of p:Point
    | private PCircle of p:Point * r:float * filled: bool
    static member Circle(p:Point,r:float,?filled:bool) =
        PCircle(p, r, defaultArg filled false)

The private modifier will hide the constructor of the PCircle case but allow matching | PCircle _ -> .

This is useful for creating friendly APIs, as the best API for describing the data does not always match the best API for consuming code to use when constructing objects.

@dsyme
Copy link
Collaborator

dsyme commented Nov 26, 2019

FWIW I considered this for F# 1.x and actually started down this path (including private entries in records) but there were a heap of bugs and technical problems - for example, you can't check completeness of matching from places that don't have access to all the union cases

Likewise you could imagine features where you trim off a capabilities off union, record and interfaces

  • restricting the ability to construct a record value
  • restricting the ability to access a particular field of a record value
  • restricting the ability to construct a particular case union
  • restricting the ability to decompose/match on a union
  • restricting the ability to extend an interface
  • restricting the ability to implement an interface

etc.

In the end we decided on the guiding principle that records and unions should focus on "simple and symmetric" data - what you put in is what you get out - and we'd use other mechanisms like signature files, active patterns, object programming and so on to hide and encapsulate.

The only thing we kept was allowing "fully private" representations for union and records.

@cartermp
Copy link
Member

@dsyme when you say "this" are you referring to the suggestion or @charlesroddie's counter-suggestion?

@voronoipotato
Copy link
Author

voronoipotato commented Nov 26, 2019

If it is about mine, I would view syntax sugar to transform into what @NinoFloris suggested as sufficient. The goal of this issue for me is to be able to constrain my input to a particular shape in such a way that people can't accidentally create a DU that is invalid, while also not making it so boilerplate heavy that it's unreasonable to expect people to implement. The value in "restricting the ability to construct" is entirely to serve the goal of constraining my space of valid DU.

Right now I can't specify a full active pattern as a type to be input into a function and so I lose information about what the actual type is. When we say something is type X = A of int | B of int it matters whether it's A or B, even though underneath it all is just an int. In a similar way, when I have a function that gets broken down via an active pattern, if I don't store the decision of the pattern, I'm losing my types.

let (|Positive|Negative|) i = if i >= 0 then Positive i else Negative i
//it's cool that I can match on positive and negative now 
//but I can't represent these in my type signature which means any function that consumes
//or composes with this function loses information about the domain.
let times (x: nothing_to_put_here)  = 
    match x with
    | Positive i -> i * 3  //composition breaks down unless the following function also uses the same match, which I can't meaningfully depend on my consumer knowing about this.
    | Negative i -> i * 5 

Brainstorming

some ideas to consider that might be garbage

If I could define the constructor and shadow the old one, that would probably also also serve my goal.

//this looks a little janky and probably breaks down if there are more than one type.
type X = A of int  | B of int
   new i = if i > 7 then A i else B i

other brainstorming idea that may be too hot for F#

// this probably breaks down if A and B are different types, but the general idea is here
type X =  (|A of int|B of int|) i -> if i > 7 then A i else B i

@dsyme
Copy link
Collaborator

dsyme commented Nov 26, 2019

@dsyme when you say "this" are you referring to the suggestion or @charlesroddie's counter-suggestion?

Yes

@abelbraaksma
Copy link

@dsyme when you say "this" are you referring to the suggestion or @charlesroddie's counter-suggestion

Yes

"do you want coffee or tea?"

Yes ;)

@dsyme
Copy link
Collaborator

dsyme commented Nov 26, 2019

"do you want coffee or tea?"

:) I'm referring to what @charlesroddie's drinking

@voronoipotato
Copy link
Author

presumably he drinks 🍵 because I drink ☕️

@davidglassborow
Copy link

I know it's not ideal, but I just rebind the constructor straight after the type definition, never had any problems (although the name will not have the try in it obviously)

type NegativeNumber =
    | NegativeNumber of int

let NegativeNumber i = if i < 0 then Some (NegativeNumber i) else None

@TIHan
Copy link

TIHan commented Dec 10, 2019

There have been times where I wanted this feature, but in the end, I would just create an object and do my encapsulation.

I don't have a strong opinion. I like the symmetry that we have now, but because you can use active patterns, from @NinoFloris 's example, to almost achieve private constructors, I'd say we should investigate more.

I will caution that adding such features does make using these types more complicated. Right now, I think of DUs and records as data that is either all public or private. With this kind of change, there would be added complexity where only part of a DU is disallowed for construction.

@charlesroddie
Copy link

charlesroddie commented Dec 10, 2019

you can use active patterns, from @NinoFloris 's example, to almost achieve private constructors

I don't think active patterns are relevant. If you take @NinoFloris 's example and remove the active patterns it still works:

type EmailAddress =
    private
    | ValidEmail of string
    | InvalidEmail of string
    static member ofString = function
        | "validEmail" -> ValidEmail "validEmail"
        | invalid -> InvalidEmail invalid 

let invalid = EmailAddress.ofString "invalid"

match invalid with
| InvalidEmail invalid -> printfn "invalid was InvalidEmail %s" invalid
| ValidEmail valid -> printfn "invalid was ValidEmail %s" valid

@dsyme for example, you can't check completeness of matching from places that don't have access to all the union cases

Since making the all DU case constructors private does not interfere with pattern matching, I am surprised that making just some of them private would interfere.

@voronoipotato If you try to make the cases private, you can construct them with an associated "tryCreate" function but you can't match on them.

That's not true. This code works OK:

type NegativeNumber =
    private
    | TheOnlyCaseWhichByTheWayIndicatesThisShouldBeARecordOrClassNotADU of int
    static member tryCreate i = if i < 0 then Some (TheOnlyCaseWhichByTheWayIndicatesThisShouldBeARecordOrClassNotADU i) else None
let x = NegativeNumber.tryCreate -1
let () =
    match NegativeNumber.tryCreate -1 with
    | Some(TheOnlyCaseWhichByTheWayIndicatesThisShouldBeARecordOrClassNotADU i) -> ()
    | _ -> ()

@pblasucci
Copy link

pblasucci commented Dec 10, 2019

@charlesroddie The example you've posted doesn't behave the way you expect if the DU is inside a module. In that case, you need the Active Patterns (a la @NinoFloris example). This is because private means "private to the enclosing scope", which (for a Union) is the nearest module or namespace.

This also explains why it seems like:

making the all DU case constructors private does not interfere with pattern matching

It doesn't -- within the scope where they are still visible.

All this having been said, I'm kinda fine with the way things work today.

@abelbraaksma
Copy link

abelbraaksma commented Dec 10, 2019

TheOnlyCaseWhichByTheWayIndicatesThisShouldBeARecordOrClassNotADU

@charlesroddie, while possibly OT, I disagree, single case unions are quite common, and in my experience easier to use than single field records. There may be times where the type of data drives the choice (I.e. some single global setting would go into a record, while a type descriptor would go into a DU), by en large either case can be beneficial, with possibly single case DU at an advantage due to more readable code and easier construction/deconstruction.

Background: https://fsharpforfunandprofit.com/posts/designing-with-types-single-case-dus/

@davidglassborow
Copy link

davidglassborow commented Dec 10, 2019

I know it's not ideal, but I just rebind the constructor straight after the type definition, never had any problems (although the name will not have the try in it obviously)

type NegativeNumber =
    | NegativeNumber of int

let NegativeNumber i = if i < 0 then Some (NegativeNumber i) else None

@abelbraaksma has reminded me of the good point that this will prevent calling any other methods defined on the type

@abelbraaksma
Copy link

abelbraaksma commented Dec 10, 2019

@davidglassborow, you meant here, right? https://gist.github.com/swlaschin/54cfff886669ccab895a.

That Gist from the creator of F# For Fun and Profit is a good source for this discussion, as it shows a bunch of ways to deal with the problem the OP describes. Each has its merits and its drawbacks.

The way I see it, I think it would be awesome if private on a DU case were to mean that you cannot construct, but you can still destruct a member. I think limiting this issue to only that (as is in the OP) possibly needs to be an F#-only thing that's enforced by the compiler, or the compiler can just emit the active patterns. But apart from (potentially hard) implementation issues, I think it will serve the majority of issues and cases discussed here and on other places.

Also, here's another way, which closely resembles the pattern with active recognizers from @NinoFloris, but uses extra indirection to give the user the feel of accessing the actual type: https://stackoverflow.com/questions/54428850/whats-the-functional-design-pattern-for-a-discriminated-union-with-protected-cr.

@charlesroddie
Copy link

charlesroddie commented Dec 11, 2019

@charlesroddie The example you've posted doesn't behave the way you expect if the DU is inside a module. This is because private means "private to the enclosing scope", which (for a Union) is the nearest module or namespace.

I see. I was assuming scoping was the same as for private contructors of classes (which are scoped to the class rather than the containing class). Is this behavior ever good and/or part of the language spec? Otherwise it seems like a bug in DUs and record types prevents not only construction but also extraction of data.

module OuterDU =
    module InnerDU =
        type NegativeNumber =
            private
            | TheOnlyCaseSoShouldBeARecordOrClass of int
            static member tryCreate i =
                if i < 0 then Some (TheOnlyCaseSoShouldBeARecordOrClass i) else None
    let i =
        match InnerDU.NegativeNumber.tryCreate -1 with
        | Some(InnerDU.TheOnlyCaseSoShouldBeARecordOrClass x) -> x // The union cases or fields... are not accessible from this code location
        | None -> 0

module OuterClass = // OK
    module InnerClass =
        type NegativeNumber private (i:int) =
            member t.Value = i
            static member tryCreate i = if i < 0 then Some (NegativeNumber i) else None
    let i =
        match InnerClass.NegativeNumber.tryCreate -1 with Some(n) -> n.Value | None -> 0

module OuterRecord =
    module InnerRecord =
        type NegativeNumber = private { Value: int }
        [<RequireQualifiedAccess>]
        module NegativeNumber =
            let tryCreate i = if i < 0 then Some {Value = i} else None
    let i =
        match InnerRecord.NegativeNumber.tryCreate -1 with
        | Some(n) -> n.Value // The union cases or fields... are not accessible from this code location
        | None -> 0

@pblasucci
Copy link

@charlesroddie the behaviour is definitely "by design" (see section 10.5 of the language specification, https://fsharp.org/specs/language-spec/4.1/FSharpSpec-4.1-latest.pdf). As to whether or not it's "good, I'll defer to the judgment of others (i.e. Don)... but I will say that, in the past 13 years, it's not posed a significant hurdle/burden for me in purely F# code bases. It used to cause mild grief when consuming F# from C# -- but being able to open static classes (modules) sorted that.

@abelbraaksma
Copy link

abelbraaksma commented Dec 11, 2019

Otherwise it seems like a bug in DUs and record types prevents not only construction but also extraction of data.

@charlesroddie Exactly, and that's the key point of this language suggestion. To allow extraction/destruction, but disallow construction.

It would make creating abstract data types a lot easier: you could then create DU's that are only behavioral (you define the operations that are allowed on them), as opposed to being just data containers (currently, you cannot control construction out of the box). In cases where the input needs to be validated beyond the limits of the contained type, I believe this to be a huge benefit to F#, at a relatively minor cost.

@7sharp9
Copy link
Member

7sharp9 commented Dec 11, 2019

Keeping things simple is definitely my preference. Ive never needed a private case in a variant type.

@voronoipotato
Copy link
Author

My proposal is not to make individual parts private, to discuss @charlesroddie 's suggestion, feel free to discuss more on their issue. This will both help Charles maintain the defense of their feature in a space they can better respond to and allow the more broad concept of constrained DU's to be discussed here. I feel like the discussion has gotten a little derailed from the original proposition.

@charlesroddie
Copy link

@voronoipotato we appreciate the respect shown to our person. But we believe that the difference between your suggestion and ours is merely that you propose making constructors private all at once, while we would specify them separately.

@voronoipotato
Copy link
Author

voronoipotato commented Feb 20, 2020

@charlesroddie That's correct, and I think the discussion has shown that the difference however small is not trivial. Given that some discussion has gone against your suggestion, and clearly you still value your suggestion and would like to defend it, I thought it would be most productive to move discussion about your suggestion, to your suggestion. This allows you to defend your idea, and it allows me to discuss the benefits or costs of my idea without getting bogged down in a issue that I'm not particularly interested in.

For example because my intent is to constrain a DU and not simply to make it private, it's possible that a different approach might attain my goal that may not even involve making fields private. Hopefully you can appreciate how yours is a potentially useful language implementation discussion, whereas mine is a discussion regarding how we can defend our code such that the names we give our types meaningfully respect the constraints we expect of those names. So to be clear it's a little more nuanced, my suggestion is an open ended attempt to solve a problem, yours is a design discussion about F# types. I think your issue is interesting and potentially valuable, but it is not the same as my issue.

@charlesroddie
Copy link

@voronoipotato at the moment my suggestion is in my first post in this thread. I haven't posted any related suggestion and the one you link isn't related as far as I can see. In this thread, @davidglassborow has an approach involving shadowing, but apart from that the only proposals so far have involved private constructors.

@voronoipotato
Copy link
Author

@charlesroddie oh huh, thanks. Not sure why I thought they were the same. Perhaps I was reading them a little too quickly. It does seem like it may warrant it's own language suggestion since the objections to it seem to be very technical/philosophical, and it may be good for that discussion to play out such that a clean answer or resolution is given. Making individual cases private is not a requirement to attain constraints for discriminated unions, so talking about it here kind of just clouds the discussion.

@dsyme dsyme changed the title Constrained Discriminated Unions Access controls for discriminated unions Apr 13, 2023
@dsyme
Copy link
Collaborator

dsyme commented Apr 13, 2023

I'm going to close this in favour of #852 - if we were to do anything it would be the more general thing

@dsyme dsyme closed this as completed Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants