Report bug SA_FIELD_SELF_ASSIGNMENT in nested classes as well

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))
 - 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))
 

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));
+    }
+}

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

@@ -69,6 +69,8 @@ public void visit(Code obj) {
 
     XField possibleOverwrite;
 
+    private int parentInstanceLoadFromRegister = -1;
+
     @Override
     public void sawOpcode(int seen) {
 
@@ -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;
                 }
@@ -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;
+        }
+    }
 }

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
+        }
+    }    
+}