Skip to content

Commit 74fa090

Browse files
Googlercopybara-github
authored andcommitted
[J2KT] Propagate nullability in method calls to lambda arguments.
It fixes problems in Javac frontend, where nullability in lambdas in not inferred from enclosing method calls. It also fixes some outstanding problems on the boundary between null-marked and non-null-marked scope. PiperOrigin-RevId: 863626947
1 parent bd9c779 commit 74fa090

File tree

10 files changed

+59
-136
lines changed

10 files changed

+59
-136
lines changed

transpiler/java/com/google/j2cl/transpiler/passes/PropagateNullability.java

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.google.common.collect.ImmutableSet;
2323
import com.google.common.collect.Ordering;
2424
import com.google.common.collect.Streams;
25+
import com.google.j2cl.common.visitor.Processor;
2526
import com.google.j2cl.transpiler.ast.AbstractRewriter;
2627
import com.google.j2cl.transpiler.ast.AbstractVisitor;
2728
import com.google.j2cl.transpiler.ast.ArrayLiteral;
@@ -31,7 +32,6 @@
3132
import com.google.j2cl.transpiler.ast.Expression;
3233
import com.google.j2cl.transpiler.ast.FunctionExpression;
3334
import com.google.j2cl.transpiler.ast.IntersectionTypeDescriptor;
34-
import com.google.j2cl.transpiler.ast.Invocation;
3535
import com.google.j2cl.transpiler.ast.MethodCall;
3636
import com.google.j2cl.transpiler.ast.MethodDescriptor;
3737
import com.google.j2cl.transpiler.ast.MethodLike;
@@ -128,7 +128,7 @@ public class PropagateNullability extends AbstractJ2ktNormalizationPass {
128128

129129
@Override
130130
public void applyTo(CompilationUnit compilationUnit) {
131-
compilationUnit.accept(
131+
Processor processor =
132132
new AbstractRewriter() {
133133
@Override
134134
public Node rewriteArrayLiteral(ArrayLiteral arrayLiteral) {
@@ -193,7 +193,17 @@ public Node rewriteMethodCall(MethodCall methodCall) {
193193
methodCall.getArguments());
194194
}
195195

196-
return Invocation.Builder.from(methodCall).setTarget(rewrittenMethodDescriptor).build();
196+
// Propagate nullability from parameters to function expression arguments.
197+
ImmutableList<Expression> rewrittenArguments =
198+
zip(
199+
methodCall.getArguments(),
200+
rewrittenMethodDescriptor.getParameterTypeDescriptors(),
201+
PropagateNullability.this::propagateNullabilityToFunctionExpression);
202+
203+
return MethodCall.Builder.from(methodCall)
204+
.setTarget(rewrittenMethodDescriptor)
205+
.setArguments(rewrittenArguments)
206+
.build();
197207
}
198208

199209
// TODO(b/406815802): See if rewriteInvocation can be refactored to seamlessly
@@ -260,7 +270,14 @@ public Node rewriteFunctionExpression(FunctionExpression functionExpression) {
260270
.setTypeDescriptor(inferredFunctionalInterface)
261271
.build();
262272
}
263-
});
273+
};
274+
275+
// Propagate nullability twice to ensure that we apply the effect of outward nullability
276+
// propagation from the first pass is used in the inward nullability propagation in the
277+
// second pass.
278+
// TODO(b/406815802): See whether this can be refactored to avoid running twice if not needed.
279+
compilationUnit.accept(processor);
280+
compilationUnit.accept(processor);
264281
}
265282

