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

False positive with `Var`

Open dclements opened this issue 6 years ago • 5 comments

Description of the problem / feature request:

VarChecker seems to behave erratically on implemented classes (and some other cases I haven't been able to fully pin down).

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

public class HealthCheckManager implements io.dropwizard.lifecycle.Managed {

  private final HealthCheckRegistry registry;
  private final ImmutableMap<String, HealthCheck> checks;

  @Inject
  HealthCheckManager(HealthCheckRegistry registry, Map<String, HealthCheck> checks) {
    this.registry = registry;
    this.checks = ImmutableMap.copyOf(checks);
  }

  @Override
  public void start() {
    checks.forEach(registry::register);
  }

  @Override
  public void stop() {}
}

Generates the following:

<path>/HealthCheckManager.java:20: error: [Var] Non-constant variable missing @Var annotation
  HealthCheckManager(HealthCheckRegistry registry, Map<String, HealthCheck> checks) {
                                         ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'HealthCheckManager(@Var HealthCheckRegistry registry, Map<String, HealthCheck> checks) {'?
<path>/HealthCheckManager.java:20: error: [Var] Non-constant variable missing @Var annotation
  HealthCheckManager(HealthCheckRegistry registry, Map<String, HealthCheck> checks) {
                                                                            ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'HealthCheckManager(HealthCheckRegistry registry, @Var Map<String, HealthCheck> checks) {'?

Emphasizing the state doesn't fix it, if I mark the two parameters in question final it compiles normally, but I get the following:

  HealthCheckManager(final HealthCheckRegistry registry, final Map<String, HealthCheck> checks) {
                                               ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'HealthCheckManager(HealthCheckRegistry registry, final Map<String, HealthCheck> checks) {'?
<path>/HealthCheckManager.java:20: error: [Var] Unnecessary 'final' modifier.
  HealthCheckManager(final HealthCheckRegistry registry, final Map<String, HealthCheck> checks) {
                                                                                        ^
    (see https://errorprone.info/bugpattern/Var)
  Did you mean 'HealthCheckManager(final HealthCheckRegistry registry, Map<String, HealthCheck> checks) {'?

Annotating them with @Var prevents the alert, but is inaccurate. Changing the names of the variables doesn't seem to help.

What version of Error Prone are you using?

2.3.2

openjdk 11 2018-09-25
OpenJDK Runtime Environment AdoptOpenJDK (build 11+28)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11+28, mixed mode)```

###  Have you found anything relevant by searching the web?

Could not find it in the github issues nor in `error-prone-discuss`. 

dclements avatar Nov 20 '18 22:11 dclements

This is also creating a false positive:

public class HCModule extends AbstractModule {

  @Override
  protected void configure() {
    requireBinding(HealthCheckRegistry.class);

    /* Other bindings */

    var healthCheckMapBinder = MapBinder.newMapBinder(binder(), String.class, HealthCheck.class);

    healthCheckMapBinder.addBinding("ax.lifecycle").to(StartupHealthCheck.class);
  }
}

var healthCheckMapBinder is generating: Non-constant variable missing @Var annotation var healthCheckMapBinder = MapBinder.newMapBinder(binder(), String.class, HealthCheck.class);

dclements avatar Nov 26 '18 17:11 dclements

Okay, digging into it further, I think I found what allows me to consistently replicate it: I need another error to fire first (e.g., I have some other file that error prone detects as having a different error). If that happens, then I can replicate it. If that error goes away, then the false positive with @Var goes away as well.

dclements avatar Nov 26 '18 20:11 dclements

Regression has been introduced in 2.3.2 version, 2.3.1 works fine for me

dudkiewicz-codes avatar Mar 22 '19 11:03 dudkiewicz-codes

Came across this today with the factory method on an AutoValue class.

public static EndpointWithSchema create(String dataEndpointId, Schema beamSchema) {
  return new AutoValue_SqlTableDefinition_EndpointWithSchema(dataEndpointId, beamSchema);
}
SqlTableDefinition.java:[23,51] error: [Var] Non-constant variable missing @Var annotation (see https://errorprone.info/bugpattern/Var)
  Did you mean 'public static EndpointWithSchema create(@Var String dataEndpointId, Schema beamSchema) {'?

SqlTableDefinition.java:[23,74] error: [Var] Non-constant variable missing @Var annotation (see https://errorprone.info/bugpattern/Var)
   Did you mean 'public static EndpointWithSchema create(String dataEndpointId, @Var Schema beamSchema) {'?

This was using error-prone 2.11.0.

$ java --version
openjdk 11.0.11 2021-04-20 LTS
OpenJDK Runtime Environment Corretto-11.0.11.9.1 (build 11.0.11+9-LTS)
OpenJDK 64-Bit Server VM Corretto-11.0.11.9.1 (build 11.0.11+9-LTS, mixed mode)
$ mvn -V
Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)
Maven home: /usr/local/Cellar/maven/3.8.4/libexec
Java version: 11.0.11, vendor: Amazon.com Inc., runtime: /Library/Java/JavaVirtualMachines/amazon-corretto-11.jdk/Contents/Home
Default locale: en_US, platform encoding: UTF-8
OS name: "mac os x", version: "11.1", arch: "x86_64", family: "mac"

danielnorberg avatar Mar 14 '22 19:03 danielnorberg

I am also seeing this issue in 2.15.0, exactly as @dclements described it.

bkrieger avatar Sep 20 '22 19:09 bkrieger

I am also seeing this issue in 2.16.0, exactly as @dclements described it. If a different error is present, we got a long list of false positives, after fixing this unrelated issue, one occurrence of LenientFormatStringValidation in my case, then the false positives goes away.

narape avatar Nov 09 '22 14:11 narape