error-prone icon indicating copy to clipboard operation
error-prone copied to clipboard

UnusedVariable sometimes gives false positives for record parameters

Open dododge opened this issue 3 years ago • 3 comments

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.

dododge avatar Nov 29 '21 09:11 dododge

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)

nvervelle avatar Jan 10 '22 08:01 nvervelle

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

imoverclocked avatar Mar 22 '22 00:03 imoverclocked

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?

knutae avatar Mar 25 '22 13:03 knutae