fsharp / fslang-suggestions

The place to make suggestions, discuss and vote on F# language and core library features
344 stars 21 forks source link

if let expression #705

Closed vasily-kirichenko closed 9 months ago

vasily-kirichenko commented 6 years ago

https://github.com/MangelMaxime/Fulma/blob/1cbbc35e046007029e4b382d502cd9b301443a69/src/Fulma/Elements/Form/Textarea.fs#L144-L151

if Option.isSome opts.Id then yield Props.Id opts.Id.Value :> IHTMLProp
if Option.isSome opts.Value then yield Props.Value opts.Value.Value :> IHTMLProp
if Option.isSome opts.DefaultValue then yield Props.DefaultValue opts.DefaultValue.Value :> IHTMLProp
if Option.isSome opts.ValueOrDefault then
    yield Props.Ref <| (fun e -> if e |> isNull |> not && !!e?value <> !!opts.ValueOrDefault.Value then e?value <- !!opts.ValueOrDefault.Value) :> IHTMLProp
if Option.isSome opts.Placeholder then yield Props.Placeholder opts.Placeholder.Value :> IHTMLProp
if Option.isSome opts.OnChange then yield DOMAttr.OnChange opts.OnChange.Value :> IHTMLProp
if Option.isSome opts.Ref then yield Prop.Ref opts.Ref.Value :> IHTMLProp

I propose we add if let <pattern> then <expression> expression, so the above code would be rewritten as following:

if let (Some id) = opts.Id then yield Props.Id id :> IHTMLProp
if let (Some value) = opts.Value then yield Props.Value value :> IHTMLProp
if let (Some defValue) = opts.DefaultValue then yield Props.DefaultValue defValue :> IHTMLProp
if let (Some value) = opts.ValueOrDefault then
    yield Props.Ref <| (fun e -> if e |> isNull |> not && !!e?value <> !!value then e?value <- !!value) :> IHTMLProp
if let (Some p) = opts.Placeholder then yield Props.Placeholder p :> IHTMLProp
if let (Some onChange) = opts.OnChange then yield DOMAttr.OnChange onChange :> IHTMLProp
if let (Some r) = opts.Ref then yield Prop.Ref r :> IHTMLProp

It's inspired by the same Rust's feature, see https://doc.rust-lang.org/book/second-edition/ch06-03-if-let.html, and the same Swift's feature, see https://medium.com/@abhimuralidharan/if-let-if-var-guard-let-and-defer-statements-in-swift-4f87fe857eb6

The existing way of approaching this problem in F# is (see the first snippet, or match ... with Some x -> ... | None -> (), which is even more verbose, but safer).

Pros and Cons

The advantages of making this adjustment to F# are:

The disadvantages of making this adjustment to F# are:

Extra information

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

Affidavit (please submit!)

Please tick this by placing a cross in the box:

Please tick all that apply:

abelbraaksma commented 6 years ago

An interesting idea, but...

In closest current syntax, I think this:

if let (Some id) = opts.Id then yield Props.Id id :> IHTMLProp

becomes:

let (Some id) = opts.Id in yield Props.Id id :> IHTMLProp

Apart from this raising a warning, isn't the current syntax actually shorter? Assuming you mean that the let if syntax should not raise a warning for the missing cases and if let (Test x) = y is short for function Test x -> y | _ -> (), right?

And in your examples, there's no if .. then .. else or if .. then .. elif .. then .. else. Should those be supported? If not, the new expression could only ever return ().

If I understand you correctly, the syntax-gain isn't in less typing, but in removing warnings by creating a short-hand for a typical case of testing for a single DU-case and ignoring anything else.

abelbraaksma commented 6 years ago

Oh wait, what I imply as "current" syntax, is a binding expression, and you mean to expand the if expression to create inner bindings similar to in, but with it returning something. Then still: what would be returned if the if test fails?

FunctionalFirst commented 6 years ago

if let is basically a single-case match without a required else or wildcard case. That means let behaves very differently when following if than it does elsewhere. But I suppose that difference is the perceived benefit of this suggestion.

I'm conflicted about whether this feels F#-ish or not.

FunctionalFirst commented 6 years ago

Then still: what would be returned if the if test fails?

I would assume unit, just like if without else returns today.

abelbraaksma commented 6 years ago

