feat: Add actionable diagnostic for missing members (#23572)

This closes #23469

LLMs were used to a moderate degree while working on this PR.

---------

Co-authored-by: ghostbuster91 <ghostbuster91@users.noreply.github.com>

Author: ghostbuster91

compiler/src/dotty/tools/dotc/core/Decorators.scala

@@ -5,7 +5,7 @@ package core
 import scala.annotation.tailrec
 import scala.collection.mutable.ListBuffer
 
-import Contexts.*, Names.*, Phases.*, Symbols.*
+import Contexts.*, Names.*, Phases.*, Symbols.*, Types.*
 import printing.{ Printer, Showable }, printing.Formatting.*, printing.Texts.*
 import transform.MegaPhase
 import reporting.{Message, NoExplanation}
@@ -80,6 +80,24 @@ object Decorators {
       NoSymbol
     }
 
+  extension (tp: Type)
+    /** Replace synthetic parameter names (`x$0`, `x$1`, ...) of any method
+     *  type group in `tp` (recursing through curried `MethodType`s and
+     *  through `PolyType` result types) with dollar-free names (`x0`,
+     *  `x1`, ...). Lets a printed signature be reused as a valid Scala
+     *  identifier - e.g. in stub implementations or "method is not
+     *  defined" diagnostics.
+     */
+    def withCleanParamNames(using Context): Type = tp match
+      case mt: MethodType if mt.allParamNamesSynthetic =>
+        val newNames = mt.paramNames.zipWithIndex.map((_, i) => termName("x" + i))
+        mt.derivedLambdaType(newNames, mt.paramInfos, mt.resType.withCleanParamNames)
+      case mt: MethodType =>
+        mt.derivedLambdaType(mt.paramNames, mt.paramInfos, mt.resType.withCleanParamNames)
+      case pt: PolyType =>
+        pt.derivedLambdaType(pt.paramNames, pt.paramInfos, pt.resType.withCleanParamNames)
+      case _ => tp
+
   inline val MaxFilterRecursions = 10
 
   /** Implements filterConserve, zipWithConserve methods

compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

@@ -246,6 +246,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case AmbiguousTemplateNameID // errorNumber: 228
   case IndentationWarningID // errorNumber: 229
   case IllegalIdentifierID // errorNumber: 230
+  case ConcreteClassHasUnimplementedMethodsID // errorNumer: 231
 
   def errorNumber = ordinal - 1
 

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -3899,3 +3899,40 @@ final class IllegalIdentifier(name: Name)(using Context) extends SyntaxMsg(Illeg
          |
          |The prohibition against explicit `$$` may be ignored by enclosing the identifier in backquotes
          |at the definition site."""
+
+class ConcreteClassHasUnimplementedMethods(
+    clazz: ClassSymbol,
+    missingMethods: List[Symbol],
+    addendum: String,
+    methodActions: List[CodeAction])(using Context)
+extends Message(ConcreteClassHasUnimplementedMethodsID), NoDisambiguation:
+
+  def kind = MessageKind.Declaration
+
+  private def showDecl(sym: Symbol)(using Context): String =
+    sym.asSeenFrom(clazz.thisType).mapInfo(_.withCleanParamNames).showDcl
+
+  private def prelude(using Context): String =
+    if clazz.isAnonymousClass || clazz.is(Module) then "object creation impossible"
+    else if clazz.is(Synthetic) then "instance cannot be created"
+    else s"$clazz needs to be abstract"
+
+  private def renderMissingMethods(using Context): List[String] =
+    val grouped = missingMethods.groupBy(_.owner).toList
+    grouped.sortBy(_._1.name).map { case (owner, members) =>
+      val sigs = members.sortBy(_.name).map(s => s"- ${showDecl(s)}")
+      s"""Members declared in ${owner.fullName}:
+         |${sigs.mkString("\n")}""".stripMargin
+    }
+
+  def msg(using Context) = missingMethods match
+    case single :: Nil =>
+      val notDefined = s"${showDecl(single)} in ${single.owner.showLocated} is not defined"
+      s"$prelude, since $notDefined$addendum"
+    case _ =>
+      s"""$prelude, since it has ${missingMethods.size} unimplemented members.
+         |
+         |${renderMissingMethods.mkString("\n\n")}""".stripMargin
+
+  def explain(using Context) = ""
+  override def actions(using Context) = methodActions

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

@@ -24,6 +24,7 @@ import Annotations.Annotation
 import Constants.Constant
 import cc.{stripCapturing, CCState}
 import cc.Mutability.isUpdateMethod
+import dotty.tools.dotc.util.Chars.{isLineBreakChar, isWhitespace}
 
 object RefChecks {
   import tpd.*
@@ -338,6 +339,29 @@ object RefChecks {
         val clazzNameEnd = clazz.srcPos.span.start + clazz.name.stripModuleClassSuffix.lastPart.length
         clazz.srcPos.sourcePos.copy(span = clazz.srcPos.span.withEnd(clazzNameEnd))
 
+    /** True when `clazz` is the synthetic anonymous class generated for an
+     *  enum case (either a simple case via `$new` or a parameterized one
+     *  marked with `EnumCase`). */
+    def isEnumAnonCls = // courtesy of Checking.checkEnum
+      clazz.isAnonymousClass
+      && clazz.owner.isTerm
+      && {
+           clazz.owner.isAllOf(EnumCase)
+        || (clazz.owner.name eq nme.DOLLAR_NEW) && clazz.owner.isAllOf(Private | Synthetic)
+      }
+
+    /** Positions at which to report a "needs to be abstract" / "object creation
+     *  impossible" error for `clazz`. Normally a single position pointing at
+     *  the class name; for enum-case anonymous classes the error is moved to
+     *  the case definition(s) so the user sees it on `case Foo` rather than on
+     *  the synthetic `$anon`. */
+    def classErrorPositions: List[util.SrcPos] =
+      if !isEnumAnonCls then clazzNamePos :: Nil
+      else if clazz.owner.isAllOf(EnumCase) then clazz.owner.srcPos :: Nil
+      else
+        val e = clazz.parentSyms.head
+        e.children.filter(_.info.typeSymbol == e).map(_.srcPos)
+
     def printMixinOverrideErrors(): Unit =
       mixinOverrideErrors.toList match {
         case Nil =>
@@ -646,15 +670,15 @@ object RefChecks {
     // Verifying a concrete class has nothing unimplemented.
     if (!clazz.isOneOf(AbstractOrTrait)) {
       val abstractErrors = new mutable.ListBuffer[String]
+      val concreteClassUnimplementedMethodError = new mutable.ListBuffer[Message]
       def abstractErrorMessage =
         // a little formatting polish
         if (abstractErrors.size <= 2) abstractErrors.mkString(" ")
         else abstractErrors.tail.mkString(abstractErrors.head + ":\n", "\n", "")
 
-      def abstractClassError(mustBeMixin: Boolean, msg: String): Unit = {
+      def abstractClassError(msg: String): Unit = {
         def prelude = (
           if (clazz.isAnonymousClass || clazz.is(Module)) "object creation impossible"
-          else if (mustBeMixin) s"$clazz needs to be a mixin"
           else if clazz.is(Synthetic) then "instance cannot be created"
           else s"$clazz needs to be abstract"
           ) + ", since"
@@ -733,56 +757,28 @@ object RefChecks {
               .distinctBy(_.signature) // Avoid duplication for similar definitions (#19731)
         }
 
-        /** Replace synthetic parameter names (x$0, x$1, ...) with
-         *  dollar-free versions (x0, x1, ...) so that stub implementations
-         *  are directly usable as valid Scala identifiers.
-         */
-        def replaceSyntheticParamNames(tp: Type): Type = tp match
-          case mt: MethodType if mt.allParamNamesSynthetic =>
-            val newNames = mt.paramNames.zipWithIndex.map((_, i) => termName("x" + i))
-            mt.derivedLambdaType(newNames, mt.paramInfos, replaceSyntheticParamNames(mt.resType))
-          case mt: MethodType =>
-            mt.derivedLambdaType(mt.paramNames, mt.paramInfos, replaceSyntheticParamNames(mt.resType))
-          case pt: PolyType =>
-            pt.derivedLambdaType(pt.paramNames, pt.paramInfos, replaceSyntheticParamNames(pt.resType))
-          case _ => tp
-
-        def stubImplementations: List[String] = {
-          // Grouping missing methods by the declaring class
-          val regrouped = missingMethods.groupBy(_.owner).toList
-          def membersStrings(members: List[Symbol]) =
-            members.sortBy(_.name.toString).map: sym =>
-              val denot = sym.asSeenFrom(clazz.thisType)
-              denot.mapInfo(replaceSyntheticParamNames).showDcl + " = ???"
-
-          if (regrouped.tail.isEmpty)
-            membersStrings(regrouped.head._2)
-          else (regrouped.sortBy(_._1.name.toString()) flatMap {
-            case (owner, members) =>
-              ("// Members declared in " + owner.fullName) +: membersStrings(members) :+ ""
-          }).init
-        }
+        def stubImplementations: List[String] =
+          missingMethods.sortBy(_.name.toString).map: sym =>
+            sym.asSeenFrom(clazz.thisType).mapInfo(_.withCleanParamNames).showDcl + " = ???"
 
-        // If there are numerous missing methods, we presume they are aware of it and
-        // give them a nicely formatted set of method signatures for implementing.
-        if (missingMethods.size > 1) {
-          abstractClassError(false, "it has " + missingMethods.size + " unimplemented members.")
-          val preface =
-            """|/** As seen from %s, the missing signatures are as follows.
-                 | *  For convenience, these are usable as stub implementations.
-                 | */
-                 |""".stripMargin.format(clazz)
-          abstractErrors += stubImplementations.map("  " + _ + "\n").mkString(preface, "", "")
+        if missingMethods.isEmpty then return
+
+        lazy val actions =
+          createAddMissingMethodsAction(clazz, stubImplementations)
+            ++ createMakeClassAbstractAction(clazz)
+
+        if missingMethods.size > 1 then
+          concreteClassUnimplementedMethodError += ConcreteClassHasUnimplementedMethods(
+            clazz, missingMethods, addendum = "", actions)
           return
-        }
 
         for (member <- missingMethods) {
           def showDclAndLocation(sym: Symbol) =
-            s"${sym.mapInfo(replaceSyntheticParamNames).showDcl} in ${sym.owner.showLocated}"
+            s"${sym.mapInfo(_.withCleanParamNames).showDcl} in ${sym.owner.showLocated}"
           def undefined(msg: String) =
-            val notdefined = s"${showDclAndLocation(member)} is not defined"
-            val text = if !msg.isEmpty then s"$notdefined $msg" else notdefined
-            abstractClassError(mustBeMixin = false, text)
+            val addendum = if msg.isEmpty then "" else s" $msg"
+            concreteClassUnimplementedMethodError += ConcreteClassHasUnimplementedMethods(
+              clazz, missingMethods, addendum, actions)
           val underlying = member.underlyingSymbol
 
           // Give a specific error message for abstract vars based on why it fails:
@@ -875,7 +871,7 @@ object RefChecks {
               val impl1 = clazz.thisType.nonPrivateMember(decl.name) // DEBUG
               report.log(i"${impl1}: ${impl1.info}") // DEBUG
               report.log(i"${clazz.thisType.memberInfo(decl)}") // DEBUG
-              abstractClassError(false, "there is a deferred declaration of " + infoString(decl) +
+              abstractClassError("there is a deferred declaration of " + infoString(decl) +
                 " which is not implemented in a subclass" + err.abstractVarMessage(decl))
           }
         if (bc.asClass.superClass.is(Abstract))
@@ -945,25 +941,17 @@ object RefChecks {
               |This is a limitation that enables better GADT constraints in case class patterns""".stripMargin
         do report.errorOrMigrationWarning(withExplain, clazz.srcPos, MigrationVersion.Scala2to3)
       checkNoAbstractMembers()
-      if (abstractErrors.isEmpty)
+      if (abstractErrors.isEmpty && concreteClassUnimplementedMethodError.isEmpty)
         checkNoAbstractDecls(clazz)
 
       if abstractErrors.nonEmpty then
-        val isEnumAnonCls = // courtesy of Checking.checkEnum
-          clazz.isAnonymousClass
-          && clazz.owner.isTerm
-          && {
-               clazz.owner.isAllOf(EnumCase)
-            || (clazz.owner.name eq nme.DOLLAR_NEW) && clazz.owner.isAllOf(Private | Synthetic)
-          }
-        if !isEnumAnonCls then
-          report.error(abstractErrorMessage, clazzNamePos)
-        else if clazz.owner.isAllOf(EnumCase) then
-          report.error(abstractErrorMessage, clazz.owner.srcPos)
-        else
-          val e = clazz.parentSyms.head
-          for child <- e.children if child.info.typeSymbol == e do // report all simple cases
-            report.error(abstractErrorMessage, child.srcPos)
+        val msg = abstractErrorMessage
+        classErrorPositions.foreach(report.error(msg, _))
+      for
+        message <- concreteClassUnimplementedMethodError
+        pos <- classErrorPositions
+      do
+        report.error(message, pos)
 
       checkMemberTypesOK()
       checkCaseClassInheritanceInvariant()
@@ -1349,7 +1337,111 @@ object RefChecks {
       tree.tpe match
         case tp: NamedType if tp.prefix.typeSymbol != ctx.enclosingClass =>
           report.warning(UnqualifiedCallToAnyRefMethod(tree, tree.symbol), tree)
+        case _ => ()
+
+  private def createAddMissingMethodsAction(clazz: ClassSymbol, methods: List[String])(using Context): List[CodeAction] =
+    // Synthetic classes (e.g. anonymous classes generated by macros) may have
+    // no corresponding node in the untyped AST. In that case there's nothing
+    // to anchor a source-level patch to, so we don't offer the action.
+    NavigateAST.untypedPath(clazz.span) match
+      case (untypedTree: untpd.Tree) :: _ =>
+        addMissingMethodsActionPatch(clazz, methods, untypedTree)
+      case _ => Nil
+
+  /** Code action that prepends the `abstract` modifier to the class declaration.
+   *
+   *  Only offered for regular classes that can legally become abstract. We skip:
+   *    - modules (`object`s are implicitly final),
+   *    - anonymous classes (no source-level keyword to modify),
+   *    - case classes (cannot be abstract),
+   *    - already-final classes (mutually exclusive with abstract),
+   *    - synthetic classes (enum case singletons, given bodies, etc.).
+   *
+   *  As with [[createAddMissingMethodsAction]], if the class has no node in
+   *  the untyped AST (e.g. macro-generated) we skip silently.
+   */
+  private def createMakeClassAbstractAction(clazz: ClassSymbol)(using Context): List[CodeAction] =
+    import dotty.tools.dotc.rewrites.Rewrites.ActionPatch
+
+    val ineligible =
+      clazz.is(Module) || clazz.isAnonymousClass || clazz.is(Case) ||
+        clazz.is(Final) || clazz.is(Synthetic)
+    if ineligible then Nil
+    else NavigateAST.untypedPath(clazz.span) match
+      case (untypedTree: untpd.Tree) :: _ =>
+        val insertPos = untypedTree.sourcePos.withSpan(Span(untypedTree.span.start))
+        val patch = ActionPatch(insertPos, "abstract ")
+        List(CodeAction(s"Make `${clazz.name.show}` abstract", None, List(patch)))
+      case _ => Nil
+
+  private def addMissingMethodsActionPatch(
+      clazz: ClassSymbol,
+      methods: List[String],
+      untypedTree: untpd.Tree)(using Context): List[CodeAction] = {
+    import dotty.tools.dotc.rewrites.Rewrites.ActionPatch
+
+    val classSrcPos = clazz.srcPos
+    val content = classSrcPos.sourcePos.source.content()
+    val span = classSrcPos.endPos.span
+
+    val classText = new String(content.slice(untypedTree.span.start, untypedTree.span.end))
+    val classHasBraces = classText.contains("{") && classText.contains("}")
+
+    // Indentation for inserted methods
+    val lineStart = content.lastIndexWhere(isLineBreakChar, end = span.end - 1) + 1
+    val baseIndent = new String(content.slice(lineStart, span.end).takeWhile(c => c == ' ' || c == '\t'))
+    val indent = baseIndent + "  "
+
+    val formattedMethods = methods.map(m => s"$indent$m").mkString(System.lineSeparator())
+
+    val isBracelessSyntax = untypedTree match
+      case untpd.TypeDef(_, tmpl: untpd.Template) =>
+        !classText.contains("{") && tmpl.body.nonEmpty
+      case _ => false
+
+    if (classHasBraces) {
+      val insertBeforeBrace = untypedTree.sourcePos.withSpan(Span(untypedTree.span.end - 1))
+      val braceStart = classText.indexOf('{')
+      val braceEnd   = classText.lastIndexOf('}')
+      val bodyBetweenBraces = classText.slice(braceStart + 1, braceEnd)
+      val bodyIsEmpty = bodyBetweenBraces.forall(_.isWhitespace)
+      val bodyContainsNewLine = bodyBetweenBraces.exists(isLineBreakChar)
+
+      val prefix = if (bodyContainsNewLine) "" else System.lineSeparator()
+      val patchText =
+        prefix +
+        formattedMethods +
+        System.lineSeparator()
+
+      val patch = ActionPatch(insertBeforeBrace, patchText)
+      List(CodeAction("Add missing methods", None, List(patch)))
+    } else if (isBracelessSyntax) {
+      val insertAfterLastDef = untypedTree match
+        case untpd.TypeDef(_, tmpl: untpd.Template) if tmpl.body.nonEmpty =>
+          val lastDef = tmpl.body.last
+          lastDef.sourcePos.withSpan(Span(lastDef.span.end))
         case _ =>
+          untypedTree.sourcePos.withSpan(Span(untypedTree.span.end))
+
+      val patchText = System.lineSeparator() + formattedMethods
+
+      val patch = ActionPatch(insertAfterLastDef, patchText)
+      List(CodeAction("Add missing methods", None, List(patch)))
+    } else {
+      // Class has no body – add whole `{ ... }` after class header, same line
+      val insertAfterHeader = untypedTree.sourcePos.withSpan(Span(untypedTree.span.end))
+
+      val patchText =
+        " {" + System.lineSeparator() +
+        formattedMethods + System.lineSeparator() +
+        "}"
+
+      val patch = ActionPatch(insertAfterHeader, patchText)
+      List(CodeAction("Add missing methods", None, List(patch)))
+    }
+  }
+
+
 }
 import RefChecks.*