UnusedVariable sometimes gives false positives for record parameters

[dododge]

Using error-prone 2.10.0.

Here's a trivial example of a record where I get a warning that the x value is never being used, even though there are public methods that access it both directly and via its getter.

public class Example
{
    private static record MyRecord(int x) { }

    public static int testMethod1(int x) {
        return new MyRecord(x).x();
    }

    public static int testMethod2(int x) {
        return new MyRecord(x).x;
    }
}

Example.java:[3,12] [UnusedVariable] The parameter 'x' is never read.

I don't know the exact conditions that trigger this, but for example if I make the record itself public the warning goes away.

[nvervelle]

Following the fix for #1648 , I also updated to 2.10.0 and tried to remove all the @SuppressWarnings("UnusedVariable") in our code. It works for most of the records, but not for some. I first thought that it was when a record variable was only accessed by the record itself or by another record. Then I found that issue, and I can confirm that for us also the remaining UnusedVariable are for records defined inside a class as private.

BTW : static is useless on a record (it's always static)

[imoverclocked]

FWIW, I still see this in errorprone 2.11.0 (and 2.13.1)

[knutae]

The following tiny example also gives errors:

package example;

import static org.junit.Assert.assertEquals;

import org.junit.Test;

public class TestRecord {
    public record MyRecord(int foo, int bar) {}

    @Test
    public void record() {
        var rec = new MyRecord(2, 3);
        assertEquals(2, rec.foo());
        assertEquals(3, rec.bar());
    }
}

Tested via bazel (5.1.0) with the following BUILD file:

load("@rules_java//java:defs.bzl", "java_test")

java_test(
    name = "TestRecord",
    size = "small",
    srcs = ["TestRecord.java"],
    deps = ["@junit"],
    javacopts = [
        "-Werror",
        "-XepAllDisabledChecksAsWarnings",
    ],
)

It fails like this:

java failed: error executing command external/remotejdk17_linux/bin/java '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ... (remaining 15 arguments skipped)
record-test/src/main/java/example/TestRecord.java:8: warning: [UnusedVariable] The field 'bar' is never read.
    public record MyRecord(int foo, int bar) {}
                                        ^
    (see https://errorprone.info/bugpattern/UnusedVariable)
  Did you mean 'public record MyRecord(int foo) {}'?
record-test/src/main/java/example/TestRecord.java:8: warning: [UnusedVariable] The field 'foo' is never read.
    public record MyRecord(int foo, int bar) {}
                               ^
    (see https://errorprone.info/bugpattern/UnusedVariable)
  Did you mean 'public record MyRecord(, int bar) {}'?
error: warnings found and -Werror specified

It seems like it doesn't understand that the field are indirectly used by the generated getter functions?

[TimvdLippe]

We recently started adopting record classes and we can still reproduce this error with similar patterns as the ones posted above. For now, we added a @SuppressWarnings to the constructor to disable the check:

public class Example {
    @SuppressWarnings("unused")
    private record MyRecord(int x) { }
}

[Marcono1234]

@knutae, which Error Prone version is your sample using (I am not very familiar with Bazel)? Does it use a version < 2.10.0 because in that version the false positives for record fields were supposedly fixed (47da3af). When I tried to recreate your sample (without Bazel) I was not able to reproduce it with the latest Error Prone version.

[knutae]

@knutae, which Error Prone version is your sample using (I am not very familiar with Bazel)? Does it use a version < 2.10.0 because in that version the false positives for record fields were supposedly fixed (47da3af). When I tried to recreate your sample (without Bazel) I was not able to reproduce it with the latest Error Prone version.

Looks like Bazel 5 uses Error Prone 2.9.0. So hopefully this will be fixed by upgrading to Bazel 6, which uses 2.11.0.

[rdesgroppes]

Problem still happens with Bazel 6.2.0 shipping with Error Prone 2.18.0.

[victornoel]

This issue is still happening with error-prone 2.19.1

[rdesgroppes]

Problem still happens with Bazel 6.3.0 shipping with Error Prone 2.20.0.

[agentgt]

I'm still getting this as well for 2.21.0.

Pretty simple to reproduce

class ErrorProneBug {

	private record Stuff(boolean stuff) {
	}

	public static void main(String[] args) {
		new Stuff(true).stuff();
	}

}

ErrorProneBug.java:[5,9] [UnusedVariable] The parameter 'stuff' is never read.

[tbroyer]

The fix was merged last week, 2.21 is 3 weeks old, so this is expected.

[agentgt]

Sorry I for some reason got confused on the timing and thought errorprone just got released.