Skip to content

Commit f18b2d1

Browse files
msridharmsridhar
committed
Merge branch 'master' into jdk-javac-plugin
2 parents 9d1b6ad + ec6d829 commit f18b2d1

12 files changed

Lines changed: 251 additions & 21 deletions

File tree

‎jdk-recent-unit-tests/build.gradle‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ dependencies {
4141
}
4242
testImplementation deps.test.jsr305Annotations
4343
testModulePath deps.test.cfQual
44+
testModulePath deps.build.jspecify
45+
testModulePath project(":test-java-module")
4446
}
4547

4648
tasks.withType(Test).configureEach { test ->
@@ -64,4 +66,6 @@ tasks.getByName('test').configure {
6466
onlyIf {
6567
deps.versions.errorProneApi == deps.versions.errorProneLatest
6668
}
69+
// we need this since we don't have an implementation / api dependence on test-java-module
70+
dependsOn ':test-java-module:jar'
6771
}

‎jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/ModuleInfoTests.java‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,88 @@ public void testModuleInfo() {
4444
"}")
4545
.doTest();
4646
}
47+
48+
@Test
49+
public void nullmarkedModule() {
50+
defaultCompilationHelper
51+
.addSourceLines(
52+
"module-info.java",
53+
"import org.jspecify.annotations.NullMarked;",
54+
"@NullMarked",
55+
"module com.example.myapp {",
56+
" exports com.example.myapp;",
57+
" requires java.base;",
58+
" requires org.jspecify;",
59+
"}")
60+
.addSourceLines(
61+
"com/example/myapp/Test.java",
62+
"package com.example.myapp;",
63+
"public class Test {",
64+
" public static void main(String[] args) {",
65+
" String s = null;",
66+
" // BUG: Diagnostic contains: dereferenced expression s is @Nullable",
67+
" s.hashCode();",
68+
" }",
69+
"}")
70+
.doTest();
71+
}
72+
73+
@Test
74+
public void nullUnmarkedPackageInNullMarkedModule() {
75+
defaultCompilationHelper
76+
.addSourceLines(
77+
"module-info.java",
78+
"import org.jspecify.annotations.NullMarked;",
79+
"@NullMarked",
80+
"module com.example.myapp {",
81+
" exports com.example.myapp;",
82+
" requires java.base;",
83+
" requires org.jspecify;",
84+
"}")
85+
.addSourceLines(
86+
"com/example/myapp/package-info.java",
87+
"@NullUnmarked package com.example.myapp;",
88+
"import org.jspecify.annotations.NullUnmarked;")
89+
.addSourceLines(
90+
"com/example/myapp/Test.java",
91+
"package com.example.myapp;",
92+
"public class Test {",
93+
" public static void main(String[] args) {",
94+
" String s = null;",
95+
" // no error since @NullUnmarked is in effect",
96+
" s.hashCode();",
97+
" }",
98+
"}")
99+
.doTest();
100+
}
101+
102+
@Test
103+
public void fromBytecode() {
104+
defaultCompilationHelper
105+
.addSourceLines(
106+
"module-info.java",
107+
"import org.jspecify.annotations.NullMarked;",
108+
"@NullMarked",
109+
"module com.example.myapp {",
110+
" requires java.base;",
111+
" requires org.jspecify;",
112+
" requires com.uber.test.java.module;",
113+
"}")
114+
.addSourceLines(
115+
"Test.java",
116+
"import org.jspecify.annotations.NullMarked;",
117+
"import com.example.nullmarked.NullMarkedFromModule;",
118+
"import com.example.nullunmarked.NullUnmarkedFromPackage;",
119+
"@NullMarked",
120+
"class Test {",
121+
" void testPositive() {",
122+
" // BUG: Diagnostic contains: passing @Nullable parameter",
123+
" NullMarkedFromModule.takesNonNull(null);",
124+
" }",
125+
" void testNegative() {",
126+
" NullUnmarkedFromPackage.takesAny(null);",
127+
" }",
128+
"}")
129+
.doTest();
130+
}
47131
}

