lintr icon indicating copy to clipboard operation
lintr copied to clipboard

Lint LHS literals in conditional expressions?

Open IndrajeetPatil opened this issue 1 year ago • 4 comments

Preamble

It can be difficult to understand conditional expressions when the numeric, string, etc. literals appear in RHS of the expression, and {lintr} can help detect such instances.

It's rare to see code like this, so I am wondering if it is worth introducing a new linter, but wanted to create an issue nonetheless.

Examples

Actual

if (0L == length(x)) 1L
if ("a" == x) 1L
if (10L < quantity) discount <- 0.1
...

Suggested/Expected

if (length(x) == 0L) 1L
if (x == "a") 1L
if (quantity > 10L) discount <- 0.1
...

Exceptions

If there are literals on both RHS and LHS. E.g.

dplyr::filter(data, 0L == 1L)

IndrajeetPatil avatar Dec 11 '22 10:12 IndrajeetPatil

This sounds related to the yoda_test_linter(). Would it be worth adding this new functionality there (and rename it with a more generic name maybe?)?

Bisaloo avatar Dec 11 '22 10:12 Bisaloo

You are right! This would be a perfect for that linter. And, yes, we will need to change the name of this linter.

How about yoda_condition_linter() (https://en.wikipedia.org/wiki/Yoda_conditions)? Generic enough to cover tests, expressions, etc.

IndrajeetPatil avatar Dec 11 '22 10:12 IndrajeetPatil

yes I had this in mind as going into yoda_test_linter()

MichaelChirico avatar Dec 11 '22 18:12 MichaelChirico

This covers a TODO as well:

https://github.com/r-lib/lintr/blob/72eeefd87596ea7fa72ee1a8b14bcb82d322c381/tests/testthat/test-yoda_test_linter.R#L73-L75

MichaelChirico avatar Dec 19 '23 06:12 MichaelChirico