diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bf9af48..013788a 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: | diff --git a/build.gradle b/build.gradle index 4b4dea5..947bb6f 100644 --- a/build.gradle +++ b/build.gradle @@ -235,6 +235,7 @@ if (!cfQualJar.toFile().exists()) { spotless { java { + target '**/*.java' googleJavaFormat() formatAnnotations() } diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index 800a55c..13c71af 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -115,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; @@ -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.UNBOUNDED_WILDCARD_UPPER_BOUND, TODO + 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) { @@ -170,6 +200,7 @@ private NullSpecAnnotatedTypeFactory( minusNull = util.minusNull; unionNull = util.unionNull; nullnessOperatorUnspecified = util.nullnessOperatorUnspecified; + parametricNull = util.parametricNull; addAliasedTypeAnnotation( "org.jspecify.annotations.NullnessUnspecified", nullnessOperatorUnspecified); @@ -187,22 +218,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.UNBOUNDED_WILDCARD_UPPER_BOUND TODO - }) + .setValue("locations", nullMarkedLocationsUnionNull) + .setValue("applyToSubpackages", false) + .build(); + AnnotationMirror nullMarkedDefaultQualParametric = + new AnnotationBuilder(processingEnv, DefaultQualifier.class) + .setValue("value", ParametricNull.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 = @@ -210,7 +244,10 @@ private NullSpecAnnotatedTypeFactory( .setValue( "value", new AnnotationMirror[] { - nullMarkedDefaultQualMinusNull, nullMarkedDefaultQualUnionNull + nullMarkedDefaultQualMinusNull, + nullMarkedDefaultQualUnionNull, + nullMarkedDefaultQualParametric, + nullMarkedDefaultQualUnspecified }) .build(); @@ -359,7 +396,8 @@ 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, ParametricNull.class)); } @Override @@ -438,10 +476,16 @@ protected Map> createDirectSuper nameToQualifierKind.get(Nullable.class.getCanonicalName()); DefaultQualifierKind nullnessOperatorUnspecified = nameToQualifierKind.get(NullnessUnspecified.class.getCanonicalName()); + DefaultQualifierKind parametricNullKind = + nameToQualifierKind.get(ParametricNull.class.getCanonicalName()); Map> supers = new HashMap<>(); - supers.put(minusNullKind, singleton(nullnessOperatorUnspecified)); + LinkedHashSet superOfMinusNull = new LinkedHashSet<>(); + superOfMinusNull.add(nullnessOperatorUnspecified); + superOfMinusNull.add(parametricNullKind); + supers.put(minusNullKind, superOfMinusNull); supers.put(nullnessOperatorUnspecified, singleton(unionNullKind)); + supers.put(parametricNullKind, singleton(unionNullKind)); supers.put(unionNullKind, emptySet()); return supers; /* @@ -457,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 @@ -706,6 +760,9 @@ private List getUpperBounds(AnnotatedTypeMirror t * * 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())) { @@ -861,8 +918,8 @@ protected AnnotatedTypeMirror substituteTypeVariable( substitute.replaceAnnotation(minusNull); } else if (argument.hasAnnotation(unionNull) || use.hasAnnotation(unionNull)) { substitute.replaceAnnotation(unionNull); - } else if (argument.hasAnnotation(nullnessOperatorUnspecified) - || use.hasAnnotation(nullnessOperatorUnspecified)) { + } else if (argument.hasEffectiveAnnotation(nullnessOperatorUnspecified) + || use.hasEffectiveAnnotation(nullnessOperatorUnspecified)) { substitute.replaceAnnotation(nullnessOperatorUnspecified); } 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 0000000..0e4c65e --- /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(Nullable.class) +@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 e6b7dc0..955bceb 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 diff --git a/src/test/java/tests/ConformanceTest.java b/src/test/java/tests/ConformanceTest.java index edfb67c..c7e2cb7 100644 --- a/src/test/java/tests/ConformanceTest.java +++ b/src/test/java/tests/ConformanceTest.java @@ -161,12 +161,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"); diff --git a/tests/ConformanceTestOnSamples-report.txt b/tests/ConformanceTestOnSamples-report.txt index d1dec5a..991f4fe 100644 --- a/tests/ConformanceTestOnSamples-report.txt +++ b/tests/ConformanceTestOnSamples-report.txt @@ -1,4 +1,4 @@ -# 67 pass; 506 fail; 573 total; 11.7% 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 @@ -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 @@ -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 @@ -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 diff --git a/tests/regression/Issue159.java b/tests/regression/Issue159.java new file mode 100644 index 0000000..ab44b01 --- /dev/null +++ b/tests/regression/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/regression/Issue163.java b/tests/regression/Issue163.java new file mode 100644 index 0000000..36de896 --- /dev/null +++ b/tests/regression/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/regression/OverrideTest.java b/tests/regression/OverrideTest.java new file mode 100644 index 0000000..c34e08c --- /dev/null +++ b/tests/regression/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 {} diff --git a/tests/regression/UnboundedDefaultsToNullable.java b/tests/regression/UnboundedDefaultsToNullable.java new file mode 100644 index 0000000..6457e3a --- /dev/null +++ b/tests/regression/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; + } +}