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

Allow read access to records, but private construction #1122

Closed
5 tasks done
brianberns opened this issue Mar 3, 2022 · 21 comments
Closed
5 tasks done

Allow read access to records, but private construction #1122

brianberns opened this issue Mar 3, 2022 · 21 comments

Comments

@brianberns
Copy link

brianberns commented Mar 3, 2022

In order to support input validation, I think it would be useful to declare a type whose fields can be accessed, but which cannot be directly created by users.

Problem

We often need to validate input when constructing a value of a particular type. For example, let's say we want a type that represents positive floating-point numbers:

type PosFloat =
    {
        Value : float
    }

module PosFloat =

    let add x y =
        {
            Value = x.Value + y.Value
        }

(Note that this could be represented by either a record or a discriminated union. I've chosen to use a record type here, but the same discussion applies equally to DUs.)

We could then use this type as follows:

let x = { Value = 1.0 }
let y = { Value = 2.0 }
PosFloat.add x y |> printfn "%A"   // { Value = 3.0 }

We want to make invalid states unrepresentable, so we need to prevent code like this:

let bad = { Value = -1.0 }

To do this, we make the fields private and provide a create function instead:

type PosFloat =
    private {
        Value : float
    }

module PosFloat =

    let create value =
        if value <= 0.0 then failwith "Must be positive"
        {
            Value = value
        }

    let add x y =
        create (x.Value + y.Value)

let x = PosFloat.create 1.0
let y = PosFloat.create 2.0
PosFloat.add x y |> printfn "%A"   // { Value = 3.0 }

let bad = PosFloat.create -1.0   // System.Exception: Must be positive

But now users can't access the internal value themselves:

module MyModule =
    let mult x y =
        PosFloat.create (x.Value * y.Value)   // The union cases or fields of the type 'PosFloat' are not accessible from this code location

How can we fix this?

Workaround

One approach is to provide a separate member for accessing the internal value:

type PosFloat =
    private {
        _Value : float
    }
    with member this.Value = this._Value

module PosFloat =

    let create value =
        if value <= 0.0 then failwith "Must be positive"
        {
            _Value = value
        }

    let add x y =
        create  (x._Value + y._Value)

But this has some major disadvantages:

  • We had to rename the private field to avoid conflicting with the name of the public member.
  • Users lose automatic type inference, so have to explicitly annotate uses of the type, which is potentially a lot of extra typing:
module MyModule =

    let badMult x y =
        PosFloat.create (x.Value * y.Value)   // ERROR: Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object.

    let mult (x : PosFloat) (y : PosFloat) =
        PosFloat.create (x.Value * y.Value)

Proposal

I think it would be useful to declare a type whose fields can be accessed, but which cannot be directly created by users. Something like this:

type PosFloat =
    protected {
        Value : float
    }

I've used protected here, but feel free to substitute another keyword of your choice. Users would then be able to access the Value field but not able to use it to construct a value of the type:

let x = { Value = 1.0 }   // prohibited
printfn "%A" y.Value      // allowed

Again, note that the same proposal applies to discriminated keywords:

type PosFloat = protected Value of float

Pros and Cons

The advantages of making this adjustment to F# are developers have a low-friction way to validate input when constructing F# records and discriminated unions.

The disadvantages of making this adjustment to F# are it requires a keyword, which is another thing that developers have to remember.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): Small. It's a single keyword that's easily parsed and enforced.

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

For Readers

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

@Tarmil
Copy link

Tarmil commented Mar 4, 2022

Related: #810

@Frassle
Copy link

Frassle commented Mar 4, 2022

protected already has a meaning in the context of the CLR so I'd think it quite confusing to use that same word for this different concept.

@charlesroddie
Copy link

Users lose automatic type inference, so have to explicitly annotate uses of the type, which is potentially a lot of extra typing:

The fact that let add x y = ... x.Value + y.Value compiles is a bug in type inference, since possessing a member Value does not imply that something is a PosFloat. It's a guess not an logical inference. So the fact that a type annotation is required here is a benefit not a disadvantage.

I would recommend

[<Struct>]
type PosFloat private (value:float) =
    member _.Value = value
    static member Create(value:float) =
        if value <= 0. then failwith "Mut be positive" else PosFloat(value)

@brianberns
Copy link
Author

protected already has a meaning in the context of the CLR so I'd think it quite confusing to use that same word for this different concept.

I'm not attached to the particular keyword. I chose protected because it's already reserved by F#, but not used anywhere. We could instead use another existing keyword, such as get, or invent a new one, such as readonly.

@brianberns
Copy link
Author

The fact that let add x y = ... x.Value + y.Value compiles is a bug in type inference, since possessing a member Value does not imply that something is a PosFloat.

That's the expected behavior for records and discriminated unions, not a bug. I think this behavior is crucial to writing clean F# code.

I would recommend ... [<Struct>] ...

Having to fall back to low-level code like this is exactly what I'm proposing we should avoid for such a basic use case.

@acowlikeobject
Copy link

At our (my) company, we recently moved our "domain-heavy" code from Python to F#. Overall, we've been very happy with our decision.

But, for a language whose selling points are correctness and conciseness, the absence of one obvious, simple way of creating constrained types is baffling. We've repeatedly felt the need for "destruct but don't construct" access.

Scott Wlaschin shows a few ways to do this. Everything there is a compromise, and has felt like a kludgy trick to newcomers; you give up the ergonomics of records and discriminated unions (a huge part of why we enjoy F#), or haven't really forced use of your validating constructor, or write tediously duplicative code in active patterns (which are limited to seven cases; not enough when making a large domain-specific DU "read only").

We'd dearly love to see something like this.

@brianberns
Copy link
Author

Thank you for those links. Using active patterns to access the fields is an interesting idea, but still seems like a clunky workaround, rather than a long-term solution. Scott Wlaschin mentions that signature files can be used to solve this, but I don't see how that would work either. There really doesn't seem to be any good way to do this yet in F# with algebraic data types (i.e. records and DU's).

@cartermp
Copy link
Member

cartermp commented Mar 9, 2022

Just a note:

Small. It's a single keyword that's easily parsed and enforced.

This is probably not accurate. This amounts to a new kind of access modifier, which is not an easy thing to add and carefully consider the design for. It's not just this, but it's carefully considering all the domain modeling scenarios, understanding which feels appropriate to bring into the core language or not, good diagnostics, and ensuring tooling behaves correctly. Parsing a keyword is just a very small part of this.

Personally, I don't see this as being particularly valuable. This kind of flexibility around construction and visibility is exactly what objects provide, and @charlesroddie's suggestion is what I would do if I needed "read but not construct" permissions on a type. Since this is already supported by objects and structs in F# today, I don't think it's terribly important to bring into DUs and records.

@brianberns
Copy link
Author

brianberns commented Mar 9, 2022

@charlesroddie's suggestion is what I would do if I needed "read but not construct" permissions on a type. Since this is already supported by objects and structs in F# today, I don't think it's terribly important to bring into DUs and records.

If this preserved type inference based on field names, I would probably accept it. Forcing users of my code to write (x : PosFloat) every time they want to access Value gets old quickly, and seems to defeat one of the major advantages of F#. I guess the bigger issue here is that F# doesn't attempt to include members in its type inference algorithm. (I'm sure that would be an even bigger enhancement request, if it's even possible in theory.)

@cartermp
Copy link
Member

cartermp commented Mar 9, 2022

Forcing users of my code to write (x : PosFloat) every time they want to access Value gets old quickly

There are also two other things to consider:

  • F# code very often tends towards type annotations or parameters to functions or methods over time, especially in a public signature
  • When used in pipelines, there is often little need to add a type annotation

@heronbpv
Copy link

Since the possibility of bringing stronger information-hiding concepts to DUs/Records seems to be too far, how about going in a different direction?
How hard would it be to bring some, or even better all, of the pattern matching capabilities enjoyed by DU and Records to F# objects instead?
I reckon they do lag behind in this area, but by how much?

@SilkyFowl
Copy link

SilkyFowl commented Mar 16, 2022

Personally, this is ideal.
If intervening in the constructor, I want to specify the validation function directly, not by keywords.

type String50 =
    [<Validate(
        fun s ->
            if String.IsNullOrEmpty(str) then
                Error [ "NullOrEmpty" ]
            elif String.length str > 50 then
                Error [ "Over 50" ]
            else
                Ok str)>]
    | String50 of string

In the other direction, what about a type like Result<'t,'error> that can enforce validation?
The idea is to store DUs/Records in a type that provides strong information hiding and operation assistance.
Source code is here.
Regardless of which direction it goes, I would be happy to see enhanced verification capabilities.

/// Unlike `Result<'t , 'error>` cannot be created freely.
type Validated<'dto, 'domain, 'error> =
    private
    | Valid of domain: 'domain
    | Invalid of error: InvalidType<'dto, 'domain, 'error>


/// Manipulate `Validated<'dto, 'domain, 'error>` in this class.
type Domain<'dto, 'domain, 'error>
    (
        fromDto: 'dto -> 'domain,
        toDto: 'domain -> 'dto,
        validate: 'dto -> Result<'dto, 'error list>
    ) =
    // The following is omitted

Domain<'dto, 'domain, 'error> provides value generation, manipulation assistance, etc.

module Domain =
    open DomainHelper
    open System

    // Omission

    /// Nested cases
    type Person =
        private
            { First: String50
              Last: String50
              Birthdate: Birthdate }

    type PersonDto =
        { First: ValidatedString50
          Last: ValidatedString50
          Birthdate: ValidatedBirthdate }

    let person =
        Domain<PersonDto, Person, string>(
            (fun p ->
                { First = string50.Value p.First
                  Last = string50.Value p.Last
                  Birthdate = birthdate.Value p.Birthdate }),
            (fun p ->
                { First = string50.ofDomain p.First
                  Last = string50.ofDomain p.Last
                  Birthdate = birthdate.ofDomain p.Birthdate }),
            (fun p ->
                match p.First, p.Last, p.Birthdate with
                | Valid _, Valid _, Valid _ -> Ok p
                | (BoxInvalid v), (BoxInvalid v'), (BoxInvalid v'') ->
                    (v @ v' @ v'')
                    |> List.map (fun e -> $"%A{e}")
                    |> Error)
        )

It would be nice to have a function to assist in creating functions for domain types.

open Domain
open System

let fooBar =
    person.Create
        { First = string50.Create "Foo"
          Last = string50.Create "Bar"
          Birthdate = birthdate.Create(DateTime(1990, 3, 4)) }

let hogeFuga =
    person.Create
        { First = string50.Create "Hoge"
          Last = string50.Create "Fuga"
          Birthdate = birthdate.Create(DateTime(2999, 2, 8)) }

module String50 =
    let concat = string50.Lift2Dto(fun s1 s2 -> s1 + s2)

module Person =
    let margeName =
        person.Lift2Dto (fun p1 p2 ->
            { p1 with
                First = String50.concat p1.First p2.First
                Last = String50.concat p1.Last p2.Last })

let marged = Person.margeName fooBar hogeFuga

// val marged: DomainHelper.Validated<PersonDto,Person,string> =
//   Valid { First = String50 "FooHoge"
//         Last = String50 "BarFuga"
//         Birthdate = Birthdate 1990/03/04 0:00:00 }

@dsyme
Copy link
Collaborator

dsyme commented Jun 16, 2022

Also related is #516

@dsyme
Copy link
Collaborator

dsyme commented Jun 16, 2022

Also related is

  1. Can record types be revealed with only members in the signature, see Allow record fields to implement signature members #1153
  2. Can record types optionally reveal only the ability to access the record (e.g. pattern matching), and not construct in the signature

Ideally anything in this area would follow the same rules we use for access control. However It's surprisingly hard to find remotely satisfying ways of declaraing this distiction in capabilities:

For the "can I create the record" then this is reasonable:

type PosFloat =
    {
        private new
        Value : float
    }

But what of restricting the ability to access it? Do we allow access controls on the members?

type PosFloat =
    {
        private Value : float
    }

Perhaps this is reasonable - though it's not clear if you could do this:

type PosFloat =
    {
        public new
        private Value : float
    }

That is construct but don't touch.

Anyway I guess private new and internal new are a reasonable kind of feature addition, consistent with signatures and existing privay mechanisms, what do you think?

@dsyme dsyme changed the title Protected fields: Allow read access, but prevent construction to support input validation Allow read access to records, but private construction Jun 16, 2022
@acowlikeobject
Copy link

Enforcing a private constructor on the entire type (but leaving the ability to destruct public) with a private new and internal new would already be incredibly useful. Member-level access control seems more complex and could be addressed later?

@dsyme Could this also be available on discriminated unions? Again, hugely useful. Current workarounds (eg, hiding cases with private and making them available via active patterns) have poor ergonomics.

@dsyme
Copy link
Collaborator

dsyme commented Jun 17, 2022

@acowlikeobject Yes, in theory. What syntax? And would it be per-union-case or not? Per-union-case construction is the capability logicall corresponding to the New* static methods.

type Union =
    | private new A of string 
    | private new B of int 

Or this, doing all at once

type Union =
    private new
    | A of string 
    | B of int 

The former is heading to a more complete state (access controls that correspond to the individual capabilities implied by the generated code for unions and records), the latter is simpler.

@dsyme
Copy link
Collaborator

dsyme commented Jun 17, 2022

Note attributes could also be used to prevent an expensive proliferation of new syntax. However that means accessibility becomes an awkward mix of attributes and keywords. Neither is ideal

@aneumann-cdlx
Copy link

aneumann-cdlx commented Jun 17, 2022

@acowlikeobject Yes, in theory. What syntax? And would it be per-union-case or not? Per-union-case construction is the capability logicall corresponding to the New* static methods.

type Union =
    | private new A of string 
    | private new B of int 

I'm definitely wading into the deeper end of the language pool than I ought to but if you will pardon me jumping in...I like per-union-case construction over per-union construction.

It appears to more closely track common cases when creating constrained types. You can add validation members just like this just like we have in the past of course, but the bonus here is that you no longer have to maintain two types, both of which are just parts of one domain entity in various stages of its lifecycle.

  1. a public creatable unvalidated type (that then must be applied to a certain function to create the next type)
  2. a private create validated type

Stealing some code from the linked example, we get an entire unvalidated->validated constrained type in just the type declaration.

type EmailAddress = 
    | Unvalidated of string
    | private new Validated of string
    static member private isValid (value: string) =
        // Validation code goes here
        true
    static member Validate(address: string) =
        if EmailAddress.isValid address then
            Validated address
        else
            Unvalidated address
    member this.Value =
        match this with 
        | Unvalidated value -> value
        | Validated value -> value

Now any functions that want to take only valid emails can just pattern match on a given EmailAddress to see if the union case is Validated or not. We no longer need a separate (private create) ValidatedEmail type to make the illegal state unrepresentable.

@acowlikeobject
Copy link

@acowlikeobject Yes, in theory. What syntax? And would it be per-union-case or not? Per-union-case construction is the capability logicall corresponding to the New* static methods.

@dsyme I'd vote for simplicity and have private new at the type-level. (If you squint enough, it's analogous to type-level private new for records?)

DUs are fantastic for modeling state machines, where you want to inspect state in several places (public destruction), but control transitions (private construction).

I'm not sure case-level control adds much in those use cases. You'd probably end up marking all or all-but-initial cases as private. On the other hand, it feels pretty natural to have a type-level private new and force use of an event handler to effect state transitions.

But obviously, you can accomplish strictly more with case-level control, so I'd still be thrilled if that's what landed. It just seems to use up the complexity budget for not a lot of additional value?

@aneumann-cdlx I'd say create and value functions work well enough for private DUs with binary Unvalidated and Validated cases. You don't destructure much with those types anyway. If you have an initial Unvalidated case and several validated cases, then it starts to look a lot like the state machine pattern above.

@SilkyFowl
Copy link

SilkyFowl commented Jun 18, 2022

@dsyme I also agree with @acowlikeobject. private new is very exciting, I'd love to see it happen, although I think it would be useful to be able to control the case level of the DU.

@acowlikeobject If this feature is realized, pattern matching will be very easy, so combining it with Result<'t,'error> etc. may be a viable option, as shown below.

type EmailAddress = 
    private new
    | EmailAddress of string
    static member private isValid (value: string) =
        // Validation code goes here
        true

    static member TryCreate(address: string) :Result<EmailAddress,string>=
        if EmailAddress.isValid address then
            Ok (EmailAddress address)
        else
            Error address

    static member TryCreateWith onError (address: string) :Result<EmailAddress,'error>=
        if EmailAddress.isValid address then
            Ok (EmailAddress address)
        else
            Error (onError address)

updated 2022/06/27:
I noticed that if we can new private, the following code may just cause unnecessary complexity.

Personally, I think it would be good to combine it with the Static Interface proposed in the following Issue.

type IValidatable<'dto,'t> =
    abstract static member Validate :('t -> 'validated) -> ('dto -> 'validated) -> 'dto -> 'validated

module Validatable =
    let validate<'Validatable when 'Validatable :> IValidatable<'dto,'t>> onValie onInvalid value =
        'Validatable.Validate onValid onInvalid value

    let create<'Validatable> x =
        validate<'Validatable> id (failwithf "Invalid: %A") x

    let createResult<'Validatable> x =
        validate<'Validatable> Ok Error x

    let createOpt<'Validatable> x =
        validate<'Validatable> Some (fun _ -> None) x

type EmailAddress = 
    private new
    | EmailAddress of string
    static member private isValid (value: string) =
        // Validation code goes here
        true

    interface IValidatable<string,EmailAddress> with
        static member Validate onValid onInvalid address =
            if EmailAddress.isValid address then
                onValid (EmailAddress address)
            else
                onInvalid address

type PosFloat =
    {
        private new
        Value : float
    }
    interface IValidatable<float,PosFloat> with
        static member Validate onValid onInvalid value =
            if value <= 0.0 then
                onValid {Value = value}
            else
                onInvalid value

@dsyme
Copy link
Collaborator

dsyme commented Apr 13, 2023

I'm going to close this in favour of #852, which we can use to track this general issue

@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