Skip to content

Apply Flexible Types to Symbols from files compiled without Explicit Nulls #22473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import dotty.tools.dotc.core.Decorators.i
* to handle the full spectrum of Scala types. Additionally, some kinds of symbols like constructors and
* enum instances get special treatment.
*/
object JavaNullInterop {
object ImplicitNullInterop {

/** Transforms the type `tp` of Java member `sym` to be explicitly nullable.
* `tp` is needed because the type inside `sym` might not be set when this method is called.
Expand All @@ -55,11 +55,11 @@ object JavaNullInterop {
*/
def nullifyMember(sym: Symbol, tp: Type, isEnumValueDef: Boolean)(using Context): Type = trace(i"nullifyMember ${sym}, ${tp}"){
assert(ctx.explicitNulls)
assert(sym.is(JavaDefined), "can only nullify java-defined members")

// Some special cases when nullifying the type
if isEnumValueDef || sym.name == nme.TYPE_ then
// Don't nullify the `TYPE` field in every class and Java enum instances
if isEnumValueDef || sym.name == nme.TYPE_ // Don't nullify the `TYPE` field in every class and Java enum instances
|| sym.is(Flags.ModuleVal) // Don't nullify Modules
then
tp
else if sym.name == nme.toString_ || sym.isConstructor || hasNotNullAnnot(sym) then
// Don't nullify the return type of the `toString` method.
Expand All @@ -80,14 +80,14 @@ object JavaNullInterop {
* but the result type is not nullable.
*/
private def nullifyExceptReturnType(tp: Type)(using Context): Type =
new JavaNullMap(outermostLevelAlreadyNullable = true)(tp)
new ImplicitNullMap(outermostLevelAlreadyNullable = true)(tp)

/** Nullifies a Java type by adding `| Null` in the relevant places. */
/** Nullifies a type by adding `| Null` in the relevant places. */
private def nullifyType(tp: Type)(using Context): Type =
new JavaNullMap(outermostLevelAlreadyNullable = false)(tp)
new ImplicitNullMap(outermostLevelAlreadyNullable = false)(tp)

/** A type map that implements the nullification function on types. Given a Java-sourced type, this adds `| Null`
* in the right places to make the nulls explicit in Scala.
/** A type map that implements the nullification function on types. Given a Java-sourced type or an
* implicitly null type, this adds `| Null` in the right places to make the nulls explicit.
*
* @param outermostLevelAlreadyNullable whether this type is already nullable at the outermost level.
* For example, `Array[String] | Null` is already nullable at the
Expand All @@ -97,7 +97,7 @@ object JavaNullInterop {
* This is useful for e.g. constructors, and also so that `A & B` is nullified
* to `(A & B) | Null`, instead of `(A | Null & B | Null) | Null`.
*/
private class JavaNullMap(var outermostLevelAlreadyNullable: Boolean)(using Context) extends TypeMap {
private class ImplicitNullMap(var outermostLevelAlreadyNullable: Boolean)(using Context) extends TypeMap {
def nullify(tp: Type): Type = if ctx.flexibleTypes then FlexibleType(tp) else OrNull(tp)

/** Should we nullify `tp` at the outermost level? */
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3427,6 +3427,7 @@ object Types extends TypeUtils {
// flexible type is always a subtype of the original type and the Object type.
// It is not necessary according to the use cases, so we choose to use a simpler
// rule.
assert(!tp.isInstanceOf[LazyType])
FlexibleType(OrNull(tp), tp)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ class ClassfileParser(
denot.info = translateTempPoly(attrCompleter.complete(denot.info, isVarargs))
if (isConstructor) normalizeConstructorInfo()

if (ctx.explicitNulls) denot.info = JavaNullInterop.nullifyMember(denot.symbol, denot.info, isEnum)
if (ctx.explicitNulls) denot.info = ImplicitNullInterop.nullifyMember(denot.symbol, denot.info, isEnum)

// seal java enums
if (isEnum) {
Expand Down
6 changes: 6 additions & 0 deletions compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,12 @@ class TreeUnpickler(reader: TastyReader,
sym.info = tpt.tpe
ValDef(tpt)
}

// If explicit nulls is enabled, and the source file did not have explicit
// nulls enabled, nullify the member to allow for compatibility.
if (ctx.explicitNulls && !explicitNulls) then
sym.info = ImplicitNullInterop.nullifyMember(sym, sym.info, sym.is(Enum))

goto(end)
setSpan(start, tree)

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1879,7 +1879,7 @@ class Namer { typer: Typer =>

val mbrTpe = paramFn(checkSimpleKinded(typedAheadType(mdef.tpt, tptProto)).tpe)
if (ctx.explicitNulls && mdef.mods.is(JavaDefined))
JavaNullInterop.nullifyMember(sym, mbrTpe, mdef.mods.isAllOf(JavaEnumValue))
ImplicitNullInterop.nullifyMember(sym, mbrTpe, mdef.mods.isAllOf(JavaEnumValue))
else mbrTpe
}

Expand Down
14 changes: 12 additions & 2 deletions compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,18 @@ class CompilationTests {
compileFilesInDir("tests/explicit-nulls/pos", explicitNullsOptions),
compileFilesInDir("tests/explicit-nulls/flexible-types-common", explicitNullsOptions),
compileFilesInDir("tests/explicit-nulls/unsafe-common", explicitNullsOptions and "-language:unsafeNulls" and "-Yno-flexible-types"),
)
}.checkCompile()
).checkCompile()

locally {
val tests = List(
compileFile("tests/explicit-nulls/flexible-unpickle/Unsafe_1.scala", explicitNullsOptions without "-Yexplicit-nulls"),
compileFile("tests/explicit-nulls/flexible-unpickle/Flexible_2.scala", explicitNullsOptions.withClasspath(
defaultOutputDir + testGroup + "/Unsafe_1/flexible-unpickle/Unsafe_1")),
).map(_.keepOutput.checkCompile())

tests.foreach(_.delete())
}
}

@Test def explicitNullsWarn: Unit = {
implicit val testGroup: TestGroup = TestGroup("explicitNullsWarn")
Expand Down
10 changes: 10 additions & 0 deletions tests/explicit-nulls/flexible-unpickle/Flexible_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import unsafeNulls.Foo.*
import unsafeNulls.Unsafe_1

@main
def Flexible_2() =
val s2: String | Null = "foo"
val unsafe = new Unsafe_1()
val s: String = unsafe.foo(s2)
unsafe.foo("")
unsafe.foo(null)
12 changes: 12 additions & 0 deletions tests/explicit-nulls/flexible-unpickle/Unsafe_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package unsafeNulls

class Unsafe_1 {
def foo(s: String): String = {
if (s == null) then "nullString"
else s
}
}

object Foo {
def bar = "bar!"
}
Loading