New linter to simplify negatives in conditional expressions?

[IndrajeetPatil]

Preamble

There ain't nobody that doesn't struggle with negatives, and it would be nice to have a linter that suggests a way to simplify boolean tests with negatives.

Here are a couple of ways to do this in a single linter.

1. Re-order if/else

Actual

if (!normal_case) {
  ...1
} else {
  ...2
}

Suggested

if (normal_case) {
  ...2
} else {
  ...1
}

2. Applying De Morgan's laws

Actual = Suggested

• (not A) or (not B) = not (A and B)

if (!has_fuel || !has_mirrors) {
  can_be_rented <- FALSE
}

if (!(has_fuel && has_mirrors)) {
  can_be_rented <- FALSE
}

• (not A) and (not B) = not (A or B)

if (!is_tank_empty && !is_mirror_broken) {
  can_be_rented <- TRUE
}

if (!(is_tank_empty || is_mirror_broken)) {
  can_be_rented <- TRUE
}

WDYT? Any other ways to simplify these?

[AshesITR]

Honestly, I prefer !has_fuel || !has_mirrors because parentheses use additional brainpower.

Also, control flow should be ordered in such a way that the surrounding code reads naturally. Therefor, it depends on context whether if (!cond) or if (cond) is more readable.

[MichaelChirico]

this is a Google linter IINM -- IfNotElseLinter() :)

On Sat, Dec 10, 2022, 3:21 PM AshesITR @.***> wrote:

Honestly, I prefer !has_fuel || !has_mirrors because parentheses use additional brainpower.

Also, control flow should be ordered in such a way that the surrounding code reads naturally. Therefor, it depends on context whether if (!cond) or if (cond) is more readable.

— Reply to this email directly, view it on GitHub https://github.com/r-lib/lintr/issues/1831#issuecomment-1345406893, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2BA5JBM6NSAY2SPIYS66TWMUF7XANCNFSM6AAAAAAS2QEHLU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[IndrajeetPatil]

Honestly, I prefer !has_fuel || !has_mirrors because parentheses use additional brainpower.

Fair enough. But my preferred approach here would be to extract the conditional expressions to a new variable so that the parentheses can be avoided. Additionally, this introduces a reusable abstraction with an informative name.

# actual
if (!has_fuel || !has_mirrors) {
  can_be_rented <- FALSE
}

# refactored
is_car_in_good_condition <- has_fuel && has_mirrors
if (!is_car_in_good_condition) {
  can_be_rented <- FALSE
}

[IndrajeetPatil]

The first part of this issue was addressed by https://github.com/r-lib/lintr/pull/2071.

I sense there is no consensus about whether the second part should be supported.

[MichaelChirico]

I sense there is no consensus about whether the second part should be supported.

I think consensus is more important for setting defaults -- it seems like a reasonable option to add, though I wouldn't enable it by default.

[IndrajeetPatil]

Sounds good to me.

Maybe we can call this parameter enforce_de_morgan_laws and set it FALSE by default?

[AshesITR]

use_de_morgans_laws = FALSE is more in line with our other parameter names. Also note the genitive "s".