Apply flexible types to files compiled without explicit nulls

compiler/src/dotty/tools/dotc/core/JavaNullInterop.scala → 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`.

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

@@ -1858,6 +1858,8 @@ object Types extends TypeUtils {
         t
       case t @ SAMType(_, _) =>
         t
+      case ft: FlexibleType =>
+        ft.underlying.findFunctionType
       case _ =>
         NoType
 
@@ -3396,15 +3398,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()
@@ -3424,6 +3428,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

@@ -918,6 +918,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 {
@@ -934,10 +940,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) {
@@ -975,6 +983,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)
@@ -983,6 +994,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

@@ -1907,7 +1907,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
   }
 

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

@@ -0,0 +1,18 @@
+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
+i7883.scala                           # Unsure
+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

@@ -205,9 +205,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()
 
@@ -217,8 +217,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,38 @@
+import unsafeNulls.Foo.*
+import unsafeNulls.Unsafe_1
+
+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()
+
+@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)

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

@@ -0,0 +1,21 @@
+package unsafeNulls
+
+class Unsafe_1 {
+  def foo(s: String): String = {
+    if (s == null) then "nullString"
+    else s
+  }
+  def bar[T >: String](s: T): T = {
+    ???
+  }
+  def bar2[T >: String | Null](s: T): T = {
+    ???
+  }
+  def bar3[T <: Function1[String,String]](g: T): T = g
+  def bar4[HK[_]](i: String): HK[String] = ???
+}
+
+object Foo {
+  def bar = "bar!"
+  def id[T](t: T): T = t
+}

tests/explicit-nulls/pos/interop-sam-src/S.scala

@@ -7,10 +7,14 @@ def m = {
 
   j.g1(f1)
   j.g1((_: String | Null) => null)
+  j.g1(_ => null)
+  j.g1(x => if (x == "") null else null)
   j.g1(null)
 
   j.g2(f2)
   j.g2((_: Int) => ())
+  j.g2(_ => ())
+  j.g2(x => if (x == 1) () else ())
   j.g2(null)
 
   j.h1(f1)