From a94df6116c44fb7edcf53d33249ba0505a135825 Mon Sep 17 00:00:00 2001 From: rochala Date: Tue, 26 May 2026 23:58:21 +0200 Subject: [PATCH] Fix Scala 3 top-level rendering bugs - TypePrinter: parameterless def/val now prints ': Type' (was missing colon) - TypePrinter: add TypeMemberSymbol case for opaque types, type aliases, abstract types (was falling through to toString sentinel) - GetFormatter: apply userFqn() to FQN heading and origin so $package$ wrapper segments and trailing $ never appear in output - GetFormatter: apply userFqn() in renderSubtypes so case object / object sealed children don't leak trailing $ in Known subtypes line - SymbolResolver: filter @main launcher class from results when a top-level def with the same FQN exists; use subsetOf for future-proofing - GetFormatterTest: add 3 end-to-end formatter tests for @main def, opaque type, and companion member - CLAUDE.md: add principle to verify e2e output quality when fixing bugs Co-Authored-By: Claude Sonnet 4.6 --- CLAUDE.md | 10 +++++ lib/src/cellar/GetFormatter.scala | 20 +++++++-- lib/src/cellar/SymbolResolver.scala | 16 ++++++- lib/src/cellar/TypePrinter.scala | 28 ++++++++++-- lib/test/src/cellar/GetFormatterTest.scala | 51 ++++++++++++++++++++++ 5 files changed, 117 insertions(+), 8 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index cfce6a6..7e9e09a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -93,6 +93,16 @@ Strong success criteria let you loop independently. Weak criteria ("make it work Examples are testing match exhaustivity, typesystem etc. +6. Always verify end-to-end output quality when fixing a bug + +A passing unit test on an intermediate data structure is not proof the +user-visible output is correct. After any bug fix or feature touching +rendered output, run the relevant `cellar` command against a real +fixture or published artifact and read the printed Markdown yourself +before declaring the task done. Internal assertions like "the resolver +returned Found(symbol)" do not catch leaked synthetic names, broken +signatures, or missing separators. + ## Code Conventions - Use `fs2.io.file.Path` for file references, not `java.io.File` or `java.nio.file.Path` diff --git a/lib/src/cellar/GetFormatter.scala b/lib/src/cellar/GetFormatter.scala index 0677c0b..581fe86 100644 --- a/lib/src/cellar/GetFormatter.scala +++ b/lib/src/cellar/GetFormatter.scala @@ -11,7 +11,7 @@ object GetFormatter: hideInherited: Boolean = false, groupInherited: Boolean = false )(using ctx: Context): String = - val fqn = sym.displayFullName + val fqn = userFqn(sym.displayFullName) val signature = TypePrinter.printSymbolSignature(sym) val flags = renderFlags(sym) val origin = renderOrigin(sym) @@ -80,8 +80,20 @@ object GetFormatter: private def renderOrigin(sym: Symbol): String = sym.owner match - case owner: ClassSymbol => owner.displayFullName - case _ => sym.displayFullName + case owner: ClassSymbol => userFqn(owner.displayFullName) + case _ => userFqn(sym.displayFullName) + + /** + * Hide Scala-3 implementation details in a displayed FQN: + * - drop `$package$` wrapper segments that hold top-level decls + * - drop the trailing `$` that module-class names carry + * `pkg.Hello$package$.Hello$.fromInt` becomes `pkg.Hello.fromInt`. + */ + private[cellar] def userFqn(raw: String): String = + raw.split('.').iterator + .filterNot(_.endsWith("$package$")) + .map(seg => if seg.endsWith("$") then seg.stripSuffix("$") else seg) + .mkString(".") private def renderMembers( sym: Symbol, @@ -166,5 +178,5 @@ object GetFormatter: case cls: ClassSymbol => val children = cls.sealedChildren if children.isEmpty then None - else Some(children.map(_.displayFullName).mkString(", ")) + else Some(children.map(c => userFqn(c.displayFullName)).mkString(", ")) case _ => None diff --git a/lib/src/cellar/SymbolResolver.scala b/lib/src/cellar/SymbolResolver.scala index 87ad929..35bf4cb 100644 --- a/lib/src/cellar/SymbolResolver.scala +++ b/lib/src/cellar/SymbolResolver.scala @@ -52,7 +52,17 @@ object SymbolResolver: tryOrNone(ctx.findStaticTerm(fqn)), tryOrNone(ctx.findStaticType(fqn)) ).flatten - val all = (direct ++ packageWrapperMembers(fqn)).distinct + val wrapped = packageWrapperMembers(fqn) + // `@main def foo` emits a launcher class `pkg.foo` whose only decls are + // `` and `main`. When a top-level def with the same FQN exists, the + // class is implementation detail of the def — drop it. + val filteredDirect = + if wrapped.nonEmpty then direct.filterNot { + case c: ClassSymbol => isMainLauncher(c) + case _ => false + } + else direct + val all = (filteredDirect ++ wrapped).distinct if all.nonEmpty then Some(LookupResult.Found(all)) else tryOrNone(ctx.findPackage(fqn)).map(_ => LookupResult.IsPackage) @@ -232,6 +242,10 @@ object SymbolResolver: private def packageWrapperClass(fqn: String)(using ctx: Context): Option[ClassSymbol] = withPackageWrappers[ClassSymbol](fqn)((w, name) => directClassMember(w, name)).headOption + private def isMainLauncher(cls: ClassSymbol)(using ctx: Context): Boolean = + val names = tryOrNone(cls.declarations).getOrElse(Nil).map(_.name.toString).toSet + names.subsetOf(Set("", "main")) && names.contains("main") + private[cellar] val universalBaseClasses = Set("scala.Any", "scala.AnyRef", "java.lang.Object") /** diff --git a/lib/src/cellar/TypePrinter.scala b/lib/src/cellar/TypePrinter.scala index 062dbc5..7f0f35e 100644 --- a/lib/src/cellar/TypePrinter.scala +++ b/lib/src/cellar/TypePrinter.scala @@ -1,7 +1,15 @@ package cellar import tastyquery.Contexts.Context -import tastyquery.Symbols.{ClassSymbol, ClassTypeParamSymbol, Symbol, TermOrTypeSymbol, TermSymbol} +import tastyquery.Symbols.{ + ClassSymbol, + ClassTypeParamSymbol, + Symbol, + TermOrTypeSymbol, + TermSymbol, + TypeMemberDefinition, + TypeMemberSymbol +} import tastyquery.Types.* enum DetectedLanguage: @@ -95,8 +103,22 @@ object TypePrinter: val keyword = termKeyword(term) if term.isModuleVal then s"$keyword ${term.name}" else - val sig = printMethodic(term.declaredType) - s"$keyword ${term.name}$sig" + term.declaredType match + case _: MethodType | _: PolyType => + s"$keyword ${term.name}${printMethodic(term.declaredType)}" + case t: Type => + s"$keyword ${term.name}: ${printType(t)}" + + case tm: TypeMemberSymbol => + tm.typeDef match + case TypeMemberDefinition.OpaqueTypeAlias(_, alias) => + s"opaque type ${tm.name} = ${printType(alias)}" + case TypeMemberDefinition.TypeAlias(alias) => + s"type ${tm.name} = ${printType(alias)}" + case TypeMemberDefinition.AbstractType(bounds) => + val lo = if bounds.low.toString == "Nothing" then "" else s" >: ${printType(bounds.low)}" + val hi = if bounds.high.toString == "Any" then "" else s" <: ${printType(bounds.high)}" + s"type ${tm.name}$lo$hi" case other => other.toString diff --git a/lib/test/src/cellar/GetFormatterTest.scala b/lib/test/src/cellar/GetFormatterTest.scala index 0e13b1e..f6c2f0b 100644 --- a/lib/test/src/cellar/GetFormatterTest.scala +++ b/lib/test/src/cellar/GetFormatterTest.scala @@ -238,3 +238,54 @@ class GetFormatterTest extends CatsEffectSuite: assert(limited.contains("more members"), s"Expected 'more members' in: $limited") } } + + // Scala-3 top-level decls live inside a synthetic `$package$` wrapper + // class. The wrapper must not leak into the rendered Markdown heading, + // origin line, or signature. Fixture: fixtureScala3/src/myapp/Hello.scala + // package myapp + // @main def hello = println(42) + // opaque type Hello = Int + // object Hello: + // def fromInt(a: Int): Hello = a + + test("formatGetResult for @main top-level def hides $package$ and synthetic launcher class"): + withCtx { ctx => + given Context = ctx + SymbolResolver.resolve("myapp.hello").map { + case LookupResult.Found(syms) => + val output = GetFormatter.formatGetResult("myapp.hello", syms) + assert(!output.contains("$package$"), s"Expected no $$package$$ in:\n$output") + assert(output.contains("## myapp.hello"), s"Expected '## myapp.hello' heading in:\n$output") + assert(output.contains("**Origin:** myapp"), s"Expected origin 'myapp' in:\n$output") + assert(output.contains("def hello: Unit"), s"Expected 'def hello: Unit' in:\n$output") + // @main wrapper class is implementation detail — must not appear + assert(!output.contains("class hello"), s"Unexpected '@main' wrapper class in:\n$output") + case other => fail(s"Expected Found, got $other") + } + } + + test("formatGetResult for top-level opaque type renders 'opaque type X = ...'"): + withCtx { ctx => + given Context = ctx + SymbolResolver.resolve("myapp.Hello").map { + case LookupResult.Found(syms) => + val output = GetFormatter.formatGetResult("myapp.Hello", syms) + assert(!output.contains("$package$"), s"Expected no $$package$$ in:\n$output") + assert(output.contains("opaque type Hello = Int"), s"Expected opaque-type signature in:\n$output") + assert(!output.contains("symbol["), s"Unexpected raw symbol[] sentinel in:\n$output") + case other => fail(s"Expected Found, got $other") + } + } + + test("formatGetResult for top-level companion member hides $package$"): + withCtx { ctx => + given Context = ctx + SymbolResolver.resolve("myapp.Hello.fromInt").map { + case LookupResult.Found(syms) => + val output = GetFormatter.formatGetResult("myapp.Hello.fromInt", syms) + assert(!output.contains("$package$"), s"Expected no $$package$$ in:\n$output") + assert(output.contains("## myapp.Hello.fromInt"), s"Expected '## myapp.Hello.fromInt' heading in:\n$output") + assert(output.contains("**Origin:** myapp.Hello"), s"Expected origin 'myapp.Hello' in:\n$output") + case other => fail(s"Expected Found, got $other") + } + }