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

[java] AvoidReassigningLoopVariables - false negatives within for-loops and skip allowed #4500

Open
adangel opened this issue Apr 23, 2023 · 0 comments
Labels
a:false-negative PMD doesn't flag a problematic piece of code
Milestone

Comments

@adangel
Copy link
Member

adangel commented Apr 23, 2023

Affects PMD Version: 7.0.0-rc1

Rule: AvoidReassigningLoopVariables

Description:

This rule might not report anymore all reassignments of the control variable in for-loops when the property forReassign is set to skip.

Code Sample demonstrating the issue:

public class Foo {
    void foo(int bar) {
        for (int i=0; i < 10; i++) {
            doSomethingWith(i);
            if (foo()) {
                i++;
                i--;
                ++i;
                --i;
                i += 1;
                i -= 1;
                doSomethingWith(i++);
                i += 2;    // not OK (line 13)
                i -= 2;    // not OK (line 14)
                i &= 1;    // not OK (line 15)
                i |= 1;    // not OK (line 16)
                i *= 1;    // not OK - is not reported anymore (line 17)
                i /= 1;    // not OK - is not reported anymore (line 18)
                i = i + 1; // not OK - is reported with 7.0.0-rc1 (line 19)
                i = i - 1; // not OK - is reported with 7.0.0-rc1 (line 20)
                i = 5;     // not OK - is reported with 7.0.0-rc1 (line 21)
                doSomethingWith(i = 5); // not OK - is reported with 7.0.0-rc1 (line 22)
            }
        }
    }
}

Expected outcome:

PMD should report violations at lines 13 to 22, but only lines 19 to 22 are reported.

More context:

  • [doc] Differences in rules in pmd 7 vs pmd 6 #3123
  • According to the rule description, only increments/decrements should be allowed in this case. That means ++, -- and += and -=. Multiplication/Division shouldn't be allowed. Also not the bitwise operators (not tested: % modulo, <</>> shift, ^ xor).
  • We probably should only allow +/- operators - as that's how increments/decrements work.
  • We probably should deny any multiplication/division -> lines 17,18
  • We probably should deny any other operators (logical, bitwise) -> lines 15,16
  • We probably should allow the longer form of increments/decrements -> lines 19,20. Note: This is then a false positive right now.
@adangel adangel added the a:false-negative PMD doesn't flag a problematic piece of code label Apr 23, 2023
adangel added a commit to adangel/pmd that referenced this issue Apr 23, 2023
adangel added a commit to adangel/pmd that referenced this issue Apr 27, 2023
@adangel adangel added this to the 7.x milestone Mar 14, 2024
@jsotuyod jsotuyod added needs:pmd7-revalidation The issue hasn't yet been retested vs PMD 7 and may be stale and removed needs:pmd7-revalidation The issue hasn't yet been retested vs PMD 7 and may be stale labels Mar 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:false-negative PMD doesn't flag a problematic piece of code
Projects
None yet
Development

No branches or pull requests

2 participants