if let is basically a single-case match without a required else or wildcard case.

@FunctionalFirst: But I assume then that the purpose is that you get the wildcard case for free, that is, it wouldn't throw an exception if the if-case isn't true. A single-case match without wildcard would throw, unless it's a single case DU, or is this syntax meant for single-case DU only? The examples speak otherwise (and there's already let-binding syntax for that anyway).

And, while we're at it, if this is accepted, it should probably support the whole range of patterns, right?

cartermp commented 6 years ago

An important distinction in the Rust docs:

Using if let means less typing, less indentation, and less boilerplate code. However, you lose the exhaustive checking that match enforces. Choosing between match and if let depends on what you’re doing in your particular situation and whether gaining conciseness is an appropriate trade-off for losing exhaustive checking.

Which makes sense, since the else is equivalent to a wildcard case.

As in Rust or Swift, the value bound with if let is only in scope for the if block.

Another interesting question would be extending this to elif:

type Foo =
    | Bar of int
    | Baz of int
    | Qux of int
    | Uhhh of string
    | Something of float

if let (Bar x) then
    printfn "Bar is %d" x
elif let (Baz x) then
    printfn "Baz is %d" x
else
    printfn "I don't care!"

The Swift use of if let is also interesting. In Swift, you can end up creating a pyramid of doom with if let. This is remediated by using guard let, which is a sort of inverse that you use to enforce invariants up front before processing data. This is done in the face of lots of optional data and only wanting to process something if all the data exists. I don't think if let would imply guard let in F#, but it's worth noting that the introduction of if let does introduce another pathway towards writing pyramids of doom.

vasily-kirichenko commented 6 years ago

To clarify, this feature's intention is simplifying two-branches match expression, when the second pattern returns unit, so the following code

[ for i in 1..10 do
    match foo i with
    | Some x -> yield bar x
    | None -> () ]

would be written as

[ for i in 1..10 do
    if let (Some x) = i then yield bar x ]

So, if let ... then ... expression always returns unit and would look like an imperative feature, but it would work really nicely inside computation expression, as shown above.

It does not yield warning on incomplete match because it's the whole purpose of the feature: it evaluates the expression after then if, and only if a value satisfies the pattern.

About if ... elif ... elif... else ..., I strongly disagree it should be implemented because it would not add anything beyond normal match expression and would read much harder.

matthid commented 6 years ago

If we implement this I'd expect else, elif and elif let to work (for consistency in the language)

abelbraaksma commented 6 years ago

If indeed one of the main use cases is to use this in CEs, I would strongly suggest to include if let! into the proposal.

vasily-kirichenko commented 6 years ago

@matthid I still disagree :) It provides a (definitely worse) alternative way to write matches, this should not be done.

realvictorprm commented 6 years ago

so the main purpose of this suggestion is to get rid of the | _ -> unit part of a match expression?

FunctionalFirst commented 6 years ago

Additional use cases would make this more compelling. @vasily-kirichenko's example can already be shortened slightly to if opts.Id.IsSome then yield opts.Id.Value :> IHTMLProp.

Option, like Nullable, has only two cases, and both of those types provide a property to check for the significant case (IsSome/HasValue). You could argue that this pattern should be used for all such types.

In other words, if let seems to solve a problem that's already solved by an established pattern. The benefit of the pattern is that it allows the author of the type to define canonical usage, which encourages consistency among its consumers.

vasily-kirichenko commented 6 years ago

if opts.Id.IsSome then yield opts.Id.Value :> IHTMLProp is not safe.

FunctionalFirst commented 6 years ago

if opts.Id.IsSome then yield opts.Id.Value :> IHTMLProp is not safe.

@vasily-kirichenko How is .IsSome different than Option.isSome?

vasily-kirichenko commented 6 years ago

@FunctionalFirst I mean, you can (and will) easily write

if opts.Id.IsNone then yield opts.Id.Value :> IHTMLProp
vasily-kirichenko commented 6 years ago

And an IDE feature for converting between match and if let could be added :)

untitled

matthid commented 6 years ago

It provides a (definitely worse) alternative way to write matches, this should not be done.

Indeed, but I feel like that is still "Ok" as you get 'punished' by having a bad and noisy syntax, if you missuse it. IMHO that is just enough to favor allowing 'else' and 'elif' and 'elif let'. This makes this a proper expression which is (again IMHO) more in line with F# philosophy.

