Skip to content

Commit 5be83f4

Browse files
lahodajlahodaj
committed
Support for type variables and wildcards.
1 parent ecfac7c commit 5be83f4

2 files changed

Lines changed: 212 additions & 24 deletions

File tree

‎java/java.hints/src/org/netbeans/modules/java/hints/bugs/NPECheck.java‎

Lines changed: 47 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@
4949
import javax.lang.model.type.ExecutableType;
5050
import javax.lang.model.type.TypeKind;
5151
import javax.lang.model.type.TypeMirror;
52+
import javax.lang.model.type.TypeVariable;
53+
import javax.lang.model.type.WildcardType;
5254
import javax.lang.model.util.ElementFilter;
5355
import org.netbeans.api.annotations.common.CheckForNull;
5456
import org.netbeans.api.java.source.CompilationInfo;
@@ -88,11 +90,6 @@ public class NPECheck {
8890
})
8991
public static ErrorDescription assignment(HintContext ctx) {
9092
Element e = ctx.getInfo().getTrees().getElement(ctx.getVariables().get("$var"));
91-
92-
if (!isVariableElement(ctx, e)) {
93-
return null;
94-
}
95-
9693
TreePath expr = ctx.getVariables().get("$expr");
9794
StateEnum r = computeExpressionsState(ctx).getOrDefault(expr.getLeaf(), DEFAULT_STATE).thisTypeState;
9895

@@ -459,7 +456,7 @@ private static boolean hasNull(HintContext ctx, BinaryTree bt) {
459456
@TriggerPattern("return $expression;")
460457
public static ErrorDescription returnNull(HintContext ctx) {
461458
TreePath expression = ctx.getVariables().get("$expression");
462-
StateEnum returnState = computeExpressionsState(ctx).getOrDefault(expression.getLeaf(), DEFAULT_STATE).thisTypeState;
459+
State returnState = computeExpressionsState(ctx).getOrDefault(expression.getLeaf(), DEFAULT_STATE);
463460

464461
if (returnState == null) return null;
465462

@@ -483,16 +480,12 @@ public static ErrorDescription returnNull(HintContext ctx) {
483480
if (el == null || el.getKind() != ElementKind.METHOD) return null;
484481

485482
State expected = getStateFromAnnotations(info, el);
486-
String key = null;
487-
488-
switch (returnState) {
489-
case NULL:
490-
if (expected.isNotNull()) key = "ERR_ReturningNullFromNonNull";
491-
break;
492-
case POSSIBLE_NULL_REPORT:
493-
if (expected.isNotNull()) key = "ERR_ReturningPossibleNullFromNonNull";
494-
break;
495-
}
483+
String key = switch (statesMatch(expected, returnState)) {
484+
case TOP_LEVEL_NULL_TO_NONNULL -> "ERR_ReturningNullFromNonNull";
485+
case TOP_LEVEL_POSSIBLE_NULL_TO_NONNULL -> "ERR_ReturningPossibleNullFromNonNull";
486+
case MISMATCH -> "ERR_TYPES_MISMATCH";
487+
default -> null;
488+
};
496489

497490
if (key != null) {
498491
String displayName = NbBundle.getMessage(NPECheck.class, key);
@@ -633,6 +626,7 @@ private static State getStateFromAnnotations(CompilationInfo info, Element e) {
633626
//XXX:
634627
//- should include with OVERRIDE_ANNOTATIONS?
635628
//- adjust default(!)
629+
//- should really ignore the defaults for local variables??? especially at validation time?
636630
result = getStateFromAnnotations(info, e.asType(), x -> null,
637631
LOCAL_VARIABLES.contains(e.getKind()) ? StateEnum.POSSIBLE_NULL : typeDefault, typeDefault);
638632
} else if (e.getKind() == ElementKind.METHOD) {
@@ -660,13 +654,42 @@ private static State getStateFromAnnotations(CompilationInfo info, TypeMirror ty
660654
}
661655

662656
private static State getStateFromAnnotations(CompilationInfo info, TypeMirror type, Function<TypeMirror, State> type2StateMapper, StateEnum topLevelFallbackState, StateEnum fallbackState) {
657+
return getStateFromAnnotations(info, type, type2StateMapper, topLevelFallbackState, fallbackState, true);
658+
}
659+
660+
private static State getStateFromAnnotations(CompilationInfo info, TypeMirror type, Function<TypeMirror, State> type2StateMapper, StateEnum topLevelFallbackState, StateEnum fallbackState, boolean recurseToGenericTypes) {
663661
State state = type2StateMapper.apply(type);
664662

665663
if (state != null) {
666664
//TODO: should presumably merge with other aspects?
667665
return state;
668666
}
669667

668+
if (recurseToGenericTypes) {
669+
if (type.getKind() == TypeKind.TYPEVAR) {
670+
StateEnum thisTypeState = getStateFromAnnotations(type.getAnnotationMirrors(), StateEnum.POSSIBLE_NULL);
671+
State fromBound = getStateFromAnnotations(info, ((TypeVariable) type).getUpperBound(), type2StateMapper, topLevelFallbackState, fallbackState);
672+
673+
if (thisTypeState != StateEnum.POSSIBLE_NULL) {
674+
return fromBound.setThisState(thisTypeState);
675+
} else {
676+
return fromBound;
677+
}
678+
} else if (type.getKind() == TypeKind.WILDCARD) {
679+
WildcardType wt = (WildcardType) type;
680+
TypeMirror base;
681+
if (wt.getExtendsBound() != null) {
682+
base = wt.getExtendsBound();
683+
} else if (wt.getSuperBound() != null) {
684+
base = wt.getSuperBound();
685+
} else {
686+
return new State(StateEnum.POSSIBLE_NULL);
687+
}
688+
689+
return getStateFromAnnotations(info, base, type2StateMapper, topLevelFallbackState, fallbackState);
690+
}
691+
}
692+
670693
StateEnum thisTypeState = getStateFromAnnotations(type.getAnnotationMirrors(), topLevelFallbackState);
671694
List<State> typeParameters = null;
672695
State arrayComponentState = null;
@@ -676,12 +699,12 @@ private static State getStateFromAnnotations(CompilationInfo info, TypeMirror ty
676699

677700
typeParameters = dt.getTypeArguments()
678701
.stream()
679-
.map(ta -> getStateFromAnnotations(info, ta, type2StateMapper, fallbackState))
702+
.map(ta -> getStateFromAnnotations(info, ta, type2StateMapper, fallbackState, fallbackState, recurseToGenericTypes))
680703
.toList();
681704
} else if (type.getKind() == TypeKind.ARRAY) {
682705
ArrayType at = (ArrayType) type;
683706

684-
arrayComponentState = getStateFromAnnotations(info, at.getComponentType(), type2StateMapper, fallbackState);
707+
arrayComponentState = getStateFromAnnotations(info, at.getComponentType(), type2StateMapper, fallbackState, fallbackState, recurseToGenericTypes);
685708
}
686709

687710
return new State(thisTypeState, typeParameters, arrayComponentState);
@@ -1263,9 +1286,12 @@ public State visitMethodInvocation(MethodInvocationTree node, Void p) {
12631286
TypeMirror instantiatedReturnType = ((ExecutableType) info.getTypes().asMemberOf(receiver, e)).getReturnType();
12641287
State instantiatedState = getStateFromAnnotations(info, instantiatedReturnType, marker2State::get, StateEnum.POSSIBLE_NULL);
12651288
TypeMirror declaredReturnType = ((ExecutableElement) e).getReturnType();
1266-
State declaredState = getStateFromAnnotations(info, declaredReturnType, null);
1289+
State declaredState = getStateFromAnnotations(info, declaredReturnType, type -> null, null, null, false);
12671290

1291+
//testTypeAnnotations2: NOT_NULL List <PNR String> + PNR T -> PNR<PNR>
1292+
//testTypeVariables4: @NN String, T -> PNR
12681293
return State.weakMerge(instantiatedState, declaredState);
1294+
// return instantiatedState;
12691295
}
12701296
}
12711297
}
@@ -1996,9 +2022,9 @@ public static State weakMerge(State s1, State s2) {
19962022
}
19972023

19982024
private static List<State> mergeTypeParams(List<State> typeParams1, List<State> typeParams2, boolean strict) {
1999-
if (typeParams1 == null) {
2025+
if (typeParams1 == null || typeParams1.isEmpty()) {
20002026
return typeParams2;
2001-
} else if (typeParams2 == null) {
2027+
} else if (typeParams2 == null || typeParams2.isEmpty()) {
20022028
return typeParams1;
20032029
} else if (typeParams1.size() == typeParams2.size()) {
20042030
List<State> typeParams = new ArrayList<>();

‎java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/NPECheckTest.java‎

Lines changed: 165 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2054,7 +2054,9 @@ public void testTypeAnnotations2() throws Exception {
20542054
import java.util.*;
20552055
public class Test {
20562056
private void test(Box<@NotNull List<@NullAllowed String>> boxOfStrings) {
2057-
boxOfStrings.get().get(1).toString();
2057+
boxOfStrings.get()
2058+
.get(1)
2059+
.toString();
20582060
}
20592061
}
20602062
class Box<T> {
@@ -2068,8 +2070,8 @@ class Box<T> {
20682070
@interface NotNull {}
20692071
""")
20702072
.run(NPECheck.class)
2071-
.assertWarnings("5:27-5:30:verifier:Possibly Dereferencing null",
2072-
"5:34-5:42:verifier:Possibly Dereferencing null");
2073+
.assertWarnings("6:21-6:24:verifier:Possibly Dereferencing null",
2074+
"7:21-7:29:verifier:Possibly Dereferencing null");
20732075
}
20742076

20752077
public void testTypeAnnotations3() throws Exception {
@@ -2396,6 +2398,166 @@ private void test(@NullAllowed String str) {
23962398
.assertWarnings();
23972399
}
23982400

2401+
public void testTypeVariables() throws Exception {
2402+
HintTest.create()
2403+
.sourceLevel("21")
2404+
.input("""
2405+
package test;
2406+
import java.lang.annotation.*;
2407+
public class Test<T1 extends @NullAllowed String, T2 extends @NotNull T1, T3 extends @NullAllowed T2> {
2408+
private void test(T1 t1, T2 t2, T3 t3) {
2409+
@NotNull String s;
2410+
s = t1;
2411+
s = t2;
2412+
s = t3;
2413+
}
2414+
}
2415+
@Target(ElementType.TYPE_USE)
2416+
@interface NullAllowed {}
2417+
@Target(ElementType.TYPE_USE)
2418+
@interface NotNull {}
2419+
""")
2420+
.run(NPECheck.class)
2421+
.assertWarnings("5:8-5:14:verifier:PANNNV",
2422+
"7:8-7:14:verifier:PANNNV");
2423+
}
2424+
2425+
public void testTypeVariables2() throws Exception {
2426+
HintTest.create()
2427+
.sourceLevel("21")
2428+
.classpath(FileUtil.urlForArchiveOrDir(new File(System.getProperty("hints-jspecify.jar.location"))))
2429+
.input("""
2430+
package test;
2431+
import org.jspecify.annotations.NullMarked;
2432+
import org.jspecify.annotations.Nullable;
2433+
@NullMarked
2434+
public class Test<T1 extends @Nullable String, T2 extends T1, T3 extends @Nullable T2> {
2435+
private String s;
2436+
private void test(T1 t1, T2 t2, T3 t3) {
2437+
s = t1;
2438+
s = t2;
2439+
s = t3;
2440+
}
2441+
}
2442+
""")
2443+
.run(NPECheck.class)
2444+
.assertWarnings("7:8-7:14:verifier:PANNNV",
2445+
"8:8-8:14:verifier:PANNNV",
2446+
"9:8-9:14:verifier:PANNNV");
2447+
}
2448+
2449+
public void testTypeVariables3() throws Exception {
2450+
HintTest.create()
2451+
.sourceLevel("21")
2452+
.classpath(FileUtil.urlForArchiveOrDir(new File(System.getProperty("hints-jspecify.jar.location"))))
2453+
.input("""
2454+
package test;
2455+
import java.util.*;
2456+
import org.jspecify.annotations.NonNull;
2457+
import org.jspecify.annotations.Nullable;
2458+
import org.jspecify.annotations.NullnessUnspecified;
2459+
public class Test {
2460+
private @NonNull String test1(Box<? extends @Nullable String> b) {
2461+
return b.t();
2462+
}
2463+
private @NonNull Object test2(Box<? super @Nullable String> b) {
2464+
return b.t();
2465+
}
2466+
private List<? extends @NonNull String> test3(Box<? extends @Nullable String> b) {
2467+
return b.tList();
2468+
}
2469+
private List<? extends @NonNull String> test4(Box<? extends @NullnessUnspecified String> b) {
2470+
return b.tList(); //unspecified, no warning
2471+
}
2472+
}
2473+
record Box<T>(T t) {
2474+
public List<T> tList() { return List.of(t()); }
2475+
}
2476+
""")
2477+
.input("org/jspecify/annotations/NullnessUnspecified.java",
2478+
"""
2479+
package org.jspecify.annotations;
2480+
import java.lang.annotation.*;
2481+
@Target(ElementType.TYPE_USE)
2482+
public @interface NullnessUnspecified {}
2483+
""")
2484+
.run(NPECheck.class)
2485+
.assertWarnings("7:17-7:18:verifier:ERR_ReturningPossibleNullFromNonNull",
2486+
"10:17-10:18:verifier:ERR_ReturningPossibleNullFromNonNull",
2487+
"13:17-13:22:verifier:Nullness states mismatch");
2488+
}
2489+
2490+
public void testTypeVariables4() throws Exception {
2491+
HintTest.create()
2492+
.sourceLevel("21")
2493+
.classpath(FileUtil.urlForArchiveOrDir(new File(System.getProperty("hints-jspecify.jar.location"))))
2494+
.input("""
2495+
package test;
2496+
import java.util.*;
2497+
import org.jspecify.annotations.NonNull;
2498+
import org.jspecify.annotations.Nullable;
2499+
import org.jspecify.annotations.NullnessUnspecified;
2500+
public class Test {
2501+
private @NonNull String test1(Box<@NonNull String> b) {
2502+
return b.t();
2503+
}
2504+
private @NonNull String test2(Box<? extends @NonNull String> b) {
2505+
return b.t();
2506+
}
2507+
private @NonNull String test3(Box<@Nullable String> b) {
2508+
return b.t();
2509+
}
2510+
private @NonNull String test4(Box<? extends @Nullable String> b) {
2511+
return b.t();
2512+
}
2513+
private @NonNull String test5(Box<@NullnessUnspecified String> b) {
2514+
return b.t();
2515+
}
2516+
private @NonNull String test6(Box<? extends @NullnessUnspecified String> b) {
2517+
return b.t();
2518+
}
2519+
}
2520+
record Box<T extends @Nullable Object>(T t) {
2521+
}
2522+
""")
2523+
.input("org/jspecify/annotations/NullnessUnspecified.java",
2524+
"""
2525+
package org.jspecify.annotations;
2526+
import java.lang.annotation.*;
2527+
@Target(ElementType.TYPE_USE)
2528+
public @interface NullnessUnspecified {}
2529+
""")
2530+
.run(NPECheck.class)
2531+
.assertWarnings("13:17-13:18:verifier:ERR_ReturningPossibleNullFromNonNull",
2532+
"16:17-16:18:verifier:ERR_ReturningPossibleNullFromNonNull");
2533+
}
2534+
2535+
public void testAssignToFields() throws Exception {
2536+
HintTest.create()
2537+
.sourceLevel("21")
2538+
.classpath(FileUtil.urlForArchiveOrDir(new File(System.getProperty("hints-jspecify.jar.location"))))
2539+
.input("""
2540+
package test;
2541+
import java.lang.annotation.*;
2542+
public class Test {
2543+
private @NotNull String nn;
2544+
private @NullAllowed String na;
2545+
private String nothing;
2546+
private void test(@NullAllowed String p) {
2547+
nn = p;
2548+
na = p;
2549+
nothing = p;
2550+
}
2551+
}
2552+
@Target(ElementType.TYPE_USE)
2553+
@interface NullAllowed {}
2554+
@Target(ElementType.TYPE_USE)
2555+
@interface NotNull {}
2556+
""")
2557+
.run(NPECheck.class)
2558+
.assertWarnings("7:8-7:14:verifier:PANNNV");
2559+
}
2560+
23992561
//TODO: NullnessUnspecified
24002562

24012563
//TODO: check full "assignment" type in hints;; needs to remap parameter types(!!!)

0 commit comments

Comments
 (0)