-
-
Notifications
You must be signed in to change notification settings - Fork 3
Description
In the following example and in the current version of the NoPrematureLetComputation
rule, the variable toBeMoved
will be moved to the Just
branch:
-- Before
let
toBeMoved = 1
in
case x of
Just y ->
Just toBeMoved
Nothing ->
Nothing
-- After
case x of
Just y ->
let
toBeMoved = 1
in
Just toBeMoved
Nothing ->
Nothing
Another way of writing this code would be with a Maybe.map
, where the rule doesn't report an error but it should probably.
-- Before
let
toBeMoved = 1
in
x
|> Maybe.map
(\y ->
Just toBeMoved
)
-- After, ideally but not currently
x
|> Maybe.map
(\y ->
let
toBeMoved = 1
in
Just toBeMoved
)
There are a bunch of other functions where this would make sense, yet also plenty where it wouldn't (because we don't want to introduce multiple recomputations of the let variable).
For instance, we would like to do it for Task.map
✅ :
let
toBeMoved = 1
in
x
|> Task.map
(\y ->
Just toBeMoved
)
but we don't for List.map
, as the value would be recomputed multiple times ❌
let
toBeMoved = 1
in
x
|> List.map
(\y ->
Just toBeMoved
)
Functions I think would be okay to move it to:
- Maybe.map, Maybe.map2-5, Maybe.andThen
- Result.map, Result.map2-5, Result.andThen, Result.mapError
- Task.map, Task.andThen
I'm wondering about lambdas given to Task.perform
, since this is more about delaying the computation (which I'm not sure is always desirable), and probably not so much about branching.
Although if the variable to be moved is used only in a branch of the lambda given to Task.perform
, then I'd say it makes sense:
let
toBeMoved = 1
in
x
|> Task.perform
(\result ->
case result of
Ok _ -> toBeMoved
Err _ -> somethingElse
)
Currently, if I remember correctly, variables are never moved across a lambda, specifically because we don't know how often the lambda will be called. So I think it makes sense for Task.perform
, but only if there's branching.
For all the other ones, including custom functions, we should not do it, to avoid false positives.
I wonder if we can somehow detect which custom functions will evaluate a callback multiple times and which ones will only call it once... 🤔