cartermp commented 6 years ago

The whole feature does sort of fall under the principle of not offering multiple ways to do the same thing, but it's not like this would be the first feature that adds a different way to do things. And there is a precedent in other languages, plus it's in the spirit of F# to make things more pattern-based.

But elif feels like it would go a bit too far in the direction of offering a different way to do things. For example, if you covered all cases of a DU type with if let and elif let, what happens with else? A warning, like adding a discard case after a complete pattern match? But there's certainly an awkwardness with having if let but no ability to do elif let.

matthid commented 6 years ago

@cartermp Yes and also the other way around

if someflag then
elif let ...
else

Why would that not be allowed?

matthid commented 6 years ago

if you covered all cases of a DU type with if let and elif let, what happens with else?

You need to answer that anyway because it can be a single case union or a record. This question only disappears when else and elif is not allowed. But in that scenario it is no longer an expression (which would IMHO change this to a "don't do it"-feature)

0x53A commented 6 years ago

I would definitely prefer it if it were implemented as narrow as possible:

I would also prefer to disallow let if for single case unions, for them you can already use

type T = | T of int
let myT = T 1
let (T i) = myT in printfn "%i" i

Edit: scratch the last part, it would still make sense for partial nested matches:

type TInner = A of int | B of int
type TOuter = | TOuter  of TInner 
let myT = TOuter  (A 1)
if let (TOuter (A i)) = myT then printfn "%i" i
mrange commented 6 years ago

I personally think for symmetry elif should be supported. I would be deeply confused if it wasn't.

Also it allows things like this (which is good or bad depending on the observer)

if let (Some x)  = a then doSomething x
elif let (Some x) = b then doSomething x
else doNothing ()
0x53A commented 6 years ago

What about

if let (Some x) = a || let (Some x) = b then doSomething x else doNothing ()

?

FunctionalFirst commented 6 years ago

I personally think for symmetry elif should be supported. I would be deeply confused if it wasn't.

Also it allows things like this (which is good or bad depending on the observer)

if let (Some x)  = a then doSomething x
elif let (Some x) = b then doSomething x
else doNothing ()

That seems like a wordier way to write this (existing syntax)—

match a, b with
| Some x, _ | _, Some x -> doSomething x
| _ -> doNothing()
cartermp commented 6 years ago

That's my primary concern with this feature. if let in isolation has prior art to look at and makes sense in isolation. But making it more complete leads us right down the path of creating an alternative to match expressions.

Risord commented 6 years ago

I personally think for symmetry elif should be supported. I would be deeply confused if it wasn't. Also it allows things like this (which is good or bad depending on the observer)

if let (Some x)  = a then doSomething x
elif let (Some x) = b then doSomething x
else doNothing ()

That seems like a wordier way to write this (existing syntax)—

match a, b with
| Some x, _ | _, Some x -> doSomething x
| _ -> doNothing()

But if it's just

if let (Some x)  = a then doSomething x
elif let (Some x) = b then doSomething x

Then it's saves the default case... which is ~only thing this feature saves on first-place. So vote for elif let for consistency if this get accepted.

ghost commented 6 years ago

I'm not sure if I like or dislike the feature. Like others have said I think it could feel confusing if elif was not supported.

I'm also not sure this is the right place for code suggestions (I'm new here :P). In this particular case however, wouldn't the clunkyness go away if instead trying to pull every value out of the Option, one were to put everything into Option and remove all the boolean logic?


