From 3b1f4492eb61192fbb48a8927ae2e6e136c0f445 Mon Sep 17 00:00:00 2001 From: cpovirk Date: Wed, 30 Jun 2021 11:50:27 -0700 Subject: [PATCH] Avoid wildcards, which confuse our nullness checker. 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 https://github.com/jspecify/checker-framework/issues/4#issuecomment-763111013 and other issues there.) RELNOTES=n/a PiperOrigin-RevId: 382352859 --- .../main/java/com/google/auto/common/AnnotationMirrors.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/common/src/main/java/com/google/auto/common/AnnotationMirrors.java b/common/src/main/java/com/google/auto/common/AnnotationMirrors.java index bc6255d2fb..9ce5cd9bc0 100644 --- a/common/src/main/java/com/google/auto/common/AnnotationMirrors.java +++ b/common/src/main/java/com/google/auto/common/AnnotationMirrors.java @@ -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; @@ -94,9 +95,10 @@ public static Equivalence equivalence() { public static ImmutableMap getAnnotationValuesWithDefaults( AnnotationMirror annotation) { ImmutableMap.Builder values = ImmutableMap.builder(); + // Use unmodifiableMap to eliminate wildcards, which cause issues for our nullness checker. @SuppressWarnings("GetElementValues") - Map declaredValues = - annotation.getElementValues(); + Map 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.