diff --git a/CHANGELOG.md b/CHANGELOG.md index 23eab9423b5..a87550caac5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2. - Bump up logback to `1.4.0` - Bump up log4j2 binding to `2.18.0` - Fixed InvalidInputException in Eclipse while bug reporting ([#2134](https://github.com/spotbugs/spotbugs/issues/2134)) +- Bug `SA_FIELD_SELF_ASSIGNMENT` is now reported from nested classes as well ([#2142](https://github.com/spotbugs/spotbugs/issues/2142)) - Avoid warning on use of security manager on Java 17 and newer. ([#1579](https://github.com/spotbugs/spotbugs/issues/1579)) - Fixed false positives `EI_EXPOSE_REP` thrown in case of fields initialized by the `of` or `copyOf` method of a `List`, `Map` or `Set` ([#1771](https://github.com/spotbugs/spotbugs/issues/1771)) - Fixed CFGBuilderException thrown when `dup_x2` is used to swap the reference and wide-value (double, long) in the stack ([#2146](https://github.com/spotbugs/spotbugs/pull/2146)) diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue2142Test.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue2142Test.java new file mode 100644 index 00000000000..abb1267cfa1 --- /dev/null +++ b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue2142Test.java @@ -0,0 +1,38 @@ +package edu.umd.cs.findbugs.detect; + +import org.junit.Test; + +import edu.umd.cs.findbugs.AbstractIntegrationTest; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder; + +import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.MatcherAssert.assertThat; + +public class Issue2142Test extends AbstractIntegrationTest { + @Test + public void test() { + performAnalysis("ghIssues/Issue2142.class", + "ghIssues/Issue2142$Inner.class"); + BugInstanceMatcher matcher = new BugInstanceMatcherBuilder() + .bugType("SA_FIELD_SELF_ASSIGNMENT").build(); + assertThat(getBugCollection(), containsExactly(2, matcher)); + + matcher = new BugInstanceMatcherBuilder() + .bugType("SA_FIELD_SELF_ASSIGNMENT") + .inClass("Issue2142") + .inMethod("foo1") + .atLine(6) + .build(); + assertThat(getBugCollection(), hasItem(matcher)); + + matcher = new BugInstanceMatcherBuilder() + .bugType("SA_FIELD_SELF_ASSIGNMENT") + .inClass("Issue2142$Inner") + .inMethod("foo2") + .atLine(10) + .build(); + assertThat(getBugCollection(), hasItem(matcher)); + } +} diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindFieldSelfAssignment.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindFieldSelfAssignment.java index fac48805c42..1ea040313b8 100644 --- a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindFieldSelfAssignment.java +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindFieldSelfAssignment.java @@ -29,6 +29,7 @@ import edu.umd.cs.findbugs.BugReporter; import edu.umd.cs.findbugs.LocalVariableAnnotation; import edu.umd.cs.findbugs.OpcodeStack; +import edu.umd.cs.findbugs.OpcodeStack.CustomUserValue; import edu.umd.cs.findbugs.OpcodeStack.Item; import edu.umd.cs.findbugs.Priorities; import edu.umd.cs.findbugs.StatelessDetector; @@ -36,10 +37,12 @@ import edu.umd.cs.findbugs.ba.XField; import edu.umd.cs.findbugs.bcel.OpcodeStackDetector; +@CustomUserValue public class FindFieldSelfAssignment extends OpcodeStackDetector implements StatelessDetector { private final BugReporter bugReporter; private static final boolean DEBUG = SystemProperties.getBoolean("fsa.debug"); + private static final int NO_REGISTER = -1; int state; public FindFieldSelfAssignment(BugReporter bugReporter) { @@ -69,6 +72,8 @@ public void visit(Code obj) { XField possibleOverwrite; + private int parentInstanceLoadFromRegister = NO_REGISTER; + @Override public void sawOpcode(int seen) { @@ -91,8 +96,11 @@ public void sawOpcode(int seen) { OpcodeStack.Item fourth = stack.getStackItem(3); XField f2 = third.getXField(); int registerNumber2 = fourth.getRegisterNumber(); - if (f2 != null && f2.equals(getXFieldOperand()) && registerNumber2 >= 0 - && registerNumber2 == third.getFieldLoadedFromRegister() + int loadedFromRegister2 = fourth.getFieldLoadedFromRegister(); + if (f2 != null && f2.equals(getXFieldOperand()) + && ((registerNumber2 >= 0 && registerNumber2 == third.getFieldLoadedFromRegister()) + || (loadedFromRegister2 >= 0 && third.getUserValue() instanceof Integer + && loadedFromRegister2 == ((Integer) third.getUserValue()).intValue())) && !third.sameValue(top) && (third.getPC() == -1 || third.getPC() > lastMethodCall)) { possibleOverwrite = f2; } @@ -161,7 +169,28 @@ public void sawOpcode(int seen) { default: break; } + if (seen == Const.GETFIELD) { + XField f = getXFieldOperand(); + OpcodeStack.Item top = stack.getStackItem(0); + XField topF = top.getXField(); + if (f == null || topF == null) { + return; + } + if ("this$0".equals(topF.getName())) { + parentInstanceLoadFromRegister = top.getFieldLoadedFromRegister(); + } + } } + @Override + public void afterOpcode(int seen) { + super.afterOpcode(seen); + + if (seen == Const.GETFIELD && parentInstanceLoadFromRegister > NO_REGISTER) { + OpcodeStack.Item top = stack.getStackItem(0); + top.setUserValue(parentInstanceLoadFromRegister); + parentInstanceLoadFromRegister = NO_REGISTER; + } + } } diff --git a/spotbugsTestCases/src/java/ghIssues/Issue2142.java b/spotbugsTestCases/src/java/ghIssues/Issue2142.java new file mode 100644 index 00000000000..942da9d37b7 --- /dev/null +++ b/spotbugsTestCases/src/java/ghIssues/Issue2142.java @@ -0,0 +1,13 @@ +package ghIssues; + +public class Issue2142 { + int foo; + void foo1() { + foo = foo++; // can report a warning in this line + } + class Inner { + void foo2() { + foo = foo++; // should report a warning in this line + } + } +}