let f g x = (g x) :> IHTMLProp
let htmlProperties = 
     List.choose id [ yield classes |> Some
                      yield Props.Disabled opts.Disabled :> IHTMLProp |> Some
                      yield Props.ReadOnly opts.IsReadOnly :> IHTMLProp |> Some
                      yield Option.map (f Props.Id) opts.Id
                      yield Option.map (f Props.Value) opts.Value 
                      // And so on.. ]
in textarea htmlProperties children
voronoipotato commented 6 years ago

The proposed syntax is surprising and unexpected, and the value it adds is honestly also unclear to me.

type TInner = A of int | B of int
type TOuter = | TOuter  of TInner 
let myT = TOuter  (A 1)
//currently works great
myT |> (fun (TOuter (A i)) -> printfn "%i" i)
//pointfree also works
(fun (TOuter (A i)) -> printfn "%i" i) myT
//less parentheses also works
match myT with TOuter (A i) -> printfn "%i" i
//this is actually longer than the traditional match because 
//like the fun it has parentheses that can't be omitted due to ambiguity
if let (TOuter (A i)) = myT then printfn "%i" i

Do we really absolutely need an nth way to destructure / match?

you can let bind, you can computation expression let bind, you can use a bind function, you can match, you can function match, you can create a function that does all this in one step.

vasily-kirichenko commented 6 years ago

@voronoipotato have you read the proposal? seen the example from a Fable app? questions left?

voronoipotato commented 6 years ago

I understand a little better having read some of the comments. It was non-obvious to me that not specifying the none case causes an exception. I haven't ever run into "not filled out the none case" as a problem. if let definitely was not obvious to me what should happen. I get that these other languages do it that way, but pulling idioms from other languages IMHO should always be done carefully as it is a good way to alienate both newcomers and people who love the language as it is. What are the odds that someone coming to F# has used rust/swift extensively given that I don't even think they interop nicely. It's VERY tempting for us to be implementing a thing in every syntax of every adjacent language, but in the end it's more valuable to be true to ourselves. We have the haskellers who want to mimic haskell's syntax and the C#'ers who want to mimic C#'s syntax, and the OCaml people, and the rust people etc. If we do that we're just risking creating an environment where people fight over what syntax is good, and each person just locks down on the version they're familiar with.

Additionally what happens when there's more than one case are we going to do "If then elseif" and this leads to my next concern. It's also entirely new syntax in an area that in my opinion is already very cluttered. See fun, function, match, let with destructuring, not to mention are we going to add if let to computation expressions. In terms of approaches there's several ways you can already go, you can create a function, you can match, you can use a function based match, a lambda destructuring. There are several existing approaches the authors of that code could have used to fix the line bloat.

If we're going to make any change (and I'm not confident we should), I would recommend extending match/fun/function to support automatic wildcard matching for "unit", since the return type must always be unit anyway.

// we could modify match so that when the return is unit
// it won't throw an exception for unmatched cases. 
// it would be essentially syntactic sugar for wildcard to unit since it must always be unit.
match t with Some(t) -> yield t
// and lambdas
fun (Some t) -> yield t
//also
fun (Some t) -> printfn "%s" t 
//functions too...
function Some t -> printfn "%s" t

I think using matches or lamdbas is much more explicit that it's an action rather than an assignment and is more in line with how F# currently reads.

vasily-kirichenko commented 6 years ago

Look, I created this suggestion when I saw that Fable code, let me repeat it here:

if Option.isSome opts.Id then yield Props.Id opts.Id.Value :> IHTMLProp
if Option.isSome opts.Value then yield Props.Value opts.Value.Value :> IHTMLProp
if Option.isSome opts.DefaultValue then yield Props.DefaultValue opts.DefaultValue.Value :> IHTMLProp
if Option.isSome opts.ValueOrDefault then
    yield Props.Ref <| (fun e -> if e |> isNull |> not && !!e?value <> !!opts.ValueOrDefault.Value then e?value <- !!opts.ValueOrDefault.Value) :> IHTMLProp
if Option.isSome opts.Placeholder then yield Props.Placeholder opts.Placeholder.Value :> IHTMLProp
if Option.isSome opts.OnChange then yield DOMAttr.OnChange opts.OnChange.Value :> IHTMLProp
if Option.isSome opts.Ref then yield Prop.Ref opts.Ref.Value :> IHTMLProp

What's wrong with it?

You say there are already too many ways to destruct values. Yes, but there is no one suitable for this particular case. Personally, I'd write it as match opts.Id with Some id -> yield Props.Id id :> IHTMLProp | None -> () (or | _ -> ()), but it's just boilerplate-full and it's clear that we do someting imperative (kind of) here anyway: we yield if it's Some, and we do nothing, if it's None. Computation expressions blur the line between pure and side effectfull code though, so if F# does not have CEs, I'd not propose this at all. But inside CEs this seems to works very nicely.

I ask you to write a variant of the following code, which will 1. Safe 2. Reasonably short. You can define helper functions if you need (you cannot define custom CE operations, because the code must work inside list/array/seq expressions).

