FinalizeLocalVariables fails when processing fields of anonymous classes

[AlejandroBertolo]

Greetings. Thanks everyone for your work in this useful library.

We've found a issue with the FinalizeLocalVariables recipe, by which fields of anonymous classes that are reassigned in overriden methods are finalized, generating compilation problems in processed files.

What version of OpenRewrite are you using?

staticAnalysis 1.0.7
RecipeBom 2.3.0
RewriteMavenPlugin 5.7.1

How are you running OpenRewrite?

Through Maven plugin, in a multi-module project.

What is the smallest, simplest way to reproduce the problem?

The following test exposes the problem.

package Test;

import javafx.beans.property.IntegerProperty;
import javafx.beans.property.SimpleIntegerProperty;

    class Test {

      private final IntegerProperty cellCount = new SimpleIntegerProperty(this, "cellCount", 0) {
        private int oldCount = 0;

        @Override
        protected void invalidated() {
          final int currentCellCount = this.get();

          final boolean countChanged = this.oldCount != currentCellCount;
          if (countChanged) {
            this.oldCount = currentCellCount;
          }
        }
      };
    }
}

What did you expect to see?

No changes should be applied to the previous code.

What did you see instead?

oldCount var was finalized, regardless of assignment made in invalidated().

package Test;

import javafx.beans.property.IntegerProperty;
import javafx.beans.property.SimpleIntegerProperty;

    class Test {

      private final IntegerProperty cellCount = new SimpleIntegerProperty(this, "cellCount", 0) {
        private final int oldCount = 0;

        @Override
        protected void invalidated() {
          final int currentCellCount = this.get();

          final boolean countChanged = this.oldCount != currentCellCount;
          if (countChanged) {
            this.oldCount = currentCellCount;
          }
        }
      };
    }
}

Are you interested in contributing a fix to OpenRewrite?

Yes, although the fix we have is to identify fields of inner classes and simply skip them. This is a workaround to avoid breaking compilation in processed projects.

[timtebeek]

Thanks for pointing this out! Seems best indeed to at the very least skip fields of anonymous classes for now, such that we don't break any code. We could then in the future revisit this with proper guards, although I'd hope this pattern to be rare.

Would appreciate if you'd indeed want to contribute that; seems like you're near that already. :)