Apply flexible types to files compiled without explicit nulls (#23386)

A fixed version of #22473 

This PR wraps the types of symbols from files compiled without explicit
nulls in flexible types.
This allows for interop between multiple files in cases where

```scala
class Unsafe_1 {
  def foo(s: String): String = {
    if (s == null) then "nullString"
    else s
  }
}
```
compiled without explicit nulls can still be used in

```scala
def Flexible_2() =
  val s2: String | Null = "foo"
  val unsafe = new Unsafe_1()
  val s: String = unsafe.foo(s2)
  unsafe.foo("")
  unsafe.foo(null)
```
whereas the argument would have been a strictly non-null String, because
of the flexible type, the function call is now permitted.

Some explicit nulls neg tests have been excluded under the scala2
library tasty tests due to technical reasons, for example:
```scala
class S {
  given Conversion[String, Array[String]] = _ => ???

  def f = {
    val s: String | Null = ???

    val x: String = s // error
    val xl = s.length // error
    val xs: Array[String | Null] | Null = s // error
  }
}
```
`val xl = s.length` will not give an error. This is because the .length
function is from the WrappedString.scala library. After desugaring, it
will become `wrapString(s).length`. Since the scala2 library is compiled
without explicit nulls, the types are flexified.`wrapString` will have
argument type FlexibleType(String) and return type FlexibleType(String),
therefore it will not give an error. Hence we need to exclude it from
the tasty tests.


[test_scala2_library_tasty]

Author: noti0na1

compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala renamed to compiler/src/dotty/tools/dotc/core/ImplicitNullInterop.scala

@@ -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.
@@ -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.
@@ -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
@@ -97,26 +97,32 @@ 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? */
     def needsNull(tp: Type): Boolean =
       if outermostLevelAlreadyNullable then false
       else tp match
-        case tp: TypeRef if
+        case tp: TypeRef if !tp.hasSimpleKind
           // We don't modify value types because they're non-nullable even in Java.
-          tp.symbol.isValueClass
+          || tp.symbol.isValueClass
           // We don't modify unit types.
           || tp.isRef(defn.UnitClass)
           // We don't modify `Any` because it's already nullable.
-          || tp.isRef(defn.AnyClass)
-          // We don't nullify Java varargs at the top level.
-          // Example: if `setNames` is a Java method with signature `void setNames(String... names)`,
-          // then its Scala signature will be `def setNames(names: (String|Null)*): Unit`.
-          // This is because `setNames(null)` passes as argument a single-element array containing the value `null`,
-          // and not a `null` array.
-          || !ctx.flexibleTypes && tp.isRef(defn.RepeatedParamClass) => false
+          || tp.isRef(defn.AnyClass) => false
+        case _ => true
+
+    // We don't nullify Java varargs at the top level.
+    // Example: if `setNames` is a Java method with signature `void setNames(String... names)`,
+    // then its Scala signature will be `def setNames(names: (String|Null)*): Unit`.
+    // This is because `setNames(null)` passes as argument a single-element array containing the value `null`,
+    // and not a `null` array.
+    def tyconNeedsNull(tp: Type): Boolean =
+      if outermostLevelAlreadyNullable then false
+      else tp match
+        case tp: TypeRef
+          if !ctx.flexibleTypes && tp.isRef(defn.RepeatedParamClass) => false
         case _ => true
 
     override def apply(tp: Type): Type = tp match {
@@ -130,7 +136,7 @@ object JavaNullInterop {
         val targs2 = targs map this
         outermostLevelAlreadyNullable = oldOutermostNullable
         val appTp2 = derivedAppliedType(appTp, tycon, targs2)
-        if needsNull(tycon) then nullify(appTp2) else appTp2
+        if tyconNeedsNull(tycon) then nullify(appTp2) else appTp2
       case ptp: PolyType =>
         derivedLambdaType(ptp)(ptp.paramInfos, this(ptp.resType))
       case mtp: MethodType =>
@@ -140,6 +146,7 @@ object JavaNullInterop {
         outermostLevelAlreadyNullable = oldOutermostNullable
         derivedLambdaType(mtp)(paramInfos2, this(mtp.resType))
       case tp: TypeAlias => mapOver(tp)
+      case tp: TypeBounds => mapOver(tp)
       case tp: AndType =>
         // nullify(A & B) = (nullify(A) & nullify(B)) | Null, but take care not to add
         // duplicate `Null`s at the outermost level inside `A` and `B`.
@@ -149,6 +156,14 @@ object JavaNullInterop {
       // In all other cases, return the type unchanged.
       // In particular, if the type is a ConstantType, then we don't nullify it because it is the
       // type of a final non-nullable field.
+      case tp: ExprType => mapOver(tp)
+      case tp: AnnotatedType => mapOver(tp)
+      case tp: OrType =>
+        outermostLevelAlreadyNullable = true
+        nullify(derivedOrType(tp, this(tp.tp1), this(tp.tp2)))
+      case tp: RefinedType =>
+        outermostLevelAlreadyNullable = true
+        nullify(mapOver(tp))
       case _ => tp
     }
   }

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -1888,6 +1888,8 @@ object Types extends TypeUtils {
         t
       case t @ SAMType(_, _) =>
         t
+      case ft: FlexibleType =>
+        ft.underlying.findFunctionType
       case _ =>
         NoType
 
@@ -3428,15 +3430,17 @@ object Types extends TypeUtils {
     override def underlying(using Context): Type = hi
 
     def derivedFlexibleType(hi: Type)(using Context): Type =
-      if hi eq this.hi then this else FlexibleType(hi)
+      if hi eq this.hi then this else FlexibleType.make(hi)
 
     override def computeHash(bs: Binders): Int = doHash(bs, hi)
 
     override final def baseClasses(using Context): List[ClassSymbol] = hi.baseClasses
   }
 
   object FlexibleType {
-    def apply(tp: Type)(using Context): FlexibleType = tp match {
+    def apply(tp: Type)(using Context): FlexibleType =
+      assert(tp.isValueType, s"Should not flexify ${tp}")
+      tp match {
       case ft: FlexibleType => ft
       case _ =>
         // val tp1 = tp.stripNull()
@@ -3456,6 +3460,15 @@ object Types extends TypeUtils {
         // rule.
         FlexibleType(OrNull(tp), tp)
     }
+
+    def make(tp: Type)(using Context): Type =
+      tp match
+        case _: FlexibleType => tp
+        case TypeBounds(lo, hi) => TypeBounds(FlexibleType.make(lo), FlexibleType.make(hi))
+        case wt: WildcardType => wt.optBounds match
+          case tb: TypeBounds => WildcardType(FlexibleType.make(tb).asInstanceOf[TypeBounds])
+          case _ => wt
+        case other => FlexibleType(tp)
   }
 
   // --- AndType/OrType ---------------------------------------------------------------

compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala

@@ -519,7 +519,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) {

compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala

@@ -921,6 +921,12 @@ class TreeUnpickler(reader: TastyReader,
 
       def ta = ctx.typeAssigner
 
+      // If explicit nulls is enabled, and the source file did not have explicit
+      // nulls enabled, nullify the member to allow for compatibility.
+      def nullify(sym: Symbol) =
+        if (ctx.explicitNulls && ctx.flexibleTypes && !explicitNulls) then
+          sym.info = ImplicitNullInterop.nullifyMember(sym, sym.info, sym.is(Enum))
+
       val name = readName()
       pickling.println(s"reading def of $name at $start")
       val tree: MemberDef = tag match {
@@ -937,10 +943,12 @@ class TreeUnpickler(reader: TastyReader,
             else
               tpt.tpe
           sym.info = methodType(paramss, resType)
+          nullify(sym)
           DefDef(paramDefss, tpt)
         case VALDEF =>
           val tpt = readTpt()(using localCtx)
           sym.info = tpt.tpe.suppressIntoIfParam(sym)
+          nullify(sym)
           ValDef(tpt)
         case TYPEDEF | TYPEPARAM =>
           if (sym.isClass) {
@@ -978,6 +986,9 @@ class TreeUnpickler(reader: TastyReader,
               sym.typeRef.recomputeDenot() // make sure we see the new bounds from now on
             else
               sym.info = info
+              if (tag == TYPEPARAM) {
+                nullify(sym)
+              }
 
             sym.resetFlag(Provisional)
             TypeDef(rhs)
@@ -986,6 +997,7 @@ class TreeUnpickler(reader: TastyReader,
           val tpt = readTpt()(using localCtx)
           assert(nothingButMods(end))
           sym.info = tpt.tpe.suppressIntoIfParam(sym)
+          nullify(sym)
           ValDef(tpt)
       }
       goto(end)

compiler/src/dotty/tools/dotc/typer/Namer.scala

@@ -1921,7 +1921,7 @@ class Namer { typer: Typer =>
         sym.resetFlag(Lazy)
       case _ =>
     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
   }
 

compiler/test/dotc/neg-explicit-nulls-scala2-library-tasty.excludelist

@@ -0,0 +1,17 @@
+byname-nullables.scala                # identity() flexified
+varargs.scala                         # Array type flexified
+flow-conservative.scala               # .length flexified
+nn-basic.scala                        # .length flexified but trim rejected
+i21380c.scala                         # .length flexified but replaceAll rejected
+unsafe-scope.scala                    # .length flexified
+i17467.scala                          # Singleton type flexified
+from-nullable.scala                   # Option argument flexified
+flow-in-block.scala                   # .length flexified
+array.scala                           # Type arugment of Array flexified
+flow-forward-ref.scala                # .length flexified, forward reference error
+flow-implicitly.scala                 # Singleton type flexified
+nn.scala                              # Flexified elided error [!]
+flow-basic.scala                      # .length flexified
+
+unsafe-cast.scala           # Array type flexified
+unsafe-extensions.scala     # Function arguments flexified

compiler/test/dotty/tools/TestSources.scala

@@ -48,13 +48,17 @@ object TestSources {
 
   def negScala2LibraryTastyExcludelistFile: String = "compiler/test/dotc/neg-scala2-library-tasty.excludelist"
   def negInitGlobalScala2LibraryTastyExcludelistFile: String = "compiler/test/dotc/neg-init-global-scala2-library-tasty.excludelist"
+  def negExplicitNullsScala2LibraryTastyExcludelistFile: String = "compiler/test/dotc/neg-explicit-nulls-scala2-library-tasty.excludelist"
 
   def negScala2LibraryTastyExcludelisted: List[String] =
     if Properties.usingScalaLibraryTasty then loadList(negScala2LibraryTastyExcludelistFile)
     else Nil
   def negInitGlobalScala2LibraryTastyExcludelisted: List[String] =
     if Properties.usingScalaLibraryTasty then loadList(negInitGlobalScala2LibraryTastyExcludelistFile)
     else Nil
+  def negExplicitNullsScala2LibraryTastyExcludelisted: List[String] =
+    if Properties.usingScalaLibraryTasty then loadList(negExplicitNullsScala2LibraryTastyExcludelistFile)
+    else Nil
 
   // patmat tests lists
 

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -208,9 +208,9 @@ class CompilationTests {
   @Test def explicitNullsNeg: Unit = {
     implicit val testGroup: TestGroup = TestGroup("explicitNullsNeg")
     aggregateTests(
-      compileFilesInDir("tests/explicit-nulls/neg", explicitNullsOptions),
+      compileFilesInDir("tests/explicit-nulls/neg", explicitNullsOptions, FileFilter.exclude(TestSources.negExplicitNullsScala2LibraryTastyExcludelisted)),
       compileFilesInDir("tests/explicit-nulls/flexible-types-common", explicitNullsOptions and "-Yno-flexible-types"),
-      compileFilesInDir("tests/explicit-nulls/unsafe-common", explicitNullsOptions and "-Yno-flexible-types"),
+      compileFilesInDir("tests/explicit-nulls/unsafe-common", explicitNullsOptions and "-Yno-flexible-types", FileFilter.exclude(TestSources.negExplicitNullsScala2LibraryTastyExcludelisted)),
     )
   }.checkExpectedErrors()
 
@@ -220,8 +220,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")

tests/explicit-nulls/flexible-unpickle/Flexible_2.scala

@@ -0,0 +1,99 @@
+import unsafeNulls.Foo.*
+import unsafeNulls.Unsafe_1
+import unsafeNulls.{A, B, C, F, G, H, I, J, L, M}
+import scala.reflect.Selectable.reflectiveSelectable
+
+class Inherit_1 extends Unsafe_1 {
+  override def foo(s: String): String = s
+  override def bar[T >: String](s: T): T = s
+  override def bar2[T >: String | Null](s: T): T = s
+  override def bar3[T <: Function1[String,String]](g: T) = g
+  override def bar4[HK[_]](i: String | Null): HK[String | Null] = ???
+}
+
+class Inherit_2 extends Unsafe_1 {
+  override def foo(s: String | Null): String | Null = null
+  override def bar[T >: String](s: T | Null): T | Null = s
+  override def bar2[T >: String](s: T): T = s
+  override def bar3[T <: Function1[(String|Null),(String|Null)]](g: T) = g
+  override def bar4[HK[_]](i: String): HK[String] = ???
+}
+
+class Inherit_3 extends Unsafe_1 {
+  override def foo(s: String): String | Null = null
+  override def bar[T >: String](s: T): T | Null = s
+}
+
+class Inherit_4 extends Unsafe_1 {
+  override def foo(s: String | Null): String = "non-null string"
+  override def bar[T >: String](s: T | Null): T = "non-null string"
+}
+
+case class cc()
+
+class K(val b: String) extends J(b) {
+}
+
+@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)
+
+
+  val a = refinement.b
+  refinement.b = null
+  val refinement2: Unsafe_1 { var b: String } = refinement
+  refinement = null
+
+  val singletonbar: bar.type = singleton
+
+  val extension: String = intersection.reverse
+
+  val stringA: String = intersection.stringA
+  val stringB: String = intersection.stringB
+  intersection.stringA = null
+  intersection.stringB = null
+
+  val intersection2: A & B = intersection
+  intersection = null
+
+  val stringC: String = union.stringC
+  union.stringC = null
+
+  val union2: A | B = union
+  union = null
+
+  val constructorTest = new Unsafe_1(null)
+  val member: String = constructorTest.member
+  constructorTest.member = null
+
+  bar match {
+    case str @ null: String => ()
+    case other => ()
+  }
+
+  val f = new F(null, G(12))
+  val F(x, y) = f
+
+  val g: (List[F] | String | List[Int]) = F.many
+  F.many = null :: null :: Nil
+  F.many = null
+
+  val h: H { val s: String } = new H { override val s: String = "foo" }
+
+  val jBox: I[J] = new I(new J(null))
+  val kBox: I[K] = new I(new K("foo"))
+
+  val box: I[J] = kBox
+
+  val jBox2: L[J] = new L[J](j => ())
+  val kBox2: L[K] = new L[K](k => ())
+
+  val box2: L[K] = jBox2
+  val box3: I[J | Null] = box
+
+  val m: String = M.test(null)
+