Thanks!

et1975 commented 6 years ago

Here's another example where it would be useful:

while Option.isSome nextMsg do 
    let msg = nextMsg.Value
Nhowka commented 6 years ago

As using if could hint in having symmetry in supporting elif and else, I propose instead that we use when without needing the let.

when (Some x) = b then doSomething x

That would make it more intuitive that it is something unity. Or even instead of then we could use do:

when (Some id) = opts.Id do yield Props.Id id :> IHTMLProp
ghost commented 5 years ago

@vasily-kirichenko

I ask you to write a variant of the following code, which will 1. Safe 2. Reasonably short. You can define helper functions if you need (you cannot define custom CE operations, because the code must work inside list/array/seq expressions).

What about my suggestion ?

In addition to that I guess you could create a CE like "optionList" and have the wrapping in Some be implicit with yield.

voronoipotato commented 5 years ago

This is my first crack at the problem. I realized that this resembled nested mapping of lists , except with an option and a list. I used choose here to gobble up our None since that's what its for. This lets us treat all our options as the things inside the options and so the code is in my opinion much easier to read. If you want me to write this with yield I'll give it a shot but I'm less experienced with yield so it might take me a little longer.

let first = Some "test"
let second = None
let third = Some "2"
let fourth = Some "3"
let fifth = None
let sixth = Some "hmm"
//Example setup... 

//Heres where you would do your (a b :> IHTMLProp) or whatever
//Use currying to permit any number of arguments, just put your option in the last position
let f (a: string) (b: string) =  (b + a) 

//the syntax of this still allows for inline declarations etc.
//i made the argument order this way to preserve the look and feel of the original code
//also it visually tracks like pipes
//choose is map but gobbles up the nones.
let l =  List.choose (fun (x,g) -> Option.map g x ) [
  first , f "ing"
  second , f "no"
  third , f "yay"
  fourth, f "woah"
  fifth, fun fifth -> fifth + "yes!"
  sixth, f "6" 
  ] 
voronoipotato commented 5 years ago
let first = Some "test"
let second = None
let third = Some "2"
let fourth = Some "3"
let fifth = None
let sixth = Some "hmm"
//Example setup... 

//goofy goober option, make an Option Pipe operator.
//it has two '>' because you're really forcing it in there 😄 
let inline (|>>) x y = Option.map y x
//Heres where you would do your (a b :> IHTMLProp) or whatever
let f (a: string) (b: string) =  (b + a) 
let g = f "got it"
//the syntax of this still allows for inline declarations etc.
//i did it this way to preserve the look and feel of the original code
//choose gobbles up the nones. Map lets us map our function over options
let test = 
  List.choose (id) [
    yield first |>> (f "test")
    yield second |> Option.map (fun _ -> "don't care")
    yield Option.map g third
    yield Option.map g fourth
    ] 
dsyme commented 5 years ago

Looking at the original sample,

match opts.Id with Some id -> yield Props.Id id :> IHTMLProp | _ -> ()

is only a handful of characters longer than this (71 to 63).

if let (Some id) = opts.Id then yield Props.Id id :> IHTMLProp

Is it really worth it for 8 characters?

voronoipotato commented 5 years ago

I'm still proud of my "fat-pipes" operator 😄

Richiban commented 5 years ago

I'm assuming that the only real reason the if statement is even in F# is because (a) it's expected for users coming from other languages and (b) it allows a 'statement' form where the else branch may be omitted.

Otherwise, the two forms seem isomorphic.

let b = true

match b with true -> printfn "Yes" | false -> ()

// vs

if b then printfn "Yes"
7sharp9 commented 5 years ago

I think in the spirit of swift the initial example would look like this:

if let id = opts.Id then yield Props.Id id :> IHTMLProp
if let value = opts.Value then yield Props.Value value :> IHTMLProp
if let defValue = opts.DefaultValue then yield Props.DefaultValue defValue :> IHTMLProp
if let value = opts.ValueOrDefault then
    yield Props.Ref <| (fun e -> if e |> isNull |> not && !!e?value <> !!value then e?value <- !!value) :> IHTMLProp
