feat: Add actionable diagnostic for missing members

ghostbuster91 · ghostbuster91 · commit a1f0bb058beb · 2025-07-19T20:26:05.000+02:00
diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala
@@ -232,6 +232,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case UnnecessaryNN // errorNumber: 216
   case ErasedNotPureID // errorNumber: 217
   case IllegalErasedDefID // errorNumber: 218
+  case ConcreteClassHasUnimplementedMethodsID // errorNumer: 219
 
   def errorNumber = ordinal - 1
 
diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala
@@ -3652,3 +3652,30 @@ final class IllegalErasedDef(sym: Symbol)(using Context) extends TypeMsg(Illegal
   override protected def explain(using Context): String =
     "Only non-lazy immutable values can be `erased`"
 end IllegalErasedDef
+
+class ConcreteClassHasUnimplementedMethods(clazz: ClassSymbol, missingMethods: List[dotty.tools.dotc.core.Symbols.Symbol], val actions: List[CodeAction])(using Context)
+extends Message(ConcreteClassHasUnimplementedMethodsID): 
+
+  def kind = MessageKind.Declaration
+
+  def renderMissingMethods: 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(_.asSeenFrom(clazz.thisType).showDcl).map(m => s"- $m")
+
+    (regrouped.sortBy(_._1.name.toString()) map {
+      case (owner, members) =>
+        s"""Members declared in ${owner.fullName}:
+        |${membersStrings(members).mkString("\n")}"""
+    })
+  }
+
+  def msg(using Context) = 
+    s"""$clazz needs to be abstract, since it has ${missingMethods.size} unimplemented members.
+    |
+    |${renderMissingMethods.mkString("\n")}
+    |""".stripMargin
+  
+  def explain(using Context) = ""
+  override def actions(using Context) = this.actions
diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala
@@ -629,15 +629,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"
@@ -717,37 +717,22 @@ object RefChecks {
         }
 
         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(_.asSeenFrom(clazz.thisType).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
+          membersStrings(missingMethods)
         }
 
-        // 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.size >= 1) {
+          concreteClassUnimplementedMethodError += ConcreteClassHasUnimplementedMethods(clazz, missingMethods, createAddMissingMethodsAction(clazz, stubImplementations))
           return
         }
 
         for (member <- missingMethods) {
           def showDclAndLocation(sym: Symbol) =
             s"${sym.showDcl} in ${sym.owner.showLocated}"
           def undefined(msg: String) =
-            abstractClassError(false, s"${showDclAndLocation(member)} is not defined $msg")
+            abstractClassError(s"${showDclAndLocation(member)} is not defined $msg")
           val underlying = member.underlyingSymbol
 
           // Give a specific error message for abstract vars based on why it fails:
@@ -840,7 +825,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))
@@ -916,6 +901,8 @@ object RefChecks {
       if (abstractErrors.nonEmpty)
         report.error(abstractErrorMessage, clazz.srcPos)
 
+      concreteClassUnimplementedMethodError.foreach(report.error(_, clazz.srcPos))
+
       checkMemberTypesOK()
       checkCaseClassInheritanceInvariant()
     }
@@ -1272,6 +1259,79 @@ object RefChecks {
         case tp: NamedType if tp.prefix.typeSymbol != ctx.owner.enclosingClass =>
           report.warning(UnqualifiedCallToAnyRefMethod(tree, tree.symbol), tree)
         case _ => ()
+
+  private def createAddMissingMethodsAction(clazz: ClassSymbol, methods: List[String])(using Context): List[CodeAction] = {
+    import dotty.tools.dotc.rewrites.Rewrites.ActionPatch
+    import dotty.tools.dotc.util.Spans
+    import dotty.tools.dotc.ast.untpd
+
+    val untypedTree = NavigateAST
+      .untypedPath(clazz.span)
+      .head
+      .asInstanceOf[untpd.Tree]
+
+    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(c => c == '\n', 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.contains(System.lineSeparator())
+
+      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.*
 
diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala
@@ -179,6 +179,118 @@ class CodeActionTest extends DottyTest:
       ctxx = ctxx
       )
 
+  @Test def insertMissingSingleMethodToNonEmptyClass =
+    checkCodeAction(
+      code =
+        """trait Animal {
+          |  def name: String
+          |}
+          |class Dog extends Animal {
+          |  def bbb = 2
+          |}
+          |""".stripMargin,
+         title = "Add missing methods",
+      expected =
+        """trait Animal {
+          |  def name: String
+          |}
+          |class Dog extends Animal {
+          |  def bbb = 2
+          |  def name: String = ???
+          |}
+          |""".stripMargin,
+          afterPhase= "erasure"
+      )
+
+  @Test def insertMissingMethodsFromMultipleSources =
+    checkCodeAction(
+      code =
+        """trait Animal {
+          |  def name: String
+          |}
+          |trait Car {
+          |  def wheels: Int
+          |}
+          |class Dog extends Animal with Car {
+          |}
+          |""".stripMargin,
+         title = "Add missing methods",
+      expected =
+        """trait Animal {
+          |  def name: String
+          |}
+          |trait Car {
+          |  def wheels: Int
+          |}
+          |class Dog extends Animal with Car {
+          |  def name: String = ???
+          |  def wheels: Int = ???
+          |}
+          |""".stripMargin,
+          afterPhase= "erasure"
+      )
+
+  @Test def insertMissingMethodIntoEmptyClassWithBrackets =
+    checkCodeAction(
+      code =
+        """trait Animal {
+          |  def name: String
+          |}
+          |class Dog extends Animal {}
+          |""".stripMargin,
+         title = "Add missing methods",
+      expected =
+        """trait Animal {
+          |  def name: String
+          |}
+          |class Dog extends Animal {
+          |  def name: String = ???
+          |}
+          |""".stripMargin,
+          afterPhase= "erasure"
+      )
+
+  @Test def insertMissingMethodIntoEmptyClassWithoutBrackets =
+    checkCodeAction(
+      code =
+        """trait Animal {
+          |  def name: String
+          |}
+          |class Dog extends Animal
+          |""".stripMargin,
+         title = "Add missing methods",
+      expected =
+        """trait Animal {
+          |  def name: String
+          |}
+          |class Dog extends Animal {
+          |  def name: String = ???
+          |}
+          |""".stripMargin,
+          afterPhase= "erasure"
+      )
+
+  @Test def insertMissingMethodIntoNonEmptyClassWithBraclessSyntax =
+    checkCodeAction(
+      code =
+        """trait Animal {
+          |  def name: String
+          |}
+          |class Dog extends Animal:
+          |  def bbb = 2
+          |""".stripMargin,
+         title = "Add missing methods",
+      expected =
+        """trait Animal {
+          |  def name: String
+          |}
+          |class Dog extends Animal:
+          |  def bbb = 2
+          |  def name: String = ???
+          |""".stripMargin,
+          afterPhase= "erasure"
+      )
+
   // Make sure we're not using the default reporter, which is the ConsoleReporter,
   // meaning they will get reported in the test run and that's it.
   private def newContext =
