Skip to content

Commit

Permalink
Avoid wildcards, which confuse our nullness checker.
Browse files Browse the repository at this point in the history
Specifically, wildcards defeat our ability to recognize that `containsKey` guarantees that `get` will return non-null.

See https://github.com/jspecify/nullness-checker-for-checker-framework/blob/a13e6921d6e23ec1349be007b5e9a6d1f279c91c/src/main/java/com/google/jspecify/nullness/NullSpecTransfer.java#L603

Without this CL, Auto-Common would break when I submit CL 382342656 to annotate the remaining immutable-map classes.

Arguably we should have a specific issue open for this problem, but:

- This may be the first time it's come up in practice.
- _Everything_ about wildcards in our checker is weird, thanks to a combination of the Checker Framework's unusual handling of them (which they are fixing by implementing capture conversion) and our hacks on top of that (which we hope can go away). (The very general project of "remove our hacks" is tracked by jspecify/checker-framework#4 (comment) and other issues there.)

RELNOTES=n/a
PiperOrigin-RevId: 382352859
  • Loading branch information
cpovirk authored and Google Java Core Libraries committed Jun 30, 2021
1 parent 9d79ce1 commit 3b1f449
Showing 1 changed file with 4 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static com.google.auto.common.MoreElements.isAnnotationPresent;
import static com.google.auto.common.MoreStreams.toImmutableSet;
import static com.google.common.base.Preconditions.checkNotNull;
import static java.util.Collections.unmodifiableMap;

import com.google.common.base.Equivalence;
import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -94,9 +95,10 @@ public static Equivalence<AnnotationMirror> equivalence() {
public static ImmutableMap<ExecutableElement, AnnotationValue> getAnnotationValuesWithDefaults(
AnnotationMirror annotation) {
ImmutableMap.Builder<ExecutableElement, AnnotationValue> values = ImmutableMap.builder();
// Use unmodifiableMap to eliminate wildcards, which cause issues for our nullness checker.
@SuppressWarnings("GetElementValues")
Map<? extends ExecutableElement, ? extends AnnotationValue> declaredValues =
annotation.getElementValues();
Map<ExecutableElement, AnnotationValue> declaredValues =
unmodifiableMap(annotation.getElementValues());
for (ExecutableElement method :
ElementFilter.methodsIn(annotation.getAnnotationType().asElement().getEnclosedElements())) {
// Must iterate and put in this order, to ensure consistency in generated code.
Expand Down

0 comments on commit 3b1f449

Please sign in to comment.