Merge pull request #4229 from dotty-staging/change-implicit-convs

Emit feature warnings for implicit conversions.

Author: odersky

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

@@ -436,6 +436,7 @@ object StdNames {
     val head: N                 = "head"
     val higherKinds: N          = "higherKinds"
     val identity: N             = "identity"
+    val implicitConversions: N  = "implicitConversions"
     val implicitly: N           = "implicitly"
     val in: N                   = "in"
     val info: N                 = "info"
@@ -488,6 +489,7 @@ object StdNames {
     val productPrefix: N        = "productPrefix"
     val readResolve: N          = "readResolve"
     val reflect : N             = "reflect"
+    val reflectiveSelectable: N = "reflectiveSelectable"
     val reify : N               = "reify"
     val rootMirror : N          = "rootMirror"
     val run: N                  = "run"

compiler/src/dotty/tools/dotc/reporting/Reporter.scala

@@ -8,7 +8,7 @@ import core.Decorators.PhaseListDecorator
 import collection.mutable
 import java.lang.System.currentTimeMillis
 import core.Mode
-import dotty.tools.dotc.core.Symbols.Symbol
+import dotty.tools.dotc.core.Symbols.{Symbol, NoSymbol}
 import diagnostic.messages._
 import diagnostic._
 import Message._
@@ -39,7 +39,7 @@ trait Reporting { this: Context =>
   def reportWarning(warning: Warning): Unit =
     if (!this.settings.silentWarnings.value) {
       if (this.settings.XfatalWarnings.value) reporter.report(warning.toError)
-      else  reporter.report(warning)
+      else reporter.report(warning)
     }
 
   def deprecationWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
@@ -170,7 +170,10 @@ abstract class Reporter extends interfaces.ReporterResult {
   def errorsReported = hasErrors
 
   private[this] var reportedFeaturesUseSites = Set[Symbol]()
-  def isReportedFeatureUseSite(featureTrait: Symbol): Boolean = reportedFeaturesUseSites.contains(featureTrait)
+
+  def isReportedFeatureUseSite(featureTrait: Symbol): Boolean =
+    featureTrait.ne(NoSymbol) && reportedFeaturesUseSites.contains(featureTrait)
+
   def reportNewFeatureUseSite(featureTrait: Symbol): Unit = reportedFeaturesUseSites += featureTrait
 
   val unreportedWarnings = new mutable.HashMap[String, Int] {
@@ -227,7 +230,7 @@ abstract class Reporter extends interfaces.ReporterResult {
     ctx.mode.is(Mode.Printing)
 
   /** Does this reporter contain not yet reported errors or warnings? */
-  def hasPending: Boolean = false
+  def hasPendingErrors: Boolean = false
 
   /** If this reporter buffers messages, remove and return all buffered messages. */
   def removeBufferedMessages(implicit ctx: Context): List[MessageContainer] = Nil

compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala

@@ -28,13 +28,8 @@ class StoreReporter(outer: Reporter) extends Reporter {
     infos += m
   }
 
-  override def hasPending: Boolean = infos != null && {
-    infos exists {
-      case _: Error => true
-      case _: Warning => true
-      case _ => false
-    }
-  }
+  override def hasPendingErrors: Boolean =
+    infos != null && infos.exists(_.isInstanceOf[Error])
 
   override def removeBufferedMessages(implicit ctx: Context): List[MessageContainer] =
     if (infos != null) try infos.toList finally infos = null

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

@@ -577,6 +577,49 @@ trait Checking {
     case _ =>
   }
 
+  /** If `sym` is an implicit conversion, check that implicit conversions are enabled.
+   *  @pre  sym.is(Implicit)
+   */
+  def checkImplicitConversionDefOK(sym: Symbol)(implicit ctx: Context): Unit = sym.info.stripPoly match {
+    case mt @ MethodType(_ :: Nil)
+    if !mt.isImplicitMethod && !sym.is(Synthetic) => // it's a conversion
+      checkFeature(
+        defn.LanguageModuleClass, nme.implicitConversions,
+        i"Definition of implicit conversion $sym",
+        ctx.owner.topLevelClass,
+        sym.pos)
+    case _ =>
+  }
+
+  /** If `sym` is an implicit conversion, check that that implicit conversions are enabled, unless
+   *    - it is synthetic
+   *    - it is has the same owner as one of the classes it converts to (modulo companions)
+   *    - it is defined in Predef
+   *    - it is the scala.reflect.Selectable.reflectiveSelectable conversion
+   */
+  def checkImplicitConversionUseOK(sym: Symbol, pos: Position)(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, pos)
+  }
+
+  /** Issue a feature warning if feature is not enabled */
+  def checkFeature(base: ClassSymbol,
+                   name: TermName,
+                   description: => String,
+                   featureUseSite: Symbol,
+                   pos: Position)(implicit ctx: Context): Unit =
+    if (!ctx.featureEnabled(base, name))
+      ctx.featureWarning(name.toString, description,
+        isScala2Feature = base.isContainedIn(defn.LanguageModuleClass),
+        featureUseSite, required = false, pos)
+
   /** Check that `tp` is a class type and that any top-level type arguments in this type
    *  are feasible, i.e. that their lower bound conforms to their upper bound. If a type
    *  argument is infeasible, issue and error and continue with upper bound.
@@ -870,6 +913,8 @@ trait NoChecking extends ReChecking {
   override def checkStable(tp: Type, pos: Position)(implicit ctx: Context): Unit = ()
   override def checkClassType(tp: Type, pos: Position, traitReq: Boolean, stablePrefixReq: Boolean)(implicit ctx: Context): Type = tp
   override def checkImplicitParamsNotSingletons(vparamss: List[List[ValDef]])(implicit ctx: Context): Unit = ()
+  override def checkImplicitConversionDefOK(sym: Symbol)(implicit ctx: Context): Unit = ()
+  override def checkImplicitConversionUseOK(sym: Symbol, pos: Position)(implicit ctx: Context): Unit = ()
   override def checkFeasibleParent(tp: Type, pos: Position, where: => String = "")(implicit ctx: Context): Type = tp
   override def checkInlineConformant(tree: Tree, isFinal: Boolean, what: => String)(implicit ctx: Context) = ()
   override def checkNoDoubleDeclaration(cls: Symbol)(implicit ctx: Context): Unit = ()
@@ -881,4 +926,5 @@ trait NoChecking extends ReChecking {
   override def checkCaseInheritance(parentSym: Symbol, caseCls: ClassSymbol, pos: Position)(implicit ctx: Context) = ()
   override def checkNoForwardDependencies(vparams: List[ValDef])(implicit ctx: Context): Unit = ()
   override def checkMembersOK(tp: Type, pos: Position)(implicit ctx: Context): Type = tp
+  override def checkFeature(base: ClassSymbol, name: TermName, description: => String, featureUseSite: Symbol, pos: Position)(implicit ctx: Context): Unit = ()
 }

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

@@ -249,7 +249,12 @@ object ProtoTypes {
           targ = arg.withType(WildcardType)
         else {
           targ = typerFn(arg)
-          if (!ctx.reporter.hasPending) {
+          if (!ctx.reporter.hasPendingErrors) {
+            // FIXME: This can swallow warnings by updating the typerstate from a nested
+            // context that gets discarded later. But we do have to update the
+            // typerstate if there are no errors. If we also omitted the next two lines
+            // when warning were emitted, `pos/t1756.scala` would fail when run with -feature.
+            // It would produce an orphan type parameter for CI when pickling.
             myTypedArg = myTypedArg.updated(arg, targ)
             evalState = evalState.updated(arg, (ctx.typerState, ctx.typerState.constraint))
           }

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

@@ -1405,7 +1405,10 @@ class Typer extends Namer
     val tparams1 = tparams mapconserve (typed(_).asInstanceOf[TypeDef])
     val vparamss1 = vparamss nestedMapconserve (typed(_).asInstanceOf[ValDef])
     vparamss1.foreach(checkNoForwardDependencies)
-    if (sym is Implicit) checkImplicitParamsNotSingletons(vparamss1)
+    if (sym is Implicit) {
+      checkImplicitParamsNotSingletons(vparamss1)
+      checkImplicitConversionDefOK(sym)
+    }
     val tpt1 = checkSimpleKinded(typedType(tpt))
 
     var rhsCtx = ctx
@@ -2441,6 +2444,7 @@ class Typer extends Namer
       if (ctx.mode.is(Mode.ImplicitsEnabled))
         inferView(tree, pt) match {
           case SearchSuccess(inferred, _, _) =>
+            checkImplicitConversionUseOK(inferred.symbol, tree.pos)
             readapt(inferred)(ctx.retractMode(Mode.ImplicitsEnabled))
           case failure: SearchFailure =>
             if (pt.isInstanceOf[ProtoType] && !failure.isAmbiguous)

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

@@ -178,6 +178,7 @@ class CompilationTests extends ParallelTesting {
     compileFilesInDir("tests/neg-kind-polymorphism", defaultOptions and "-Ykind-polymorphism") +
     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/i3246.scala", scala2Mode) +
     compileFile("tests/neg-custom-args/overrideClass.scala", scala2Mode) +
     compileFile("tests/neg-custom-args/autoTuplingTest.scala", defaultOptions.and("-language:noAutoTupling")) +

compiler/test/dotty/tools/dotc/reporting/UserDefinedErrorMessages.scala

@@ -88,6 +88,7 @@ class UserDefinedErrorMessages extends ErrorMessagesTest {
     checkMessagesAfter("frontend") {
       """
         |class C {
+        |  import scala.language.implicitConversions
         |  @annotation.implicitAmbiguous("msg A=${A}")
         |  implicit def f[A](x: Int): String = "f was here"
         |  implicit def g(x: Int): String = "f was here"

tests/neg-custom-args/impl-conv/A.scala

@@ -0,0 +1,11 @@
+package implConv
+
+object A {
+
+  implicit def s2i(x: String): Int = Integer.parseInt(x) // error: feature
+
+  implicit class Foo(x: String) {
+    def foo = x.length
+  }
+
+}

tests/neg-custom-args/impl-conv/B.scala

@@ -0,0 +1,10 @@
+package implConv
+
+object B {
+  import A._
+
+  "".foo
+
+  val x: Int = ""  // error: feature
+
+}