From a1f0bb058beb8943c11973eb0641ff06218d2491 Mon Sep 17 00:00:00 2001 From: ghostbuster91 Date: Sat, 19 Jul 2025 20:00:08 +0200 Subject: [PATCH 1/3] feat: Add actionable diagnostic for missing members --- .../tools/dotc/reporting/ErrorMessageID.scala | 1 + .../dotty/tools/dotc/reporting/messages.scala | 27 +++++ .../dotty/tools/dotc/typer/RefChecks.scala | 104 ++++++++++++---- .../tools/dotc/reporting/CodeActionTest.scala | 112 ++++++++++++++++++ 4 files changed, 222 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index 2dedcd788ec4..5b9ad65fce29 100644 --- 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 index eb5e692b2915..4621ca02ec80 100644 --- 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 index a79408b756ee..ef47cd1d35ca 100644 --- 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,29 +717,14 @@ 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 } @@ -747,7 +732,7 @@ object RefChecks { 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 index 91074110389e..20a96248a949 100644 --- 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 = From e90f1900360462d41377e181c9c8963de6d8e6a5 Mon Sep 17 00:00:00 2001 From: ghostbuster91 Date: Sat, 19 Jul 2025 20:08:12 +0200 Subject: [PATCH 2/3] Add test for multiline class def --- .../dotty/tools/dotc/typer/RefChecks.scala | 5 ++-- .../tools/dotc/reporting/CodeActionTest.scala | 30 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index ef47cd1d35ca..0d6aab6285be 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -22,6 +22,7 @@ import config.Printers.refcheck import reporting.* import Constants.Constant import cc.{stripCapturing, isUpdateMethod, CCState} +import dotty.tools.dotc.util.Chars.{isLineBreakChar, isWhitespace} object RefChecks { import tpd.* @@ -1278,7 +1279,7 @@ object RefChecks { val classHasBraces = classText.contains("{") && classText.contains("}") // Indentation for inserted methods - val lineStart = content.lastIndexWhere(c => c == '\n', end = span.end - 1) + 1 + 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 + " " @@ -1295,7 +1296,7 @@ object RefChecks { val braceEnd = classText.lastIndexOf('}') val bodyBetweenBraces = classText.slice(braceStart + 1, braceEnd) val bodyIsEmpty = bodyBetweenBraces.forall(_.isWhitespace) - val bodyContainsNewLine = bodyBetweenBraces.contains(System.lineSeparator()) + val bodyContainsNewLine = bodyBetweenBraces.exists(isLineBreakChar) val prefix = if (bodyContainsNewLine) "" else System.lineSeparator() val patchText = diff --git a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala index 20a96248a949..5c653dc0e57a 100644 --- a/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala +++ b/compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala @@ -291,6 +291,36 @@ class CodeActionTest extends DottyTest: afterPhase= "erasure" ) + @Test def insertMissingMethodsIntoMultilineClassDefinition = + 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" + ) + // 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 = From 7f9b524ec09cea5412b511c92c7a7492e5b7692d Mon Sep 17 00:00:00 2001 From: ghostbuster91 Date: Sun, 20 Jul 2025 17:40:21 +0200 Subject: [PATCH 3/3] Render different message depending on missingMethods count --- .../src/dotty/tools/dotc/reporting/messages.scala | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 4621ca02ec80..c9179fce5dde 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -3671,11 +3671,15 @@ extends Message(ConcreteClassHasUnimplementedMethodsID): }) } - def msg(using Context) = - s"""$clazz needs to be abstract, since it has ${missingMethods.size} unimplemented members. - | - |${renderMissingMethods.mkString("\n")} - |""".stripMargin + def msg(using Context) = missingMethods match + case single :: Nil => + def showDclAndLocation(sym: Symbol) = s"${sym.showDcl} in ${sym.owner.showLocated}" + s"$clazz needs to be abstract, since ${showDclAndLocation(single)} is not defined" + case _ => + 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