Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bool comparison false positive #53

Open
nrdxp opened this issue Sep 8, 2022 · 5 comments
Open

bool comparison false positive #53

nrdxp opened this issue Sep 8, 2022 · 5 comments

Comments

@nrdxp
Copy link

nrdxp commented Sep 8, 2022

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
Copy link

SuperSandro2000 commented Sep 8, 2022

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
Copy link
Author

nrdxp commented Sep 8, 2022

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
Copy link
Owner

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

@ilkecan
Copy link
Contributor

ilkecan commented Sep 24, 2022

@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
Copy link

spikespaz commented Sep 21, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants