google / google/error-prone

CompileTimeConstant checks are bypassed depending on the reference type

Open
#1,965 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.