‎nullaway/src/main/java/com/uber/nullaway/CodeAnnotationInfo.java‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public static CodeAnnotationInfo instance(Context context) {
7171
/**
7272
* Checks if a symbol comes from an annotated package, as determined by either configuration flags
7373
* (e.g. {@code -XepOpt:NullAway::AnnotatedPackages}) or package level annotations (e.g. {@code
74-
* org.jspecify.annotations.NullMarked}).
74+
* org.jspecify.annotations.NullMarked}) or module level annotations.
7575
*
7676
* @param outermostClassSymbol symbol for class (must be an outermost class)
7777
* @param config NullAway config
@@ -84,9 +84,7 @@ private static boolean fromAnnotatedPackage(
8484
String className = outermostClassSymbol.getQualifiedName().toString();
8585
Symbol.PackageSymbol enclosingPackage = ASTHelpers.enclosingPackage(outermostClassSymbol);
8686
if (!config.fromExplicitlyAnnotatedPackage(className)
87-
&& !(enclosingPackage != null
88-
&& hasDirectAnnotationWithSimpleName(
89-
enclosingPackage, NullabilityUtil.NULLMARKED_SIMPLE_NAME))) {
87+
&& !(enclosingPackage != null && explicitlyNullMarkedPackageOrModule(enclosingPackage))) {
9088
// By default, unknown code is unannotated unless @NullMarked or configured as annotated by
9189
// package name
9290
return false;
@@ -105,6 +103,18 @@ && hasDirectAnnotationWithSimpleName(
105103
return true;
106104
}
107105

106+
private static boolean explicitlyNullMarkedPackageOrModule(
107+
Symbol.PackageSymbol enclosingPackage) {
108+
if (hasDirectAnnotationWithSimpleName(
109+
enclosingPackage, NullabilityUtil.NULLMARKED_SIMPLE_NAME)) {
110+
return true;
111+
}
112+
Symbol enclosingModule = enclosingPackage.getEnclosingElement();
113+
return enclosingModule != null
114+
&& hasDirectAnnotationWithSimpleName(
115+
enclosingModule, NullabilityUtil.NULLMARKED_SIMPLE_NAME);
116+
}
117+
108118
/**
109119
* Check if a symbol comes from generated code.
110120
*

‎nullaway/src/main/java/com/uber/nullaway/NullAway.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1924,9 +1924,10 @@ private Description handleInvocation(
19241924
argumentPositionNullness[i] =
19251925
Nullness.paramHasNullableAnnotation(methodSymbol, i, config)
19261926
? Nullness.NULLABLE
1927-
: ((config.isJSpecifyMode() && tree instanceof MethodInvocationTree)
1927+
: ((config.isJSpecifyMode()
1928+
&& (tree instanceof MethodInvocationTree || tree instanceof NewClassTree))
19281929
? genericsChecks.getGenericParameterNullnessAtInvocation(
1929-
i, methodSymbol, (MethodInvocationTree) tree, state, config)
1930+
i, methodSymbol, tree, state, config)
19301931
: Nullness.NONNULL);
19311932
}
19321933
}

‎nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java‎

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import static com.google.common.base.Verify.verify;
44
import static com.uber.nullaway.NullabilityUtil.castToNonNull;
55

6+
import com.google.common.base.Verify;
67
import com.google.errorprone.VisitorState;
78
import com.google.errorprone.util.ASTHelpers;
89
import com.sun.source.tree.AnnotatedTypeTree;
@@ -54,8 +55,8 @@ public final class GenericsChecks {
5455
* from type variables for the method to their inferred type arguments (most importantly with
5556
* inferred nullability information).
5657
*/
57-
private final Map<MethodInvocationTree, Map<TypeVariable, Type>>
58-
inferredSubstitutionsForGenericMethodCalls = new LinkedHashMap<>();
58+
private final Map<Tree, Map<TypeVariable, Type>> inferredSubstitutionsForGenericMethodCalls =
59+
new LinkedHashMap<>();
5960

6061
/**
6162
* Checks that for an instantiated generic type, {@code @Nullable} types are only used for type
@@ -943,31 +944,31 @@ private static com.sun.tools.javac.util.List<Type> convertTreesToTypes(
943944
/**
944945
* Substitutes the type arguments from a generic method invocation into the method's type.
945946
*
946-
* @param methodInvocationTree the method invocation tree
947+
* @param tree the method invocation tree
947948
* @param methodSymbol symbol for the invoked generic method
948949
* @param state the visitor state
949950
* @param config the NullAway config
950951
* @return the substituted method type for the generic method
951952
*/
952953
private Type substituteTypeArgsInGenericMethodType(
953-
MethodInvocationTree methodInvocationTree,
954-
Symbol.MethodSymbol methodSymbol,
955-
VisitorState state,
956-
Config config) {
954+
Tree tree, Symbol.MethodSymbol methodSymbol, VisitorState state, Config config) {
957955

958-
List<? extends Tree> typeArgumentTrees = methodInvocationTree.getTypeArguments();
956+
List<? extends Tree> typeArgumentTrees =
957+
(tree instanceof MethodInvocationTree)
958+
? ((MethodInvocationTree) tree).getTypeArguments()
959+
: ((NewClassTree) tree).getTypeArguments();
959960
com.sun.tools.javac.util.List<Type> explicitTypeArgs = convertTreesToTypes(typeArgumentTrees);
960961

961962
Type.ForAll forAllType = (Type.ForAll) methodSymbol.type;
962963
Type.MethodType underlyingMethodType = (Type.MethodType) forAllType.qtype;
963964

964965
// There are no explicit type arguments, so use the inferred types
965966
if (explicitTypeArgs.isEmpty()) {
966-
if (inferredSubstitutionsForGenericMethodCalls.containsKey(methodInvocationTree)) {
967+
if (inferredSubstitutionsForGenericMethodCalls.containsKey(tree)) {
967968
return substituteInferredTypesForTypeVariables(
968969
state,
969970
underlyingMethodType,
970-
inferredSubstitutionsForGenericMethodCalls.get(methodInvocationTree),
971+
inferredSubstitutionsForGenericMethodCalls.get(tree),
971972
config);
972973
}
973974
}
@@ -1012,7 +1013,7 @@ private Type substituteTypeArgsInGenericMethodType(
10121013
public Nullness getGenericParameterNullnessAtInvocation(
10131014
int paramIndex,
10141015
Symbol.MethodSymbol invokedMethodSymbol,
1015-
MethodInvocationTree tree,
1016+
Tree tree,
10161017
VisitorState state,
10171018
Config config) {
10181019
boolean isVarargsParam =
@@ -1035,11 +1036,27 @@ public Nullness getGenericParameterNullnessAtInvocation(
10351036
}
10361037
}
10371038

1038-
if (!(tree.getMethodSelect() instanceof MemberSelectTree) || invokedMethodSymbol.isStatic()) {
1039-
return Nullness.NONNULL;
1039+
if (tree instanceof MethodInvocationTree) {
1040+
if (!(((MethodInvocationTree) tree).getMethodSelect() instanceof MemberSelectTree)
1041+
|| invokedMethodSymbol.isStatic()) {
1042+
return Nullness.NONNULL;
1043+
}
10401044
}
1041-
Type enclosingType =
1042-
getTreeType(((MemberSelectTree) tree.getMethodSelect()).getExpression(), config);
1045+
1046+
Type enclosingType = null;
1047+
if (tree instanceof MethodInvocationTree) {
1048+
enclosingType =
1049+
getTreeType(
1050+
((MemberSelectTree) ((MethodInvocationTree) tree).getMethodSelect()).getExpression(),
1051+
config);
1052+
1053+
} else {
1054+
Verify.verify(tree instanceof NewClassTree);
1055+
// for a constructor invocation, the type from the invocation itself is the "enclosing type"
1056+
// for the purposes of determining type arguments
1057+
enclosingType = getTreeType(tree, config);
1058+
}
1059+
10431060
return getGenericMethodParameterNullness(
10441061
paramIndex, invokedMethodSymbol, enclosingType, state, config);
10451062
}

‎nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2324,6 +2324,70 @@ public void issue1129() {
23242324
.doTest();
23252325
}
23262326

2327+
@Ignore("https://github.com/uber/NullAway/issues/1155")
2328+
@Test
2329+
public void callWithConstructorReceiver() {
2330+
makeHelper()
2331+
.addSourceLines(
2332+
"Test.java",
2333+
"import org.jspecify.annotations.NullMarked;",
2334+
"import org.jspecify.annotations.Nullable;",
2335+
"@NullMarked",
2336+
"public class Test {",
2337+
" private static class Inner<T extends @Nullable Object> {",
2338+
" Inner<T> identity() { return this; }",
2339+
" }",
2340+
" Inner<@Nullable Object> mThing = new Inner<@Nullable Object>().identity();",
2341+
"}")
2342+
.doTest();
2343+
}
2344+
2345+
@Test
2346+
public void newNullableWithArg() {
2347+
makeHelper()
2348+
.addSourceLines(
2349+
"Test.java",
2350+
"import org.jspecify.annotations.NullMarked;",
2351+
"import org.jspecify.annotations.Nullable;",
2352+
"@NullMarked",
2353+
"public class Test {",
2354+
" private static class Wrapper<T extends @Nullable String> {",
2355+
" private final T value;",
2356+
" Wrapper(T value) {",
2357+
" this.value = value;",
2358+
" }",
2359+
" }",
2360+
" Wrapper<@Nullable String> testConstructorCall() {",
2361+
" return new Wrapper<@Nullable String>(null);",
2362+
" }",
2363+
"}")
2364+
.doTest();
2365+
}
2366+
2367+
@Test
2368+
public void newNullableWithArgAndConstructorType() {
2369+
makeHelper()
2370+
.addSourceLines(
2371+
"Holder.java",
2372+
"import org.jspecify.annotations.NullMarked;",
2373+
"import org.jspecify.annotations.Nullable;",
2374+
"@NullMarked",
2375+
"public class Holder {",
2376+
" String s;",
2377+
" public <U extends @Nullable Object> Holder(U value) {",
2378+
" s = String.valueOf(value);",
2379+
" }",
2380+
" static Holder testNegative() {",
2381+
" return new <@Nullable String>Holder(null);",
2382+
" }",
2383+
" static Holder testPositive() {",
2384+
" // BUG: Diagnostic contains: passing @Nullable parameter 'null' where @NonNull is required",
2385+
" return new <String>Holder(null);",
2386+
" }",
2387+
"}")
2388+
.doTest();
2389+
}
2390+
23272391
@Test
23282392
public void issue1156() {
23292393
makeHelper()

‎settings.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ include ':sample-library-model'
1717
include ':sample'
1818
include ':test-java-lib'
1919
include ':test-java-lib-lombok'
20+
include ':test-java-module'
2021
include ':test-library-models'
2122
include ':jar-infer:android-jarinfer-models-sdk28'
2223
include ':jar-infer:android-jarinfer-models-sdk29'

‎test-java-module/build.gradle‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright (C) 2025. Uber Technologies
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
plugins {
18+
id "java-library"
19+
}
20+
21+
dependencies {
22+
implementation deps.build.jspecify
23+
}

‎test-java-module/src/main/java/com/example/nullmarked/NullMarkedFromModule.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package com.example.nullmarked;
2+
3+
public class NullMarkedFromModule {
4+
5+
public static void takesNonNull(Object o) {}
6+
}

‎test-java-module/src/main/java/com/example/nullunmarked/NullUnmarkedFromPackage.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package com.example.nullunmarked;
2+
3+
public class NullUnmarkedFromPackage {
4+
5+
public static void takesAny(Object o) {}
6+
}

0 commit comments

Comments
 (0)