Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CompileTimeConstant checks are bypassed depending on the reference type #1965

Open
carterkozak opened this issue Nov 25, 2020 · 0 comments
Open

Comments

@carterkozak
Copy link
Contributor

Description of the problem / feature request:

CompileTimeConstant parameter validation is only done when the type which is directly annotated is referenced. Annotated interfaces are ignored when using a reference to a concrete implementation, and implementation annotations are ignored if a superclass or superinterface reference type is used.

Replace this line with your answer.

Feature requests: what underlying problem are you trying to solve with this feature?

Ideally there would be a mechanism to prevent CompileTimeConstant checks from being accidentally bypassed. I have
implemented an errorprone check to require type hierarchies provide consistent annotations here:
palantir/gradle-baseline#1559
But there are other ways such validation could work.

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

First case:

The check needs additional data at this point. I would recommend a second check which validates that all superclasses and superinterfaces are annotated matching the annotated implementation.

interface Iface {
  void accept(String value);
}

class Impl implements Iface {
  @Override
  public void accept(@CompileTimeConstant String value) {}
}

Then:

String dynamic = System.getProperty("java.specification.version");
Iface value0 = new Impl();
// Passes compilation as expected
value0.accept(dynamic);
Impl value1 = new Impl();
// Fails compilation
value1.accept(dynamic);

Second:

Implementing an interface with methods that contain constant parameters does not detect the constant annotation on the super-interface, allowing non-constant values and violating the contract. Likely a larger problem as the new var keyword becomes widespread.

This could be solved by expanding the compile-time-constant check to check super-interfaces, or by requiring subtypes add matching annotations (writing an automated fix is trivial). For what it's worth, I think it's helpful to add the annotation to all implementations because the code becomes more obvious to developers.

Given:

interface Iface {
  void accept(@CompileTimeConstant String value);
}

class Impl implements Iface {
  @Override
  public void accept(String value) {}
}

Then:

String dynamic = System.getProperty("java.specification.version");
Iface value0 = new Impl();
// Fails compilation as expected
value0.accept(dynamic);
Impl value1 = new Impl();
// Passes compilation
value1.accept(dynamic);

What version of Error Prone are you using?

2.4.0

Have you found anything relevant by searching the web?

no

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant