Skip to content

Commit e1069a2

Browse files
wmdietlwmdietl
authored
Introduce a NullSpecTypeValidator to ensure unspecified type variables are ok (#178)
1 parent 39764fb commit e1069a2

File tree

7 files changed

+90
-5
lines changed

7 files changed

+90
-5
lines changed

‎src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ final class NullSpecAnnotatedTypeFactory
145145
new TypeUseLocation[] {
146146
TypeUseLocation.CONSTRUCTOR_RESULT,
147147
TypeUseLocation.EXCEPTION_PARAMETER,
148+
TypeUseLocation.IMPLICIT_LOWER_BOUND,
148149
TypeUseLocation.RECEIVER,
149150
};
150151

@@ -157,10 +158,6 @@ final class NullSpecAnnotatedTypeFactory
157158

158159
private static final TypeUseLocation[] defaultLocationsUnspecified =
159160
new TypeUseLocation[] {
160-
// Lower bounds could be MinusNull, but all uses in unmarked code would become unspecified
161-
// anyways.
162-
// Revisit once https://github.com/eisop/checker-framework/issues/741 is fixed.
163-
TypeUseLocation.IMPLICIT_LOWER_BOUND,
164161
TypeUseLocation.IMPLICIT_WILDCARD_UPPER_BOUND_NO_SUPER,
165162
TypeUseLocation.TYPE_VARIABLE_USE,
166163
TypeUseLocation.OTHERWISE

‎src/main/java/com/google/jspecify/nullness/NullSpecTypeValidator.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Copyright 2020 The JSpecify Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.jspecify.nullness;
16+
17+
import javax.lang.model.element.AnnotationMirror;
18+
import org.checkerframework.common.basetype.BaseTypeChecker;
19+
import org.checkerframework.common.basetype.BaseTypeValidator;
20+
import org.checkerframework.framework.type.AnnotatedTypeMirror;
21+
22+
final class NullSpecTypeValidator extends BaseTypeValidator {
23+
private final AnnotationMirror nullnessOperatorUnspecified;
24+
25+
/** Constructor. */
26+
NullSpecTypeValidator(
27+
BaseTypeChecker checker,
28+
NullSpecVisitor visitor,
29+
NullSpecAnnotatedTypeFactory atypeFactory,
30+
Util util) {
31+
super(checker, visitor, atypeFactory);
32+
33+
nullnessOperatorUnspecified = util.nullnessOperatorUnspecified;
34+
}
35+
36+
@Override
37+
public boolean areBoundsValid(AnnotatedTypeMirror upperBound, AnnotatedTypeMirror lowerBound) {
38+
if (upperBound.hasAnnotation(nullnessOperatorUnspecified)
39+
|| lowerBound.hasAnnotation(nullnessOperatorUnspecified)) {
40+
return true;
41+
} else {
42+
return super.areBoundsValid(upperBound, lowerBound);
43+
}
44+
}
45+
}

‎src/main/java/com/google/jspecify/nullness/NullSpecVisitor.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
import javax.lang.model.type.DeclaredType;
7272
import javax.lang.model.type.TypeMirror;
7373
import org.checkerframework.common.basetype.BaseTypeVisitor;
74+
import org.checkerframework.common.basetype.TypeValidator;
7475
import org.checkerframework.framework.type.AnnotatedTypeMirror;
7576
import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedDeclaredType;
7677
import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedExecutableType;
@@ -637,4 +638,9 @@ protected NullSpecAnnotatedTypeFactory createTypeFactory() {
637638
// Reading util this way is ugly but necessary. See discussion in NullSpecChecker.
638639
return new NullSpecAnnotatedTypeFactory(checker, ((NullSpecChecker) checker).util);
639640
}
641+
642+
@Override
643+
protected TypeValidator createTypeValidator() {
644+
return new NullSpecTypeValidator(checker, this, atypeFactory, ((NullSpecChecker) checker).util);
645+
}
640646
}

‎src/test/java/tests/NullSpecTest.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,18 @@ public static String[] getTestDirs() {
5454
}
5555
}
5656

