Patch empty implicit parens on error recovery

compiler/src/dotty/tools/dotc/ast/Trees.scala

@@ -509,9 +509,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)

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}
@@ -1593,6 +1593,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) =

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}
@@ -602,7 +602,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")
@@ -757,13 +757,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
@@ -1096,6 +1102,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 {
@@ -1141,51 +1162,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)
@@ -1223,10 +1252,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()
@@ -1236,10 +1269,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

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

@@ -615,6 +615,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

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -88,7 +88,8 @@ class CompilationTests {
       compileFile("tests/rewrites/ambiguous-named-tuple-assignment.scala", defaultOptions.and("-rewrite", "-source:3.6-migration")),
       compileFile("tests/rewrites/i21382.scala", defaultOptions.and("-indent", "-rewrite")),
       compileFile("tests/rewrites/unused.scala", defaultOptions.and("-rewrite", "-Wunused:all")),
-      compileFile("tests/rewrites/i22440.scala", defaultOptions.and("-rewrite"))
+      compileFile("tests/rewrites/i22440.scala", defaultOptions.and("-rewrite")),
+      compileFile("tests/rewrites/i22792.scala", defaultOptions.and("-rewrite")),
     ).checkRewrites()
   }
 

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`

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.
+   ---------------------------------------------------------------------------------------------------------------------

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

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

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