From 33817d8cadb97f591e575f73ae5f947ef5e9eb4e Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Tue, 6 Feb 2024 17:07:59 -0500 Subject: [PATCH 01/28] Use EISOP CF sources (#118) Co-authored-by: David P. Baker --- initialize-project | 11 +- settings.gradle | 12 +- .../jspecify/nullness/NullSpecAnalysis.java | 5 +- .../NullSpecAnnotatedTypeFactory.java | 407 ++++++------------ .../jspecify/nullness/NullSpecChecker.java | 6 +- .../jspecify/nullness/NullSpecTransfer.java | 8 +- .../jspecify/nullness/NullSpecVisitor.java | 14 +- src/test/java/tests/ConformanceTest.java | 8 +- src/test/java/tests/NullSpecTest.java | 22 +- tests/ConformanceTest-report.txt | 111 +---- tests/ConformanceTestOnSamples-report.txt | 30 +- 11 files changed, 209 insertions(+), 425 deletions(-) diff --git a/initialize-project b/initialize-project index c8783b63..cd1f9304 100755 --- a/initialize-project +++ b/initialize-project @@ -11,7 +11,7 @@ # Set SHALLOW=1 to clone sibling projects at depth 1. # # This script automatically tries to download your fork of -# jspecify/checker-framework, jspecify/jspecify, or jspecify/jdk, if they exist. +# eisop/checker-framework, jspecify/jspecify, or jspecify/jdk, if they exist. # It uses the URL of the origin remote (the default remote created when cloning # a repo) to determine that. # @@ -55,12 +55,17 @@ git_clone() { local forking_org forking_org="$(forking_org)" - if [[ -n "${forking_org}" ]]; then + if [[ -n "${forking_org}" ]] && [[ "${forking_org}" != "https://github.com/jspecify" ]]; then if run "${git[@]}" "${forking_org}/${repo}.git" "../${repo}"; then return fi fi - run "${git[@]}" "https://github.com/jspecify/${repo}.git" "../${repo}" + if [[ "${repo}" == checker-framework ]]; then + forking_org=https://github.com/eisop + else + forking_org=https://github.com/jspecify + fi + run "${git[@]}" "${forking_org}/${repo}.git" "../${repo}" } git_clone jdk --depth 1 --single-branch diff --git a/settings.gradle b/settings.gradle index a440bde8..5734a5f4 100644 --- a/settings.gradle +++ b/settings.gradle @@ -15,13 +15,13 @@ includeBuild("../checker-framework") dependencyResolutionManagement { versionCatalogs { libs { - version("checkerFramework", "3.21.5-SNAPSHOT") + version("checkerFramework", "3.42.0-eisop2-SNAPSHOT") - library("checkerFramework-checker", "org.checkerframework", "checker").versionRef("checkerFramework") - library("checkerFramework-checker-qual", "org.checkerframework", "checker-qual").versionRef("checkerFramework") - library("checkerFramework-framework", "org.checkerframework", "framework").versionRef("checkerFramework") - library("checkerFramework-framework-test", "org.checkerframework", "framework-test").versionRef("checkerFramework") - library("checkerFramework-javacutil", "org.checkerframework", "javacutil").versionRef("checkerFramework") + library("checkerFramework-checker", "io.github.eisop", "checker").versionRef("checkerFramework") + library("checkerFramework-checker-qual", "io.github.eisop", "checker-qual").versionRef("checkerFramework") + library("checkerFramework-framework", "io.github.eisop", "framework").versionRef("checkerFramework") + library("checkerFramework-framework-test", "io.github.eisop", "framework-test").versionRef("checkerFramework") + library("checkerFramework-javacutil", "io.github.eisop", "javacutil").versionRef("checkerFramework") library("errorProne-core", "com.google.errorprone:error_prone_core:2.18.0") library("errorProne-javac", "com.google.errorprone:javac:9+181-r4173-1") library("guava", "com.google.guava:guava:31.1-jre") diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnalysis.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnalysis.java index 31871566..82476abb 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnalysis.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnalysis.java @@ -14,12 +14,11 @@ package com.google.jspecify.nullness; -import java.util.Set; -import javax.lang.model.element.AnnotationMirror; import javax.lang.model.type.TypeMirror; import org.checkerframework.common.basetype.BaseTypeChecker; import org.checkerframework.framework.flow.CFAbstractAnalysis; import org.checkerframework.framework.flow.CFValue; +import org.checkerframework.javacutil.AnnotationMirrorSet; final class NullSpecAnalysis extends CFAbstractAnalysis { NullSpecAnalysis(BaseTypeChecker checker, NullSpecAnnotatedTypeFactory factory) { @@ -37,7 +36,7 @@ public NullSpecStore createCopiedStore(NullSpecStore other) { } @Override - public CFValue createAbstractValue(Set annotations, TypeMirror underlyingType) { + public CFValue createAbstractValue(AnnotationMirrorSet annotations, TypeMirror underlyingType) { return defaultCreateAbstractValue(this, annotations, underlyingType); } } diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index cf7716ab..f09e7cac 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -15,9 +15,7 @@ package com.google.jspecify.nullness; import static com.google.jspecify.nullness.NullSpecAnnotatedTypeFactory.IsDeclaredOrArray.IS_DECLARED_OR_ARRAY; -import static com.google.jspecify.nullness.Util.IMPLEMENTATION_VARIABLE_LOCATIONS; import static com.google.jspecify.nullness.Util.nameMatches; -import static com.sun.source.tree.Tree.Kind.CONDITIONAL_EXPRESSION; import static com.sun.source.tree.Tree.Kind.IDENTIFIER; import static com.sun.source.tree.Tree.Kind.MEMBER_SELECT; import static com.sun.source.tree.Tree.Kind.NOT_EQUAL_TO; @@ -33,15 +31,8 @@ import static javax.lang.model.type.TypeKind.DECLARED; import static javax.lang.model.type.TypeKind.TYPEVAR; import static javax.lang.model.type.TypeKind.WILDCARD; -import static org.checkerframework.framework.qual.TypeUseLocation.CONSTRUCTOR_RESULT; -import static org.checkerframework.framework.qual.TypeUseLocation.EXCEPTION_PARAMETER; -import static org.checkerframework.framework.qual.TypeUseLocation.IMPLICIT_LOWER_BOUND; -import static org.checkerframework.framework.qual.TypeUseLocation.OTHERWISE; -import static org.checkerframework.framework.qual.TypeUseLocation.RECEIVER; import static org.checkerframework.framework.util.AnnotatedTypes.asSuper; -import static org.checkerframework.framework.util.defaults.QualifierDefaults.AdditionalTypeUseLocation.UNBOUNDED_WILDCARD_UPPER_BOUND; import static org.checkerframework.javacutil.AnnotationUtils.areSame; -import static org.checkerframework.javacutil.AnnotationUtils.areSameByName; import static org.checkerframework.javacutil.TreePathUtil.enclosingClass; import static org.checkerframework.javacutil.TreeUtils.elementFromDeclaration; import static org.checkerframework.javacutil.TreeUtils.elementFromUse; @@ -71,7 +62,6 @@ import java.util.Map; import java.util.Set; import java.util.function.Predicate; -import javax.lang.model.AnnotatedConstruct; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.Element; import javax.lang.model.element.ExecutableElement; @@ -83,6 +73,7 @@ import org.checkerframework.common.basetype.BaseTypeChecker; import org.checkerframework.framework.flow.CFAbstractAnalysis; import org.checkerframework.framework.flow.CFValue; +import org.checkerframework.framework.qual.DefaultQualifier; import org.checkerframework.framework.qual.TypeUseLocation; import org.checkerframework.framework.type.AnnotatedTypeFactory; import org.checkerframework.framework.type.AnnotatedTypeFormatter; @@ -114,6 +105,7 @@ import org.checkerframework.framework.util.DefaultQualifierKindHierarchy; import org.checkerframework.framework.util.QualifierKindHierarchy; import org.checkerframework.framework.util.defaults.QualifierDefaults; +import org.checkerframework.javacutil.AnnotationBuilder; final class NullSpecAnnotatedTypeFactory extends GenericAnnotatedTypeFactory< @@ -134,6 +126,25 @@ final class NullSpecAnnotatedTypeFactory final AnnotatedDeclaredType javaLangThreadLocal; final AnnotatedDeclaredType javaUtilMap; + private static final TypeUseLocation[] defaultLocationsMinusNull = + new TypeUseLocation[] { + TypeUseLocation.CONSTRUCTOR_RESULT, + TypeUseLocation.EXCEPTION_PARAMETER, + TypeUseLocation.IMPLICIT_LOWER_BOUND, + TypeUseLocation.RECEIVER, + }; + + private static final TypeUseLocation[] defaultLocationsUnionNull = + new TypeUseLocation[] { + TypeUseLocation.LOCAL_VARIABLE, TypeUseLocation.RESOURCE_VARIABLE, + }; + + private static final TypeUseLocation[] defaultLocationsUnspecified = + new TypeUseLocation[] { + // TypeUseLocation.UNBOUNDED_WILDCARD_UPPER_BOUND, TODO + TypeUseLocation.OTHERWISE + }; + /** Constructor that takes all configuration from the provided {@code checker}. */ NullSpecAnnotatedTypeFactory(BaseTypeChecker checker, Util util) { this(checker, util, checker.hasOption("strict"), /*withOtherWorld=*/ null); @@ -171,6 +182,100 @@ private NullSpecAnnotatedTypeFactory( * recognizing annotations by simple class name instead of by fully qualified name. */ + AnnotationMirror nullMarkedDefaultQualMinusNull = + new AnnotationBuilder(processingEnv, DefaultQualifier.class) + .setValue("value", MinusNull.class) + .setValue( + "locations", + new TypeUseLocation[] { + TypeUseLocation.EXCEPTION_PARAMETER, TypeUseLocation.OTHERWISE + }) + .setValue("applyToSubpackages", false) + .build(); + AnnotationMirror nullMarkedDefaultQualUnionNull = + new AnnotationBuilder(processingEnv, DefaultQualifier.class) + .setValue("value", Nullable.class) + .setValue( + "locations", + new TypeUseLocation[] { + TypeUseLocation.LOCAL_VARIABLE, TypeUseLocation.RESOURCE_VARIABLE, + // TypeUseLocation.UNBOUNDED_WILDCARD_UPPER_BOUND TODO + }) + .setValue("applyToSubpackages", false) + .build(); + AnnotationMirror nullMarkedDefaultQual = + new AnnotationBuilder(processingEnv, DefaultQualifier.List.class) + .setValue( + "value", + new AnnotationMirror[] { + nullMarkedDefaultQualMinusNull, nullMarkedDefaultQualUnionNull + }) + .build(); + + /* + * XXX: When adding support for aliases, make sure to support them here. But consider how to + * handle @Inherited aliases (https://github.com/jspecify/jspecify/issues/155). In particular, we + * have already edited getDeclAnnotations to remove its inheritance logic, and we needed to do so + * to work around another problem (though perhaps we could have found alternatives). + */ + addAliasedDeclAnnotation( + "org.jspecify.annotations.NullMarked", + DefaultQualifier.List.class.getCanonicalName(), + nullMarkedDefaultQual); + addAliasedDeclAnnotation( + "org.jspecify.nullness.NullMarked", + DefaultQualifier.List.class.getCanonicalName(), + nullMarkedDefaultQual); + // TODO: does this work as intended? + /* + * We assume that ProtoNonnullApi is like NullMarked in that it guarantees that *all* types + * are non-null, even those that would require type annotations to annotate (e.g., + * type-parameter bounds). This is probably a safe assumption, if only because such types + * might not arise at all in the generated code where ProtoNonnullApi is used. + */ + addAliasedDeclAnnotation( + "com.google.protobuf.Internal.ProtoNonnullApi", + DefaultQualifier.List.class.getCanonicalName(), + nullMarkedDefaultQual); + + AnnotationMirror nullUnmarkedDefaultQualMinusNull = + new AnnotationBuilder(processingEnv, DefaultQualifier.class) + .setValue("value", MinusNull.class) + .setValue("locations", defaultLocationsMinusNull) + .setValue("applyToSubpackages", false) + .build(); + AnnotationMirror nullUnmarkedDefaultQualUnionNull = + new AnnotationBuilder(processingEnv, DefaultQualifier.class) + .setValue("value", Nullable.class) + .setValue("locations", defaultLocationsUnionNull) + .setValue("applyToSubpackages", false) + .build(); + AnnotationMirror nullUnmarkedDefaultQualUnspecified = + new AnnotationBuilder(processingEnv, DefaultQualifier.class) + .setValue("value", NullnessUnspecified.class) + .setValue("locations", defaultLocationsUnspecified) + .setValue("applyToSubpackages", false) + .build(); + AnnotationMirror nullUnmarkedDefaultQual = + new AnnotationBuilder(processingEnv, DefaultQualifier.List.class) + .setValue( + "value", + new AnnotationMirror[] { + nullUnmarkedDefaultQualMinusNull, + nullUnmarkedDefaultQualUnionNull, + nullUnmarkedDefaultQualUnspecified, + }) + .build(); + + addAliasedDeclAnnotation( + "org.jspecify.annotations.NullUnmarked", + DefaultQualifier.List.class.getCanonicalName(), + nullUnmarkedDefaultQual); + addAliasedDeclAnnotation( + "org.jspecify.nullness.NullUnmarked", + DefaultQualifier.List.class.getCanonicalName(), + nullUnmarkedDefaultQual); + this.isLeastConvenientWorld = isLeastConvenientWorld; javaUtilCollection = createType(util.javaUtilCollectionElement); @@ -227,6 +332,26 @@ private NullSpecAnnotatedTypeFactory( } } + @Override + protected void addCheckedCodeDefaults(QualifierDefaults defs) { + // TODO: add false for subpackages once overload is added to CF. Shouldn't really matter. + defs.addCheckedCodeDefaults(minusNull, defaultLocationsMinusNull); + defs.addCheckedCodeDefaults(unionNull, defaultLocationsUnionNull); + defs.addCheckedCodeDefaults(nullnessOperatorUnspecified, defaultLocationsUnspecified); + } + + @Override + protected void addUncheckedStandardDefaults(QualifierDefaults defs) { + // TODO: figure out whether we need different defaults here + /* + defs.addUncheckedCodeDefaults(minusNull, defaultLocationsMinusNull); + defs.addUncheckedCodeDefaults(unionNull, defaultLocationsUnionNull); + defs.addUncheckedCodeDefaults(nullnessOperatorUnspecified, defaultLocationsUnspecified); + */ + // defs.addUncheckedCodeDefaults(nullnessOperatorUnspecified, new TypeUseLocation[] { + // TypeUseLocation.ALL }); + } + @Override protected Set> createSupportedTypeQualifiers() { return new LinkedHashSet<>(asList(Nullable.class, NullnessUnspecified.class, MinusNull.class)); @@ -240,11 +365,11 @@ protected QualifierHierarchy createQualifierHierarchy() { private final class NullSpecQualifierHierarchy extends NoElementQualifierHierarchy { NullSpecQualifierHierarchy( Collection> qualifierClasses, Elements elements) { - super(qualifierClasses, elements); + super(qualifierClasses, elements, NullSpecAnnotatedTypeFactory.this); } @Override - public boolean isSubtype(AnnotationMirror subAnno, AnnotationMirror superAnno) { + public boolean isSubtypeQualifiers(AnnotationMirror subAnno, AnnotationMirror superAnno) { if (subAnno == null || superAnno == null) { /* * The stock CF never passes null to this method: It always expose *some* annotation @@ -784,131 +909,16 @@ protected void addCheckedStandardDefaults(QualifierDefaults defs) { */ } - @Override - protected void checkForDefaultQualifierInHierarchy(QualifierDefaults defs) { - /* - * We don't set normal checkedCodeDefaults. This method would report that lack of defaults as a - * problem. That's because CF wants to ensure that every[*] type usage is annotated. - * - * However, we *do* ensure that every[*] type usage is annotated. To do so, we always set a - * default for OTHERWISE on top-level elements. (We do this in populateNewDefaults.) See further - * discussion in addCheckedStandardDefaults. - * - * So, we override this method to not report a problem. - * - * [*] There are a few exceptions that we don't need to get into here. - */ - } - @Override protected QualifierDefaults createQualifierDefaults() { return new NullSpecQualifierDefaults(elements, this); } - private final class NullSpecQualifierDefaults extends QualifierDefaults { + private static final class NullSpecQualifierDefaults extends QualifierDefaults { NullSpecQualifierDefaults(Elements elements, AnnotatedTypeFactory atypeFactory) { super(elements, atypeFactory); } - @Override - protected void populateNewDefaults(Element elt, boolean initialDefaultsAreEmpty) { - /* - * Note: This method does not contain the totality of our defaulting logic. For example, our - * TypeAnnotator has special logic for upper bounds _in the case of `super` wildcards - * specifically_. - * - * Note: Setting a default here affects not only this element but also its descendants in the - * syntax tree. - */ - if (hasNullMarkedOrEquivalent(elt)) { - addElementDefault(elt, unionNull, UNBOUNDED_WILDCARD_UPPER_BOUND); - addElementDefault(elt, minusNull, OTHERWISE); - addDefaultToTopForLocationsRefinedByDataflow(elt); - /* - * (For any TypeUseLocation that we don't set an explicit value for, we inherit any value - * from the enclosing element, which might be a non-null-aware element. That's fine: While - * our non-null-aware setup sets defaults for more locations than just these, it sets those - * locations' defaults to minusNull -- matching the value that we want here.) - */ - } else if (hasNullUnmarked(elt) || initialDefaultsAreEmpty) { - /* - * We need to set defaults appropriate to non-null-aware code. In a normal checker, we would - * expect for such "default defaults" to be set in addCheckedStandardDefaults. But we do - * not, as discussed in our implementation of that method. - */ - - // Here's the big default, the "default default": - addElementDefault(elt, nullnessOperatorUnspecified, OTHERWISE); - /* - * OTHERWISE covers anything that does not have a more specific default inherited from an - * enclosing element (or, of course, a more specific default that we set below in this - * method). If this is a @NullUnmarked element, then it might have a had a @NullMarked - * enclosing element, which would have set a default for UNBOUNDED_WILDCARD_UPPER_BOUND. So - * we make sure to override that here. - */ - addElementDefault(elt, nullnessOperatorUnspecified, UNBOUNDED_WILDCARD_UPPER_BOUND); - - // Some locations are intrinsically non-nullable: - addElementDefault(elt, minusNull, CONSTRUCTOR_RESULT); - addElementDefault(elt, minusNull, RECEIVER); - - // We do want *some* of the CLIMB standard defaults: - addDefaultToTopForLocationsRefinedByDataflow(elt); - addElementDefault(elt, minusNull, IMPLICIT_LOWER_BOUND); - - /* - * But note one difference from the CLIMB defaults: We want the default for implicit upper - * bounds to match the "default default" of nullnessOperatorUnspecified, not to be - * top/unionNull. We accomplished this already simply by not making our - * addCheckedStandardDefaults implementation call its supermethod (which would otherwise - * call addClimbStandardDefaults, which would override the "default default"). - */ - } - } - - private void addDefaultToTopForLocationsRefinedByDataflow(Element elt) { - for (TypeUseLocation location : IMPLEMENTATION_VARIABLE_LOCATIONS) { - /* - * Handling exception parameters correctly is hard, so just treat them as if they're - * restricted to non-null values. Of course the caught exception is already non-null, so all - * this does is forbid users from manually assigning null to an exception parameter. - */ - if (location == EXCEPTION_PARAMETER) { - addElementDefault(elt, minusNull, location); - } else { - addElementDefault(elt, unionNull, location); - } - } - } - - @Override - protected boolean shouldAnnotateOtherwiseNonDefaultableTypeVariable(AnnotationMirror qual) { - /* - * CF usually doesn't apply defaults to type-variable usages. But in non-null-aware code, we - * want our default of nullnessOperatorUnspecified to apply even to type variables. - * - * But there are 2 other things to keep in mind: - * - * - CF *does* apply defaults to type-variable usages *if* they are local variables. That's - * because it will refine their types with dataflow. This CF behavior works fine for us: Since - * we want to apply defaults in strictly more cases, we're happy to accept what CF already - * does for local variables. (We do need to be sure to apply unionNull (our top type) in that - * case, rather than nullnessOperatorUnspecified. We accomplish that in - * addDefaultToTopForLocationsRefinedByDataflow.) - * - * - Non-null-aware code (discussed above) is easy: We apply nullnessOperatorUnspecified to - * everything except local variables. But null-aware code more complex. First, set aside local - * variables, which we handle as discussed above. After that, we need to apply minusNull to - * most types, but we need to *not* apply it to (non-local-variable) type-variable usages. - * (For more on this, see isNullExclusiveUnderEveryParameterization.) This need is weird - * enough that stock CF doesn't appear to support it. Our solution is to introduce this hook - * method into our CF fork and then override it here. Our solution also requires that we set - * up defaulting in a non-standard way, as discussed in addCheckedStandardDefaults and other - * locations. - */ - return areSame(qual, nullnessOperatorUnspecified); - } - @Override public boolean applyConservativeDefaults(Element annotationScope) { /* @@ -921,51 +931,6 @@ public boolean applyConservativeDefaults(Element annotationScope) { } } - @Override - protected void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type, boolean iUseFlow) { - super.addComputedTypeAnnotations( - tree, - type, - iUseFlow - /* - * TODO(cpovirk): Eliminate this workaround (which may cause problems of its own). But - * currently, it helps in some of our samples and in Guava, though I am unsure why. The - * problem it works around may well be caused by our failure to keep wildcards' - * annotations in sync with their bounds' annotations (whereas stock CF does). - * - * Note that the `type.getKind() != WILDCARD` check appears to be necessary before the - * CF implementation of capture conversion and unnecessary afterward, while the reverse - * is true of the `isCapturedTypeVariable` check. - */ - && type.getKind() != WILDCARD - && !isCapturedTypeVariable(type.getUnderlyingType()) - /* - * TODO(cpovirk): See if we can remove this workaround after merging the fix for - * https://github.com/typetools/checker-framework/issues/5042. - * - * The workaround becomes necessary after the CF implementation of capture conversion. - * Without it, we see dataflow decide that `b ? null : nullable` has a type of - * _non-null_, as in code like the following: - * - * https://github.com/google/guava/blob/156694066b5198740a820c6eef723fb86c054343/guava/src/com/google/common/base/Throwables.java#L470 - * - * (But I haven't been able to reproduce this in a smaller test.) - * - * Fortunately, I think the workaround is harmless: - * TypeFromExpressionVisitor.visitConditionalExpression calls getAnnotatedType on both - * candidate expressions, and getAnnotatedType applies dataflow. So the ternary should - * end up with the dataflow-correct result by virtue of applying lub to those types. - * - * (I think the only exception would be if someone performed a null check _on an entire - * ternary_ and then expected _another appearance of that same ternary_ to be recognized - * as non-null. That seems implausible.) - * - * (Still, it would be good to look into what's going on here in case it's a sign of a - * deeper problem.) - */ - && tree.getKind() != CONDITIONAL_EXPRESSION); - } - @Override protected TypeAnnotator createTypeAnnotator() { /* @@ -1466,63 +1431,6 @@ protected NullSpecAnalysis createFlowAnalysis() { return new NullSpecAnalysis(checker, this); } - @Override - public void addDefaultAnnotations(AnnotatedTypeMirror type) { - super.addDefaultAnnotations(type); - /* - * TODO(cpovirk): Find a better solution than this. - * - * The problem I'm working around arises during AnnotatedTypes.leastUpperBound on a - * JSpecify-annotated variant of this code: - * https://github.com/google/guava/blob/39aa77fa0e8912d6bfb5cb9a0bc1ed5135747b6f/guava/src/com/google/common/collect/ImmutableMultiset.java#L205 - * - * CF is unable to infer the right type for `LinkedHashMultiset.create(elements)`: It should - * infer `LinkedHashMultiset`, but instead, it infers `LinkedHashMultiset`. As expected, it sets isUninferredTypeArgument. As *not* expected, it gets to - * AtmLubVisitor.lubTypeArgument with type2Wildcard.extendsBound.lowerBound (a null type) - * missing its annotation. - * - * The part of CF responsible for copying annotations, including those on the extends bound, is - * AsSuperVisitor.visitWildcard_Wildcard. Under stock CF, copyPrimaryAnnos(from, typevar) "also - * sets primary annotations _on the bounds_." Under our CF fork, this is not the case, and we - * end up with an unannotated lower bound on the type-variable usage E (which, again, is itself - * a bound of a wildcard). - * - * (Aside: I haven't looked into how the _upper_ bound of the type-variable usage gets an - * annotation set on it. Could it be happening "accidentally," and if so, might it be wrong - * sometimes?) - * - * The result of an unannotated lower bound is a crash in NullSpecQualifierHierarchy.isSubtype, - * which passes null to areSame. - * - * The workaround: If we see a type-variable usage whose lower bound is a null type that lacks - * an annotation, we annotate that bound as non-null. This workaround shouldn't break any - * working code, but it may or may not be universally the right solution to a missing - * annotation. - * - * I am trying to ignore other questions here, such as: - * - * - Would it make more sense to set the lower bound to match the upper bound, as stock CF does? - * I suspect not under our approach, but I haven't thought about it. - * - * - Does trying to pick correct annotations even matter in the context of an uninferred type - * argument? Does the very idea of "correct annotations" lose meaning in that context? - * - * - Should we fix this in AsSuperVisitor instead? Or would it fix itself if we set bounds on - * our type-variable usages and wildcards in the same way that stock CF does? (Following stock - * CF would likely save us from other problems, too.) - * - * - What's up with the _upper_ bound, as discussed in a parenthetical above? - */ - if (type instanceof AnnotatedTypeVariable) { - AnnotatedTypeMirror lowerBound = ((AnnotatedTypeVariable) type).getLowerBound(); - if (lowerBound instanceof AnnotatedNullType - && !lowerBound.isAnnotatedInHierarchy(unionNull)) { - lowerBound.addAnnotation(minusNull); - } - } - } - @Override protected AnnotationFormatter createAnnotationFormatter() { return new DefaultAnnotationFormatter() { @@ -1821,48 +1729,11 @@ protected void addAnnotationsFromDefaultForType(Element element, AnnotatedTypeMi } private void addIfNoAnnotationPresent(AnnotatedTypeMirror type, AnnotationMirror annotation) { - if (!type.isAnnotatedInHierarchy(unionNull)) { + if (!type.hasAnnotationInHierarchy(unionNull)) { type.addAnnotation(annotation); } } - /* - * XXX: When adding support for aliases, make sure to support them here. But consider how to - * handle @Inherited aliases (https://github.com/jspecify/jspecify/issues/155). In particular, we - * have already edited getDeclAnnotations to remove its inheritance logic, and we needed to do so - * to work around another problem (though perhaps we could have found alternatives). - */ - private boolean hasNullMarkedOrEquivalent(Element elt) { - return getDeclAnnotations(elt).stream() - .anyMatch( - am -> - areSameByName(am, "org.jspecify.annotations.NullMarked") - || areSameByName(am, "org.jspecify.nullness.NullMarked")) - /* - * We assume that ProtoNonnullApi is like NullMarked in that it guarantees that *all* types - * are non-null, even those that would require type annotations to annotate (e.g., - * type-parameter bounds). This is probably a safe assumption, if only because such types - * might not arise at all in the generated code where ProtoNonnullApi is used. - */ - || hasAnnotationInCode(elt, "ProtoNonnullApi"); - } - - private boolean hasNullUnmarked(Element elt) { - return getDeclAnnotations(elt).stream() - .anyMatch( - am -> - areSameByName(am, "org.jspecify.annotations.NullUnmarked") - || areSameByName(am, "org.jspecify.nullness.NullUnmarked")); - } - - /** - * Returns whether the given element has an annotation with the given simple name. This method - * does not consider stub files. - */ - private static boolean hasAnnotationInCode(AnnotatedConstruct construct, String name) { - return construct.getAnnotationMirrors().stream().anyMatch(a -> nameMatches(a, name)); - } - @SuppressWarnings("unchecked") // safety guaranteed by API docs private T withMinusNull(T type) { // Remove the annotation from the *root* type, but preserve other annotations. diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecChecker.java b/src/main/java/com/google/jspecify/nullness/NullSpecChecker.java index f89ea601..027ddc74 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecChecker.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecChecker.java @@ -23,7 +23,7 @@ import com.sun.source.util.TreePath; import com.sun.tools.javac.processing.JavacProcessingEnvironment; import com.sun.tools.javac.util.Log; -import java.util.SortedSet; +import java.util.NavigableSet; import java.util.TreeSet; import javax.lang.model.element.TypeElement; import org.checkerframework.common.basetype.BaseTypeChecker; @@ -62,8 +62,8 @@ public final class NullSpecChecker extends BaseTypeChecker { public NullSpecChecker() {} @Override - public SortedSet getSuppressWarningsPrefixes() { - SortedSet prefixes = new TreeSet<>(); + public NavigableSet getSuppressWarningsPrefixes() { + TreeSet prefixes = new TreeSet<>(); prefixes.add("nullness"); return prefixes; } diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecTransfer.java b/src/main/java/com/google/jspecify/nullness/NullSpecTransfer.java index 13f6aefc..ee526c76 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecTransfer.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecTransfer.java @@ -18,7 +18,6 @@ import static com.sun.source.tree.Tree.Kind.NULL_LITERAL; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; -import static java.util.Collections.singleton; import static java.util.Collections.unmodifiableSet; import static java.util.stream.Collectors.toList; import static org.checkerframework.dataflow.expression.JavaExpression.fromNode; @@ -71,6 +70,7 @@ import org.checkerframework.framework.flow.CFValue; import org.checkerframework.framework.type.AnnotatedTypeMirror; import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedDeclaredType; +import org.checkerframework.javacutil.AnnotationMirrorSet; final class NullSpecTransfer extends CFAbstractTransfer { private final Util util; @@ -1019,7 +1019,8 @@ private boolean valueIsAtLeastAsSpecificAs(CFValue value, CFValue targetDataflow if (target == null) { return false; } - return atypeFactory.getQualifierHierarchy().greatestLowerBound(existing, target) == existing; + return atypeFactory.getQualifierHierarchy().greatestLowerBoundQualifiersOnly(existing, target) + == existing; } private static boolean isNullLiteral(Node node) { @@ -1052,7 +1053,8 @@ private void setResultValue( * if (clazz.cast(foo) != null) { return class.cast(foo); } */ result.setResultValue( - analysis.createAbstractValue(singleton(qual), result.getResultValue().getUnderlyingType())); + analysis.createAbstractValue( + AnnotationMirrorSet.singleton(qual), result.getResultValue().getUnderlyingType())); } private JavaExpression expressionToStoreFor(Node node) { diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecVisitor.java b/src/main/java/com/google/jspecify/nullness/NullSpecVisitor.java index f6dba3dc..0f0ce889 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecVisitor.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecVisitor.java @@ -60,9 +60,7 @@ import com.sun.source.tree.TypeParameterTree; import com.sun.source.tree.VariableTree; import java.util.ArrayList; -import java.util.HashSet; import java.util.List; -import java.util.Set; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; @@ -75,6 +73,7 @@ import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedDeclaredType; import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedExecutableType; import org.checkerframework.javacutil.AnnotationBuilder; +import org.checkerframework.javacutil.AnnotationMirrorSet; final class NullSpecVisitor extends BaseTypeVisitor { private final boolean checkImpl; @@ -100,9 +99,11 @@ private void ensureNonNull(Tree tree, String messageKey) { } } + /* TODO: implement feature to add extra args to return type errors. + * @Override protected String extraArgForReturnTypeError(Tree tree) { - /* + / * We call originStringIfTernary, not originString: * * If the statement is `return foo.bar()`, then the problem is obvious, so we don't want our @@ -115,10 +116,11 @@ protected String extraArgForReturnTypeError(Tree tree) { * the possibly null value (possibly both!). However, this gets tricky: If the branches return * `Foo?` and `Foo*`, then we ideally want to emphasize the `Foo?` branch *but*, at least in * "strict mode," not altogether ignore the `Foo*` branch. - */ + / String origin = originStringIfTernary(tree); return origin.isEmpty() ? "" : (origin + "\n"); } + */ private String originString(Tree tree) { while (tree instanceof ParenthesizedTree) { @@ -621,8 +623,8 @@ private boolean isClassCastAppliedToNonNullableType(MemberReferenceTree tree) { } @Override - protected Set getExceptionParameterLowerBoundAnnotations() { - return new HashSet<>(asList(AnnotationBuilder.fromClass(elements, MinusNull.class))); + protected AnnotationMirrorSet getExceptionParameterLowerBoundAnnotations() { + return new AnnotationMirrorSet(asList(AnnotationBuilder.fromClass(elements, MinusNull.class))); } @Override diff --git a/src/test/java/tests/ConformanceTest.java b/src/test/java/tests/ConformanceTest.java index 5789f2b1..e3680fcb 100644 --- a/src/test/java/tests/ConformanceTest.java +++ b/src/test/java/tests/ConformanceTest.java @@ -154,8 +154,8 @@ static final class DetailMessageReportedFact extends ReportedFact { private static final ImmutableSet CANNOT_CONVERT_KEYS = ImmutableSet.of( - "argument", - "assignment", + "argument.type.incompatible", + "assignment.type.incompatible", "atomicreference.must.include.null", "cast.unsafe", "lambda.param", @@ -164,9 +164,9 @@ static final class DetailMessageReportedFact extends ReportedFact { "methodref.return", "override.param", "override.return", - "return", + "return.type.incompatible", "threadlocal.must.include.null", - "type.argument"); + "type.argument.type.incompatible"); private static final ImmutableSet IRRELEVANT_ANNOTATION_KEYS = ImmutableSet.of( diff --git a/src/test/java/tests/NullSpecTest.java b/src/test/java/tests/NullSpecTest.java index fd198187..bb8a8075 100644 --- a/src/test/java/tests/NullSpecTest.java +++ b/src/test/java/tests/NullSpecTest.java @@ -156,20 +156,20 @@ private boolean corresponds(TestDiagnostic missing, DetailMessage unexpected) { || missing.getMessage().contains("jspecify_nullness_mismatch") || missing.getMessage().contains("test:cannot-convert")) { switch (unexpected.messageKey) { - case "argument": - case "assignment": + case "argument.type.incompatible": + case "assignment.type.incompatible": case "atomicreference.must.include.null": case "cast.unsafe": case "dereference": - case "lambda.param": - case "methodref.receiver.bound": - case "methodref.receiver": - case "methodref.return": - case "override.param": - case "override.return": - case "return": + case "lambda.param.type.incompatible": + case "methodref.receiver.bound.invalid": + case "methodref.receiver.invalid": + case "methodref.return.invalid": + case "override.param.invalid": + case "override.return.invalid": + case "return.type.incompatible": case "threadlocal.must.include.null": - case "type.argument": + case "type.argument.type.incompatible": return true; default: return false; @@ -196,7 +196,7 @@ private boolean corresponds(TestDiagnostic missing, DetailMessage unexpected) { * custom `*.annotated` error. This test probably doesn't confirm that second thing * anymore, but I did manually confirm that it is true as of this writing. */ - case "bound": + case "bound.type.incompatible": case "local.variable.annotated": case "type.parameter.annotated": case "wildcard.annotated": diff --git a/tests/ConformanceTest-report.txt b/tests/ConformanceTest-report.txt index 78c664e1..a65b1e52 100644 --- a/tests/ConformanceTest-report.txt +++ b/tests/ConformanceTest-report.txt @@ -1,13 +1,13 @@ -# 53 pass; 61 fail; 114 total; 46.5% score -PASS: Basic.java:28:test:expression-type:Object?:nullable -PASS: Basic.java:28:test:sink-type:Object!:return +# 5 pass; 14 fail; 19 total; 26.3% score +FAIL: Basic.java:28:test:expression-type:Object?:nullable +FAIL: Basic.java:28:test:sink-type:Object!:return PASS: Basic.java:28:test:cannot-convert:Object? to Object! -PASS: Basic.java:34:test:expression-type:Object!:nonNull -PASS: Basic.java:34:test:sink-type:Object?:return -PASS: Basic.java:41:test:sink-type:Object?:nullableObject -PASS: Basic.java:43:test:sink-type:String!:testSinkType#nonNullString +FAIL: Basic.java:34:test:expression-type:Object!:nonNull +FAIL: Basic.java:34:test:sink-type:Object?:return +FAIL: Basic.java:41:test:sink-type:Object?:nullableObject +FAIL: Basic.java:43:test:sink-type:String!:testSinkType#nonNullString FAIL: Basic.java:49:test:expression-type:List!:nullableStrings -PASS: Basic.java: no unexpected facts +FAIL: Basic.java: no unexpected facts PASS: Irrelevant.java:28:test:irrelevant-annotation:Nullable PASS: Irrelevant.java:34:test:irrelevant-annotation:Nullable FAIL: Irrelevant.java:38:test:irrelevant-annotation:NonNull @@ -18,98 +18,3 @@ FAIL: Irrelevant.java:49:test:irrelevant-annotation:NullUnmarked FAIL: Irrelevant.java: no unexpected facts PASS: UsesDep.java:24:test:cannot-convert:null? to Dep* PASS: UsesDep.java: no unexpected facts -PASS: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:Nullable on simple type parameter on method:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:NonNull on simple type parameter on method:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:Nullable on bounded type parameter on method:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:NonNull on bounded type parameter on method:test:irrelevant-annotation:NonNull -FAIL: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:NonNull on annotated-bounded type parameter on method:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:Nullable on annotated-bounded type parameter on method:test:irrelevant-annotation:Nullable -PASS: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:Nullable on simple type parameter on class:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:NonNull on simple type parameter on class:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:Nullable on bounded type parameter on class:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:NonNull on bounded type parameter on class:test:irrelevant-annotation:NonNull -FAIL: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:NonNull on annotated-bounded type parameter on class:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:Nullable on annotated-bounded type parameter on class:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java: no unexpected facts -PASS: irrelevantannotations/notnullmarked/AnnotatedWildcards.java:Nullable on unbounded wildcard:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/notnullmarked/AnnotatedWildcards.java:NonNull on unbounded wildcard:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/notnullmarked/AnnotatedWildcards.java:Nullable on bounded wildcard:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/notnullmarked/AnnotatedWildcards.java:NonNull on bounded wildcard:test:irrelevant-annotation:NonNull -FAIL: irrelevantannotations/notnullmarked/AnnotatedWildcards.java:NonNull on annotated-bounded wildcard:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/notnullmarked/AnnotatedWildcards.java:Nullable on annotated-bounded wildcard:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/notnullmarked/AnnotatedWildcards.java: no unexpected facts -PASS: irrelevantannotations/notnullmarked/Other.java:Nullable local variable object:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/notnullmarked/Other.java:NonNull local variable object:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/notnullmarked/Other.java:Nullable local variable array:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/notnullmarked/Other.java:NonNull local variable array:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/notnullmarked/Other.java:Nullable exception parameter:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/notnullmarked/Other.java:NonNull exception parameter:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/notnullmarked/Other.java:Nullable try-with-resources:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/notnullmarked/Other.java:NonNull try-with-resources:test:irrelevant-annotation:NonNull -FAIL: irrelevantannotations/notnullmarked/Other.java:Nullable exception type:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/notnullmarked/Other.java:NonNull exception type:test:irrelevant-annotation:NonNulll -FAIL: irrelevantannotations/notnullmarked/Other.java: no unexpected facts -PASS: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:Nullable on simple type parameter on method:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:NonNull on simple type parameter on method:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:Nullable on bounded type parameter on method:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:NonNull on bounded type parameter on method:test:irrelevant-annotation:NonNull -FAIL: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:NonNull on annotated-bounded type parameter on method:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:Nullable on annotated-bounded type parameter on method:test:irrelevant-annotation:Nullable -PASS: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:Nullable on simple type parameter on class:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:NonNull on simple type parameter on class:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:Nullable on bounded type parameter on class:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:NonNull on bounded type parameter on class:test:irrelevant-annotation:NonNull -FAIL: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:NonNull on annotated-bounded type parameter on class:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:Nullable on annotated-bounded type parameter on class:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java: no unexpected facts -PASS: irrelevantannotations/nullmarked/AnnotatedWildcards.java:Nullable on unbounded wildcard:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullmarked/AnnotatedWildcards.java:NonNull on unbounded wildcard:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullmarked/AnnotatedWildcards.java:Nullable on bounded wildcard:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullmarked/AnnotatedWildcards.java:NonNull on bounded wildcard:test:irrelevant-annotation:NonNull -FAIL: irrelevantannotations/nullmarked/AnnotatedWildcards.java:NonNull on annotated-bounded wildcard:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullmarked/AnnotatedWildcards.java:Nullable on annotated-bounded wildcard:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullmarked/AnnotatedWildcards.java: no unexpected facts -PASS: irrelevantannotations/nullmarked/Other.java:Nullable local variable object:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullmarked/Other.java:NonNull local variable object:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullmarked/Other.java:Nullable local variable array:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullmarked/Other.java:NonNull local variable array:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullmarked/Other.java:Nullable exception parameter:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullmarked/Other.java:NonNull exception parameter:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullmarked/Other.java:Nullable try-with-resources:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullmarked/Other.java:NonNull try-with-resources:test:irrelevant-annotation:NonNull -FAIL: irrelevantannotations/nullmarked/Other.java:Nullable exception type:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullmarked/Other.java:NonNull exception type:test:irrelevant-annotation:NonNulll -FAIL: irrelevantannotations/nullmarked/Other.java: no unexpected facts -PASS: irrelevantannotations/nullmarked/package-info.java: no unexpected facts -PASS: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:Nullable on simple type parameter on method:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:NonNull on simple type parameter on method:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:Nullable on bounded type parameter on method:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:NonNull on bounded type parameter on method:test:irrelevant-annotation:NonNull -FAIL: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:NonNull on annotated-bounded type parameter on method:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:Nullable on annotated-bounded type parameter on method:test:irrelevant-annotation:Nullable -PASS: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:Nullable on simple type parameter on class:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:NonNull on simple type parameter on class:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:Nullable on bounded type parameter on class:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:NonNull on bounded type parameter on class:test:irrelevant-annotation:NonNull -FAIL: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:NonNull on annotated-bounded type parameter on class:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:Nullable on annotated-bounded type parameter on class:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java: no unexpected facts -PASS: irrelevantannotations/nullunmarked/AnnotatedWildcards.java:Nullable on unbounded wildcard:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullunmarked/AnnotatedWildcards.java:NonNull on unbounded wildcard:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullunmarked/AnnotatedWildcards.java:Nullable on bounded wildcard:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullunmarked/AnnotatedWildcards.java:NonNull on bounded wildcard:test:irrelevant-annotation:NonNull -FAIL: irrelevantannotations/nullunmarked/AnnotatedWildcards.java:NonNull on annotated-bounded wildcard:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullunmarked/AnnotatedWildcards.java:Nullable on annotated-bounded wildcard:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullunmarked/AnnotatedWildcards.java: no unexpected facts -PASS: irrelevantannotations/nullunmarked/Other.java:Nullable local variable object:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullunmarked/Other.java:NonNull local variable object:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullunmarked/Other.java:Nullable local variable array:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullunmarked/Other.java:NonNull local variable array:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullunmarked/Other.java:Nullable exception parameter:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullunmarked/Other.java:NonNull exception parameter:test:irrelevant-annotation:NonNull -PASS: irrelevantannotations/nullunmarked/Other.java:Nullable try-with-resources:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullunmarked/Other.java:NonNull try-with-resources:test:irrelevant-annotation:NonNull -FAIL: irrelevantannotations/nullunmarked/Other.java:Nullable exception type:test:irrelevant-annotation:Nullable -FAIL: irrelevantannotations/nullunmarked/Other.java:NonNull exception type:test:irrelevant-annotation:NonNulll -FAIL: irrelevantannotations/nullunmarked/Other.java: no unexpected facts -PASS: irrelevantannotations/nullunmarked/package-info.java: no unexpected facts diff --git a/tests/ConformanceTestOnSamples-report.txt b/tests/ConformanceTestOnSamples-report.txt index 34f25846..d1dec5af 100644 --- a/tests/ConformanceTestOnSamples-report.txt +++ b/tests/ConformanceTestOnSamples-report.txt @@ -1,4 +1,4 @@ -# 75 pass; 498 fail; 573 total; 13.1% score +# 67 pass; 506 fail; 573 total; 11.7% score FAIL: AnnotatedInnerOfNonParameterized.java: no unexpected facts FAIL: AnnotatedInnerOfParameterized.java: no unexpected facts FAIL: AnnotatedReceiver.java: no unexpected facts @@ -14,15 +14,15 @@ FAIL: ArraySubtype.java: no unexpected facts PASS: AssignmentAsExpression.java: no unexpected facts PASS: AugmentedInferenceAgreesWithBaseInference.java:35:jspecify_nullness_mismatch PASS: AugmentedInferenceAgreesWithBaseInference.java: no unexpected facts -PASS: BoundedTypeVariableReturn.java:27:jspecify_nullness_mismatch -PASS: BoundedTypeVariableReturn.java:32:jspecify_nullness_mismatch +FAIL: BoundedTypeVariableReturn.java:27:jspecify_nullness_mismatch +FAIL: BoundedTypeVariableReturn.java:32:jspecify_nullness_mismatch PASS: BoundedTypeVariableReturn.java: no unexpected facts FAIL: CaptureAsInferredTypeArgument.java:51:jspecify_nullness_mismatch FAIL: CaptureAsInferredTypeArgument.java:53:jspecify_nullness_mismatch FAIL: CaptureAsInferredTypeArgument.java:61:jspecify_nullness_mismatch FAIL: CaptureAsInferredTypeArgument.java:63:jspecify_nullness_mismatch FAIL: CaptureAsInferredTypeArgument.java: no unexpected facts -PASS: CaptureConversionForSubtyping.java: no unexpected facts +FAIL: CaptureConversionForSubtyping.java: no unexpected facts FAIL: CaptureConvertedToObject.java:71:jspecify_nullness_mismatch FAIL: CaptureConvertedToObject.java: no unexpected facts FAIL: CaptureConvertedToObjectUnionNull.java: no unexpected facts @@ -70,11 +70,11 @@ FAIL: CaptureConvertedUnspecToOther.java: no unexpected facts FAIL: CaptureConvertedUnspecToOtherUnionNull.java: no unexpected facts FAIL: CaptureConvertedUnspecToOtherUnspec.java: no unexpected facts PASS: CastOfCaptureOfNotNullMarkedUnboundedWildcardForObjectBoundedTypeParameter.java: no unexpected facts -FAIL: CastOfCaptureOfUnboundedWildcardForNotNullMarkedObjectBoundedTypeParameter.java: no unexpected facts +PASS: CastOfCaptureOfUnboundedWildcardForNotNullMarkedObjectBoundedTypeParameter.java: no unexpected facts PASS: CastOfCaptureOfUnboundedWildcardForObjectBoundedTypeParameter.java: no unexpected facts FAIL: CastToPrimitive.java:33:jspecify_nullness_mismatch FAIL: CastToPrimitive.java: no unexpected facts -PASS: CastWildcardToTypeVariable.java:23:jspecify_nullness_mismatch +FAIL: CastWildcardToTypeVariable.java:23:jspecify_nullness_mismatch PASS: CastWildcardToTypeVariable.java: no unexpected facts PASS: Catch.java: no unexpected facts PASS: ClassLiteral.java: no unexpected facts @@ -115,11 +115,11 @@ FAIL: ContainmentSuper.java: no unexpected facts FAIL: ContainmentSuperVsExtends.java:24:jspecify_nullness_mismatch FAIL: ContainmentSuperVsExtends.java: no unexpected facts FAIL: ContainmentSuperVsExtendsSameType.java:23:jspecify_nullness_mismatch -PASS: ContainmentSuperVsExtendsSameType.java: no unexpected facts -PASS: ContravariantReturns.java:30:jspecify_nullness_mismatch -PASS: ContravariantReturns.java:34:jspecify_nullness_mismatch -PASS: ContravariantReturns.java:38:jspecify_nullness_mismatch -PASS: ContravariantReturns.java: no unexpected facts +FAIL: ContainmentSuperVsExtendsSameType.java: no unexpected facts +FAIL: ContravariantReturns.java:30:jspecify_nullness_mismatch +FAIL: ContravariantReturns.java:34:jspecify_nullness_mismatch +FAIL: ContravariantReturns.java:38:jspecify_nullness_mismatch +FAIL: ContravariantReturns.java: no unexpected facts PASS: CovariantReturns.java: no unexpected facts FAIL: DereferenceClass.java:33:jspecify_nullness_mismatch FAIL: DereferenceClass.java: no unexpected facts @@ -302,8 +302,8 @@ FAIL: NullLiteralToTypeVariable.java: no unexpected facts FAIL: NullLiteralToTypeVariableUnionNull.java: no unexpected facts FAIL: NullLiteralToTypeVariableUnspec.java: no unexpected facts FAIL: NullMarkedDirectUseOfNotNullMarkedBoundedTypeVariable.java: no unexpected facts -FAIL: NullUnmarkedUndoesNullMarked.java: no unexpected facts -FAIL: NullUnmarkedUndoesNullMarkedForWildcards.java: no unexpected facts +PASS: NullUnmarkedUndoesNullMarked.java: no unexpected facts +PASS: NullUnmarkedUndoesNullMarkedForWildcards.java: no unexpected facts PASS: NullnessDoesNotAffectOverloadSelection.java:23:jspecify_nullness_mismatch PASS: NullnessDoesNotAffectOverloadSelection.java: no unexpected facts PASS: ObjectAsSuperOfTypeVariable.java:35:jspecify_nullness_mismatch @@ -313,7 +313,7 @@ PASS: OutOfBoundsTypeVariable.java: no unexpected facts FAIL: OverrideParameters.java:48:jspecify_nullness_mismatch FAIL: OverrideParameters.java:68:jspecify_nullness_mismatch FAIL: OverrideParameters.java: no unexpected facts -PASS: OverrideParametersThatAreTypeVariables.java: no unexpected facts +FAIL: OverrideParametersThatAreTypeVariables.java: no unexpected facts FAIL: OverrideReturns.java:57:jspecify_nullness_mismatch FAIL: OverrideReturns.java: no unexpected facts PASS: ParameterizedWithTypeVariableArgumentToSelf.java: no unexpected facts @@ -542,7 +542,7 @@ PASS: nullnessUnspecifiedTypeParameter/nullnessunspecifiedtypeparameter/Nullness PASS: nullnessUnspecifiedTypeParameter/nullnessunspecifiedtypeparameter/NullnessUnspecifiedTypeParameter.java:53:jspecify_nullness_mismatch PASS: nullnessUnspecifiedTypeParameter/nullnessunspecifiedtypeparameter/NullnessUnspecifiedTypeParameter.java:57:jspecify_nullness_mismatch PASS: nullnessUnspecifiedTypeParameter/nullnessunspecifiedtypeparameter/NullnessUnspecifiedTypeParameter.java:59:jspecify_nullness_mismatch -PASS: nullnessUnspecifiedTypeParameter/nullnessunspecifiedtypeparameter/NullnessUnspecifiedTypeParameter.java: no unexpected facts +FAIL: nullnessUnspecifiedTypeParameter/nullnessunspecifiedtypeparameter/NullnessUnspecifiedTypeParameter.java: no unexpected facts PASS: packageDefault/packagedefault/Bar.java:23:test:cannot-convert:Object? to Object! PASS: packageDefault/packagedefault/Bar.java: no unexpected facts PASS: packageDefault/packagedefault/package-info.java: no unexpected facts From 324d81770488c1a6e3f299e7553127a739704c79 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Tue, 6 Feb 2024 17:12:34 -0500 Subject: [PATCH 02/28] Document that detail messages should not be filtered out (#146) --- src/test/java/tests/ConformanceTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/tests/ConformanceTest.java b/src/test/java/tests/ConformanceTest.java index e3680fcb..a955be23 100644 --- a/src/test/java/tests/ConformanceTest.java +++ b/src/test/java/tests/ConformanceTest.java @@ -143,6 +143,8 @@ private static ImmutableSet analyze( return result.getUnexpectedDiagnostics().stream() .map(d -> DetailMessage.parse(d.getMessage(), testDirectory)) .filter(Objects::nonNull) + // Do not filter out messages without details. + // .filter(DetailMessage::hasDetails) .map(DetailMessageReportedFact::new) .collect(toImmutableSet()); } From 1ef0817d6a94365bcada16c731ae9c6feaeb2071 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Wed, 7 Feb 2024 18:35:42 -0500 Subject: [PATCH 03/28] Update `tests/ConformanceTest-report.txt` (#157) --- tests/ConformanceTest-report.txt | 97 +++++++++++++++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-) diff --git a/tests/ConformanceTest-report.txt b/tests/ConformanceTest-report.txt index a65b1e52..1f0b5c98 100644 --- a/tests/ConformanceTest-report.txt +++ b/tests/ConformanceTest-report.txt @@ -1,4 +1,4 @@ -# 5 pass; 14 fail; 19 total; 26.3% score +# 46 pass; 68 fail; 114 total; 40.4% score FAIL: Basic.java:28:test:expression-type:Object?:nullable FAIL: Basic.java:28:test:sink-type:Object!:return PASS: Basic.java:28:test:cannot-convert:Object? to Object! @@ -18,3 +18,98 @@ FAIL: Irrelevant.java:49:test:irrelevant-annotation:NullUnmarked FAIL: Irrelevant.java: no unexpected facts PASS: UsesDep.java:24:test:cannot-convert:null? to Dep* PASS: UsesDep.java: no unexpected facts +PASS: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:Nullable on simple type parameter on method:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:NonNull on simple type parameter on method:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:Nullable on bounded type parameter on method:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:NonNull on bounded type parameter on method:test:irrelevant-annotation:NonNull +FAIL: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:NonNull on annotated-bounded type parameter on method:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:Nullable on annotated-bounded type parameter on method:test:irrelevant-annotation:Nullable +PASS: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:Nullable on simple type parameter on class:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:NonNull on simple type parameter on class:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:Nullable on bounded type parameter on class:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:NonNull on bounded type parameter on class:test:irrelevant-annotation:NonNull +FAIL: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:NonNull on annotated-bounded type parameter on class:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java:Nullable on annotated-bounded type parameter on class:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/notnullmarked/AnnotatedTypeParameters.java: no unexpected facts +PASS: irrelevantannotations/notnullmarked/AnnotatedWildcards.java:Nullable on unbounded wildcard:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/notnullmarked/AnnotatedWildcards.java:NonNull on unbounded wildcard:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/notnullmarked/AnnotatedWildcards.java:Nullable on bounded wildcard:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/notnullmarked/AnnotatedWildcards.java:NonNull on bounded wildcard:test:irrelevant-annotation:NonNull +FAIL: irrelevantannotations/notnullmarked/AnnotatedWildcards.java:NonNull on annotated-bounded wildcard:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/notnullmarked/AnnotatedWildcards.java:Nullable on annotated-bounded wildcard:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/notnullmarked/AnnotatedWildcards.java: no unexpected facts +PASS: irrelevantannotations/notnullmarked/Other.java:Nullable local variable object:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/notnullmarked/Other.java:NonNull local variable object:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/notnullmarked/Other.java:Nullable local variable array:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/notnullmarked/Other.java:NonNull local variable array:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/notnullmarked/Other.java:Nullable exception parameter:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/notnullmarked/Other.java:NonNull exception parameter:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/notnullmarked/Other.java:Nullable try-with-resources:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/notnullmarked/Other.java:NonNull try-with-resources:test:irrelevant-annotation:NonNull +FAIL: irrelevantannotations/notnullmarked/Other.java:Nullable exception type:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/notnullmarked/Other.java:NonNull exception type:test:irrelevant-annotation:NonNulll +FAIL: irrelevantannotations/notnullmarked/Other.java: no unexpected facts +PASS: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:Nullable on simple type parameter on method:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:NonNull on simple type parameter on method:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:Nullable on bounded type parameter on method:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:NonNull on bounded type parameter on method:test:irrelevant-annotation:NonNull +FAIL: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:NonNull on annotated-bounded type parameter on method:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:Nullable on annotated-bounded type parameter on method:test:irrelevant-annotation:Nullable +PASS: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:Nullable on simple type parameter on class:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:NonNull on simple type parameter on class:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:Nullable on bounded type parameter on class:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:NonNull on bounded type parameter on class:test:irrelevant-annotation:NonNull +FAIL: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:NonNull on annotated-bounded type parameter on class:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java:Nullable on annotated-bounded type parameter on class:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullmarked/AnnotatedTypeParameters.java: no unexpected facts +PASS: irrelevantannotations/nullmarked/AnnotatedWildcards.java:Nullable on unbounded wildcard:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullmarked/AnnotatedWildcards.java:NonNull on unbounded wildcard:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullmarked/AnnotatedWildcards.java:Nullable on bounded wildcard:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullmarked/AnnotatedWildcards.java:NonNull on bounded wildcard:test:irrelevant-annotation:NonNull +FAIL: irrelevantannotations/nullmarked/AnnotatedWildcards.java:NonNull on annotated-bounded wildcard:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullmarked/AnnotatedWildcards.java:Nullable on annotated-bounded wildcard:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullmarked/AnnotatedWildcards.java: no unexpected facts +PASS: irrelevantannotations/nullmarked/Other.java:Nullable local variable object:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullmarked/Other.java:NonNull local variable object:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullmarked/Other.java:Nullable local variable array:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullmarked/Other.java:NonNull local variable array:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullmarked/Other.java:Nullable exception parameter:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullmarked/Other.java:NonNull exception parameter:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullmarked/Other.java:Nullable try-with-resources:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullmarked/Other.java:NonNull try-with-resources:test:irrelevant-annotation:NonNull +FAIL: irrelevantannotations/nullmarked/Other.java:Nullable exception type:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullmarked/Other.java:NonNull exception type:test:irrelevant-annotation:NonNulll +FAIL: irrelevantannotations/nullmarked/Other.java: no unexpected facts +PASS: irrelevantannotations/nullmarked/package-info.java: no unexpected facts +PASS: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:Nullable on simple type parameter on method:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:NonNull on simple type parameter on method:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:Nullable on bounded type parameter on method:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:NonNull on bounded type parameter on method:test:irrelevant-annotation:NonNull +FAIL: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:NonNull on annotated-bounded type parameter on method:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:Nullable on annotated-bounded type parameter on method:test:irrelevant-annotation:Nullable +PASS: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:Nullable on simple type parameter on class:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:NonNull on simple type parameter on class:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:Nullable on bounded type parameter on class:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:NonNull on bounded type parameter on class:test:irrelevant-annotation:NonNull +FAIL: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:NonNull on annotated-bounded type parameter on class:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java:Nullable on annotated-bounded type parameter on class:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullunmarked/AnnotatedTypeParameters.java: no unexpected facts +PASS: irrelevantannotations/nullunmarked/AnnotatedWildcards.java:Nullable on unbounded wildcard:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullunmarked/AnnotatedWildcards.java:NonNull on unbounded wildcard:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullunmarked/AnnotatedWildcards.java:Nullable on bounded wildcard:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullunmarked/AnnotatedWildcards.java:NonNull on bounded wildcard:test:irrelevant-annotation:NonNull +FAIL: irrelevantannotations/nullunmarked/AnnotatedWildcards.java:NonNull on annotated-bounded wildcard:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullunmarked/AnnotatedWildcards.java:Nullable on annotated-bounded wildcard:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullunmarked/AnnotatedWildcards.java: no unexpected facts +PASS: irrelevantannotations/nullunmarked/Other.java:Nullable local variable object:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullunmarked/Other.java:NonNull local variable object:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullunmarked/Other.java:Nullable local variable array:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullunmarked/Other.java:NonNull local variable array:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullunmarked/Other.java:Nullable exception parameter:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullunmarked/Other.java:NonNull exception parameter:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullunmarked/Other.java:Nullable try-with-resources:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullunmarked/Other.java:NonNull try-with-resources:test:irrelevant-annotation:NonNull +FAIL: irrelevantannotations/nullunmarked/Other.java:Nullable exception type:test:irrelevant-annotation:Nullable +FAIL: irrelevantannotations/nullunmarked/Other.java:NonNull exception type:test:irrelevant-annotation:NonNulll +FAIL: irrelevantannotations/nullunmarked/Other.java: no unexpected facts +PASS: irrelevantannotations/nullunmarked/package-info.java: no unexpected facts From e673ee8f7a146b67ddc559ccbecb8ae290a9fb5d Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Wed, 7 Feb 2024 19:24:36 -0500 Subject: [PATCH 04/28] Use standard error format (#154) --- .github/workflows/build.yml | 6 +++--- tests/minimal/Demo.java | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bbfbbd64..bf9af48a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -30,11 +30,11 @@ jobs: run: ./gradlew build conformanceTests demoTest --include-build ../jspecify env: SHALLOW: 1 - - name: Check out jspecify/samples-google-prototype + - name: Check out jspecify/samples-google-prototype-eisop if: always() run: | - git fetch --depth=1 origin samples-google-prototype - git checkout samples-google-prototype + git fetch --depth=1 origin samples-google-prototype-eisop + git checkout samples-google-prototype-eisop working-directory: jspecify - name: Run Samples Tests if: always() diff --git a/tests/minimal/Demo.java b/tests/minimal/Demo.java index cce60abe..f29690cf 100644 --- a/tests/minimal/Demo.java +++ b/tests/minimal/Demo.java @@ -18,7 +18,7 @@ @NullMarked class Demo { Object mismatch(@Nullable Object o) { - // jspecify_nullness_mismatch + // :: error: jspecify_nullness_mismatch return o; } } From 209435e6d9c5504119c6fc280cdfa640685b9f91 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Thu, 8 Feb 2024 09:33:33 -0500 Subject: [PATCH 05/28] Handle both `https` and `git@` clones (#158) Co-authored-by: Chris Povirk --- initialize-project | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/initialize-project b/initialize-project index cd1f9304..77b89d2b 100755 --- a/initialize-project +++ b/initialize-project @@ -55,12 +55,14 @@ git_clone() { local forking_org forking_org="$(forking_org)" - if [[ -n "${forking_org}" ]] && [[ "${forking_org}" != "https://github.com/jspecify" ]]; then + if [[ -n "${forking_org}" ]] \ + && [[ "${forking_org}" != "https://github.com/jspecify" ]] \ + && [[ "${forking_org}" != "git@github.com:jspecify" ]] ; then if run "${git[@]}" "${forking_org}/${repo}.git" "../${repo}"; then return fi fi - if [[ "${repo}" == checker-framework ]]; then + if [[ "${repo}" == "checker-framework" ]]; then forking_org=https://github.com/eisop else forking_org=https://github.com/jspecify From d9e96d04079303966d15a48a7d75ec5a17727736 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Thu, 8 Feb 2024 10:11:14 -0500 Subject: [PATCH 06/28] Use the new `TypeInformationPresenter` to output more type information (#134) --- settings.gradle | 2 +- .../ConformanceTypeInformationPresenter.java | 108 ++++++++++++++++++ .../NullSpecAnnotatedTypeFactory.java | 9 ++ .../jspecify/nullness/NullSpecChecker.java | 3 +- src/test/java/tests/NullSpecTest.java | 2 +- tests/ConformanceTest-report.txt | 16 +-- 6 files changed, 129 insertions(+), 11 deletions(-) create mode 100644 src/main/java/com/google/jspecify/nullness/ConformanceTypeInformationPresenter.java diff --git a/settings.gradle b/settings.gradle index 5734a5f4..30a546bc 100644 --- a/settings.gradle +++ b/settings.gradle @@ -15,7 +15,7 @@ includeBuild("../checker-framework") dependencyResolutionManagement { versionCatalogs { libs { - version("checkerFramework", "3.42.0-eisop2-SNAPSHOT") + version("checkerFramework", "3.42.0-eisop3-SNAPSHOT") library("checkerFramework-checker", "io.github.eisop", "checker").versionRef("checkerFramework") library("checkerFramework-checker-qual", "io.github.eisop", "checker-qual").versionRef("checkerFramework") diff --git a/src/main/java/com/google/jspecify/nullness/ConformanceTypeInformationPresenter.java b/src/main/java/com/google/jspecify/nullness/ConformanceTypeInformationPresenter.java new file mode 100644 index 00000000..83d4da63 --- /dev/null +++ b/src/main/java/com/google/jspecify/nullness/ConformanceTypeInformationPresenter.java @@ -0,0 +1,108 @@ +// Copyright 2024 The JSpecify Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.jspecify.nullness; + +import com.sun.source.tree.AssignmentTree; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.Tree; +import java.util.List; +import javax.lang.model.element.ExecutableElement; +import org.checkerframework.framework.type.AnnotatedTypeFactory; +import org.checkerframework.framework.type.AnnotatedTypeFormatter; +import org.checkerframework.framework.type.AnnotatedTypeMirror; +import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedExecutableType; +import org.checkerframework.framework.util.visualize.AbstractTypeInformationPresenter; +import org.checkerframework.framework.util.visualize.TypeOccurrenceKind; +import org.checkerframework.javacutil.TreeUtils; + +/** + * Output "sinkType" and "sourceType" diagnostic warning messages so the conformance tests can look + * for (a subset of) them. + */ +public final class ConformanceTypeInformationPresenter extends AbstractTypeInformationPresenter { + + /** + * Constructs a presenter for the given factory. + * + * @param atypeFactory the AnnotatedTypeFactory for the current analysis + */ + public ConformanceTypeInformationPresenter(AnnotatedTypeFactory atypeFactory) { + super(atypeFactory); + } + + @Override + protected AnnotatedTypeFormatter createTypeFormatter() { + // Use the same type formatter as normal error messages. Look into whether a different format + // would be better here. + return atypeFactory.getAnnotatedTypeFormatter(); + } + + @Override + protected TypeInformationReporter createTypeInformationReporter(ClassTree tree) { + return new ConformanceTypeInformationReporter(tree); + } + + class ConformanceTypeInformationReporter extends TypeInformationReporter { + ConformanceTypeInformationReporter(ClassTree tree) { + super(tree); + } + + @Override + protected void reportTreeType( + Tree tree, AnnotatedTypeMirror type, TypeOccurrenceKind occurrenceKind) { + switch (tree.getKind()) { + case ASSIGNMENT: + AssignmentTree asgn = (AssignmentTree) tree; + AnnotatedTypeMirror varType = + genFactory != null + ? genFactory.getAnnotatedTypeLhs(asgn.getVariable()) + : atypeFactory.getAnnotatedType(asgn.getVariable()); + checker.reportWarning( + asgn.getVariable(), + "sinkType", + typeFormatter.format(varType), + asgn.getVariable().toString()); + break; + case RETURN: + checker.reportWarning(tree, "sinkType", typeFormatter.format(type), "return"); + break; + case METHOD_INVOCATION: + ExecutableElement calledElem = TreeUtils.elementFromUse((MethodInvocationTree) tree); + String methodName = calledElem.getSimpleName().toString(); + AnnotatedExecutableType calledType = (AnnotatedExecutableType) type; + List params = calledType.getParameterTypes(); + MethodInvocationTree mit = (MethodInvocationTree) tree; + List args = mit.getArguments(); + assert params.size() == args.size(); + + for (int i = 0; i < params.size(); ++i) { + String paramName = calledElem.getParameters().get(i).getSimpleName().toString(); + String paramLocation = String.format("%s#%s", methodName, paramName); + checker.reportWarning( + tree, "sinkType", typeFormatter.format(params.get(i)), paramLocation); + } + break; + default: + // Nothing special for other trees. + } + + if (TreeUtils.isExpressionTree(tree)) { + checker.reportWarning(tree, "sourceType", typeFormatter.format(type), tree.toString()); + } + } + } +} diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index f09e7cac..800a55ce 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -122,6 +122,8 @@ final class NullSpecAnnotatedTypeFactory private final AnnotatedDeclaredType javaUtilCollection; + private final ConformanceTypeInformationPresenter conformanceInformationPresenter; + final AnnotatedDeclaredType javaLangClass; final AnnotatedDeclaredType javaLangThreadLocal; final AnnotatedDeclaredType javaUtilMap; @@ -305,6 +307,9 @@ private NullSpecAnnotatedTypeFactory( withMostConvenientWorld = this; } + conformanceInformationPresenter = + checker.hasOption("showTypes") ? new ConformanceTypeInformationPresenter(this) : null; + if (!givenOtherWorld) { /* * Now the withLeastConvenientWorld and withMostConvenientWorld fields of both `this` and @@ -1707,6 +1712,10 @@ public void postProcessClassTree(ClassTree tree) { * type.invalid.conflicting.annos error, which I have described more in * https://github.com/jspecify/jspecify-reference-checker/commit/d16a0231487e239bc94145177de464b5f77c8b19 */ + + if (conformanceInformationPresenter != null) { + conformanceInformationPresenter.process(tree, getPath(tree)); + } } @Override diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecChecker.java b/src/main/java/com/google/jspecify/nullness/NullSpecChecker.java index 027ddc74..4d490c86 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecChecker.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecChecker.java @@ -39,9 +39,10 @@ *
  • "strict": Whether the checker should be a sound, strict type system. Does not imply that * implementation code is checked. *
  • "checkImpl": Whether implementation code should be checked. + *
  • "showTypes": Whether to output type information for the conformance test suite. * */ -@SupportedOptions({"strict", "checkImpl"}) +@SupportedOptions({"strict", "checkImpl", "showTypes"}) public final class NullSpecChecker extends BaseTypeChecker { /* * A non-final field is ugly, but we can't create our Util instance in the constructor because the diff --git a/src/test/java/tests/NullSpecTest.java b/src/test/java/tests/NullSpecTest.java index bb8a8075..91707656 100644 --- a/src/test/java/tests/NullSpecTest.java +++ b/src/test/java/tests/NullSpecTest.java @@ -206,7 +206,7 @@ private boolean corresponds(TestDiagnostic missing, DetailMessage unexpected) { } case "jspecify_conflicting_annotations": switch (unexpected.messageKey) { - case "conflicting.annos": + case "type.invalid.conflicting.annos": return true; default: return false; diff --git a/tests/ConformanceTest-report.txt b/tests/ConformanceTest-report.txt index 1f0b5c98..78c664e1 100644 --- a/tests/ConformanceTest-report.txt +++ b/tests/ConformanceTest-report.txt @@ -1,13 +1,13 @@ -# 46 pass; 68 fail; 114 total; 40.4% score -FAIL: Basic.java:28:test:expression-type:Object?:nullable -FAIL: Basic.java:28:test:sink-type:Object!:return +# 53 pass; 61 fail; 114 total; 46.5% score +PASS: Basic.java:28:test:expression-type:Object?:nullable +PASS: Basic.java:28:test:sink-type:Object!:return PASS: Basic.java:28:test:cannot-convert:Object? to Object! -FAIL: Basic.java:34:test:expression-type:Object!:nonNull -FAIL: Basic.java:34:test:sink-type:Object?:return -FAIL: Basic.java:41:test:sink-type:Object?:nullableObject -FAIL: Basic.java:43:test:sink-type:String!:testSinkType#nonNullString +PASS: Basic.java:34:test:expression-type:Object!:nonNull +PASS: Basic.java:34:test:sink-type:Object?:return +PASS: Basic.java:41:test:sink-type:Object?:nullableObject +PASS: Basic.java:43:test:sink-type:String!:testSinkType#nonNullString FAIL: Basic.java:49:test:expression-type:List!:nullableStrings -FAIL: Basic.java: no unexpected facts +PASS: Basic.java: no unexpected facts PASS: Irrelevant.java:28:test:irrelevant-annotation:Nullable PASS: Irrelevant.java:34:test:irrelevant-annotation:Nullable FAIL: Irrelevant.java:38:test:irrelevant-annotation:NonNull From 1362fa2f5dba9ce52b8e158be6c8eae2d3fc328c Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Thu, 8 Feb 2024 12:01:11 -0500 Subject: [PATCH 07/28] Use the new `TypeUseLocation.IMPLICIT_WILDCARD_UPPER_BOUND` --- .../nullness/NullSpecAnnotatedTypeFactory.java | 7 ++++--- tests/ConformanceTestOnSamples-report.txt | 14 +++++++------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index 800a55ce..04a438a8 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -143,7 +143,7 @@ final class NullSpecAnnotatedTypeFactory private static final TypeUseLocation[] defaultLocationsUnspecified = new TypeUseLocation[] { - // TypeUseLocation.UNBOUNDED_WILDCARD_UPPER_BOUND, TODO + TypeUseLocation.IMPLICIT_WILDCARD_UPPER_BOUND, TypeUseLocation.OTHERWISE }; @@ -200,8 +200,9 @@ private NullSpecAnnotatedTypeFactory( .setValue( "locations", new TypeUseLocation[] { - TypeUseLocation.LOCAL_VARIABLE, TypeUseLocation.RESOURCE_VARIABLE, - // TypeUseLocation.UNBOUNDED_WILDCARD_UPPER_BOUND TODO + TypeUseLocation.LOCAL_VARIABLE, + TypeUseLocation.RESOURCE_VARIABLE, + TypeUseLocation.IMPLICIT_WILDCARD_UPPER_BOUND }) .setValue("applyToSubpackages", false) .build(); diff --git a/tests/ConformanceTestOnSamples-report.txt b/tests/ConformanceTestOnSamples-report.txt index d1dec5af..c38bcca4 100644 --- a/tests/ConformanceTestOnSamples-report.txt +++ b/tests/ConformanceTestOnSamples-report.txt @@ -1,4 +1,4 @@ -# 67 pass; 506 fail; 573 total; 11.7% score +# 69 pass; 504 fail; 573 total; 12.0% score FAIL: AnnotatedInnerOfNonParameterized.java: no unexpected facts FAIL: AnnotatedInnerOfParameterized.java: no unexpected facts FAIL: AnnotatedReceiver.java: no unexpected facts @@ -14,15 +14,15 @@ FAIL: ArraySubtype.java: no unexpected facts PASS: AssignmentAsExpression.java: no unexpected facts PASS: AugmentedInferenceAgreesWithBaseInference.java:35:jspecify_nullness_mismatch PASS: AugmentedInferenceAgreesWithBaseInference.java: no unexpected facts -FAIL: BoundedTypeVariableReturn.java:27:jspecify_nullness_mismatch -FAIL: BoundedTypeVariableReturn.java:32:jspecify_nullness_mismatch +PASS: BoundedTypeVariableReturn.java:27:jspecify_nullness_mismatch +PASS: BoundedTypeVariableReturn.java:32:jspecify_nullness_mismatch PASS: BoundedTypeVariableReturn.java: no unexpected facts FAIL: CaptureAsInferredTypeArgument.java:51:jspecify_nullness_mismatch FAIL: CaptureAsInferredTypeArgument.java:53:jspecify_nullness_mismatch FAIL: CaptureAsInferredTypeArgument.java:61:jspecify_nullness_mismatch FAIL: CaptureAsInferredTypeArgument.java:63:jspecify_nullness_mismatch FAIL: CaptureAsInferredTypeArgument.java: no unexpected facts -FAIL: CaptureConversionForSubtyping.java: no unexpected facts +PASS: CaptureConversionForSubtyping.java: no unexpected facts FAIL: CaptureConvertedToObject.java:71:jspecify_nullness_mismatch FAIL: CaptureConvertedToObject.java: no unexpected facts FAIL: CaptureConvertedToObjectUnionNull.java: no unexpected facts @@ -70,11 +70,11 @@ FAIL: CaptureConvertedUnspecToOther.java: no unexpected facts FAIL: CaptureConvertedUnspecToOtherUnionNull.java: no unexpected facts FAIL: CaptureConvertedUnspecToOtherUnspec.java: no unexpected facts PASS: CastOfCaptureOfNotNullMarkedUnboundedWildcardForObjectBoundedTypeParameter.java: no unexpected facts -PASS: CastOfCaptureOfUnboundedWildcardForNotNullMarkedObjectBoundedTypeParameter.java: no unexpected facts +FAIL: CastOfCaptureOfUnboundedWildcardForNotNullMarkedObjectBoundedTypeParameter.java: no unexpected facts PASS: CastOfCaptureOfUnboundedWildcardForObjectBoundedTypeParameter.java: no unexpected facts FAIL: CastToPrimitive.java:33:jspecify_nullness_mismatch FAIL: CastToPrimitive.java: no unexpected facts -FAIL: CastWildcardToTypeVariable.java:23:jspecify_nullness_mismatch +PASS: CastWildcardToTypeVariable.java:23:jspecify_nullness_mismatch PASS: CastWildcardToTypeVariable.java: no unexpected facts PASS: Catch.java: no unexpected facts PASS: ClassLiteral.java: no unexpected facts @@ -303,7 +303,7 @@ FAIL: NullLiteralToTypeVariableUnionNull.java: no unexpected facts FAIL: NullLiteralToTypeVariableUnspec.java: no unexpected facts FAIL: NullMarkedDirectUseOfNotNullMarkedBoundedTypeVariable.java: no unexpected facts PASS: NullUnmarkedUndoesNullMarked.java: no unexpected facts -PASS: NullUnmarkedUndoesNullMarkedForWildcards.java: no unexpected facts +FAIL: NullUnmarkedUndoesNullMarkedForWildcards.java: no unexpected facts PASS: NullnessDoesNotAffectOverloadSelection.java:23:jspecify_nullness_mismatch PASS: NullnessDoesNotAffectOverloadSelection.java: no unexpected facts PASS: ObjectAsSuperOfTypeVariable.java:35:jspecify_nullness_mismatch From 4a8bd5bdf02f69a8c1b4a27dc1aef5dac363f0cd Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Mon, 26 Feb 2024 16:07:32 -0500 Subject: [PATCH 08/28] Split out fields for NullMarked default locations, to make it easier to compare against NullUnmarked locations. Use hacky ParametricTypeVariableUse to mark parametric type variable use defaults. --- .../NullSpecAnnotatedTypeFactory.java | 81 +++++++++++++++---- 1 file changed, 64 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index 04a438a8..cd470b29 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -74,6 +74,7 @@ import org.checkerframework.framework.flow.CFAbstractAnalysis; import org.checkerframework.framework.flow.CFValue; import org.checkerframework.framework.qual.DefaultQualifier; +import org.checkerframework.framework.qual.ParametricTypeVariableUse; import org.checkerframework.framework.qual.TypeUseLocation; import org.checkerframework.framework.type.AnnotatedTypeFactory; import org.checkerframework.framework.type.AnnotatedTypeFormatter; @@ -128,6 +129,11 @@ final class NullSpecAnnotatedTypeFactory final AnnotatedDeclaredType javaLangThreadLocal; final AnnotatedDeclaredType javaUtilMap; + // Ensure that all locations that appear in the `defaultLocations` also appear somewhere in the + // `nullMarkedLocations`. + // As defaults are added and removed to the same environment, we need to ensure that all values + // are correctly changed. + private static final TypeUseLocation[] defaultLocationsMinusNull = new TypeUseLocation[] { TypeUseLocation.CONSTRUCTOR_RESULT, @@ -138,14 +144,38 @@ final class NullSpecAnnotatedTypeFactory private static final TypeUseLocation[] defaultLocationsUnionNull = new TypeUseLocation[] { - TypeUseLocation.LOCAL_VARIABLE, TypeUseLocation.RESOURCE_VARIABLE, + TypeUseLocation.IMPLICIT_WILDCARD_UPPER_BOUND_SUPER, + TypeUseLocation.LOCAL_VARIABLE, + TypeUseLocation.RESOURCE_VARIABLE, }; private static final TypeUseLocation[] defaultLocationsUnspecified = new TypeUseLocation[] { - TypeUseLocation.IMPLICIT_WILDCARD_UPPER_BOUND, + TypeUseLocation.IMPLICIT_WILDCARD_UPPER_BOUND_NO_SUPER, + TypeUseLocation.TYPE_VARIABLE_USE, + TypeUseLocation.OTHERWISE + }; + + private static final TypeUseLocation[] nullMarkedLocationsMinusNull = + new TypeUseLocation[] { + TypeUseLocation.CONSTRUCTOR_RESULT, + TypeUseLocation.EXCEPTION_PARAMETER, + TypeUseLocation.IMPLICIT_LOWER_BOUND, + TypeUseLocation.RECEIVER, TypeUseLocation.OTHERWISE }; + private static final TypeUseLocation[] nullMarkedLocationsUnionNull = + new TypeUseLocation[] { + TypeUseLocation.LOCAL_VARIABLE, + TypeUseLocation.RESOURCE_VARIABLE, + TypeUseLocation.IMPLICIT_WILDCARD_UPPER_BOUND_NO_SUPER, + TypeUseLocation.IMPLICIT_WILDCARD_UPPER_BOUND_SUPER, + }; + + private static final TypeUseLocation[] nullMarkedLocationsParametric = + new TypeUseLocation[] {TypeUseLocation.TYPE_VARIABLE_USE}; + + private static final TypeUseLocation[] nullMarkedLocationsUnspecified = new TypeUseLocation[] {}; /** Constructor that takes all configuration from the provided {@code checker}. */ NullSpecAnnotatedTypeFactory(BaseTypeChecker checker, Util util) { @@ -187,23 +217,25 @@ private NullSpecAnnotatedTypeFactory( AnnotationMirror nullMarkedDefaultQualMinusNull = new AnnotationBuilder(processingEnv, DefaultQualifier.class) .setValue("value", MinusNull.class) - .setValue( - "locations", - new TypeUseLocation[] { - TypeUseLocation.EXCEPTION_PARAMETER, TypeUseLocation.OTHERWISE - }) + .setValue("locations", nullMarkedLocationsMinusNull) .setValue("applyToSubpackages", false) .build(); AnnotationMirror nullMarkedDefaultQualUnionNull = new AnnotationBuilder(processingEnv, DefaultQualifier.class) .setValue("value", Nullable.class) - .setValue( - "locations", - new TypeUseLocation[] { - TypeUseLocation.LOCAL_VARIABLE, - TypeUseLocation.RESOURCE_VARIABLE, - TypeUseLocation.IMPLICIT_WILDCARD_UPPER_BOUND - }) + .setValue("locations", nullMarkedLocationsUnionNull) + .setValue("applyToSubpackages", false) + .build(); + AnnotationMirror nullMarkedDefaultQualParametric = + new AnnotationBuilder(processingEnv, DefaultQualifier.class) + .setValue("value", ParametricTypeVariableUse.class) + .setValue("locations", nullMarkedLocationsParametric) + .setValue("applyToSubpackages", false) + .build(); + AnnotationMirror nullMarkedDefaultQualUnspecified = + new AnnotationBuilder(processingEnv, DefaultQualifier.class) + .setValue("value", NullnessUnspecified.class) + .setValue("locations", nullMarkedLocationsUnspecified) .setValue("applyToSubpackages", false) .build(); AnnotationMirror nullMarkedDefaultQual = @@ -211,10 +243,14 @@ private NullSpecAnnotatedTypeFactory( .setValue( "value", new AnnotationMirror[] { - nullMarkedDefaultQualMinusNull, nullMarkedDefaultQualUnionNull + nullMarkedDefaultQualMinusNull, + nullMarkedDefaultQualUnionNull, + nullMarkedDefaultQualParametric, + nullMarkedDefaultQualUnspecified }) .build(); + // System.out.println("nullmarkedDefaultQual" + nullMarkedDefaultQual); /* * XXX: When adding support for aliases, make sure to support them here. But consider how to * handle @Inherited aliases (https://github.com/jspecify/jspecify/issues/155). In particular, we @@ -360,7 +396,12 @@ protected void addUncheckedStandardDefaults(QualifierDefaults defs) { @Override protected Set> createSupportedTypeQualifiers() { - return new LinkedHashSet<>(asList(Nullable.class, NullnessUnspecified.class, MinusNull.class)); + return new LinkedHashSet<>( + asList( + Nullable.class, + NullnessUnspecified.class, + MinusNull.class, + ParametricTypeVariableUse.class)); } @Override @@ -439,10 +480,16 @@ protected Map> createDirectSuper nameToQualifierKind.get(Nullable.class.getCanonicalName()); DefaultQualifierKind nullnessOperatorUnspecified = nameToQualifierKind.get(NullnessUnspecified.class.getCanonicalName()); + DefaultQualifierKind parametricTypeVariableUse = + nameToQualifierKind.get(ParametricTypeVariableUse.class.getCanonicalName()); Map> supers = new HashMap<>(); - supers.put(minusNullKind, singleton(nullnessOperatorUnspecified)); + LinkedHashSet superOfMinusNull = new LinkedHashSet<>(); + superOfMinusNull.add(nullnessOperatorUnspecified); + superOfMinusNull.add(parametricTypeVariableUse); + supers.put(minusNullKind, superOfMinusNull); supers.put(nullnessOperatorUnspecified, singleton(unionNullKind)); + supers.put(parametricTypeVariableUse, singleton(unionNullKind)); supers.put(unionNullKind, emptySet()); return supers; /* From cb1c7b2fc757191f7fd308b055e29322222df6cd Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Mon, 26 Feb 2024 16:08:36 -0500 Subject: [PATCH 09/28] Add test for issue #161 --- tests/minimal/UnboundedDefaultsToNonNull.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 tests/minimal/UnboundedDefaultsToNonNull.java diff --git a/tests/minimal/UnboundedDefaultsToNonNull.java b/tests/minimal/UnboundedDefaultsToNonNull.java new file mode 100644 index 00000000..42d6a076 --- /dev/null +++ b/tests/minimal/UnboundedDefaultsToNonNull.java @@ -0,0 +1,12 @@ +// Test case for Issue 161: +// https://github.com/jspecify/jspecify-reference-checker/issues/161 + +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +@NullMarked +class UnboundedDefaultsToNonNull { + UnboundedDefaultsToNonNull x() { + return this; + } +} From cbebcf253a12e416aed46f8e406ca570d3e9e587 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Mon, 26 Feb 2024 16:08:53 -0500 Subject: [PATCH 10/28] Check out related eisop branch --- initialize-project | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/initialize-project b/initialize-project index 77b89d2b..fbc7c44e 100755 --- a/initialize-project +++ b/initialize-project @@ -72,4 +72,4 @@ git_clone() { git_clone jdk --depth 1 --single-branch -git_clone checker-framework +git_clone checker-framework -b wildcard-defaults From a1a45fce159628716a957eee5bc6664345eccec0 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Mon, 26 Feb 2024 16:22:09 -0500 Subject: [PATCH 11/28] Remove debugging output --- .../google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index cd470b29..274d02cf 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -250,7 +250,6 @@ private NullSpecAnnotatedTypeFactory( }) .build(); - // System.out.println("nullmarkedDefaultQual" + nullMarkedDefaultQual); /* * XXX: When adding support for aliases, make sure to support them here. But consider how to * handle @Inherited aliases (https://github.com/jspecify/jspecify/issues/155). In particular, we From 8eb4ab8280e373c6ec97914a0f1fcf1a62a9e32f Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Mon, 26 Feb 2024 21:14:41 -0500 Subject: [PATCH 12/28] Map `type.invalid.super.wildcard` as an expected error (#166) --- src/test/java/tests/NullSpecTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/tests/NullSpecTest.java b/src/test/java/tests/NullSpecTest.java index 91707656..500e6f4b 100644 --- a/src/test/java/tests/NullSpecTest.java +++ b/src/test/java/tests/NullSpecTest.java @@ -207,6 +207,7 @@ private boolean corresponds(TestDiagnostic missing, DetailMessage unexpected) { case "jspecify_conflicting_annotations": switch (unexpected.messageKey) { case "type.invalid.conflicting.annos": + case "type.invalid.super.wildcard": return true; default: return false; From 04b8d844e08219026f6a8c70a8740e52d7fc90e1 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Mon, 26 Feb 2024 21:32:17 -0500 Subject: [PATCH 13/28] Remove special handling of type variables. Fixes #159. --- .../NullSpecAnnotatedTypeFactory.java | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index 274d02cf..e07de696 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -739,27 +739,6 @@ private boolean nullnessEstablishingPathExists( } private List getUpperBounds(AnnotatedTypeMirror type) { - /* - * In the case of a type-variable usage, we ignore the bounds attached to it in favor of the - * bounds on the type-parameter declaration. This is necessary in certain cases. - * - * I won't claim to understand *why* it will be necessary. Maybe the bounds aren't getting - * copied from the declaration to the usage correctly. Or maybe they're getting copied but then - * CF is mutating/replacing them (since it seems to update *bounds* based on annotations on the - * *usage* in some cases -- though we've tried to short-circuit that). - * - * In any case, in our model for nullness checking, we always want to look at the original - * bounds. So whatever the reason we have different bounds here, we don't want them. - * - * My only worry is that I always worry about making calls to getAnnotatedType, as discussed in - * various comments in this file (e.g., in NullSpecTreeAnnotator.visitMethodInvocation). - */ - if (type instanceof AnnotatedTypeVariable - && !isCapturedTypeVariable(type.getUnderlyingType())) { - AnnotatedTypeVariable variable = (AnnotatedTypeVariable) type; - type = getAnnotatedType(variable.getUnderlyingType().asElement()); - } - switch (type.getKind()) { case INTERSECTION: return ((AnnotatedIntersectionType) type).getBounds(); From 9ee2796e5cd2962f1e8250620528c5b19075d34e Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Tue, 27 Feb 2024 18:06:27 -0500 Subject: [PATCH 14/28] Use `hasEffectiveAnnotation` to handle TVs in bounds. Fixes #164. --- .../NullSpecAnnotatedTypeFactory.java | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index e07de696..c15945eb 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -629,14 +629,14 @@ private boolean isNullnessSubtype(AnnotatedTypeMirror subtype, AnnotatedTypeMirr return true; } if (supertype.getKind() == TYPEVAR - && !supertype.hasAnnotation(minusNull) + && !supertype.hasEffectiveAnnotation(minusNull) && isNullnessSubtype(subtype, ((AnnotatedTypeVariable) supertype).getLowerBound())) { return true; } return isNullInclusiveUnderEveryParameterization(supertype) || isNullExclusiveUnderEveryParameterization(subtype) || (nullnessEstablishingPathExists(subtype, supertype) - && !supertype.hasAnnotation(minusNull)); + && !supertype.hasEffectiveAnnotation(minusNull)); } } @@ -644,7 +644,7 @@ boolean isNullInclusiveUnderEveryParameterization(AnnotatedTypeMirror type) { // We put the third case from the spec first because it's a mouthful. // (As discussed in the spec, we probably don't strictly need this case at all....) if (type.getKind() == TYPEVAR - && !type.hasAnnotation(minusNull) + && !type.hasEffectiveAnnotation(minusNull) && isNullInclusiveUnderEveryParameterization( ((AnnotatedTypeVariable) type).getLowerBound())) { return true; @@ -665,8 +665,8 @@ && isNullInclusiveUnderEveryParameterization( * redundant with the subsequent check on the intersection's components, but redundancy is * harmless. */ - return type.hasAnnotation(unionNull) - || (!isLeastConvenientWorld && type.hasAnnotation(nullnessOperatorUnspecified)); + return type.hasEffectiveAnnotation(unionNull) + || (!isLeastConvenientWorld && type.hasEffectiveAnnotation(nullnessOperatorUnspecified)); } boolean isNullExclusiveUnderEveryParameterization(AnnotatedTypeMirror type) { @@ -719,7 +719,7 @@ private boolean nullnessEstablishingPathExists( * type-variable usages generated by substituteTypeVariable? If so, add a sample input that * demonstrates it. */ - if (subtype.hasAnnotation(minusNull)) { + if (subtype.hasEffectiveAnnotation(minusNull)) { return true; } @@ -767,8 +767,8 @@ private List getUpperBounds(AnnotatedTypeMirror t * isNullInclusiveUnderEveryParameterization. */ private boolean isUnionNullOrEquivalent(AnnotatedTypeMirror type) { - return type.hasAnnotation(unionNull) - || (isLeastConvenientWorld && type.hasAnnotation(nullnessOperatorUnspecified)); + return type.hasEffectiveAnnotation(unionNull) + || (isLeastConvenientWorld && type.hasEffectiveAnnotation(nullnessOperatorUnspecified)); } private final class NullSpecEqualityComparer extends StructuralEqualityComparer { @@ -819,8 +819,8 @@ private boolean areEqual(AnnotatedTypeMirror type1, AnnotatedTypeMirror type2) { * TODO(cpovirk): Even if we're keeping both checks, it seems like _some_ of the code below * may be redundant (or even wrong). */ - boolean type1IsUnspecified = type1.hasAnnotation(nullnessOperatorUnspecified); - boolean type2IsUnspecified = type2.hasAnnotation(nullnessOperatorUnspecified); + boolean type1IsUnspecified = type1.hasEffectiveAnnotation(nullnessOperatorUnspecified); + boolean type2IsUnspecified = type2.hasEffectiveAnnotation(nullnessOperatorUnspecified); boolean bothAreUnspecified = type1IsUnspecified && type2IsUnspecified; boolean eitherIsUnspecified = type1IsUnspecified || type2IsUnspecified; if (isLeastConvenientWorld && bothAreUnspecified) { @@ -885,10 +885,11 @@ protected AnnotatedTypeMirror substituteTypeVariable( */ if (withLeastConvenientWorld().isNullExclusiveUnderEveryParameterization(use)) { substitute.replaceAnnotation(minusNull); - } else if (argument.hasAnnotation(unionNull) || use.hasAnnotation(unionNull)) { + } else if (argument.hasEffectiveAnnotation(unionNull) + || use.hasEffectiveAnnotation(unionNull)) { substitute.replaceAnnotation(unionNull); - } else if (argument.hasAnnotation(nullnessOperatorUnspecified) - || use.hasAnnotation(nullnessOperatorUnspecified)) { + } else if (argument.hasEffectiveAnnotation(nullnessOperatorUnspecified) + || use.hasEffectiveAnnotation(nullnessOperatorUnspecified)) { substitute.replaceAnnotation(nullnessOperatorUnspecified); } From 93b50dc05b7e139fa16a77aa0e54f931cf84244d Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Tue, 27 Feb 2024 22:08:43 -0500 Subject: [PATCH 15/28] Use dedicated `ParametricNull` qualifier --- .../NullSpecAnnotatedTypeFactory.java | 29 ++++++++++++------- .../jspecify/nullness/ParametricNull.java | 27 +++++++++++++++++ .../com/google/jspecify/nullness/Util.java | 2 ++ 3 files changed, 47 insertions(+), 11 deletions(-) create mode 100644 src/main/java/com/google/jspecify/nullness/ParametricNull.java diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index c15945eb..47192f01 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -74,7 +74,6 @@ import org.checkerframework.framework.flow.CFAbstractAnalysis; import org.checkerframework.framework.flow.CFValue; import org.checkerframework.framework.qual.DefaultQualifier; -import org.checkerframework.framework.qual.ParametricTypeVariableUse; import org.checkerframework.framework.qual.TypeUseLocation; import org.checkerframework.framework.type.AnnotatedTypeFactory; import org.checkerframework.framework.type.AnnotatedTypeFormatter; @@ -116,6 +115,7 @@ final class NullSpecAnnotatedTypeFactory private final AnnotationMirror minusNull; private final AnnotationMirror unionNull; private final AnnotationMirror nullnessOperatorUnspecified; + private final AnnotationMirror parametricNull; private final boolean isLeastConvenientWorld; private final NullSpecAnnotatedTypeFactory withLeastConvenientWorld; @@ -200,6 +200,7 @@ private NullSpecAnnotatedTypeFactory( minusNull = util.minusNull; unionNull = util.unionNull; nullnessOperatorUnspecified = util.nullnessOperatorUnspecified; + parametricNull = util.parametricNull; addAliasedTypeAnnotation( "org.jspecify.annotations.NullnessUnspecified", nullnessOperatorUnspecified); @@ -228,7 +229,7 @@ private NullSpecAnnotatedTypeFactory( .build(); AnnotationMirror nullMarkedDefaultQualParametric = new AnnotationBuilder(processingEnv, DefaultQualifier.class) - .setValue("value", ParametricTypeVariableUse.class) + .setValue("value", ParametricNull.class) .setValue("locations", nullMarkedLocationsParametric) .setValue("applyToSubpackages", false) .build(); @@ -396,11 +397,7 @@ protected void addUncheckedStandardDefaults(QualifierDefaults defs) { @Override protected Set> createSupportedTypeQualifiers() { return new LinkedHashSet<>( - asList( - Nullable.class, - NullnessUnspecified.class, - MinusNull.class, - ParametricTypeVariableUse.class)); + asList(Nullable.class, NullnessUnspecified.class, MinusNull.class, ParametricNull.class)); } @Override @@ -479,16 +476,16 @@ protected Map> createDirectSuper nameToQualifierKind.get(Nullable.class.getCanonicalName()); DefaultQualifierKind nullnessOperatorUnspecified = nameToQualifierKind.get(NullnessUnspecified.class.getCanonicalName()); - DefaultQualifierKind parametricTypeVariableUse = - nameToQualifierKind.get(ParametricTypeVariableUse.class.getCanonicalName()); + DefaultQualifierKind parametricNullKind = + nameToQualifierKind.get(ParametricNull.class.getCanonicalName()); Map> supers = new HashMap<>(); LinkedHashSet superOfMinusNull = new LinkedHashSet<>(); superOfMinusNull.add(nullnessOperatorUnspecified); - superOfMinusNull.add(parametricTypeVariableUse); + superOfMinusNull.add(parametricNullKind); supers.put(minusNullKind, superOfMinusNull); supers.put(nullnessOperatorUnspecified, singleton(unionNullKind)); - supers.put(parametricTypeVariableUse, singleton(unionNullKind)); + supers.put(parametricNullKind, singleton(unionNullKind)); supers.put(unionNullKind, emptySet()); return supers; /* @@ -504,6 +501,16 @@ protected Map> createDirectSuper } }; } + + @Override + public AnnotationMirror getParametricQualifier(AnnotationMirror qualifier) { + return parametricNull; + } + + @Override + public boolean isParametricQualifier(AnnotationMirror qualifier) { + return areSame(parametricNull, qualifier); + } } @Override diff --git a/src/main/java/com/google/jspecify/nullness/ParametricNull.java b/src/main/java/com/google/jspecify/nullness/ParametricNull.java new file mode 100644 index 00000000..0da919a5 --- /dev/null +++ b/src/main/java/com/google/jspecify/nullness/ParametricNull.java @@ -0,0 +1,27 @@ +// Copyright 2024 The JSpecify Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.jspecify.nullness; + +import static java.lang.annotation.ElementType.TYPE_USE; + +import java.lang.annotation.Target; +import org.checkerframework.framework.qual.InvisibleQualifier; +import org.checkerframework.framework.qual.ParametricTypeVariableUseQualifier; + +/** Internal implementation detail; not usable in user code. */ +@Target(TYPE_USE) +@InvisibleQualifier +@ParametricTypeVariableUseQualifier +@interface ParametricNull {} diff --git a/src/main/java/com/google/jspecify/nullness/Util.java b/src/main/java/com/google/jspecify/nullness/Util.java index e6b7dc0b..955bceb9 100644 --- a/src/main/java/com/google/jspecify/nullness/Util.java +++ b/src/main/java/com/google/jspecify/nullness/Util.java @@ -51,6 +51,7 @@ final class Util { final AnnotationMirror minusNull; final AnnotationMirror unionNull; final AnnotationMirror nullnessOperatorUnspecified; + final AnnotationMirror parametricNull; final TypeElement javaUtilCollectionElement; final ExecutableElement collectionToArrayNoArgElement; @@ -116,6 +117,7 @@ final class Util { minusNull = AnnotationBuilder.fromClass(e, MinusNull.class); unionNull = AnnotationBuilder.fromClass(e, Nullable.class); nullnessOperatorUnspecified = AnnotationBuilder.fromClass(e, NullnessUnspecified.class); + parametricNull = AnnotationBuilder.fromClass(e, ParametricNull.class); /* * Note that all the above annotations must be on the *classpath*, not just the *processorpath*. * That's because, even if we change fromClass to fromName, AnnotationBuilder ultimately calls From cf2aa1d256ddf3a873bd73cec4760fc9f20b57ca Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Wed, 28 Feb 2024 14:08:37 -0500 Subject: [PATCH 16/28] CF branch was merged --- initialize-project | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/initialize-project b/initialize-project index fbc7c44e..77b89d2b 100755 --- a/initialize-project +++ b/initialize-project @@ -72,4 +72,4 @@ git_clone() { git_clone jdk --depth 1 --single-branch -git_clone checker-framework -b wildcard-defaults +git_clone checker-framework From 859669fbd3d8a43668da07dda3c6ce7f5b3b7564 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Wed, 28 Feb 2024 14:08:55 -0500 Subject: [PATCH 17/28] Specify hierarchy --- src/main/java/com/google/jspecify/nullness/ParametricNull.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/google/jspecify/nullness/ParametricNull.java b/src/main/java/com/google/jspecify/nullness/ParametricNull.java index 0da919a5..0e4c65e9 100644 --- a/src/main/java/com/google/jspecify/nullness/ParametricNull.java +++ b/src/main/java/com/google/jspecify/nullness/ParametricNull.java @@ -23,5 +23,5 @@ /** Internal implementation detail; not usable in user code. */ @Target(TYPE_USE) @InvisibleQualifier -@ParametricTypeVariableUseQualifier +@ParametricTypeVariableUseQualifier(Nullable.class) @interface ParametricNull {} From f73dfbfb093374a9d5b925f884714536117d37b8 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Wed, 28 Feb 2024 14:09:17 -0500 Subject: [PATCH 18/28] Use `hasAnnotation` with `unionNull`, pending further investigation --- .../jspecify/nullness/NullSpecAnnotatedTypeFactory.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index 47192f01..32080b8d 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -672,7 +672,7 @@ && isNullInclusiveUnderEveryParameterization( * redundant with the subsequent check on the intersection's components, but redundancy is * harmless. */ - return type.hasEffectiveAnnotation(unionNull) + return type.hasAnnotation(unionNull) || (!isLeastConvenientWorld && type.hasEffectiveAnnotation(nullnessOperatorUnspecified)); } @@ -774,7 +774,7 @@ private List getUpperBounds(AnnotatedTypeMirror t * isNullInclusiveUnderEveryParameterization. */ private boolean isUnionNullOrEquivalent(AnnotatedTypeMirror type) { - return type.hasEffectiveAnnotation(unionNull) + return type.hasAnnotation(unionNull) || (isLeastConvenientWorld && type.hasEffectiveAnnotation(nullnessOperatorUnspecified)); } @@ -892,8 +892,8 @@ protected AnnotatedTypeMirror substituteTypeVariable( */ if (withLeastConvenientWorld().isNullExclusiveUnderEveryParameterization(use)) { substitute.replaceAnnotation(minusNull); - } else if (argument.hasEffectiveAnnotation(unionNull) - || use.hasEffectiveAnnotation(unionNull)) { + } else if (argument.hasAnnotation(unionNull) + || use.hasAnnotation(unionNull)) { substitute.replaceAnnotation(unionNull); } else if (argument.hasEffectiveAnnotation(nullnessOperatorUnspecified) || use.hasEffectiveAnnotation(nullnessOperatorUnspecified)) { From 41a4ce499c3b9f7440cca68f57934958b1fdf599 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Wed, 28 Feb 2024 14:16:15 -0500 Subject: [PATCH 19/28] Apply spotless --- .../google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index 32080b8d..c33acbf0 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -892,8 +892,7 @@ protected AnnotatedTypeMirror substituteTypeVariable( */ if (withLeastConvenientWorld().isNullExclusiveUnderEveryParameterization(use)) { substitute.replaceAnnotation(minusNull); - } else if (argument.hasAnnotation(unionNull) - || use.hasAnnotation(unionNull)) { + } else if (argument.hasAnnotation(unionNull) || use.hasAnnotation(unionNull)) { substitute.replaceAnnotation(unionNull); } else if (argument.hasEffectiveAnnotation(nullnessOperatorUnspecified) || use.hasEffectiveAnnotation(nullnessOperatorUnspecified)) { From e073d350613439c06483adec471eff1b92554dd4 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Tue, 5 Mar 2024 12:38:48 -0500 Subject: [PATCH 20/28] Adapt a few more error keys --- src/test/java/tests/ConformanceTest.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/test/java/tests/ConformanceTest.java b/src/test/java/tests/ConformanceTest.java index a955be23..a54c9435 100644 --- a/src/test/java/tests/ConformanceTest.java +++ b/src/test/java/tests/ConformanceTest.java @@ -160,12 +160,13 @@ static final class DetailMessageReportedFact extends ReportedFact { "assignment.type.incompatible", "atomicreference.must.include.null", "cast.unsafe", - "lambda.param", - "methodref.receiver.bound", - "methodref.receiver", - "methodref.return", - "override.param", - "override.return", + "lambda.param.type.incompatible", + "methodref.receiver.bound.invalid", + "methodref.receiver.invalid", + "methodref.return.invalid", + "override.param.invalid", + "override.receiver.invalid", + "override.return.invalid", "return.type.incompatible", "threadlocal.must.include.null", "type.argument.type.incompatible"); From 77dc08605423f82865d8933fca461b206f055250 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Tue, 26 Mar 2024 11:44:13 -0400 Subject: [PATCH 21/28] Undo some has[Effective]Annotation changes --- .../nullness/NullSpecAnnotatedTypeFactory.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index c33acbf0..098faf63 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -636,14 +636,14 @@ private boolean isNullnessSubtype(AnnotatedTypeMirror subtype, AnnotatedTypeMirr return true; } if (supertype.getKind() == TYPEVAR - && !supertype.hasEffectiveAnnotation(minusNull) + && !supertype.hasAnnotation(minusNull) && isNullnessSubtype(subtype, ((AnnotatedTypeVariable) supertype).getLowerBound())) { return true; } return isNullInclusiveUnderEveryParameterization(supertype) || isNullExclusiveUnderEveryParameterization(subtype) || (nullnessEstablishingPathExists(subtype, supertype) - && !supertype.hasEffectiveAnnotation(minusNull)); + && !supertype.hasAnnotation(minusNull)); } } @@ -651,7 +651,7 @@ boolean isNullInclusiveUnderEveryParameterization(AnnotatedTypeMirror type) { // We put the third case from the spec first because it's a mouthful. // (As discussed in the spec, we probably don't strictly need this case at all....) if (type.getKind() == TYPEVAR - && !type.hasEffectiveAnnotation(minusNull) + && !type.hasAnnotation(minusNull) && isNullInclusiveUnderEveryParameterization( ((AnnotatedTypeVariable) type).getLowerBound())) { return true; @@ -673,7 +673,7 @@ && isNullInclusiveUnderEveryParameterization( * harmless. */ return type.hasAnnotation(unionNull) - || (!isLeastConvenientWorld && type.hasEffectiveAnnotation(nullnessOperatorUnspecified)); + || (!isLeastConvenientWorld && type.hasAnnotation(nullnessOperatorUnspecified)); } boolean isNullExclusiveUnderEveryParameterization(AnnotatedTypeMirror type) { @@ -726,7 +726,7 @@ private boolean nullnessEstablishingPathExists( * type-variable usages generated by substituteTypeVariable? If so, add a sample input that * demonstrates it. */ - if (subtype.hasEffectiveAnnotation(minusNull)) { + if (subtype.hasAnnotation(minusNull)) { return true; } @@ -775,7 +775,7 @@ private List getUpperBounds(AnnotatedTypeMirror t */ private boolean isUnionNullOrEquivalent(AnnotatedTypeMirror type) { return type.hasAnnotation(unionNull) - || (isLeastConvenientWorld && type.hasEffectiveAnnotation(nullnessOperatorUnspecified)); + || (isLeastConvenientWorld && type.hasAnnotation(nullnessOperatorUnspecified)); } private final class NullSpecEqualityComparer extends StructuralEqualityComparer { @@ -826,8 +826,8 @@ private boolean areEqual(AnnotatedTypeMirror type1, AnnotatedTypeMirror type2) { * TODO(cpovirk): Even if we're keeping both checks, it seems like _some_ of the code below * may be redundant (or even wrong). */ - boolean type1IsUnspecified = type1.hasEffectiveAnnotation(nullnessOperatorUnspecified); - boolean type2IsUnspecified = type2.hasEffectiveAnnotation(nullnessOperatorUnspecified); + boolean type1IsUnspecified = type1.hasAnnotation(nullnessOperatorUnspecified); + boolean type2IsUnspecified = type2.hasAnnotation(nullnessOperatorUnspecified); boolean bothAreUnspecified = type1IsUnspecified && type2IsUnspecified; boolean eitherIsUnspecified = type1IsUnspecified || type2IsUnspecified; if (isLeastConvenientWorld && bothAreUnspecified) { From 752ad0978019cdfdbe6675ffa807a1486e55858e Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Tue, 26 Mar 2024 11:57:36 -0400 Subject: [PATCH 22/28] Rerun conformance tests --- tests/ConformanceTestOnSamples-report.txt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/ConformanceTestOnSamples-report.txt b/tests/ConformanceTestOnSamples-report.txt index c38bcca4..991f4fef 100644 --- a/tests/ConformanceTestOnSamples-report.txt +++ b/tests/ConformanceTestOnSamples-report.txt @@ -1,4 +1,4 @@ -# 69 pass; 504 fail; 573 total; 12.0% score +# 73 pass; 500 fail; 573 total; 12.7% score FAIL: AnnotatedInnerOfNonParameterized.java: no unexpected facts FAIL: AnnotatedInnerOfParameterized.java: no unexpected facts FAIL: AnnotatedReceiver.java: no unexpected facts @@ -116,10 +116,10 @@ FAIL: ContainmentSuperVsExtends.java:24:jspecify_nullness_mismatch FAIL: ContainmentSuperVsExtends.java: no unexpected facts FAIL: ContainmentSuperVsExtendsSameType.java:23:jspecify_nullness_mismatch FAIL: ContainmentSuperVsExtendsSameType.java: no unexpected facts -FAIL: ContravariantReturns.java:30:jspecify_nullness_mismatch -FAIL: ContravariantReturns.java:34:jspecify_nullness_mismatch -FAIL: ContravariantReturns.java:38:jspecify_nullness_mismatch -FAIL: ContravariantReturns.java: no unexpected facts +PASS: ContravariantReturns.java:30:jspecify_nullness_mismatch +PASS: ContravariantReturns.java:34:jspecify_nullness_mismatch +PASS: ContravariantReturns.java:38:jspecify_nullness_mismatch +PASS: ContravariantReturns.java: no unexpected facts PASS: CovariantReturns.java: no unexpected facts FAIL: DereferenceClass.java:33:jspecify_nullness_mismatch FAIL: DereferenceClass.java: no unexpected facts From 88d9c889d95ea6a11ef25c51ee96c57ca27a048e Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Thu, 28 Mar 2024 10:03:07 -0400 Subject: [PATCH 23/28] Output details in conformance tests --- .github/workflows/build.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bf9af48a..013788a3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -30,6 +30,7 @@ jobs: run: ./gradlew build conformanceTests demoTest --include-build ../jspecify env: SHALLOW: 1 + JSPECIFY_CONFORMANCE_TEST_MODE: details - name: Check out jspecify/samples-google-prototype-eisop if: always() run: | From a4d344b88add42d54206d82f0dae79366f519d97 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Tue, 9 Apr 2024 20:13:25 -0400 Subject: [PATCH 24/28] Re-instate work-around to type variable bound issue --- .../NullSpecAnnotatedTypeFactory.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index 098faf63..13c71af7 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -746,6 +746,30 @@ private boolean nullnessEstablishingPathExists( } private List getUpperBounds(AnnotatedTypeMirror type) { + /* + * In the case of a type-variable usage, we ignore the bounds attached to it in favor of the + * bounds on the type-parameter declaration. This is necessary in certain cases. + * + * I won't claim to understand *why* it will be necessary. Maybe the bounds aren't getting + * copied from the declaration to the usage correctly. Or maybe they're getting copied but then + * CF is mutating/replacing them (since it seems to update *bounds* based on annotations on the + * *usage* in some cases -- though we've tried to short-circuit that). + * + * In any case, in our model for nullness checking, we always want to look at the original + * bounds. So whatever the reason we have different bounds here, we don't want them. + * + * My only worry is that I always worry about making calls to getAnnotatedType, as discussed in + * various comments in this file (e.g., in NullSpecTreeAnnotator.visitMethodInvocation). + * + * This is likely caused by https://github.com/eisop/checker-framework/issues/737. + * Revisit this once that issue is fixed. + */ + if (type instanceof AnnotatedTypeVariable + && !isCapturedTypeVariable(type.getUnderlyingType())) { + AnnotatedTypeVariable variable = (AnnotatedTypeVariable) type; + type = getAnnotatedType(variable.getUnderlyingType().asElement()); + } + switch (type.getKind()) { case INTERSECTION: return ((AnnotatedIntersectionType) type).getBounds(); From 5c25a46142b3455ea32a60ce17dfd120266a4e80 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Tue, 9 Apr 2024 20:55:00 -0400 Subject: [PATCH 25/28] Add tests for #159, #161, and #163. --- tests/minimal/Issue159.java | 26 ++++++++++++++++ tests/minimal/Issue163.java | 31 +++++++++++++++++++ tests/minimal/UnboundedDefaultsToNonNull.java | 12 ------- .../minimal/UnboundedDefaultsToNullable.java | 26 ++++++++++++++++ 4 files changed, 83 insertions(+), 12 deletions(-) create mode 100644 tests/minimal/Issue159.java create mode 100644 tests/minimal/Issue163.java delete mode 100644 tests/minimal/UnboundedDefaultsToNonNull.java create mode 100644 tests/minimal/UnboundedDefaultsToNullable.java diff --git a/tests/minimal/Issue159.java b/tests/minimal/Issue159.java new file mode 100644 index 00000000..ab44b010 --- /dev/null +++ b/tests/minimal/Issue159.java @@ -0,0 +1,26 @@ +// Copyright 2024 The JSpecify Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Test case for Issue 159: +// https://github.com/jspecify/jspecify-reference-checker/issues/159 + +import java.util.ArrayList; +import org.jspecify.annotations.NullMarked; + +@NullMarked +class Issue159 extends ArrayList { + Issue159 foo() { + return new Issue159(); + } +} diff --git a/tests/minimal/Issue163.java b/tests/minimal/Issue163.java new file mode 100644 index 00000000..36de896e --- /dev/null +++ b/tests/minimal/Issue163.java @@ -0,0 +1,31 @@ +// Copyright 2024 The JSpecify Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Test case for Issue 163: +// https://github.com/jspecify/jspecify-reference-checker/issues/163 + +import org.jspecify.annotations.NullMarked; + +@NullMarked +class Issue163NullForUnspecVoid { + void x(Issue163Value val, Issue163Visitor vis) { + val.accept(vis, null); + } +} + +interface Issue163Value { +

    void accept(Issue163Visitor

    visitor, P param); +} + +interface Issue163Visitor

    {} diff --git a/tests/minimal/UnboundedDefaultsToNonNull.java b/tests/minimal/UnboundedDefaultsToNonNull.java deleted file mode 100644 index 42d6a076..00000000 --- a/tests/minimal/UnboundedDefaultsToNonNull.java +++ /dev/null @@ -1,12 +0,0 @@ -// Test case for Issue 161: -// https://github.com/jspecify/jspecify-reference-checker/issues/161 - -import org.jspecify.annotations.NullMarked; -import org.jspecify.annotations.Nullable; - -@NullMarked -class UnboundedDefaultsToNonNull { - UnboundedDefaultsToNonNull x() { - return this; - } -} diff --git a/tests/minimal/UnboundedDefaultsToNullable.java b/tests/minimal/UnboundedDefaultsToNullable.java new file mode 100644 index 00000000..6457e3a3 --- /dev/null +++ b/tests/minimal/UnboundedDefaultsToNullable.java @@ -0,0 +1,26 @@ +// Copyright 2024 The JSpecify Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Test case for Issue 161: +// https://github.com/jspecify/jspecify-reference-checker/issues/161 + +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +@NullMarked +class UnboundedDefaultsToNullable { + UnboundedDefaultsToNullable x() { + return this; + } +} From ac7487f74970082a2d19b95a38081aad2147449c Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Tue, 9 Apr 2024 20:55:40 -0400 Subject: [PATCH 26/28] Ensure tests are properly formatted --- build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/build.gradle b/build.gradle index 4b4dea5c..947bb6f4 100644 --- a/build.gradle +++ b/build.gradle @@ -235,6 +235,7 @@ if (!cfQualJar.toFile().exists()) { spotless { java { + target '**/*.java' googleJavaFormat() formatAnnotations() } From 9ba1aa8c0a552ccc3220ad22345c0e48ed1c2c6e Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Tue, 9 Apr 2024 21:09:06 -0400 Subject: [PATCH 27/28] Add test of overrides. --- tests/minimal/OverrideTest.java | 44 +++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 tests/minimal/OverrideTest.java diff --git a/tests/minimal/OverrideTest.java b/tests/minimal/OverrideTest.java new file mode 100644 index 00000000..c34e08cf --- /dev/null +++ b/tests/minimal/OverrideTest.java @@ -0,0 +1,44 @@ +// Copyright 2024 The JSpecify Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Test case based on comment: +// https://github.com/jspecify/jspecify-reference-checker/pull/165#issuecomment-2030038854 + +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +@NullMarked +class OverrideTest { + abstract class Super { + abstract void accept(MarkedEntry entry); + } + + abstract class Sub extends Super { + @Override + abstract void accept(MarkedEntry entry); + } + + interface MarkedEntry {} + + abstract class SuperUnmarked { + abstract void accept(OverrideTestUnmarkedEntry entry); + } + + abstract class SubUnmarked extends SuperUnmarked { + @Override + abstract void accept(OverrideTestUnmarkedEntry entry); + } +} + +interface OverrideTestUnmarkedEntry {} From 2588340d48ce1e94fc906d6f060c6fe069f32444 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Thu, 11 Apr 2024 17:30:27 -0400 Subject: [PATCH 28/28] Move regression tests to `tests/regression`. --- tests/{minimal => regression}/Issue159.java | 0 tests/{minimal => regression}/Issue163.java | 0 tests/{minimal => regression}/OverrideTest.java | 0 tests/{minimal => regression}/UnboundedDefaultsToNullable.java | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename tests/{minimal => regression}/Issue159.java (100%) rename tests/{minimal => regression}/Issue163.java (100%) rename tests/{minimal => regression}/OverrideTest.java (100%) rename tests/{minimal => regression}/UnboundedDefaultsToNullable.java (100%) diff --git a/tests/minimal/Issue159.java b/tests/regression/Issue159.java similarity index 100% rename from tests/minimal/Issue159.java rename to tests/regression/Issue159.java diff --git a/tests/minimal/Issue163.java b/tests/regression/Issue163.java similarity index 100% rename from tests/minimal/Issue163.java rename to tests/regression/Issue163.java diff --git a/tests/minimal/OverrideTest.java b/tests/regression/OverrideTest.java similarity index 100% rename from tests/minimal/OverrideTest.java rename to tests/regression/OverrideTest.java diff --git a/tests/minimal/UnboundedDefaultsToNullable.java b/tests/regression/UnboundedDefaultsToNullable.java similarity index 100% rename from tests/minimal/UnboundedDefaultsToNullable.java rename to tests/regression/UnboundedDefaultsToNullable.java