266283
/**
@@ -447,6 +464,19 @@ private MethodDescriptor propagateNullabilityFromArguments(
447464
methodDescriptor, typeParameterDescriptors, inferredTypeArgumentDescriptors);
448465
}
449466

467+
private Expression propagateNullabilityToFunctionExpression(
468+
Expression toExpression, TypeDescriptor fromTypeDescriptor) {
469+
return switch (toExpression) {
470+
case FunctionExpression functionExpression ->
471+
FunctionExpression.Builder.from(functionExpression)
472+
.setTypeDescriptor(
473+
propagateNullabilityTo(
474+
toExpression.getTypeDescriptor(), fromTypeDescriptor, ImmutableSet.of()))
475+
.build();
476+
default -> toExpression;
477+
};
478+
}
479+
450480
// TODO(b/406815802): Add JavaDoc
451481
private MethodDescriptor propagateNullabilityFromQualifier(
452482
MethodDescriptor methodDescriptor,

transpiler/javatests/com/google/j2cl/integration/java/nullability/Main.java

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import static com.google.j2cl.integration.testing.Asserts.assertNull;
1919
import static com.google.j2cl.integration.testing.Asserts.assertTrue;
20-
import static com.google.j2cl.integration.testing.TestUtils.isJ2Kt;
2120

2221
import java.util.function.Function;
2322
import org.jspecify.annotations.NullMarked;
@@ -311,12 +310,7 @@ private static void testNullWildcardInLambda() {
311310
}
312311

313312
private static void testLambdaParameterTypeInference() {
314-
if (!isJ2Kt()) {
315-
// TODO(b/454662844): The function here gets inferred to Function<Integer, Integer> then
316-
// cast to Function<in Integer?, Integer>, which at runtime has a bridge/adaptor
317-
// that does an implicit unboxing and ends in NPE.
318-
assertTrue(1 == Main.<Integer>applyToNull(value -> (value == null) ? 1 : value + 1));
319-
}
313+
assertTrue(1 == Main.<Integer>applyToNull(value -> (value == null) ? 1 : value + 1));
320314
}
321315

322316
private static <T> int applyToNull(Function<? super @Nullable T, Integer> function) {

transpiler/javatests/com/google/j2cl/readable/java/j2kt/NullabilityInferenceProblem10.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ interface Observer<V extends @Nullable Object> {
4343
}
4444

4545
static void test(Lazy<Observable<Foo>> lazyObservable) {
46-
// TODO(b/...): Uncomment when fixed
47-
// lazyObservable.get().addObserver(InNullMarked::observeNullable);
46+
lazyObservable.get().addObserver(InNullMarked::observeNullable);
4847
}
4948
}
5049
}

transpiler/javatests/com/google/j2cl/readable/java/j2kt/output_kt/NullabilityInferenceProblem10.kt.txt

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,13 @@ open class NullabilityInferenceProblem10 {
3838
}
3939

4040
@JvmStatic
41-
internal fun test_pp_j2kt(lazyObservable: Lazy<Observable<Foo>>) {}
41+
internal fun test_pp_j2kt(lazyObservable: Lazy<Observable<Foo>>) {
42+
lazyObservable.get()!!.addObserver(
43+
Observer { arg0: Foo? ->
44+
return@Observer InNullMarked.observeNullable_pp_j2kt(arg0)
45+
},
46+
)
47+
}
4248
}
4349

4450
interface Foo

transpiler/javatests/com/google/j2cl/readable/java/j2ktjavac/output_kt/LambdaParameterTypeInference.kt.txt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,14 @@ open class LambdaParameterTypeInference {
5757
fun testInference(map: MutableMap<String, Int>) {
5858
map.compute(
5959
"",
60-
BiFunction { key: String, value: Int ->
61-
return@BiFunction if (value == null) 1 else value.toInt() + 1
62-
} as BiFunction<in String, in Int?, out Int?>,
60+
BiFunction { key: String, value: Int? ->
61+
return@BiFunction if (value == null) 1 else value!!.toInt() + 1
62+
},
6363
)
6464
LambdaParameterTypeInference.applyToNull<Int>(
65-
Function { value_1: Int ->
66-
return@Function if (value_1 == null) 1 else value_1.toInt() + 1
67-
} as Function<in Int?, Int>,
65+
Function { value_1: Int? ->
66+
return@Function if (value_1 == null) 1 else value_1!!.toInt() + 1
67+
},
6868
)
6969
}
7070

transpiler/javatests/com/google/j2cl/readable/java/j2ktjavac/output_kt/Nullability.kt.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ open class Nullability {
3737

3838
internal open fun testBoxedHolder_pp_j2ktjavac(holder: NullMarkedClass.Holder<Int>) {
3939
holder.acceptNullableConsumer_pp_j2ktjavac(
40-
Consumer { s: Int ->
41-
} as Consumer<Int?>,
40+
Consumer { s: Int? ->
41+
},
4242
)
4343
}
4444

transpiler/javatests/com/google/j2cl/readable/java/j2ktjdt/NullabilityInferenceProblem10.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ interface Observer<V extends @Nullable Object> {
4343
}
4444

4545
static void test(Lazy<Observable<Foo>> lazyObservable) {
46-
// TODO(b/...): Uncomment when fixed
47-
// lazyObservable.get().addObserver(InNullMarked::observeNullable);
46+
lazyObservable.get().addObserver(InNullMarked::observeNullable);
4847
}
4948
}
5049
}

transpiler/javatests/com/google/j2cl/readable/java/j2ktjdt/output_kt/NullabilityInferenceProblem10.kt.txt

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,13 @@ open class NullabilityInferenceProblem10 {
3838
}
3939

4040
@JvmStatic
41-
internal fun test_pp_j2ktjdt(lazyObservable: Lazy<Observable<Foo>>) {}
41+
internal fun test_pp_j2ktjdt(lazyObservable: Lazy<Observable<Foo>>) {
42+
lazyObservable.get()!!.addObserver(
43+
Observer { arg0: Foo? ->
44+
return@Observer InNullMarked.observeNullable_pp_j2ktjdt(arg0)
45+
},
46+
)
47+
}
4248
}
4349

4450
interface Foo

transpiler/javatests/com/google/j2cl/readable/java/j2ktnotpassing/NullabilityInferenceProblem10.java

Lines changed: 0 additions & 49 deletions
This file was deleted.

transpiler/javatests/com/google/j2cl/readable/java/j2ktnotpassing/output_kt/NullabilityInferenceProblem10.kt.txt

Lines changed: 0 additions & 62 deletions
This file was deleted.

0 commit comments

Comments
 (0)