CompileTimeConstant checks are bypassed depending on the reference type
- Dominant language
- Java
- Stars
- 7.2k
- Forks
- 820
- Avg merge
- 5h 9m
- Merged PRs (30d)
- 50
Description
### 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:
https://github.com/palantir/gradle-baseline/pull/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.
```java
interface Iface {
void accept(String value);
}
class Impl implements Iface {
@Override
public void accept(@CompileTimeConstant String value) {}
}
```
Then:
```java
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:
```java
interface Iface {
void accept(@CompileTimeConstant String value);
}
class Impl implements Iface {
@Override
public void accept(String value) {}
}
```
Then:
```java
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
Contributor guide
Assessment
This issue has not been assessed yet.