Skip to content

Commit

Permalink
Use the new TypeUseLocation default locations (#165)
Browse files Browse the repository at this point in the history
  • Loading branch information
wmdietl authored Apr 11, 2024
1 parent 11a2980 commit 76b33c9
Show file tree
Hide file tree
Showing 11 changed files with 251 additions and 35 deletions.
1 change: 1 addition & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ if (!cfQualJar.toFile().exists()) {

spotless {
java {
target '**/*.java'
googleJavaFormat()
formatAnnotations()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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) {
Expand All @@ -170,6 +200,7 @@ private NullSpecAnnotatedTypeFactory(
minusNull = util.minusNull;
unionNull = util.unionNull;
nullnessOperatorUnspecified = util.nullnessOperatorUnspecified;
parametricNull = util.parametricNull;

addAliasedTypeAnnotation(
"org.jspecify.annotations.NullnessUnspecified", nullnessOperatorUnspecified);
Expand All @@ -187,30 +218,36 @@ 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 =
new AnnotationBuilder(processingEnv, DefaultQualifier.List.class)
.setValue(
"value",
new AnnotationMirror[] {
nullMarkedDefaultQualMinusNull, nullMarkedDefaultQualUnionNull
nullMarkedDefaultQualMinusNull,
nullMarkedDefaultQualUnionNull,
nullMarkedDefaultQualParametric,
nullMarkedDefaultQualUnspecified
})
.build();

Expand Down Expand Up @@ -359,7 +396,8 @@ protected void addUncheckedStandardDefaults(QualifierDefaults defs) {

@Override
protected Set<Class<? extends Annotation>> createSupportedTypeQualifiers() {
return new LinkedHashSet<>(asList(Nullable.class, NullnessUnspecified.class, MinusNull.class));
return new LinkedHashSet<>(
asList(Nullable.class, NullnessUnspecified.class, MinusNull.class, ParametricNull.class));
}

@Override
Expand Down Expand Up @@ -438,10 +476,16 @@ protected Map<DefaultQualifierKind, Set<DefaultQualifierKind>> createDirectSuper
nameToQualifierKind.get(Nullable.class.getCanonicalName());
DefaultQualifierKind nullnessOperatorUnspecified =
nameToQualifierKind.get(NullnessUnspecified.class.getCanonicalName());
DefaultQualifierKind parametricNullKind =
nameToQualifierKind.get(ParametricNull.class.getCanonicalName());

Map<DefaultQualifierKind, Set<DefaultQualifierKind>> supers = new HashMap<>();
supers.put(minusNullKind, singleton(nullnessOperatorUnspecified));
LinkedHashSet<DefaultQualifierKind> 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;
/*
Expand All @@ -457,6 +501,16 @@ protected Map<DefaultQualifierKind, Set<DefaultQualifierKind>> createDirectSuper
}
};
}

@Override
public AnnotationMirror getParametricQualifier(AnnotationMirror qualifier) {
return parametricNull;
}

@Override
public boolean isParametricQualifier(AnnotationMirror qualifier) {
return areSame(parametricNull, qualifier);
}
}

@Override
Expand Down Expand Up @@ -706,6 +760,9 @@ private List<? extends AnnotatedTypeMirror> 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())) {
Expand Down Expand Up @@ -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);
}

Expand Down
27 changes: 27 additions & 0 deletions src/main/java/com/google/jspecify/nullness/ParametricNull.java
Original file line number Diff line number Diff line change
@@ -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 {}
2 changes: 2 additions & 0 deletions src/main/java/com/google/jspecify/nullness/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
13 changes: 7 additions & 6 deletions src/test/java/tests/ConformanceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
22 changes: 11 additions & 11 deletions tests/ConformanceTestOnSamples-report.txt
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions tests/regression/Issue159.java
Original file line number Diff line number Diff line change
@@ -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<E> extends ArrayList<E> {
<F> Issue159<F> foo() {
return new Issue159<F>();
}
}
31 changes: 31 additions & 0 deletions tests/regression/Issue163.java
Original file line number Diff line number Diff line change
@@ -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<Void> vis) {
val.accept(vis, null);
}
}

interface Issue163Value {
<P> void accept(Issue163Visitor<P> visitor, P param);
}

interface Issue163Visitor<P> {}
Loading

0 comments on commit 76b33c9

Please sign in to comment.