57+
/** A small set of strict regression tests. */
58+
public static class RegressionStrict extends NullSpecTest {
59+
public RegressionStrict(List<File> testFiles) {
60+
super(testFiles, true);
61+
}
62+
63+
@Parameters
64+
public static String[] getTestDirs() {
65+
return new String[] {"regression-strict"};
66+
}
67+
}
68+
5769
/** A test that ignores cases where there is limited nullness information. */
5870
public static class Lenient extends NullSpecTest {
5971
public Lenient(List<File> testFiles) {

‎tests/ConformanceTest-report.txt‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# 90 pass; 24 fail; 114 total; 78.9% score
1+
# 93 pass; 24 fail; 117 total; 79.5% score
22
PASS: Basic.java:28:test:expression-type:Object?:nullable
33
PASS: Basic.java:28:test:sink-type:Object!:return
44
PASS: Basic.java:28:test:cannot-convert:Object? to Object!
@@ -44,6 +44,7 @@ PASS: irrelevantannotations/notnullmarked/Other.java:Nullable local variable arr
4444
PASS: irrelevantannotations/notnullmarked/Other.java:NonNull local variable array:test:irrelevant-annotation:NonNull
4545
PASS: irrelevantannotations/notnullmarked/Other.java:Nullable exception parameter:test:irrelevant-annotation:Nullable
4646
PASS: irrelevantannotations/notnullmarked/Other.java:NonNull exception parameter:test:irrelevant-annotation:NonNull
47+
PASS: irrelevantannotations/notnullmarked/Other.java:Intrinsically NonNull exception parameter cannot be assigned null:test:cannot-convert:null? to RuntimeException!
4748
PASS: irrelevantannotations/notnullmarked/Other.java:Nullable try-with-resources:test:irrelevant-annotation:Nullable
4849
FAIL: irrelevantannotations/notnullmarked/Other.java:NonNull try-with-resources:test:irrelevant-annotation:NonNull
4950
FAIL: irrelevantannotations/notnullmarked/Other.java:Nullable exception type:test:irrelevant-annotation:Nullable
@@ -75,6 +76,7 @@ PASS: irrelevantannotations/nullmarked/Other.java:Nullable local variable array:
7576
PASS: irrelevantannotations/nullmarked/Other.java:NonNull local variable array:test:irrelevant-annotation:NonNull
7677
PASS: irrelevantannotations/nullmarked/Other.java:Nullable exception parameter:test:irrelevant-annotation:Nullable
7778
PASS: irrelevantannotations/nullmarked/Other.java:NonNull exception parameter:test:irrelevant-annotation:NonNull
79+
PASS: irrelevantannotations/nullmarked/Other.java:Intrinsically NonNull exception parameter cannot be assigned null:test:cannot-convert:null? to RuntimeException!
7880
PASS: irrelevantannotations/nullmarked/Other.java:Nullable try-with-resources:test:irrelevant-annotation:Nullable
7981
FAIL: irrelevantannotations/nullmarked/Other.java:NonNull try-with-resources:test:irrelevant-annotation:NonNull
8082
FAIL: irrelevantannotations/nullmarked/Other.java:Nullable exception type:test:irrelevant-annotation:Nullable
@@ -107,6 +109,7 @@ PASS: irrelevantannotations/nullunmarked/Other.java:Nullable local variable arra
107109
PASS: irrelevantannotations/nullunmarked/Other.java:NonNull local variable array:test:irrelevant-annotation:NonNull
108110
PASS: irrelevantannotations/nullunmarked/Other.java:Nullable exception parameter:test:irrelevant-annotation:Nullable
109111
PASS: irrelevantannotations/nullunmarked/Other.java:NonNull exception parameter:test:irrelevant-annotation:NonNull
112+
PASS: irrelevantannotations/nullunmarked/Other.java:Intrinsically NonNull exception parameter cannot be assigned null:test:cannot-convert:null? to RuntimeException!
110113
PASS: irrelevantannotations/nullunmarked/Other.java:Nullable try-with-resources:test:irrelevant-annotation:Nullable
111114
FAIL: irrelevantannotations/nullunmarked/Other.java:NonNull try-with-resources:test:irrelevant-annotation:NonNull
112115
FAIL: irrelevantannotations/nullunmarked/Other.java:Nullable exception type:test:irrelevant-annotation:Nullable

‎tests/regression-strict/Issue177.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright 2024 The JSpecify Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
// Test case for Issue 177:
16+
// https://github.com/jspecify/jspecify-reference-checker/issues/177
17+
18+
class Issue177<T> {}

‎tests/regression/Issue172.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ class Issue172<E> {
2121
E e() {
2222
return null;
2323
}
24+
25+
void p(E p) {
26+
p = null;
27+
}
2428
}
2529

2630
class Issue172UnmarkedUse {

0 commit comments

Comments
 (0)