diff --git a/compiler/src/dotty/tools/dotc/ast/Trees.scala b/compiler/src/dotty/tools/dotc/ast/Trees.scala index c6cde66374b3..8749f7ddc10c 100644 --- a/compiler/src/dotty/tools/dotc/ast/Trees.scala +++ b/compiler/src/dotty/tools/dotc/ast/Trees.scala @@ -512,9 +512,9 @@ object Trees { /** The kind of application */ enum ApplyKind: - case Regular // r.f(x) - case Using // r.f(using x) - case InfixTuple // r f (x1, ..., xN) where N != 1; needs to be treated specially for an error message in typedApply + case Regular // r.f(x) + case Using // r.f(using x) + case InfixTuple // r f (x1, ..., xN) where N != 1; needs to be treated specially for an error message in typedApply /** fun(args) */ case class Apply[+T <: Untyped] private[ast] (fun: Tree[T], args: List[Tree[T]])(implicit @constructorOnly src: SourceFile) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index d08d08fe4412..f69dfea0007a 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -14,7 +14,7 @@ import printing.Highlighting.* import printing.Formatting import ErrorMessageID.* import ast.Trees -import config.{Feature, ScalaVersion} +import config.{Feature, MigrationVersion, ScalaVersion} import transform.patmat.Space import transform.patmat.SpaceEngine import typer.ErrorReporting.{err, matchReductionAddendum, substitutableTypeSymbolsInScope} @@ -1592,6 +1592,14 @@ class MissingArgument(pname: Name, methString: String)(using Context) else s"missing argument for parameter $pname of $methString" def explain(using Context) = "" +class MissingImplicitParameterInEmptyArguments(pname: Name, methString: String)(using Context) + extends MissingArgument(pname, methString): + override def msg(using Context) = + val mv = MigrationVersion.ImplicitParamsWithoutUsing + super.msg.concat(Message.rewriteNotice("This code", mv.patchFrom)) // patch emitted up the stack + override def explain(using Context) = + "Old-style implicit argument lists may be omitted but not empty; this syntax was corrected in 3.7." + class MissingArgumentList(method: String, sym: Symbol)(using Context) extends TypeMsg(MissingArgumentListID) { def msg(using Context) = diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index cf44dd639752..6499d806a9a5 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -23,7 +23,7 @@ import ProtoTypes.* import Inferencing.* import reporting.* import Nullables.*, NullOpsDecorator.* -import config.{Feature, SourceVersion} +import config.{Feature, MigrationVersion, SourceVersion} import collection.mutable import config.Printers.{overload, typr, unapp} @@ -34,10 +34,10 @@ import Constants.{Constant, IntTag} import Denotations.SingleDenotation import annotation.threadUnsafe -import scala.util.control.NonFatal -import dotty.tools.dotc.inlines.Inlines import scala.annotation.tailrec +import scala.util.control.NonFatal import dotty.tools.dotc.cc.isRetains +import dotty.tools.dotc.inlines.Inlines object Applications { import tpd.* @@ -607,7 +607,7 @@ trait Applications extends Compatibility { fail(TypeMismatch(methType.resultType, resultType, None)) // match all arguments with corresponding formal parameters - if success then matchArgs(orderedArgs, methType.paramInfos, 0) + if success then matchArgs(orderedArgs, methType.paramInfos, n = 0) case _ => if (methType.isError) ok = false else fail(em"$methString does not take parameters") @@ -765,13 +765,19 @@ trait Applications extends Compatibility { } else defaultArgument(normalizedFun, n, testOnly) + // a bug allowed empty parens to expand to implicit args: fail empty args for rewrite on migration + def canSupplyImplicits = + inline def failEmptyArgs: false = + if Application.this.args.isEmpty then + fail(MissingImplicitParameterInEmptyArguments(methodType.paramNames(n), methString)) + false + methodType.isImplicitMethod && (applyKind == ApplyKind.Using || failEmptyArgs) + if !defaultArg.isEmpty then defaultArg.tpe.widen match case _: MethodOrPoly if testOnly => matchArgs(args1, formals1, n + 1) case _ => matchArgs(args1, addTyped(treeToArg(defaultArg)), n + 1) - else if (methodType.isContextualMethod || applyKind == ApplyKind.Using && methodType.isImplicitMethod) - && ctx.mode.is(Mode.ImplicitsEnabled) - then + else if (methodType.isContextualMethod || canSupplyImplicits) && ctx.mode.is(Mode.ImplicitsEnabled) then val implicitArg = implicitArgTree(formal, appPos.span) matchArgs(args1, addTyped(treeToArg(implicitArg)), n + 1) else @@ -1104,6 +1110,21 @@ trait Applications extends Compatibility { then originalProto.tupledDual else originalProto + /* TODO (*) Get rid of this case. It is still syntax-based, therefore unreliable. + * It is necessary for things like `someDynamic[T](...)`, because in that case, + * somehow typedFunPart returns a tree that was typed as `TryDynamicCallType`, + * so clearly with the view that an apply insertion was necessary, but doesn't + * actually insert the apply! + * This is probably something wrong in apply insertion, but I (@sjrd) am out of + * my depth there. + * In the meantime, this makes tests pass. + */ + def isInsertedApply = fun1 match + case Select(_, nme.apply) => fun1.span.isSynthetic + case TypeApply(sel @ Select(_, nme.apply), _) => sel.span.isSynthetic + case TypeApply(fun, _) => !fun.isInstanceOf[Select] // (*) see explanatory comment + case _ => false + /** Type application where arguments come from prototype, and no implicits are inserted */ def simpleApply(fun1: Tree, proto: FunProto)(using Context): Tree = methPart(fun1).tpe match { @@ -1149,51 +1170,59 @@ trait Applications extends Compatibility { } } + def tryWithUsing(fun1: Tree, proto: FunProto)(using Context): Option[Tree] = + tryEither(Option(simpleApply(fun1, proto.withApplyKind(ApplyKind.Using)))): (_, _) => + None + /** If the applied function is an automatically inserted `apply` - * method and one of its arguments has a type mismatch , append - * a note to the error message that explains where the required - * type comes from. See #19680 and associated test case. + * method and one of its arguments has a type mismatch , append + * a note to the error message that explains where the required + * type comes from. See #19680 and associated test case. */ def maybeAddInsertedApplyNote(failedState: TyperState, fun1: Tree)(using Context): Unit = if fun1.symbol.name == nme.apply && fun1.span.isSynthetic then fun1 match - case Select(qualifier, _) => - def mapMessage(dia: Diagnostic): Diagnostic = - dia match - case dia: Diagnostic.Error => - dia.msg match - case msg: TypeMismatch => - msg.inTree match - case Some(arg) if tree.args.exists(_.span == arg.span) => - val noteText = - i"""The required type comes from a parameter of the automatically - |inserted `apply` method of `${qualifier.tpe}`.""".stripMargin - Diagnostic.Error(msg.appendExplanation("\n\n" + noteText), dia.pos) - case _ => dia - case msg => dia - case dia => dia - failedState.reporter.mapBufferedMessages(mapMessage) - case _ => () - else () + case Select(qualifier, _) => + failedState.reporter.mapBufferedMessages: + case dia: Diagnostic.Error => + dia.msg match + case msg: TypeMismatch => + msg.inTree match + case Some(arg) if tree.args.exists(_.span == arg.span) => + val noteText = + i"""The required type comes from a parameter of the automatically + |inserted `apply` method of `${qualifier.tpe}`.""".stripMargin + Diagnostic.Error(msg.appendExplanation("\n\n" + noteText), dia.pos) + case _ => dia + case msg => dia + case dia => dia + case _ => () + end if + + def maybePatchBadParensForImplicit(failedState: TyperState)(using Context): Boolean = + def rewrite(): Unit = + val replace = + if isInsertedApply then ".apply" // x() -> x.apply + else "" // f() -> f where fun1.span.end == tree.span.point + rewrites.Rewrites.patch(tree.span.withStart(fun1.span.end), replace) + var retry = false + failedState.reporter.mapBufferedMessages: + case err: Diagnostic.Error => + err.msg match + case msg: MissingImplicitParameterInEmptyArguments => + val mv = MigrationVersion.ImplicitParamsWithoutUsing + if mv.needsPatch then + retry = true + rewrite() + Diagnostic.Warning(err.msg, err.pos) + else err + case _ => err + case dia => dia + retry val result = fun1.tpe match { case err: ErrorType => cpy.Apply(tree)(fun1, proto.typedArgs()).withType(err) case TryDynamicCallType => - val isInsertedApply = fun1 match { - case Select(_, nme.apply) => fun1.span.isSynthetic - case TypeApply(sel @ Select(_, nme.apply), _) => sel.span.isSynthetic - /* TODO Get rid of this case. It is still syntax-based, therefore unreliable. - * It is necessary for things like `someDynamic[T](...)`, because in that case, - * somehow typedFunPart returns a tree that was typed as `TryDynamicCallType`, - * so clearly with the view that an apply insertion was necessary, but doesn't - * actually insert the apply! - * This is probably something wrong in apply insertion, but I (@sjrd) am out of - * my depth there. - * In the meantime, this makes tests pass. - */ - case TypeApply(fun, _) => !fun.isInstanceOf[Select] - case _ => false - } val tree1 = fun1 match case Select(_, nme.apply) => tree case _ => untpd.Apply(fun1, tree.args) @@ -1231,10 +1260,14 @@ trait Applications extends Compatibility { errorTree(tree, em"argument to summonFrom must be a pattern matching closure") } else - tryEither { - simpleApply(fun1, proto) - } { - (failedVal, failedState) => + tryEither(simpleApply(fun1, proto)): (failedVal, failedState) => + // a bug allowed empty parens to expand to implicit args, offer rewrite only on migration, + // then retry with using to emulate the bug since rewrites are ignored on error. + if proto.args.isEmpty && maybePatchBadParensForImplicit(failedState) then + tryWithUsing(fun1, proto).getOrElse: + failedState.commit() + failedVal + else def fail = maybeAddInsertedApplyNote(failedState, fun1) failedState.commit() @@ -1244,10 +1277,9 @@ trait Applications extends Compatibility { // The reason we need to try both is that the decision whether to use tupled // or not was already taken but might have to be revised when an implicit // is inserted on the qualifier. - tryWithImplicitOnQualifier(fun1, originalProto).getOrElse( + tryWithImplicitOnQualifier(fun1, originalProto).getOrElse: if (proto eq originalProto) fail - else tryWithImplicitOnQualifier(fun1, proto).getOrElse(fail)) - } + else tryWithImplicitOnQualifier(fun1, proto).getOrElse(fail) } if result.tpe.isNothingType then diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala index d0baa3373c2a..0b6688c6f5fe 100644 --- a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala @@ -623,6 +623,10 @@ object ProtoTypes { override def withContext(newCtx: Context): ProtoType = if newCtx `eq` protoCtx then this else new FunProto(args, resType)(typer, applyKind, state)(using newCtx) + + def withApplyKind(applyKind: ApplyKind) = + if applyKind == this.applyKind then this + else new FunProto(args, resType)(typer, applyKind, state) } /** A prototype for expressions that appear in function position diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index 90764f4ec981..6c763dbdbd65 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -86,7 +86,8 @@ class CompilationTests { compileFile("tests/rewrites/i22440.scala", defaultOptions.and("-rewrite")), compileFile("tests/rewrites/i22731.scala", defaultOptions.and("-rewrite", "-source:3.7-migration")), compileFile("tests/rewrites/i22731b.scala", defaultOptions.and("-rewrite", "-source:3.7-migration")), - compileFile("tests/rewrites/implicit-to-given.scala", defaultOptions.and("-rewrite", "-Yimplicit-to-given")) + compileFile("tests/rewrites/implicit-to-given.scala", defaultOptions.and("-rewrite", "-Yimplicit-to-given")), + compileFile("tests/rewrites/i22792.scala", defaultOptions.and("-rewrite")), ).checkRewrites() } diff --git a/tests/neg/i22439.check b/tests/neg/i22439.check index 471ed68d81d1..3b4dffd59aca 100644 --- a/tests/neg/i22439.check +++ b/tests/neg/i22439.check @@ -2,6 +2,9 @@ 7 | f() // error f() missing arg | ^^^ | missing argument for parameter i of method f: (implicit i: Int, j: Int): Int + | This code can be rewritten automatically under -rewrite -source 3.7-migration. + | + | longer explanation available when compiling with `-explain` -- [E050] Type Error: tests/neg/i22439.scala:8:2 ----------------------------------------------------------------------- 8 | g() // error g(given_Int, given_Int)() doesn't take more params | ^ @@ -24,3 +27,6 @@ 21 | val (ws, zs) = vs.unzip() // error! | ^^^^^^^^^^ |missing argument for parameter asPair of method unzip in trait StrictOptimizedIterableOps: (implicit asPair: ((Int, Int)) => (A1, A2)): (List[A1], List[A2]) + |This code can be rewritten automatically under -rewrite -source 3.7-migration. + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/i22792.check b/tests/neg/i22792.check new file mode 100644 index 000000000000..bde1a0e97f9c --- /dev/null +++ b/tests/neg/i22792.check @@ -0,0 +1,10 @@ +-- [E171] Type Error: tests/neg/i22792.scala:8:30 ---------------------------------------------------------------------- +8 |@main def Test = new Foo().run() // error + | ^^^^^^^^^^^^^^^ + | missing argument for parameter ev of method run in class Foo: (implicit ev: Permit): Unit + | This code can be rewritten automatically under -rewrite -source 3.7-migration. + |--------------------------------------------------------------------------------------------------------------------- + | Explanation (enabled by `-explain`) + |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + | Old-style implicit argument lists may be omitted but not empty; this syntax was corrected in 3.7. + --------------------------------------------------------------------------------------------------------------------- diff --git a/tests/neg/i22792.scala b/tests/neg/i22792.scala new file mode 100644 index 000000000000..10205573e1fa --- /dev/null +++ b/tests/neg/i22792.scala @@ -0,0 +1,8 @@ +//> using options -explain + +trait Permit +class Foo: + def run(implicit ev: Permit): Unit = ??? + +given Permit = ??? +@main def Test = new Foo().run() // error diff --git a/tests/rewrites/i22792.check b/tests/rewrites/i22792.check new file mode 100644 index 000000000000..6bb8d5f013c7 --- /dev/null +++ b/tests/rewrites/i22792.check @@ -0,0 +1,15 @@ +//> using options -source 3.7-migration + +trait Permit +class Foo: + def run(implicit ev: Permit): Unit = ??? + def apply(implicit ev: Permit): Unit = ??? + +given Permit = ??? +@main def Test = new Foo().run + +def ctorProxy = Foo().run + +def otherSyntax = new Foo().apply // Foo().apply does not work + +def kwazySyntax = new Foo() . run // that was fun diff --git a/tests/rewrites/i22792.scala b/tests/rewrites/i22792.scala new file mode 100644 index 000000000000..c8c6c4164a0b --- /dev/null +++ b/tests/rewrites/i22792.scala @@ -0,0 +1,15 @@ +//> using options -source 3.7-migration + +trait Permit +class Foo: + def run(implicit ev: Permit): Unit = ??? + def apply(implicit ev: Permit): Unit = ??? + +given Permit = ??? +@main def Test = new Foo().run() + +def ctorProxy = Foo().run() + +def otherSyntax = new Foo()() // Foo().apply does not work + +def kwazySyntax = new Foo() . run ( /* your args here! */ ) // that was fun