if let p = opts.Placeholder then yield Props.Placeholder p :> IHTMLProp
if let onChange = opts.OnChange then yield DOMAttr.OnChange onChange :> IHTMLProp
if let r = opts.Ref then yield Prop.Ref r :> IHTMLProp

where option are desugared on application to an if let. If mainly avoids the pyramid of doom that you get with nested option matches that are common in F#.

Ideally you would combine this with option binding on property values which further mitigates the pyramid of doom:

if let subprop = opts?id?subprop
charlesroddie commented 5 years ago

I don't like the if let syntax. It seems to misuse both keywords, where neither means what it usually means.

For options, the example in the OP could be fixed by allowing:

opts.Id |> Option.iter (fun id -> yield Props.Id id)

(currently doesn't work because of yield restrictions) or

for id in opts.Id do yield Props.Id id

(currently doesn't work because you can't iterate over options)

For something more general, we can extend the suggestion to match?, a match expression where each branch must return unit and where complete pattern matching is not necessary, which does nothing if no case is matched.

match? opts.Id with Some id -> yield Props.Id id
abelbraaksma commented 5 years ago

(currently doesn't work because you can't iterate over options)

I like that idea, and a simple implementation of an Option.toSeq would solve that.

match?

Interesting idea. Though it does remove completeness checking (mostly problematic when you add cases to an existing DU), but I guess that's the goal here.

theprash commented 5 years ago

@charlesroddie:

for id in opts.Id do yield Props.Id id

(currently doesn't work because you can't iterate over options)

I really like this! @bruinbrown has just pointed out to me that you could also then write this:

yield! opts.Id |> Option.map Props.Id

Is there some other pitfall of making Option directly implement seq?

vasily-kirichenko commented 5 years ago

It should implement Product, not Seq, like Scala does https://www.scala-lang.org/api/current/scala/Product.html

charlesroddie commented 5 years ago

@cartermp Anything tricky about having Option implement Seq? Is this something we just haven't got round to, or does the null representation of None cause a problem?

cartermp commented 5 years ago

I don't think I'd support having Option implement Seq. It's not a sequence of (potentially infinite) items, it's either something or nothing. It also feels weird to me that for id in ... implicitly pattern matches the value out, if there is one.

At least in terms of solving the initial problem, I think if let does so the cleanest and in a way that has parallels in other programming languages that also support pattern matching and optional types.

theprash commented 5 years ago

@cartermp Then I'll just put here some alternative workarounds that I don't think have yet been fully written out here yet:

[ yield! opts.Id |> Option.toList |> Seq.map (fun x -> Props.Id x :> IHTMLProp) ]
module Option =
    // for want of a better name and implementation
    let toSeqMap f = Option.toList >> Seq.map f

[ yield! opts.Id |> Option.toSeqMap (fun x -> Props.Id x :> IHTMLProp) ]

This isn't much longer than the proposed syntax. And in practice, I'd probably still go for List.choose id as suggested by @ghost (deleted user!) above. I don't feel the need for additional syntax, especially as the proposed syntax is unintuitive to me.

vasily-kirichenko commented 5 years ago

@theprash About intuition, ask any non-F# developer, what yield! means, you'll be surprised :)

charlesroddie commented 5 years ago

@cartermp I don't think I'd support having Option implement Seq.

Since there is some disagreement (which I didn't expect) I created a separate language request for this.

isaacabraham commented 5 years ago

Looking at the original sample, match opts.Id with Some id -> yield Props.Id id :> IHTMLProp | _ -> () is only a handful of characters longer than this (71 to 63). if let (Some id) = opts.Id then yield Props.Id id :> IHTMLProp Is it really worth it for 8 characters?

Leaving aside the shadowing of "id" here..., with the introduction of implicit yields this is now yet more concise:

if let (Some id) = opts.Id then Props.Id id :> IHTMLProp

I'm already seeing cases when, where the LHS type is already unified e.g. strings, simple records, DU cases, I'm creating matches to conditionally yield for two-case DUs etc.:

match x with Some x -> x | None -> ()

when what would probably be more readable and concise would be some way of doing something like:

match x with Some x -> x // no need for None, implicitly returns () (not sure about this!) if let (Some v) = x then v if x = (Some v) then v

Or some other ability to conditionally yield & pattern match. I don't know that I have a good idea for solving this but this feels like something that could be made a little more succinct and readable in the future.