Skip to content

Commit

Permalink
Report bug SA_FIELD_SELF_ASSIGNMENT in nested classes as well
Browse files Browse the repository at this point in the history
Bug `SA_FIELD_SELF_ASSIGNMENT` was not reported in nested classes, only in the outer class. This lead to inconsistent behavior. See issue ([spotbugs#2142](spotbugs#2142)). This PR fixes this issue.
  • Loading branch information
Ádám Balogh committed Sep 1, 2022
1 parent bfe835a commit aad527f
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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))

## 4.7.1 - 2022-06-26
### Fixed
Expand Down
@@ -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));
}
}
Expand Up @@ -69,6 +69,8 @@ public void visit(Code obj) {

XField possibleOverwrite;

private int parentInstanceLoadFromRegister = -1;

@Override
public void sawOpcode(int seen) {

Expand All @@ -91,8 +93,12 @@ 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() != null
&& third.getUserValue() instanceof Integer
&& loadedFromRegister2 == (Integer) third.getUserValue()))
&& !third.sameValue(top) && (third.getPC() == -1 || third.getPC() > lastMethodCall)) {
possibleOverwrite = f2;
}
Expand Down Expand Up @@ -161,7 +167,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 > -1) {
OpcodeStack.Item top = stack.getStackItem(0);
top.setUserValue(parentInstanceLoadFromRegister);
parentInstanceLoadFromRegister = -1;
}
}
}
13 changes: 13 additions & 0 deletions 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
}
}
}

0 comments on commit aad527f

Please sign in to comment.