AppliedFix sometimes lies about removing lines

[commonquail]

In v2.36.0, given

package example;

public final class Foo {
    public static void bar() {}
}

PrivateConstructorForUtilityClass suggests that I delete the class outright:

$ ./mvnw clean test-compile -DepFlags='-XepDisableAllChecks -Xep:PrivateConstructorForUtilityClass:ERROR'
[INFO] Scanning for projects...
...
[ERROR] /.../src/main/java/example/Foo.java:[3,14] [PrivateConstructorForUtilityClass] Classes which are not intended to be instantiated should be made non-instantiable with a private constructor. This includes utility classes (classes with only static members), and the main class.
    (see https://errorprone.info/bugpattern/PrivateConstructorForUtilityClass)
  Did you mean to remove this line?

But if I actually apply PrivateConstructorForUtilityClass's fix, fortunately, it correctly inserts a private constructor and the code subsequently passes the check:

$ ./mvnw clean test-compile -DepFlags='-XepDisableAllChecks -Xep:PrivateConstructorForUtilityClass:ERROR' -DepPatchChecks=PrivateConstructorForUtilityClass
[INFO] Scanning for projects...
...
Refactoring changes were successfully applied to file:///.../src/main/java/example/Foo.java, please check the refactored code and recompile.
[INFO] /.../src/main/java/example/Foo.java:[3,14] [PrivateConstructorForUtilityClass] Classes which are not intended to be instantiated should be made non-instantiable with a private constructor. This includes utility classes (classes with only static members), and the main class.
    (see https://errorprone.info/bugpattern/PrivateConstructorForUtilityClass)
  Did you mean to remove this line?

package example;

public final class Foo {
    public static void bar() {}


private Foo() {}
}

I believe this happens because of an interaction between some of the general helpers used by PrivateConstructorForUtilityClass's fix and a heuristic in AppliedFix that determines whether a fix is a "removal". Consider the following debugger state from a variation of com.google.errorprone.bugpatterns.PrivateConstructorForUtilityClassTest#b30170662 where Foo is already final:

Because snippet will eventually evaluate to empty, the resulting AppliedFix will have isRemoveLine=true, hence the fix suggestion.

[UKR555]

package example;

public final class Foo {
// Private constructor to prevent instantiation
private Foo() {
throw new AssertionError("Utility class - do not instantiate");
}

public static void bar() {}

}

Fix: Ensure Foo utility class is non-instantiable

Error Prone incorrectly suggested removing Foo due to a bug in AppliedFix.
This commit adds a private constructor to Foo to prevent instantiation,
resolving the PrivateConstructorForUtilityClass warning.

Reference: https://errorprone.info/bugpattern/PrivateConstructorForUtilityClass