unnecessary_lambda_linter() misses '!' when suggesting removing a lambda

[MichaelChirico]

lint(text = "sapply(x, function(xi) !all.equal(xi, y))", linters = unnecessary_lambda_linter())
# <text>:1:11: warning: [unnecessary_lambda_linter] Pass all.equal directly as a symbol to sapply() instead of wrapping it in an unnecessary anonymous function. For example, prefer lapply(DF, sum) to lapply(DF, function(x) sum(x)).
# sapply(x, function(xi) !all.equal(xi, y))
#           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

We could do sapply(x, Negate(all.equal), y), I think, but that's hardly a strict improvement.

[AshesITR]

Is !all.equal(x, y) even well formed, given that all.equal() can return character?

Even so, would !sapply(x, all.equal, y) not work?

[MichaelChirico]

Indeed that usage should be changed to !isTRUE(all.equal(...)). Some one else's code, I promise 😜

I also saw it hitting on !is.null(.) which is more normal.

I think you're right moving the ! out is usually preferable but I couldn't quickly convince myself it's always possible, so I think it makes sense to be conservative and not lint. And presumably it misses other unary operators for the same reason.

[MichaelChirico]

Indeed:

lint(text = "sapply(x, function(xi) -foo(xi))", linters = unnecessary_lambda_linter())
# <text>:1:11: warning: [unnecessary_lambda_linter] Pass foo directly as a symbol to sapply() instead of wrapping it in an unnecessary anonymous function. For example, prefer lapply(DF, sum) to lapply(DF, function(x) sum(x)).
# sapply(x, function(xi) -foo(xi))
#           ^~~~~~~~~~~~~~~~~~~~~

Still - (and even more rarely, unary +) usually should be moved outside...

[AshesITR]

I would suspect this is only a problem when the unary operations dispatch to different implementations within the loop (say the inner function can return different types with different negation semantics and the sapply() unifies the types)

I'd keep it as a feature unless a real life false positive pops up.

[MichaelChirico]

I would suspect this is only a problem when the unary operations dispatch to different implementations within the loop (say the inner function can return different types with different negation semantics and the sapply() unifies the types)

Right, exactly.

At a minimum, we need to fix the lint message, which is incorrect in this case.