From b566be7d8da3b8ef1659698a1614a6bfe18814ff Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 30 Jul 2026 19:28:44 +0200 Subject: [PATCH 1/2] Tighten rules for implicit conversions Make use of a given (Scala-3 style) implicit Conversion instance without a `language.implicitConversions` language import a warning in 3.10 and an error in 3.11. --- .../tools/dotc/config/SourceVersion.scala | 3 + compiler/src/dotty/tools/dotc/report.scala | 32 +++++++--- .../src/dotty/tools/dotc/typer/Checking.scala | 17 +++-- .../src/dotty/tools/dotc/typer/Typer.scala | 6 +- library/src/scala/language.scala | 64 +++++++++++++------ tests/neg/conversion-lang-import.check | 9 +++ tests/neg/conversion-lang-import.scala | 11 ++++ tests/neg/into-inferred.check | 56 +++++++++------- tests/warn/conditionalWarnings.scala | 6 +- tests/warn/conversion-lang-import.check | 10 +++ tests/warn/conversion-lang-import.scala | 10 +++ 11 files changed, 161 insertions(+), 63 deletions(-) create mode 100644 tests/neg/conversion-lang-import.check create mode 100644 tests/neg/conversion-lang-import.scala create mode 100644 tests/warn/conversion-lang-import.check create mode 100644 tests/warn/conversion-lang-import.scala diff --git a/compiler/src/dotty/tools/dotc/config/SourceVersion.scala b/compiler/src/dotty/tools/dotc/config/SourceVersion.scala index 799b5be9d0e5..03464bf4a45f 100644 --- a/compiler/src/dotty/tools/dotc/config/SourceVersion.scala +++ b/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 */ diff --git a/compiler/src/dotty/tools/dotc/report.scala b/compiler/src/dotty/tools/dotc/report.scala index 359e3e17e21a..e7251a5fbf6c 100644 --- a/compiler/src/dotty/tools/dotc/report.scala +++ b/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,11 +40,11 @@ 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) @@ -48,16 +52,26 @@ object report: |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)) diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 895a7142a7bf..16a3720b1916 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/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 { diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 287c6addb5ba..011c68b97665 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/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) diff --git a/library/src/scala/language.scala b/library/src/scala/language.scala index df54f0791546..03cdcb13ac2e 100644 --- a/library/src/scala/language.scala +++ b/library/src/scala/language.scala @@ -99,14 +99,13 @@ 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" @@ -114,23 +113,50 @@ object language { * 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 */ diff --git a/tests/neg/conversion-lang-import.check b/tests/neg/conversion-lang-import.check new file mode 100644 index 000000000000..4dffe47f341c --- /dev/null +++ b/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. diff --git a/tests/neg/conversion-lang-import.scala b/tests/neg/conversion-lang-import.scala new file mode 100644 index 000000000000..ad4ec4b61424 --- /dev/null +++ b/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 diff --git a/tests/neg/into-inferred.check b/tests/neg/into-inferred.check index 6f97e883aa29..b30d8e9e4b47 100644 --- a/tests/neg/into-inferred.check +++ b/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) diff --git a/tests/warn/conditionalWarnings.scala b/tests/warn/conditionalWarnings.scala index 5ab63c263abf..8c2d60f7d8d1 100644 --- a/tests/warn/conditionalWarnings.scala +++ b/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 diff --git a/tests/warn/conversion-lang-import.check b/tests/warn/conversion-lang-import.check new file mode 100644 index 000000000000..f43a242b90cf --- /dev/null +++ b/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) diff --git a/tests/warn/conversion-lang-import.scala b/tests/warn/conversion-lang-import.scala new file mode 100644 index 000000000000..31986ac85835 --- /dev/null +++ b/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 From 1a341ab2e479f50fe1b84dcf99f3d1da11679131 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 30 Jul 2026 20:28:08 +0200 Subject: [PATCH 2/2] Change ScriptedTest --- sbt-test/compilerReporter/i14576/Test.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sbt-test/compilerReporter/i14576/Test.scala b/sbt-test/compilerReporter/i14576/Test.scala index 4f65c2267134..cce4ee6500ef 100644 --- a/sbt-test/compilerReporter/i14576/Test.scala +++ b/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")