From 3c9356277f43ae4cce6df8491442d0bb08265cba Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 28 Jun 2025 18:45:05 +0200 Subject: [PATCH] Disallow Scala 2 implicits under -source future - Offer a rewrite for implicit parameters to using clauses - Provide hints in the error message on how to rewrite the rest --- .../tools/dotc/config/MigrationVersion.scala | 1 + .../dotty/tools/dotc/parsing/Parsers.scala | 12 +++ .../tools/dotc/printing/PlainPrinter.scala | 2 +- .../dotty/tools/dotc/printing/Printer.scala | 5 +- .../src/dotty/tools/dotc/typer/Checking.scala | 78 ++++++++++++++++++- tests/neg/i22440.check | 5 ++ tests/neg/i22440.scala | 2 +- tests/neg/i8896-b.scala | 2 +- tests/neg/implicit-migration.check | 64 +++++++++++++++ tests/neg/implicit-migration.scala | 17 ++++ tests/pos/i3920.scala | 3 +- tests/pos/infer-tracked-1.scala | 3 +- tests/rewrites/implicit-rewrite.check | 9 +++ tests/rewrites/implicit-rewrite.scala | 9 +++ tests/warn/i9266.check | 18 +++-- tests/warn/i9266.scala | 3 +- 16 files changed, 217 insertions(+), 16 deletions(-) create mode 100644 tests/neg/implicit-migration.check create mode 100644 tests/neg/implicit-migration.scala create mode 100644 tests/rewrites/implicit-rewrite.check create mode 100644 tests/rewrites/implicit-rewrite.scala diff --git a/compiler/src/dotty/tools/dotc/config/MigrationVersion.scala b/compiler/src/dotty/tools/dotc/config/MigrationVersion.scala index f77aa0b06308..a6c9f6e85748 100644 --- a/compiler/src/dotty/tools/dotc/config/MigrationVersion.scala +++ b/compiler/src/dotty/tools/dotc/config/MigrationVersion.scala @@ -33,6 +33,7 @@ enum MigrationVersion(val warnFrom: SourceVersion, val errorFrom: SourceVersion) case XmlLiteral extends MigrationVersion(future, future) case GivenSyntax extends MigrationVersion(future, never) case ImplicitParamsWithoutUsing extends MigrationVersion(`3.7`, future) + case Scala2Implicits extends MigrationVersion(future, future) require(warnFrom.ordinal <= errorFrom.ordinal) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index f141da79da79..e4314b27a32c 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -2377,6 +2377,9 @@ object Parsers { val start = in.offset in.token match case IMPLICIT => + report.errorOrMigrationWarning( + em"`implicit` lambdas are no longer supported, use a lambda with `?=>` instead", + in.sourcePos(), MigrationVersion.Scala2Implicits) closure(start, location, modifiers(BitSet(IMPLICIT))) case LBRACKET => val start = in.offset @@ -3557,11 +3560,17 @@ object Parsers { def paramMods() = if in.token == IMPLICIT then + report.errorOrMigrationWarning( + em"`implicit` parameters are no longer supported, use a `using` clause instead${rewriteNotice(`future-migration`)}", + in.sourcePos(), MigrationVersion.Scala2Implicits) + val startImplicit = in.offset addParamMod(() => if ctx.settings.YimplicitToGiven.value then patch(Span(in.lastOffset - 8, in.lastOffset), "using") Mod.Implicit() ) + if MigrationVersion.Scala2Implicits.needsPatch then + patch(source, Span(startImplicit, in.lastOffset), "using") else if isIdent(nme.using) then if initialMods.is(Given) then syntaxError(em"`using` is already implied here, should not be given explicitly", in.offset) @@ -4769,6 +4778,9 @@ object Parsers { else if (isExprIntro) stats += expr(Location.InBlock) else if in.token == IMPLICIT && !in.inModifierPosition() then + report.errorOrMigrationWarning( + em"`implicit` lambdas are no longer supported, use a lambda with `?=>` instead", + in.sourcePos(), MigrationVersion.Scala2Implicits) stats += closure(in.offset, Location.InBlock, modifiers(BitSet(IMPLICIT))) else if isIdent(nme.extension) && followingIsExtension() then stats += extension() diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index f88e9bac4c73..f6e60cd990d6 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -382,7 +382,7 @@ class PlainPrinter(_ctx: Context) extends Printer { protected def specialAnnotText(sym: ClassSymbol, tp: Type): Text = Str(s"@${sym.name} ").provided(tp.hasAnnotation(sym)) - protected def paramsText(lam: LambdaType): Text = { + def paramsText(lam: LambdaType): Text = { def paramText(ref: ParamRef) = val erased = ref.underlying.hasAnnotation(defn.ErasedParamAnnot) keywordText("erased ").provided(erased) diff --git a/compiler/src/dotty/tools/dotc/printing/Printer.scala b/compiler/src/dotty/tools/dotc/printing/Printer.scala index 9b37589585f0..761e6a6bb6ba 100644 --- a/compiler/src/dotty/tools/dotc/printing/Printer.scala +++ b/compiler/src/dotty/tools/dotc/printing/Printer.scala @@ -4,7 +4,7 @@ package printing import core.* import Texts.*, ast.Trees.* -import Types.{Type, SingletonType, LambdaParam, NamedType, RefinedType}, +import Types.{Type, SingletonType, LambdaParam, LambdaType, NamedType, RefinedType}, Symbols.Symbol, Scopes.Scope, Constants.Constant, Names.Name, Denotations._, Annotations.Annotation, Contexts.Context import typer.Implicits.* @@ -147,6 +147,9 @@ abstract class Printer { /** Textual representation of lambda param */ def toText(tree: LambdaParam): Text + /** textual representation of parameters of function type */ + def paramsText(lam: LambdaType): Text + /** Textual representation of all symbols in given list, * using `dclText` for displaying each. */ diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index df4350f1eb05..73918dcaeb84 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -512,6 +512,80 @@ object Checking { } } + def checkScala2Implicit(sym: Symbol)(using Context): Unit = + def migration(msg: Message) = + report.errorOrMigrationWarning(msg, sym.srcPos, MigrationVersion.Scala2Implicits) + def info = sym match + case sym: ClassSymbol => sym.primaryConstructor.info + case _ => sym.info + def paramName = info.firstParamNames match + case pname :: _ => pname.show + case _ => "x" + def paramTypeStr = info.firstParamTypes match + case pinfo :: _ => pinfo.show + case _ => "T" + def toFunctionStr(info: Type): String = info match + case ExprType(resType) => + i"() => $resType" + case info: MethodType => + i"(${ctx.printer.paramsText(info).mkString()}) => ${toFunctionStr(info.resType)}" + case info: PolyType => + i"[${ctx.printer.paramsText(info).mkString()}] => ${toFunctionStr(info.resType)}" + case _ => + info.show + + if sym.isClass then + migration( + em"""`implicit` classes are no longer supported. They can usually be replaced + |by extension methods. Example: + | + | extension ($paramName: $paramTypeStr) + | // class methods go here, replace `this` by `$paramName` + | + |Alternatively, convert to a regular class and define + |a given `Conversion` instance into that class. Example: + | + | class ${sym.name} ... + | given Conversion[$paramTypeStr, ${sym.name}] = ${sym.name}($paramName) + | + |""") + else if sym.isOldStyleImplicitConversion(directOnly = true) then + migration( + em"""`implicit` conversion methods are no longer supported. They can usually be + |replaced by given instances of class `Conversion`. Example: + | + | given Conversion[$paramTypeStr, ${sym.info.finalResultType}] = $paramName => ... + | + |""") + else if sym.is(Method) then + if !sym.isOldStyleImplicitConversion(forImplicitClassOnly = true) then + migration( + em"""`implicit` defs are no longer supported, use a `given` clause instead. Example: + | + | given ${sym.name}: ${toFunctionStr(sym.info)} = ... + | + |""") + else if sym.isTerm && !sym.isOneOf(TermParamOrAccessor) then + def note = + if sym.is(Lazy) then "" + else + i""" + | + |Note: given clauses are evaluated lazily unless the right hand side is + |a simple reference. If eager evaluation of the value's right hand side + |is important, you can define a regular val and a given instance like this: + | + | val ${sym.name} = ... + | given ${sym.info} = ${sym.name}""" + + migration( + em"""`implicit` vals are no longer supported, use a `given` clause instead. Example: + | + | given ${sym.name}: ${sym.info} = ...$note + | + |""") + end checkScala2Implicit + /** Check that symbol's definition is well-formed. */ def checkWellFormed(sym: Symbol)(using Context): Unit = { def fail(msg: Message) = report.error(msg, sym.srcPos) @@ -537,11 +611,11 @@ object Checking { fail(ParamsNoInline(sym.owner)) if sym.isInlineMethod && !sym.is(Deferred) && sym.allOverriddenSymbols.nonEmpty then checkInlineOverrideParameters(sym) - if (sym.is(Implicit)) { + if sym.is(Implicit) then assert(!sym.owner.is(Package), s"top-level implicit $sym should be wrapped by a package after typer") if sym.isType && (!sym.isClass || sym.is(Trait)) then fail(TypesAndTraitsCantBeImplicit()) - } + else checkScala2Implicit(sym) if sym.is(Transparent) then if sym.isType then if !sym.isExtensibleClass then fail(em"`transparent` can only be used for extensible classes and traits") diff --git a/tests/neg/i22440.check b/tests/neg/i22440.check index f5df4916db87..5dece3235c5f 100644 --- a/tests/neg/i22440.check +++ b/tests/neg/i22440.check @@ -1,3 +1,8 @@ +-- Error: tests/neg/i22440.scala:3:8 ----------------------------------------------------------------------------------- +3 |def foo(implicit x: Int) = x // error + | ^ + | `implicit` parameters are no longer supported, use a `using` clause instead + | This construct can be rewritten automatically under -rewrite -source future-migration. -- Error: tests/neg/i22440.scala:4:12 ---------------------------------------------------------------------------------- 4 |val _ = foo(1) // error | ^ diff --git a/tests/neg/i22440.scala b/tests/neg/i22440.scala index 79de3b71d2ec..d476cd922dba 100644 --- a/tests/neg/i22440.scala +++ b/tests/neg/i22440.scala @@ -1,4 +1,4 @@ //> using options -source future -def foo(implicit x: Int) = x +def foo(implicit x: Int) = x // error val _ = foo(1) // error diff --git a/tests/neg/i8896-b.scala b/tests/neg/i8896-b.scala index f562d2d3b719..6592dba8f4ad 100644 --- a/tests/neg/i8896-b.scala +++ b/tests/neg/i8896-b.scala @@ -7,7 +7,7 @@ object Example { given Foo[Int]() def foo0[A: Foo]: A => A = identity - def foo1[A](implicit foo: Foo[A]): A => A = identity + def foo1[A](implicit foo: Foo[A]): A => A = identity // error def foo2[A](using Foo[A]): A => A = identity def test(): Unit = { diff --git a/tests/neg/implicit-migration.check b/tests/neg/implicit-migration.check new file mode 100644 index 000000000000..c7a8f681e1ea --- /dev/null +++ b/tests/neg/implicit-migration.check @@ -0,0 +1,64 @@ +-- Error: tests/neg/implicit-migration.scala:16:21 --------------------------------------------------------------------- +16 | implicit def ol[T](implicit x: Ord[T]): Ord[List[T]] = new Ord[List[T]]() // error // error + | ^ + | `implicit` parameters are no longer supported, use a `using` clause instead + | This construct can be rewritten automatically under -rewrite -source future-migration. +-- Error: tests/neg/implicit-migration.scala:9:15 ---------------------------------------------------------------------- +9 | implicit def convert(x: String): Int = x.length // error + | ^ + | `implicit` conversion methods are no longer supported. They can usually be + | replaced by given instances of class `Conversion`. Example: + | + | given Conversion[String, Int] = x => ... + | +-- Error: tests/neg/implicit-migration.scala:11:15 --------------------------------------------------------------------- +11 | implicit val ob: Ord[Boolean] = Ord[Boolean]() // error + | ^ + | `implicit` vals are no longer supported, use a `given` clause instead. Example: + | + | given ob: Ord[Boolean] = ... + | + | Note: given clauses are evaluated lazily unless the right hand side is + | a simple reference. If eager evaluation of the value's right hand side + | is important, you can define a regular val and a given instance like this: + | + | val ob = ... + | given Ord[Boolean] = ob + | +-- Error: tests/neg/implicit-migration.scala:12:20 --------------------------------------------------------------------- +12 | lazy implicit val oi: Ord[Int] = Ord[Int]() // error + | ^ + | `implicit` vals are no longer supported, use a `given` clause instead. Example: + | + | given oi: Ord[Int] = ... + | +-- Error: tests/neg/implicit-migration.scala:14:15 --------------------------------------------------------------------- +14 | implicit def of: Ord[Float] = Ord[Float]() // error + | ^ + | `implicit` defs are no longer supported, use a `given` clause instead. Example: + | + | given of: () => Ord[Float] = ... + | +-- Error: tests/neg/implicit-migration.scala:16:15 --------------------------------------------------------------------- +16 | implicit def ol[T](implicit x: Ord[T]): Ord[List[T]] = new Ord[List[T]]() // error // error + | ^ + | `implicit` defs are no longer supported, use a `given` clause instead. Example: + | + | given ol: [T] => (x: Ord[T]) => Ord[List[T]] = ... + | +-- Error: tests/neg/implicit-migration.scala:3:15 ---------------------------------------------------------------------- +3 |implicit class C(x: String): // error + | ^ + | `implicit` classes are no longer supported. They can usually be replaced + | by extension methods. Example: + | + | extension (x: String) + | // class methods go here, replace `this` by `x` + | + | Alternatively, convert to a regular class and define + | a given `Conversion` instance into that class. Example: + | + | class C ... + | given Conversion[String, C] = C(x) + | +there was 1 feature warning; re-run with -feature for details diff --git a/tests/neg/implicit-migration.scala b/tests/neg/implicit-migration.scala new file mode 100644 index 000000000000..f5e2e2802fbe --- /dev/null +++ b/tests/neg/implicit-migration.scala @@ -0,0 +1,17 @@ +import language.future + +implicit class C(x: String): // error + def l: Int = x.length + +class Ord[T] + +object Test: + implicit def convert(x: String): Int = x.length // error + + implicit val ob: Ord[Boolean] = Ord[Boolean]() // error + lazy implicit val oi: Ord[Int] = Ord[Int]() // error + + implicit def of: Ord[Float] = Ord[Float]() // error + + implicit def ol[T](implicit x: Ord[T]): Ord[List[T]] = new Ord[List[T]]() // error // error + diff --git a/tests/pos/i3920.scala b/tests/pos/i3920.scala index 6cd74187098f..61d01156427a 100644 --- a/tests/pos/i3920.scala +++ b/tests/pos/i3920.scala @@ -8,11 +8,10 @@ class SetFunctor(tracked val ord: Ordering) { type Set = List[ord.T] def empty: Set = Nil - implicit class helper(s: Set) { + extension (s: Set) def add(x: ord.T): Set = x :: remove(x) def remove(x: ord.T): Set = s.filter(e => ord.compare(x, e) != 0) def member(x: ord.T): Boolean = s.exists(e => ord.compare(x, e) == 0) - } } object Test { diff --git a/tests/pos/infer-tracked-1.scala b/tests/pos/infer-tracked-1.scala index b4976a963074..0b7568d54159 100644 --- a/tests/pos/infer-tracked-1.scala +++ b/tests/pos/infer-tracked-1.scala @@ -10,11 +10,10 @@ class SetFunctor(val ord: Ordering) { type Set = List[ord.T] def empty: Set = Nil - implicit class helper(s: Set) { + extension (s: Set) def add(x: ord.T): Set = x :: remove(x) def remove(x: ord.T): Set = s.filter(e => ord.compare(x, e) != 0) def member(x: ord.T): Boolean = s.exists(e => ord.compare(x, e) == 0) - } } object Test { diff --git a/tests/rewrites/implicit-rewrite.check b/tests/rewrites/implicit-rewrite.check new file mode 100644 index 000000000000..47f5c6c24059 --- /dev/null +++ b/tests/rewrites/implicit-rewrite.check @@ -0,0 +1,9 @@ +//> using options source `future-migration` -rewrite + +class Ord[T] + +object Test: + + implicit def ol[T](using x: Ord[T]): Ord[List[T]] = foo[T] // error // error + + def foo[T](using x: Ord[T]): Ord[List[T]] = new Ord[List[T]]() diff --git a/tests/rewrites/implicit-rewrite.scala b/tests/rewrites/implicit-rewrite.scala new file mode 100644 index 000000000000..424300a30499 --- /dev/null +++ b/tests/rewrites/implicit-rewrite.scala @@ -0,0 +1,9 @@ +//> using options source `future-migration` -rewrite + +class Ord[T] + +object Test: + + implicit def ol[T](implicit x: Ord[T]): Ord[List[T]] = foo[T] + + def foo[T](implicit x: Ord[T]): Ord[List[T]] = new Ord[List[T]]() diff --git a/tests/warn/i9266.check b/tests/warn/i9266.check index 90dfe43bd2b2..ff8f57c9ded1 100644 --- a/tests/warn/i9266.check +++ b/tests/warn/i9266.check @@ -1,5 +1,13 @@ --- Migration Warning: tests/warn/i9266.scala:5:22 ---------------------------------------------------------------------- -5 |def test = { implicit x: Int => x + x } // warn - | ^ - | This syntax is no longer supported; parameter needs to be enclosed in (...) - | This construct can be rewritten automatically under -rewrite -source future-migration. +-- Migration Warning: tests/warn/i9266.scala:5:14 ---------------------------------------------------------------------- +5 |def test1 = { implicit (x: Int) => x + x } // warn + | ^ + | `implicit` lambdas are no longer supported, use a lambda with `?=>` instead +-- Migration Warning: tests/warn/i9266.scala:7:14 ---------------------------------------------------------------------- +7 |def test2 = { implicit x: Int => x + x } // warn // warn + | ^ + | `implicit` lambdas are no longer supported, use a lambda with `?=>` instead +-- Migration Warning: tests/warn/i9266.scala:7:23 ---------------------------------------------------------------------- +7 |def test2 = { implicit x: Int => x + x } // warn // warn + | ^ + | This syntax is no longer supported; parameter needs to be enclosed in (...) + | This construct can be rewritten automatically under -rewrite -source future-migration. diff --git a/tests/warn/i9266.scala b/tests/warn/i9266.scala index c621e9e20b99..0d5f10687063 100644 --- a/tests/warn/i9266.scala +++ b/tests/warn/i9266.scala @@ -2,5 +2,6 @@ import language.`future-migration` -def test = { implicit x: Int => x + x } // warn +def test1 = { implicit (x: Int) => x + x } // warn +def test2 = { implicit x: Int => x + x } // warn // warn