Faulty rule for not empty

[macalimlim]

There is a faulty rule for (not (empty? coll)). it suggests (seq coll)... Am I right to say that these two expressions should be referential transparent?

[danielcompton]

You're correct that these two expressions will return different things. The theory behind this is that in usage, (seq coll) still returns a truthy value like (not (empty? coll)).

The docstring for empty? says:

Returns true if coll has no items - same as (not (seq coll)). Please use the idiom (seq x) rather than (not (empty? x))

which is where the rule probably came from.

[macalimlim]

I can raise a PR to fix this rule.. this rule only works if the expression is used inside a conditional (e.g. if, when, etc.). but when it is used outside a conditional then it will not be same result..

inside a conditional... (when (not (empty? coll)) "mike") is same as (when (seq coll) "mike")

outside a conditional... (let [is-empty (not (empty? coll))] is-empty) is not same as (let [is-empty (seq coll)] is-empty)

[danielcompton]

I think this would probably need to be a configurable rule, there are also lots of conditionals to check for (including custom ones). I'm probably more inclined to leave it for now, but do you have a real-world example you could show? That might help give me some more context.

[macalimlim]

I don't currently have a real world example to show, but I think the example I gave above commonly happen in day to day work. I just found it strange that kibit suggested it. Would replacing line 17 of collections.clj with [(when (not (empty? ?x)) . ?y) (when (seq ?x) . ?y)] be enough?

[danielcompton]

Maybe? I'm a little bit conflicted here TBH.

[macalimlim]

what do the other maintainers think about my suggested change?

[macalimlim]

any update on this one?

[danielcompton]

I'm going to have put this in the hammock to think about. I don't have a lot of time to put into kibit at the moment.