Add two rules for when-let and if-let.

[edtsech]

[Scriptor]

Maybe a third rule should be added for when the user uses a combination of let and when (only combinations of let and if are covered):

[(let [?x ?y] (if ?x ?then ?else)) (if-let [?x ?y] ?then ?else)]
[(let [?x ?y] (if ?x ?expr nil)) (when-let [?x ?y] ?expr)]
[(let [?x ?y] (when ?x ?expr)) (when-let [?x ?y] ?expr)] ; New rule

[ljos]

Do these rules make sense? This is not how I would use if-let/when-let or the let/if constructs. I think this should convert to something like

(let [?x ?y] (if ?x ?then ?else)) 
    => (if ?y ?then ?else)

instead, or am I just misunderstanding the use-case here?

[joelittlejohn]

@ljos your example seems to show a rule that identifies a redundant let that can be removed. I think the goal of this change is something different, it is to identify places where the let is required (because the binding is necessary) but:

• the let has only a single binding
• the let body contains a single if or when

so an if-let or when-let can be used instead.

[ljos]

You are probably right, but in the context of how the substitution works, these two rules are equivalent. The reason they are equivalent is that we do not know if the ?x is in any of the clauses.

[joelittlejohn]

Since we don't know if ?x is in any of the clauses, surely this means that your suggested rule is unsafe? I don't think the original proposed rules suffer from this problem.

So to recap, yes I think these rules make sense and they probably shouldn't convert to something like the alternative you mentioned above.

[jonase]

@ljos Your suggestion would turn

(let [x (f y)] 
   (if x (g x) (h x)))

into

(if (f y) (g x) (h x))

while the original patch would suggests

(if-let [x (f y)]
  (g x) (h x))

which is correct.

I'd merge the pull request but it doesn't merge right now. Also, could we change the second rule's head to when as suggested by @Scriptor

[ljos]

@jonase, but the original would turn

(let [x (f y)]
    (if x (g e) (h f))

into

(if-let [x (f y)]
   (g e) (h f))

which is also incorrect.

Granted, this version does actually work after compilation, though it is bad style.

The rule has to check if the x is used in the then part or it is going to do the wrong thing some of the time.

It would also turn

(let [x (f y)]
    (if x  (h x) (g x))

into

(if-let [x (f y)]
    (h x) (g x))

Which is the wrong thing because the x is out of scope from the else.

The thing I am reacting to is that there is no way of telling what someone is doing with that x because we do not know what is happening in the then and else parts.

[danielcompton]

I'm still interested in merging this, I just need to get a few things sorted first.