Linter: should not unconditionally evaluate to "TRUE" or "FALSE"

Created on 10 Apr 2017  路  3Comments  路  Source: dart-lang/linter

image

  Future<Null> pageChanged(int newPage) async {
    if (newPage - 1 == currentPage) {
      return;
    }
    if (newPage - 1 > currentPage) {
      while (newPage - 1 > currentPage) {
        await nextPage();
      }
    } else {
      while (newPage - 1 < currentPage) {
        await previousPage();
      }
    }
  }

await nextPage() updates currentPage, therefore this will eventually evaluate to true

bug false positive

Most helpful comment

It seems to me (based on research that's been done in this area) that it's better to have no false positives and a weak rule, then work toward making the rule catch more and more issues, than to have a rule that catches 150% of the real problems. False positives cause people to disable lints, and nothing triggers them to re-enable them later.

I think that if a condition includes a reference to any non-local variable or any function (including getters and local functions) that we should assume that we don't know what the truth value of the expression will be. Also note that state mutation can be difficult to detect because the state might be far removed from the object causing it to change.

All 3 comments

This is a known limitation of the rule. We thought it was too expensive to look for all places that modify involved variables.

If currentPage is a getter, then we could probably change the rule to ignore them. Thoughts? It might be the right thing to do since is like a method without arguments, there is a convention to no mutate state in them, but still allowed.

It seems to me (based on research that's been done in this area) that it's better to have no false positives and a weak rule, then work toward making the rule catch more and more issues, than to have a rule that catches 150% of the real problems. False positives cause people to disable lints, and nothing triggers them to re-enable them later.

I think that if a condition includes a reference to any non-local variable or any function (including getters and local functions) that we should assume that we don't know what the truth value of the expression will be. Also note that state mutation can be difficult to detect because the state might be far removed from the object causing it to change.

Here's a related case that should be simple to detect, as the variable in the "invariant" condition is directly mutated inside the if block.

screen shot 2017-05-18 at 10 05 06 pm

I did a little investigating and realized the lint correctly detects that the variable changes if it's mutated within the if block, but _before_ the while loop. The issue occurs however if the variable is mutated within the loop. Perhaps the reasoning here is that the loop condition is guaranteed to pass the first iteration for for and while loops, so it's initially a wasted check, but for do-while this is not the case as the variable will have been mutated before the condition is checked.

Was this page helpful?
0 / 5 - 0 ratings