Merge pull request #5886 from dotty-staging/change-implicit-conversions-2

Fix implicit conversion warnings

Author: odersky

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

@@ -634,17 +634,23 @@ trait Checking {
    *    - it is defined in Predef
    *    - it is the scala.reflect.Selectable.reflectiveSelectable conversion
    */
-  def checkImplicitConversionUseOK(sym: Symbol, posd: Positioned)(implicit ctx: Context): Unit = {
-    val conversionOK =
-      !sym.exists ||
-      sym.is(Synthetic) ||
-      sym.info.finalResultType.classSymbols.exists(_.owner.isLinkedWith(sym.owner)) ||
-      defn.isPredefClass(sym.owner) ||
-      sym.name == nme.reflectiveSelectable && sym.maybeOwner.maybeOwner.maybeOwner == defn.ScalaPackageClass
-    if (!conversionOK)
-      checkFeature(defn.LanguageModuleClass, nme.implicitConversions,
-        i"Use of implicit conversion ${sym.showLocated}", NoSymbol, posd.sourcePos)
-  }
+  def checkImplicitConversionUseOK(sym: Symbol, posd: Positioned)(implicit ctx: Context): Unit =
+    if (sym.exists) {
+      val conv =
+        if (sym.is(Implicit)) sym
+        else {
+          assert(sym.name == nme.apply)
+          sym.owner
+        }
+      val conversionOK =
+        conv.is(Synthetic) ||
+        sym.info.finalResultType.classSymbols.exists(_.isLinkedWith(conv.owner)) ||
+        defn.isPredefClass(conv.owner) ||
+        conv.name == nme.reflectiveSelectable && conv.maybeOwner.maybeOwner.maybeOwner == defn.ScalaPackageClass
+      if (!conversionOK)
+        checkFeature(defn.LanguageModuleClass, nme.implicitConversions,
+          i"Use of implicit conversion ${conv.showLocated}", NoSymbol, posd.sourcePos)
+    }
 
   /** Issue a feature warning if feature is not enabled */
   def checkFeature(base: ClassSymbol,

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

@@ -149,6 +149,8 @@ class CompilationTests extends ParallelTesting {
     compileFilesInDir("tests/neg-custom-args/fatal-warnings", defaultOptions.and("-Xfatal-warnings")) +
     compileFilesInDir("tests/neg-custom-args/allow-double-bindings", allowDoubleBindings) +
     compileDir("tests/neg-custom-args/impl-conv", defaultOptions.and("-Xfatal-warnings", "-feature")) +
+    compileFile("tests/neg-custom-args/implicit-conversions.scala", defaultOptions.and("-Xfatal-warnings", "-feature")) +
+    compileFile("tests/neg-custom-args/implicit-conversions-old.scala", defaultOptions.and("-Xfatal-warnings", "-feature")) +
     compileFile("tests/neg-custom-args/i3246.scala", scala2Mode) +
     compileFile("tests/neg-custom-args/overrideClass.scala", scala2Mode) +
     compileFile("tests/neg-custom-args/autoTuplingTest.scala", defaultOptions.and("-language:noAutoTupling")) +

library/src/scalaShadowing/language.scala

@@ -92,8 +92,10 @@ object language {
    */
   @volatile implicit lazy val reflectiveCalls: reflectiveCalls = languageFeature.reflectiveCalls
 
-  /** Only where enabled, definitions of implicit conversions are allowed. An
-   *  implicit conversion is an implicit value of unary function type `A => B`,
+  /** Only where enabled, definitions of legacy implicit conversions and certain uses
+   *  of implicit conversions are allowed.
+   *
+   *  A legacy implicit conversion is an implicit value of unary function type `A => B`,
    *  or an implicit method that has in its first parameter section a single,
    *  non-implicit parameter. Examples:
    *
@@ -103,17 +105,36 @@ object language {
    *     implicit def listToX(xs: List[T])(implicit f: T => X): X = ...
    *  }}}
    *
-   *  implicit values of other types are not affected, and neither are implicit
-   *  classes.
+   *  Implicit values of other types are not affected, and neither are implicit
+   *  classes. In particular, implied instances of the scala.Conversion class can be
+   *  defined without having to import the language feature.
+   *
+   *  The language import is also required to enable _uses_ of implicit conversions
+   *  unless the conversion in question is co-defined with the type to which it maps.
+   *  Co-defined means: defined in the companion object of the class of the result type.
+   *  Examples:
+   *
+   *  {{{
+   *      class A
+   *      class B
+   *      object B {
+   *        implied a2b for Conversion[A, B] { ... }
+   *      }
+   *      object C {
+   *        implied b2a for Conversion[B, A] { ... }
+   *      }
+   *      import implied B._
+   *      import implied C._
+   *      val x: A = new B     // language import required
+   *      val x: B = new A     // no import necessary since a2b is co-defined with B
+   *  }}}
    *
    *  '''Why keep the feature?''' Implicit conversions are central to many aspects
    *  of Scala’s core libraries.
    *
    *  '''Why control it?''' Implicit conversions are known to cause many pitfalls
-   *  if over-used. And there is a tendency to over-use them because they look
-   *  very powerful and their effects seem to be easy to understand. Also, in
-   *  most situations using implicit parameters leads to a better design than
-   *  implicit conversions.
+   *  if over-used. This holds in particular for implicit conversions defined after
+   *  the fact between unrelated types.
    *
    *  @group production
    */

tests/neg-custom-args/implicit-conversions-old.scala

@@ -0,0 +1,23 @@
+class A
+class B
+
+object A {
+
+  implicit def a2b(x: A): B = ??? // error under -Xfatal-warnings -feature
+
+  implicit def b2a(x: B): A = ??? // error under -Xfatal-warnings -feature
+}
+
+class C
+
+object D {
+  implicit def a2c(x: A): C = ??? // error under -Xfatal-warnings -feature
+}
+
+object Test {
+  import D._
+
+  val x1: A = new B
+  val x2: B = new A  // error under -Xfatal-warnings -feature
+  val x3: C = new A  // error under -Xfatal-warnings -feature
+}

tests/neg-custom-args/implicit-conversions.scala

@@ -0,0 +1,29 @@
+class A
+class B
+
+object A {
+
+  implied for Conversion[A, B] {
+    def apply(x: A): B = ???
+  }
+
+  implied for Conversion[B, A] {
+    def apply(x: B): A = ???
+  }
+}
+
+class C
+
+object D {
+  implied for Conversion[A, C] {
+    def apply(x: A): C = ???
+  }
+}
+
+object Test {
+  import implied D._
+
+  val x1: A = new B
+  val x2: B = new A  // error under -Xfatal-warnings -feature
+  val x3: C = new A  // error under -Xfatal-warnings -feature
+}