From 556893bdf097cd63f985401eb89d28998c7ec19f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 2 Oct 2020 13:46:09 +0200 Subject: [PATCH] Change implicit conversions warnings Warn with a feature warning if - an old-style implicit def conversion is defined, or - a new style scala.Conversion instance is used as an implicit conversion Do not warn if an old-style conversion is used. There are too many warnings for this to work well. Instead, rely on the fact that old-style implicit conversions will be deprecated over time. --- .../src/dotty/tools/dotc/typer/Checking.scala | 38 ++++++++----------- .../src/dotty/tools/dotc/typer/Typer.scala | 2 +- library/src/scala/Conversion.scala | 3 +- tests/neg-custom-args/impl-conv/A.scala | 2 +- tests/neg-custom-args/impl-conv/B.scala | 5 +-- .../implicit-conversions-old.scala | 4 +- .../implicit-conversions.scala | 2 +- 7 files changed, 24 insertions(+), 32 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 9a01bb16a096..6d4bb91b8621 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -761,29 +761,21 @@ trait Checking { } } - /** 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 + /** If `tree` is an application of a new-style implicit conversion (using the apply + * method of a `scala.Conversion` instance), check that implicit conversions are + * enabled. */ - def checkImplicitConversionUseOK(sym: Symbol, pos: SrcPos)(using Context): Unit = - if (sym.exists) { - val conv = - if (sym.isOneOf(GivenOrImplicit) || sym.info.isErroneous) sym - else { - assert(sym.name == nme.apply || ctx.reporter.errorsReported) - 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(nme.implicitConversions, - i"Use of implicit conversion ${conv.showLocated}", NoSymbol, pos) - } + def checkImplicitConversionUseOK(tree: Tree)(using Context): Unit = + val sym = tree.symbol + if sym.name == nme.apply + && sym.owner.derivesFrom(defn.ConversionClass) + && !sym.info.isErroneous + then + def conv = methPart(tree) match + case Select(qual, _) => qual.symbol.orElse(sym.owner) + case _ => sym.owner + checkFeature(nme.implicitConversions, + i"Use of implicit conversion ${conv.showLocated}", NoSymbol, tree.srcPos) private def infixOKSinceFollowedBy(tree: untpd.Tree): Boolean = tree match { case _: untpd.Block | _: untpd.Match => true @@ -1244,7 +1236,7 @@ trait NoChecking extends ReChecking { override def checkStable(tp: Type, pos: SrcPos, kind: String)(using Context): Unit = () override def checkClassType(tp: Type, pos: SrcPos, traitReq: Boolean, stablePrefixReq: Boolean)(using Context): Type = tp override def checkImplicitConversionDefOK(sym: Symbol)(using Context): Unit = () - override def checkImplicitConversionUseOK(sym: Symbol, pos: SrcPos)(using Context): Unit = () + override def checkImplicitConversionUseOK(tree: Tree)(using Context): Unit = () override def checkFeasibleParent(tp: Type, pos: SrcPos, where: => String = "")(using Context): Type = tp override def checkInlineConformant(tpt: Tree, tree: Tree, sym: Symbol)(using Context): Unit = () override def checkNoAlphaConflict(stats: List[Tree])(using Context): Unit = () diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 9e9e8ba05ec4..bb92346812bf 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -3490,7 +3490,7 @@ class Typer extends Namer case SearchSuccess(found: ExtMethodApply, _, _) => found // nothing to check or adapt for extension method applications case SearchSuccess(found, _, _) => - checkImplicitConversionUseOK(found.symbol, tree.srcPos) + checkImplicitConversionUseOK(found) withoutMode(Mode.ImplicitsEnabled)(readapt(found)) case failure: SearchFailure => if (pt.isInstanceOf[ProtoType] && !failure.isAmbiguous) then diff --git a/library/src/scala/Conversion.scala b/library/src/scala/Conversion.scala index fd7aa0b4b01f..bb66a068e09c 100644 --- a/library/src/scala/Conversion.scala +++ b/library/src/scala/Conversion.scala @@ -19,4 +19,5 @@ package scala * from two to one. */ @java.lang.FunctionalInterface -abstract class Conversion[-T, +U] extends Function1[T, U] +abstract class Conversion[-T, +U] extends Function1[T, U]: + def apply(x: T): U diff --git a/tests/neg-custom-args/impl-conv/A.scala b/tests/neg-custom-args/impl-conv/A.scala index 6a2de9e6dac4..4598172fd18a 100644 --- a/tests/neg-custom-args/impl-conv/A.scala +++ b/tests/neg-custom-args/impl-conv/A.scala @@ -3,7 +3,7 @@ package implConv object A { implicit def s2i(x: String): Int = Integer.parseInt(x) // error: feature - implicit val i2s: Conversion[Int, String] = _.toString // error: feature + given i2s as Conversion[Int, String] = _.toString // ok implicit class Foo(x: String) { def foo = x.length diff --git a/tests/neg-custom-args/impl-conv/B.scala b/tests/neg-custom-args/impl-conv/B.scala index 3ea37791dac3..219b0617c563 100644 --- a/tests/neg-custom-args/impl-conv/B.scala +++ b/tests/neg-custom-args/impl-conv/B.scala @@ -1,11 +1,10 @@ package implConv object B { - import A._ + import A.{_, given _} "".foo - val x: Int = "" // error: feature + val x: Int = "" // ok val y: String = 1 // error: feature - } diff --git a/tests/neg-custom-args/implicit-conversions-old.scala b/tests/neg-custom-args/implicit-conversions-old.scala index 6abc2a33dfca..ca2bf150ddf5 100644 --- a/tests/neg-custom-args/implicit-conversions-old.scala +++ b/tests/neg-custom-args/implicit-conversions-old.scala @@ -18,6 +18,6 @@ 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 + val x2: B = new A // ok, since it's an old-style comversion + val x3: C = new A // ok, since it's an old-style comversion } \ No newline at end of file diff --git a/tests/neg-custom-args/implicit-conversions.scala b/tests/neg-custom-args/implicit-conversions.scala index 5eeb971aabf0..95148ff72344 100644 --- a/tests/neg-custom-args/implicit-conversions.scala +++ b/tests/neg-custom-args/implicit-conversions.scala @@ -23,7 +23,7 @@ object D { object Test { import D.{given _} - val x1: A = new B + val x1: A = new B // error under -Xfatal-warnings -feature val x2: B = new A // error under -Xfatal-warnings -feature val x3: C = new A // error under -Xfatal-warnings -feature } \ No newline at end of file