Tighten rules for implicit conversions

compiler/src/dotty/tools/dotc/config/SourceVersion.scala

@@ -53,6 +53,9 @@ enum SourceVersion:
 
   def requiresNewSyntax = isAtLeast(future)
 
+  def warnOnConversion = isAtLeast(`3.10`)
+  def errorOnConversion = isAtLeast(`3.11`)
+
 object SourceVersion extends Property.Key[SourceVersion]:
 
   /* The default source version used by the built compiler */

compiler/src/dotty/tools/dotc/report.scala

@@ -10,6 +10,10 @@ import java.lang.System.currentTimeMillis
 
 object report:
 
+  /** What kind of diagnostics to give for a feature warning */
+  enum Severity:
+    case FeatureWarning, Warning, Error, WarningThenError
+
   /** For sending messages that are printed only if -verbose is set */
   def inform(msg: => String, pos: SrcPos = NoSourcePosition)(using Context): Unit =
     if ctx.settings.verbose.value then echo(msg, pos)
@@ -36,28 +40,38 @@ object report:
     issueWarning(new FeatureWarning(msg, pos.sourcePos))
 
   def featureWarning(feature: String, featureDescription: => String,
-      featureUseSite: Symbol, required: Boolean, pos: SrcPos)(using Context): Unit =
-    val req = if required then "needs to" else "should"
-    val fqname = s"scala.language.$feature"
+      featureUseSite: Symbol, severity: Severity, pos: SrcPos)(using Context): Unit = {
+    val req = if severity == Severity.FeatureWarning then "should" else "needs to"
+    def fqname = s"scala.language.$feature"
 
-    val explain =
+    def explain =
       if ctx.reporter.isReportedFeatureUseSite(featureUseSite) then ""
       else
         ctx.reporter.reportNewFeatureUseSite(featureUseSite)
         s"""
            |See the Scala docs for value $fqname for a discussion
            |why the feature $req be explicitly enabled.""".stripMargin
 
+    def postscript =
+      if severity == Severity.WarningThenError
+      then "\n(This will be an error in later Scala versions)"
+      else ""
+
     def msg = em"""$featureDescription $req be enabled
                   |by adding the import clause 'import $fqname'
-                  |or by setting the compiler option -language:$feature.$explain"""
-    if required then error(msg, pos)
-    else issueWarning(new FeatureWarning(msg, pos.sourcePos))
-  end featureWarning
+                  |or by setting the compiler option -language:$feature.$explain$postscript"""
+    severity match
+      case Severity.FeatureWarning =>
+        issueWarning(new FeatureWarning(msg, pos.sourcePos))
+      case Severity.Warning | Severity.WarningThenError =>
+        warning(msg, pos)
+      case Severity.Error =>
+        error(msg, pos)
+  }
 
   def optimizerWarning(msg: String, site: String, pos: SrcPos)(using Context): Unit =
     issueWarning(new OptimizerWarning(em"$msg", site, addInlineds(pos)))
-  
+
   def warning(msg: Message, pos: SrcPos, origin: String)(using Context): Unit =
     issueWarning(LintWarning(msg, addInlineds(pos), origin))
 

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

@@ -41,7 +41,7 @@ import cc.{isCaptureChecking, RetainingAnnotation, isRetainsLike, isDisallowedIn
 import cc.Mutability.isUpdateMethod
 
 import collection.mutable
-import reporting.*
+import reporting.*, report.Severity
 import Annotations.ExperimentalAnnotation
 
 object Checking {
@@ -1275,7 +1275,13 @@ trait Checking {
         case Select(qual, _) => qual.symbol.orElse(sym.owner)
         case _ => sym.owner
       checkFeature(nme.implicitConversions,
-        i"Use of implicit conversion ${conv.showLocated}", NoSymbol, tree.srcPos)
+        i"""Implicit conversion to type $expected is not to an `into[...]` type.
+           |Therefore, the use of the implicit conversion ${conv.showLocated}""",
+        NoSymbol, tree.srcPos,
+        severity =
+          if sourceVersion.errorOnConversion then Severity.Error
+          else if sourceVersion.warnOnConversion then Severity.WarningThenError
+          else Severity.FeatureWarning)
 
   private def infixOKSinceFollowedBy(tree: untpd.Tree): Boolean = tree match {
     case _: untpd.Block | _: untpd.Match => true
@@ -1325,9 +1331,10 @@ trait Checking {
   def checkFeature(name: TermName,
                    description: => String,
                    featureUseSite: Symbol,
-                   pos: SrcPos)(using Context): Unit =
+                   pos: SrcPos,
+                   severity: Severity = Severity.FeatureWarning)(using Context): Unit =
     if !Feature.enabled(name) then
-      report.featureWarning(name.toString, description, featureUseSite, required = false, pos)
+      report.featureWarning(name.toString, description, featureUseSite, severity, 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
@@ -1810,7 +1817,7 @@ trait ReChecking extends Checking {
   override def checkCanThrow(tp: Type, span: Span)(using Context): Tree = EmptyTree
   override def checkCatch(pat: Tree, guard: Tree)(using Context): Unit = ()
   override def checkNoContextFunctionType(tree: Tree)(using Context): Unit = ()
-  override def checkFeature(name: TermName, description: => String, featureUseSite: Symbol, pos: SrcPos)(using Context): Unit = ()
+  override def checkFeature(name: TermName, description: => String, featureUseSite: Symbol, pos: SrcPos, severity: Severity)(using Context): Unit = ()
 }
 
 trait NoChecking extends ReChecking {

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

@@ -3542,8 +3542,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         cdef1.tpe.derivesFrom(defn.DynamicClass) &&
         !Feature.dynamicsEnabled
       if (reportDynamicInheritance) {
-        val isRequired = parents1.exists(_.tpe.isRef(defn.DynamicClass))
-        report.featureWarning(nme.dynamics.toString, "extension of type scala.Dynamic", cls, isRequired, cdef.srcPos)
+        val severity =
+          if parents1.exists(_.tpe.isRef(defn.DynamicClass)) then report.Severity.Error
+          else report.Severity.FeatureWarning
+        report.featureWarning(nme.dynamics.toString, "extension of type scala.Dynamic", cls, severity, cdef.srcPos)
       }
 
       checkNonCyclicInherited(cls.thisType, cls.info.parents, cls.info.decls, cdef.srcPos)

library/src/scala/language.scala

@@ -99,38 +99,64 @@ object language {
    */
   implicit lazy val reflectiveCalls: reflectiveCalls = languageFeature.reflectiveCalls
 
-  /** Where this feature is enabled, definitions of implicit conversion methods are allowed.
-   *  If `implicitConversions` is not enabled, the definition of an implicit
-   *  conversion method will trigger a warning from the compiler.
-   *
-   *  An 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:
+  /** There are two kinds of implicit conversions, depending on whether we are in
+   *  Scala 2 or 3. This language import has different meanings depending on which kind
+   *  of conversion is used.
    *
+   *  In Scala 2, an implicit conversion is an implicit method that has in its first
+   *  parameter section a single, non-implicit parameter, or it is an implicit value
+   *  of function type A => B. Examples:
    *  ```
    *     implicit def intToString(i: Int): String = s"\$i"
    *     implicit val conv: Int => String = i => s"\$i"
    *     implicit val numerals: List[String] = List("zero", "one", "two", "three")
    *     implicit val strlen: String => Int = _.length
    *     implicit def listToInt[T](xs: List[T])(implicit f: T => Int): Int = xs.map(f).sum
    *  ```
+   *  The language import controls whether _definitions_ of these Scala 2 conversions
+   *  are allowed. If `implicitConversions` is not enabled, the definition of an implicit
+   *  conversion method will trigger a warning from the compiler. The warning is only for
+   *  implicit conversions introduced by methods, implicit values are unaffected.
+   *  Implicit class definitions, which introduce a conversion to the wrapping class,
+   *  also do not warn.
    *
-   *  This language feature warns only for implicit conversions introduced by methods.
+   *  In Scala 3, an implicit conversion is a given of type `scala.Conversion`.
+   *  Examples:
+   *  ```
+   *      given Conversion[Int, BigInt] = (i: Int) => BigInt(i)
+   *      given [T] => Conversion[T, Option[T]] = Some(_)
+   *  ```
+   *  As of Scala 3.9, the language still supports Scala-2 style conversions, but these
+   *  are slated to be phased out.
    *
-   *  Other values, including functions or data types which extend `Function1`,
-   *  such as `Map`, `Set`, and `List`, do not warn.
+   *  In Scala 3, the _use_ of a (new style) implicit conversion triggers a warning or an error
+   *  if the `implicitConversions` language import is not given. Exempted are only uses that
+   *  map a value into an `into[...]` type, since `into[...]` signifies that an implicit
+   *  conversion is allowed. Everwhere else the language import has to be given. Example:
+   *  ```
+   *      import Conversion.into
+   *      given [T] => Conversion[T, Option[T]] = Some(_)
    *
-   *  Implicit class definitions, which introduce a conversion to the wrapping class,
-   *  also do not warn.
+   *      def strict(x: Option[String]) = ()
+   *      def lenient(x: into[Option[String]]) = ()
+   *
+   *      strict("abc")    // error or warning: language import needed
+   *      lenient("abc")   // ok
+   *  ```
+   *  The precise kind of diagnostic depends on the Scala version:
+   *
+   *   - For Scala 3.0 - 3.9: feature warning.
+   *   - For Scala 3.10: regular warning.
+   *   - For Scala 3.11 and higher: error.
    *
-   *  **Why keep the feature?** Implicit conversions are central to many aspects
-   *  of Scala’s core libraries.
+   *  **Why keep the feature?** Implicit conversions can add flexibility. They are
+   *  widely used in many libraries including Scala’s collection library.
    *
-   *  **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.
+   *  **Why control it?** Implicit conversions may have unexpected global effects,
+   *  potentially causing unexpected behavior or performance degradation.
+   *  Their presence also imposes a tax on precise type inference. It's therefore
+   *  better to limit their use to specific situations where a library explicitly allows
+   *  them by wrapping expected argument types in `into`.
    *
    *  @group production
    */

sbt-test/compilerReporter/i14576/Test.scala

@@ -6,7 +6,7 @@ class Text(val str: String)
 
 object Test:
   // lampepfl/dotty#14500, requires implicitConversions feature
-  given Conversion[String, Text] = Text(_)
+  implicit def f(x: String): Int = x.length
   def f(x: Text) = println(x.str)
   f("abc")
 

tests/neg/conversion-lang-import.check

@@ -0,0 +1,9 @@
+-- Error: tests/neg/conversion-lang-import.scala:10:6 ------------------------------------------------------------------
+10 |  foo("abc")  // error
+   |      ^^^^^
+   |      Implicit conversion to type Option[String] is not to an `into[...]` type.
+   |      Therefore, the use of the implicit conversion given instance given_Conversion_A_Option needs to be enabled
+   |      by adding the import clause 'import scala.language.implicitConversions'
+   |      or by setting the compiler option -language:implicitConversions.
+   |      See the Scala docs for value scala.language.implicitConversions for a discussion
+   |      why the feature needs to be explicitly enabled.

tests/neg/conversion-lang-import.scala

@@ -0,0 +1,11 @@
+//> using options -source 3.11
+import Conversion.into
+
+given [A] => Conversion[A, Option[A]] = Some(_)
+
+def foo(x: Option[String]) = ()
+def bar(x: into[Option[String]]) = ()
+
+def test =
+  foo("abc")  // error
+  bar("abc")  // ok

tests/neg/into-inferred.check

@@ -5,35 +5,43 @@
    |                                Required: List[Conversion.into[Keyword]]
    |
    | longer explanation available when compiling with `-explain`
--- Feature Warning: tests/neg/into-inferred.scala:21:43 ----------------------------------------------------------------
+-- Warning: tests/neg/into-inferred.scala:21:43 ------------------------------------------------------------------------
 21 |  val ys: List[into[Keyword]] = List(ifKW, "then", "else")         // warn // warn
    |                                           ^^^^^^
-   |      Use of implicit conversion given instance given_Conversion_String_Keyword in object Test should be enabled
-   |      by adding the import clause 'import scala.language.implicitConversions'
-   |      or by setting the compiler option -language:implicitConversions.
-   |      See the Scala docs for value scala.language.implicitConversions for a discussion
-   |      why the feature should be explicitly enabled.
--- Feature Warning: tests/neg/into-inferred.scala:21:51 ----------------------------------------------------------------
+   |Implicit conversion to type A is not to an `into[...]` type.
+   |Therefore, the use of the implicit conversion given instance given_Conversion_String_Keyword in object Test needs to be enabled
+   |by adding the import clause 'import scala.language.implicitConversions'
+   |or by setting the compiler option -language:implicitConversions.
+   |See the Scala docs for value scala.language.implicitConversions for a discussion
+   |why the feature needs to be explicitly enabled.
+   |(This will be an error in later Scala versions)
+-- Warning: tests/neg/into-inferred.scala:21:51 ------------------------------------------------------------------------
 21 |  val ys: List[into[Keyword]] = List(ifKW, "then", "else")         // warn // warn
    |                                                   ^^^^^^
-   |      Use of implicit conversion given instance given_Conversion_String_Keyword in object Test should be enabled
-   |      by adding the import clause 'import scala.language.implicitConversions'
-   |      or by setting the compiler option -language:implicitConversions.
-   |      See the Scala docs for value scala.language.implicitConversions for a discussion
-   |      why the feature should be explicitly enabled.
--- Feature Warning: tests/neg/into-inferred.scala:34:42 ----------------------------------------------------------------
+   |Implicit conversion to type A is not to an `into[...]` type.
+   |Therefore, the use of the implicit conversion given instance given_Conversion_String_Keyword in object Test needs to be enabled
+   |by adding the import clause 'import scala.language.implicitConversions'
+   |or by setting the compiler option -language:implicitConversions.
+   |See the Scala docs for value scala.language.implicitConversions for a discussion
+   |why the feature needs to be explicitly enabled.
+   |(This will be an error in later Scala versions)
+-- Warning: tests/neg/into-inferred.scala:34:42 ------------------------------------------------------------------------
 34 |  val l2: List[into[Keyword]] = l ++ List("then", "else")  // warn // warn
    |                                          ^^^^^^
-   |      Use of implicit conversion given instance given_Conversion_String_Keyword in object Test should be enabled
-   |      by adding the import clause 'import scala.language.implicitConversions'
-   |      or by setting the compiler option -language:implicitConversions.
-   |      See the Scala docs for value scala.language.implicitConversions for a discussion
-   |      why the feature should be explicitly enabled.
--- Feature Warning: tests/neg/into-inferred.scala:34:50 ----------------------------------------------------------------
+   |Implicit conversion to type A is not to an `into[...]` type.
+   |Therefore, the use of the implicit conversion given instance given_Conversion_String_Keyword in object Test needs to be enabled
+   |by adding the import clause 'import scala.language.implicitConversions'
+   |or by setting the compiler option -language:implicitConversions.
+   |See the Scala docs for value scala.language.implicitConversions for a discussion
+   |why the feature needs to be explicitly enabled.
+   |(This will be an error in later Scala versions)
+-- Warning: tests/neg/into-inferred.scala:34:50 ------------------------------------------------------------------------
 34 |  val l2: List[into[Keyword]] = l ++ List("then", "else")  // warn // warn
    |                                                  ^^^^^^
-   |      Use of implicit conversion given instance given_Conversion_String_Keyword in object Test should be enabled
-   |      by adding the import clause 'import scala.language.implicitConversions'
-   |      or by setting the compiler option -language:implicitConversions.
-   |      See the Scala docs for value scala.language.implicitConversions for a discussion
-   |      why the feature should be explicitly enabled.
+   |Implicit conversion to type A is not to an `into[...]` type.
+   |Therefore, the use of the implicit conversion given instance given_Conversion_String_Keyword in object Test needs to be enabled
+   |by adding the import clause 'import scala.language.implicitConversions'
+   |or by setting the compiler option -language:implicitConversions.
+   |See the Scala docs for value scala.language.implicitConversions for a discussion
+   |why the feature needs to be explicitly enabled.
+   |(This will be an error in later Scala versions)

tests/warn/conditionalWarnings.scala

@@ -1,13 +1,11 @@
-//> using options -deprecation 
+//> using options -deprecation
 
 object Test {
   @deprecated def foo = ???
 
-  given Conversion[String, Int] = _.length
-
   foo    // warn
 
-  val x: Int = "abc"
+  implicit def c(x: Int): String = "abc"
     // OK, since -feature warnings are not enabled.
     // The program compiles with final line
     // there was 1 feature warning; re-run with -feature for details

tests/warn/conversion-lang-import.check

@@ -0,0 +1,10 @@
+-- Warning: tests/warn/conversion-lang-import.scala:9:6 ----------------------------------------------------------------
+9 |  foo("abc")  // warn
+  |      ^^^^^
+  |      Implicit conversion to type Option[String] is not to an `into[...]` type.
+  |      Therefore, the use of the implicit conversion given instance given_Conversion_A_Option needs to be enabled
+  |      by adding the import clause 'import scala.language.implicitConversions'
+  |      or by setting the compiler option -language:implicitConversions.
+  |      See the Scala docs for value scala.language.implicitConversions for a discussion
+  |      why the feature needs to be explicitly enabled.
+  |      (This will be an error in later Scala versions)

tests/warn/conversion-lang-import.scala

@@ -0,0 +1,10 @@
+import Conversion.into
+
+given [A] => Conversion[A, Option[A]] = Some(_)
+
+def foo(x: Option[String]) = ()
+def bar(x: into[Option[String]]) = ()
+
+def test =
+  foo("abc")  // warn
+  bar("abc")  // ok