oppiliappan / statix

lints and suggestions for the nix programming language
https://git.peppe.rs/languages/statix/about
MIT License
552 stars 21 forks source link

bool comparison false positive #53

Open nrdxp opened 1 year ago

nrdxp commented 1 year ago

There are cases where we may want to test a value that may or may not be a boolean like so if v == true. Statix currently suggests I reduce this to if v.

If I do the reduction, and v is not a boolean, evaluation aborts with a type mismatch error. I could change it to if builtins.isBool v && v, but if v == true seems more concise in that case.

SuperSandro2000 commented 1 year ago

I could change it to if builtins.isBool v && v

I think that would make it obvious that the value might not be a boolean whereas v == true seems pretty redundant and also without statix could easily be refactored out.

nrdxp commented 1 year ago

One is more explicit the other more concise, I guess my main concern is that if v == true and if v are not semantically equivalent.

oppiliappan commented 1 year ago

would you have a few examples of this scenario? perhaps there are some heuristics to determine and avoid such cases.

ilkecan commented 1 year ago

@nerdypepper For a snippet like this:

let
  f = value:
    if <condition>
      then <first branch>
      else <second branch>;
in
f <argument>

The below is a table that shows the evaluation result when <condition> and <argument> are replaced with different values:

condition value argument evaluation result
value true <first branch>
value 2 error: value is an integer while a Boolean was expected
value == true true <first branch>
value == true 2 <second branch>
isBool value && value true <first branch>
isBool value && value 2 <second branch>

statix prints the following output when the condition is value == true:

[W01] Warning: Unnecessary comparison with boolean
   ╭─[default.nix:3:8]
   │
 3 │     if value == true
   ·        ──────┬──────
   ·              ╰──────── Comparing value with boolean literal true
───╯

The warning message suggests that value == true and value are equivalent but this is not true. value == true can be replaced with isBool value && value but not with value.

spikespaz commented 11 months ago

This is a function that I use:

let
  # Imply that `cond` is not falsy, if it is,
  # return `default` and otherwise `val`.
  imply = cond: val: implyDefault cond null val;
  implyDefault = cond: default: val:
    if (cond == null) || cond == false || cond == { } || cond == [ ] || cond == ""
    || cond == 0 then
      default
    else
      val;
in

Statiz wants to change cond == false to !cond, which is incorrect.