New unnecessary_lambda_linter

[MichaelChirico]

Part of #884. Closes #1531.

cc @Bisaloo for viz.

WIP: I will have to add extra logic for customizing the lint message. Filing now for early feedback.

[MichaelChirico]

Looks like we'll need a new name -- 37 characters is too long 🫣

open to ideas

[IndrajeetPatil]

Looks like we'll need a new name -- 37 characters is too long

How about unnecessary_lambda_linter()?

[MichaelChirico]

How about unnecessary_lambda_linter()?

makes sense to me, but i wonder if less-experienced users will be confused by this name

[IndrajeetPatil]

makes sense to me, but i wonder if less-experienced users will be confused by this name

IMO, given the kind of users that use linter or know what lints are, I think we can safely assume that they either know what this means or have the wherewithal to figure it out on their own. Plus, the doc should help as well.

[AshesITR]

I think we should test multi-line functions and braces more thoroughly, especially the false negative case.

e.g.
lapply(1:10, function(x) { print(x) }), which should lint

and

lapply(1:10, function(x) { print(x); x^2 }), which should not lint.

[MichaelChirico]

I think we should test multi-line functions and braces more thoroughly

Great idea! Indeed caught some false negatives. Let's leave them for follow-up (#1567) -- false negatives are more tolerable than false positives.

[AshesITR]

Am I missing the positive braced test?
e.g. function(x) { is.data.frame(x) }.

Otherwise looks good so far

[MichaelChirico]

that's marked for follow-up (#1567), is that a blocker to merge?

[AshesITR]

Ah sorry I missed that issue. Is it referenced somewhere in the code or tests?

[MichaelChirico]

There wasn't! Just added, it slipped through the cracks.

[MichaelChirico]

@AshesITR need your approval now since the Changes requested is still pending