Skip to content

Commit

Permalink
[J2KT] Propagate inferred nullability in array literals and new array…
Browse files Browse the repository at this point in the history
… expressions, which were causing insertion of `!!` in value expressions.

PiperOrigin-RevId: 735341118
  • Loading branch information
Googler authored and copybara-github committed Mar 10, 2025
1 parent 541a73a commit b72dfe2
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@
import com.google.j2cl.transpiler.passes.PreventSmartCasts;
import com.google.j2cl.transpiler.passes.PropagateCompileTimeConstants;
import com.google.j2cl.transpiler.passes.PropagateConstants;
import com.google.j2cl.transpiler.passes.PropagateNullability;
import com.google.j2cl.transpiler.passes.PropagateNullabilityInOverrides;
import com.google.j2cl.transpiler.passes.RecoverShortcutBooleanOperator;
import com.google.j2cl.transpiler.passes.RemoveCustomIsInstanceMethods;
Expand Down Expand Up @@ -728,6 +729,7 @@ public ImmutableList<Supplier<NormalizationPass>> getPassFactories(BackendOption
FixJavaKotlinMethodOverrideMismatch::new,
AnnotateProtobufMethodsAsKtProperties::new,
RewriteAnnotationTypesJ2kt::new,
PropagateNullability::new,
NormalizeNullLiterals::new,
NormalizeMinValueIntegralLiterals::new,
CreateImplicitConstructors::new,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* Copyright 2024 Google Inc.
*
* 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.j2cl.transpiler.passes;

import com.google.j2cl.transpiler.ast.AbstractRewriter;
import com.google.j2cl.transpiler.ast.ArrayLiteral;
import com.google.j2cl.transpiler.ast.ArrayTypeDescriptor;
import com.google.j2cl.transpiler.ast.CompilationUnit;
import com.google.j2cl.transpiler.ast.Expression;
import com.google.j2cl.transpiler.ast.NewArray;
import com.google.j2cl.transpiler.ast.Node;
import com.google.j2cl.transpiler.ast.TypeDescriptor;
import java.util.stream.Stream;

/**
* Propagates nullability in inferred types from actual nullability in expressions.
*
* <p>At this moment it propagates nullability of inferred component types in array literals, by
* inferring them from value expressions. Eventually, it'll also fix type arguments in invocations
* by inferring them from arguments.
*
* <p>In the following statement:
*
* <pre>{@code
* @Nullable String[] arr = {null};
* }</pre>
*
* the inferred type of array literal is {@code String[]} instead of {@code @Nullable String[]}. It
* causes insertion of {@code !!} in transpiled Kotlin code:
*
* <pre>{@code
* val arr: Array<String?> = arrayOf<String>(null!!) as Array<String?>
* }</pre>
*
* This pass fixes the inferred array component type to be {@code @Nullable String[]}, which fixes
* the problem in transpiled Kotlin code:
*
* <pre>{@code
* val arr: Array<String?> = arrayOf<String?>(null)
* }</pre>
*/
public class PropagateNullability extends AbstractJ2ktNormalizationPass {

@Override
public void applyTo(CompilationUnit compilationUnit) {
compilationUnit.accept(
new AbstractRewriter() {
@Override
public Node rewriteArrayLiteral(ArrayLiteral arrayLiteral) {
return propagateNullabilityFromValueExpressions(arrayLiteral);
}

@Override
public Node rewriteNewArray(NewArray newArray) {
Expression initializer = newArray.getInitializer();
if (initializer == null) {
return newArray;
}
// Update type of NewArray expression from rewritten initializer.
return NewArray.Builder.from(newArray)
.setTypeDescriptor((ArrayTypeDescriptor) initializer.getTypeDescriptor())
.build();
}
});
}

private ArrayLiteral propagateNullabilityFromValueExpressions(ArrayLiteral arrayLiteral) {
ArrayTypeDescriptor arrayTypeDescriptor = arrayLiteral.getTypeDescriptor();
TypeDescriptor componentTypeDescriptor =
propagateNullabilityFrom(
arrayTypeDescriptor.getComponentTypeDescriptor(),
arrayLiteral.getValueExpressions().stream().map(Expression::getTypeDescriptor));
return arrayLiteral.toBuilder()
.setTypeDescriptor(
ArrayTypeDescriptor.Builder.from(arrayTypeDescriptor)
.setComponentTypeDescriptor(componentTypeDescriptor)
.build())
.build();
}

private static TypeDescriptor propagateNullabilityFrom(
TypeDescriptor typeDescriptor, TypeDescriptor from) {
return !from.equals(typeDescriptor) && from.canBeNull()
? typeDescriptor.toNullable()
: typeDescriptor;
}

private static TypeDescriptor propagateNullabilityFrom(
TypeDescriptor typeDescriptor, Stream<TypeDescriptor> fromTypeDescriptors) {
return fromTypeDescriptors.reduce(
typeDescriptor, PropagateNullability::propagateNullabilityFrom, (a, b) -> a);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,23 +89,17 @@ private static final class Box<T extends @Nullable Object> {
}

private static void testArrayLiteral() {
// TODO(b/324550390): Remove the condition when the bug is fixed.
if (!isJ2Kt()) {
@Nullable String[] unusedArray1 = {STRING, NULL_STRING};
@Nullable String[] unusedArray2 = {NULL_STRING, STRING};
}
@Nullable String[] unusedArray1 = {STRING, NULL_STRING};
@Nullable String[] unusedArray2 = {NULL_STRING, STRING};
}

private static void testNewArray() {
@Nullable String[] unusedArray1 = new @Nullable String[] {STRING, NULL_STRING};
@Nullable String[] unusedArray2 = new @Nullable String[] {NULL_STRING, STRING};

// TODO(b/324550390): Remove the condition when the bug is fixed.
if (!isJ2Kt()) {
// Lack of @Nullable annotation in array creation expression should not cause NULL_STRING!!
@Nullable String[] unusedArray3 = new String[] {STRING, NULL_STRING};
@Nullable String[] unusedArray4 = new String[] {NULL_STRING, STRING};
}
// Lack of @Nullable annotation in array creation expression should not cause NULL_STRING!!
@Nullable String[] unusedArray3 = new String[] {STRING, NULL_STRING};
@Nullable String[] unusedArray4 = new String[] {NULL_STRING, STRING};
}

private static void testExplicitInvocationTypeArguments() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ open class NotNullAssertionProblems {
@ObjCName("withNSString") string: String,
@ObjCName("withNSString") nullableString: String?
) {
val array0: Array<String?> = arrayOf<String>(null!!) as Array<String?>
val array1: Array<String?> = arrayOf<String>(string, null!!) as Array<String?>
val array2: Array<String?> = arrayOf<String>(string, nullableString!!) as Array<String?>
val array3: Array<String?> = arrayOf<String>(null!!, string) as Array<String?>
val array4: Array<String?> = arrayOf<String>(nullableString!!, string) as Array<String?>
val array0: Array<String?> = arrayOf<String?>(null)
val array1: Array<String?> = arrayOf<String?>(string, null)
val array2: Array<String?> = arrayOf<String?>(string, nullableString)
val array3: Array<String?> = arrayOf<String?>(null, string)
val array4: Array<String?> = arrayOf<String?>(nullableString, string)
}

@ObjCName("testNewArray")
Expand Down Expand Up @@ -104,16 +104,13 @@ open class NotNullAssertionProblems {
)
NotNullAssertionProblems.accept2<String?>(nullableString, string)
NotNullAssertionProblems.acceptVararg<String>(
string,
null!!,
*(arrayOf<String?>(string, null) as Array<String>)!!,
)
NotNullAssertionProblems.acceptVararg<String>(
string,
nullableString!!,
*(arrayOf<String?>(string, nullableString) as Array<String>)!!,
)
NotNullAssertionProblems.acceptVararg<String>(
null!!,
string,
*(arrayOf<String?>(null, string) as Array<String>)!!,
)
NotNullAssertionProblems.acceptVararg<String?>(nullableString, string)
NotNullAssertionProblems.acceptGeneric<String>(
Expand Down Expand Up @@ -174,16 +171,13 @@ open class NotNullAssertionProblems {
)
NotNullAssertionProblems.Consumer<String?>(nullableString, string)
NotNullAssertionProblems.VarargConsumer<String>(
string,
null!!,
*(arrayOf<String?>(string, null) as Array<String>)!!,
)
NotNullAssertionProblems.VarargConsumer<String>(
string,
nullableString!!,
*(arrayOf<String?>(string, nullableString) as Array<String>)!!,
)
NotNullAssertionProblems.VarargConsumer<String>(
null!!,
string,
*(arrayOf<String?>(null, string) as Array<String>)!!,
)
NotNullAssertionProblems.VarargConsumer<String?>(nullableString, string)
NotNullAssertionProblems.GenericConsumer<String>(
Expand Down Expand Up @@ -292,8 +286,7 @@ open class NotNullAssertionProblems {
null!!,
)
NotNullAssertionProblems.VarargConsumer<String>(
string,
null!!,
*(arrayOf<String?>(string, null) as Array<String>)!!,
).accept(
null!!,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ open class NullabilityInferenceWithLocalVariables {
@ObjCName("testArray")
fun testArray(): Array<String> {
val local: String = ""
return arrayOf<String>(local, "")
return arrayOf<String?>(local, "") as Array<String>
}

@JvmStatic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@ import java.util.stream.Collector
import java.util.stream.Collectors
import java.util.stream.Stream
import kotlin.Any
import kotlin.Array
import kotlin.Boolean
import kotlin.OptIn
import kotlin.String
import kotlin.Suppress
import kotlin.arrayOf
import kotlin.collections.MutableList
import kotlin.experimental.ExperimentalObjCName
import kotlin.jvm.JvmStatic
Expand Down Expand Up @@ -144,16 +146,15 @@ open class StreamCollectWildcardProblem {
@ObjCName("withBoolean") b: Boolean
): Stream<String> {
return Stream.of<Stream<String>>(
(if (b) stream1.map<String>(
*(arrayOf<Stream<String>?>(if (b) stream1.map<String>(
Function/* <in StreamCollectWildcardProblem.Foo<Any>, out String> */ { arg0: StreamCollectWildcardProblem.Foo<Any> ->
return@Function arg0.getString()
},
) else null)!!,
(if (b) stream2.map<String>(
) else null, if (b) stream2.map<String>(
Function/* <in StreamCollectWildcardProblem.Foo<Any>, out String> */ { arg0_1: StreamCollectWildcardProblem.Foo<Any> ->
return@Function arg0_1.getString()
},
) else null)!!,
) else null) as Array<Stream<String>>)!!,
).filter(
Predicate/* <in Stream<String>> */ { arg0_2: Stream<String> ->
return@Predicate Objects.nonNull(arg0_2)
Expand Down

0 comments on commit b72dfe2

Please sign in to comment.