Skip to content

Commit 7c835b6

Browse files
Balogh, ÁdámKengoTODA
Balogh, Ádám
and
KengoTODA
authored
Report bug SA_FIELD_SELF_ASSIGNMENT in nested classes as well (#2161)
* Report bug `SA_FIELD_SELF_ASSIGNMENT` in nested classes as well Bug `SA_FIELD_SELF_ASSIGNMENT` was not reported in nested classes, only in the outer class. This lead to inconsistent behavior. See issue ([#2142](#2142)). This PR fixes this issue. * Update spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindFieldSelfAssignment.java Co-authored-by: Kengo TODA <skypencil+github@gmail.com> * Update spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindFieldSelfAssignment.java Co-authored-by: Kengo TODA <skypencil+github@gmail.com> * Fixed according to the comments of @KengoTODA Co-authored-by: Kengo TODA <skypencil@gmail.com> Co-authored-by: Kengo TODA <skypencil+github@gmail.com>
1 parent 4c0c1b9 commit 7c835b6

4 files changed

Lines changed: 83 additions & 2 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
1111
- Bump up logback to `1.4.0`
1212
- Bump up log4j2 binding to `2.18.0`
1313
- Fixed InvalidInputException in Eclipse while bug reporting ([#2134](https://github.com/spotbugs/spotbugs/issues/2134))
14+
- Bug `SA_FIELD_SELF_ASSIGNMENT` is now reported from nested classes as well ([#2142](https://github.com/spotbugs/spotbugs/issues/2142))
1415
- Avoid warning on use of security manager on Java 17 and newer. ([#1579](https://github.com/spotbugs/spotbugs/issues/1579))
1516
- 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))
1617
- 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))

‎spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue2142Test.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package edu.umd.cs.findbugs.detect;
2+
3+
import org.junit.Test;
4+
5+
import edu.umd.cs.findbugs.AbstractIntegrationTest;
6+
import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher;
7+
import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder;
8+
9+
import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly;
10+
import static org.hamcrest.Matchers.hasItem;
11+
import static org.hamcrest.MatcherAssert.assertThat;
12+
13+
public class Issue2142Test extends AbstractIntegrationTest {
14+
@Test
15+
public void test() {
16+
performAnalysis("ghIssues/Issue2142.class",
17+
"ghIssues/Issue2142$Inner.class");
18+
BugInstanceMatcher matcher = new BugInstanceMatcherBuilder()
19+
.bugType("SA_FIELD_SELF_ASSIGNMENT").build();
20+
assertThat(getBugCollection(), containsExactly(2, matcher));
21+
22+
matcher = new BugInstanceMatcherBuilder()
23+
.bugType("SA_FIELD_SELF_ASSIGNMENT")
24+
.inClass("Issue2142")
25+
.inMethod("foo1")
26+
.atLine(6)
27+
.build();
28+
assertThat(getBugCollection(), hasItem(matcher));
29+
30+
matcher = new BugInstanceMatcherBuilder()
31+
.bugType("SA_FIELD_SELF_ASSIGNMENT")
32+
.inClass("Issue2142$Inner")
33+
.inMethod("foo2")
34+
.atLine(10)
35+
.build();
36+
assertThat(getBugCollection(), hasItem(matcher));
37+
}
38+
}

‎spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindFieldSelfAssignment.java‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,20 @@
2929
import edu.umd.cs.findbugs.BugReporter;
3030
import edu.umd.cs.findbugs.LocalVariableAnnotation;
3131
import edu.umd.cs.findbugs.OpcodeStack;
32+
import edu.umd.cs.findbugs.OpcodeStack.CustomUserValue;
3233
import edu.umd.cs.findbugs.OpcodeStack.Item;
3334
import edu.umd.cs.findbugs.Priorities;
3435
import edu.umd.cs.findbugs.StatelessDetector;
3536
import edu.umd.cs.findbugs.SystemProperties;
3637
import edu.umd.cs.findbugs.ba.XField;
3738
import edu.umd.cs.findbugs.bcel.OpcodeStackDetector;
3839

40+
@CustomUserValue
3941
public class FindFieldSelfAssignment extends OpcodeStackDetector implements StatelessDetector {
4042
private final BugReporter bugReporter;
4143

4244
private static final boolean DEBUG = SystemProperties.getBoolean("fsa.debug");
45+
private static final int NO_REGISTER = -1;
4346
int state;
4447

4548
public FindFieldSelfAssignment(BugReporter bugReporter) {
@@ -69,6 +72,8 @@ public void visit(Code obj) {
6972

7073
XField possibleOverwrite;
7174

75+
private int parentInstanceLoadFromRegister = NO_REGISTER;
76+
7277
@Override
7378
public void sawOpcode(int seen) {
7479

@@ -91,8 +96,11 @@ public void sawOpcode(int seen) {
9196
OpcodeStack.Item fourth = stack.getStackItem(3);
9297
XField f2 = third.getXField();
9398
int registerNumber2 = fourth.getRegisterNumber();
94-
if (f2 != null && f2.equals(getXFieldOperand()) && registerNumber2 >= 0
95-
&& registerNumber2 == third.getFieldLoadedFromRegister()
99+
int loadedFromRegister2 = fourth.getFieldLoadedFromRegister();
100+
if (f2 != null && f2.equals(getXFieldOperand())
101+
&& ((registerNumber2 >= 0 && registerNumber2 == third.getFieldLoadedFromRegister())
102+
|| (loadedFromRegister2 >= 0 && third.getUserValue() instanceof Integer
103+
&& loadedFromRegister2 == ((Integer) third.getUserValue()).intValue()))
96104
&& !third.sameValue(top) && (third.getPC() == -1 || third.getPC() > lastMethodCall)) {
97105
possibleOverwrite = f2;
98106
}
@@ -161,7 +169,28 @@ public void sawOpcode(int seen) {
161169
default:
162170
break;
163171
}
172+
if (seen == Const.GETFIELD) {
173+
XField f = getXFieldOperand();
174+
OpcodeStack.Item top = stack.getStackItem(0);
175+
XField topF = top.getXField();
176+
if (f == null || topF == null) {
177+
return;
178+
}
164179

180+
if ("this$0".equals(topF.getName())) {
181+
parentInstanceLoadFromRegister = top.getFieldLoadedFromRegister();
182+
}
183+
}
165184
}
166185

186+
@Override
187+
public void afterOpcode(int seen) {
188+
super.afterOpcode(seen);
189+
190+
if (seen == Const.GETFIELD && parentInstanceLoadFromRegister > NO_REGISTER) {
191+
OpcodeStack.Item top = stack.getStackItem(0);
192+
top.setUserValue(parentInstanceLoadFromRegister);
193+
parentInstanceLoadFromRegister = NO_REGISTER;
194+
}
195+
}
167196
}

‎spotbugsTestCases/src/java/ghIssues/Issue2142.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package ghIssues;
2+
3+
public class Issue2142 {
4+
int foo;
5+
void foo1() {
6+
foo = foo++; // can report a warning in this line
7+
}
8+
class Inner {
9+
void foo2() {
10+
foo = foo++; // should report a warning in this line
11+
}
12+
}
13+
}

0 commit comments

Comments
 (0)