Skip to content

Commit

Permalink
Fix #56 (#57)
Browse files Browse the repository at this point in the history
  • Loading branch information
kubukoz authored Jul 12, 2021
1 parent 87470c9 commit f80f946
Show file tree
Hide file tree
Showing 10 changed files with 165 additions and 24 deletions.
3 changes: 2 additions & 1 deletion .scalafmt.conf
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
version = "3.0.0-RC5"
version = "3.0.0-RC6"
runner.dialect = scala3
rewrite.scala3.insertEndMarkerMinLines = 50
maxColumn = 140
align.preset = some
align.tokens.add = [
Expand Down
6 changes: 4 additions & 2 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,10 @@ val tests = project.settings(
case "3.0.0-RC2" => "0.7.25"
case _ => "0.7.26"
}) % Test
)
)
),
buildInfoKeys ++= Seq(scalaVersion),
buildInfoPackage := "b2s.buildinfo"
).enablePlugins(BuildInfoPlugin)

val betterToString =
project
Expand Down
3 changes: 3 additions & 0 deletions plugin/src/main/scala-2/BetterToStringPlugin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ final class BetterToStringPluginComponent(val global: Global) extends PluginComp
private def modifyClasses(tree: Tree, enclosingObject: Option[ModuleDef]): Tree =
tree match {
case p: PackageDef => p.copy(stats = p.stats.map(modifyClasses(_, None)))
// https://github.com/polyvariant/better-tostring/issues/59
// start here - ModuleDef which is a case object should be transformed.
// We might need to change the type of CompilerApi#Clazz to allow objects.
case m: ModuleDef =>
m.copy(impl = m.impl.copy(body = m.impl.body.map(modifyClasses(_, Some(m)))))
case clazz: ClassDef =>
Expand Down
6 changes: 4 additions & 2 deletions plugin/src/main/scala-2/Scala2CompilerApi.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,13 @@ object Scala2CompilerApi {
def addMethod(clazz: Clazz, method: Method): Clazz =
clazz.copy(impl = clazz.impl.copy(body = clazz.impl.body :+ method))

def methodNames(clazz: Clazz): List[String] = clazz.impl.body.collect { case d: DefDef =>
d.name.toString
def methodNames(clazz: Clazz): List[String] = clazz.impl.body.collect {
case d: DefDef => d.name.toString
case d: ValDef => d.name.toString
}

def isCaseClass(clazz: Clazz): Boolean = clazz.mods.hasFlag(Flags.CASE)
def isObject(clazz: Clazz): Boolean = clazz.mods.hasFlag(Flags.MODULE)
}

}
3 changes: 2 additions & 1 deletion plugin/src/main/scala-3/BetterToStringPlugin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ import dotty.tools.dotc.core.Symbols
import dotty.tools.dotc.plugins.PluginPhase
import dotty.tools.dotc.plugins.StandardPlugin
import dotty.tools.dotc.typer.FrontEnd
import tpd.*

import scala.annotation.tailrec

import tpd.*

final class BetterToStringPlugin extends StandardPlugin:
override val name: String = "better-tostring"
override val description: String = "scala compiler plugin for better default toString implementations"
Expand Down
31 changes: 21 additions & 10 deletions plugin/src/main/scala-3/Scala3CompilerApi.scala
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
package com.kubukoz

import dotty.tools.dotc.ast.Trees
import dotty.tools.dotc.ast.tpd
import dotty.tools.dotc.core.Constants.Constant
import dotty.tools.dotc.core.Contexts.Context
import dotty.tools.dotc.core.Symbols
import dotty.tools.dotc.core.Decorators.*
import dotty.tools.dotc.core.Flags
import dotty.tools.dotc.core.Flags.CaseAccessor
import dotty.tools.dotc.core.Flags.CaseClass
import dotty.tools.dotc.core.Flags.Module
import dotty.tools.dotc.core.Flags.Override
import dotty.tools.dotc.core.Flags.Package
import dotty.tools.dotc.core.Types
import dotty.tools.dotc.core.Names
import dotty.tools.dotc.core.Constants.Constant
import dotty.tools.dotc.core.Symbols
import dotty.tools.dotc.core.Symbols.ClassSymbol
import dotty.tools.dotc.core.Decorators.*
import dotty.tools.dotc.ast.Trees
import dotty.tools.dotc.core.Types

import tpd.*

trait Scala3CompilerApi extends CompilerApi:
Expand All @@ -34,7 +37,8 @@ object Scala3CompilerApi:
case v: ValDef if v.mods.is(CaseAccessor) => v
}

def className(clazz: Clazz): String = clazz.clazz.name.toString
def className(clazz: Clazz): String =
clazz.clazz.originalName.toString

def isPackageOrPackageObject(enclosingObject: EnclosingObject): Boolean =
enclosingObject.is(Package) || enclosingObject.isPackageObject
Expand Down Expand Up @@ -68,10 +72,17 @@ object Scala3CompilerApi:
def addMethod(clazz: Clazz, method: Method): Clazz =
clazz.mapTemplate(t => cpy.Template(t)(body = t.body :+ method))

def methodNames(clazz: Clazz): List[String] = clazz.t.body.collect { case d: DefDef =>
d.name.toString
}
// note: also returns vals because why not
def methodNames(clazz: Clazz): List[String] =
clazz.t.body.collect { case d: (DefDef | ValDef) =>
d.name.toString
}

def isCaseClass(clazz: Clazz): Boolean =
// for some reason, this is true for case objects too
clazz.clazz.flags.is(CaseClass)

def isCaseClass(clazz: Clazz): Boolean = clazz.clazz.flags.is(CaseClass)
def isObject(clazz: Clazz): Boolean =
clazz.clazz.flags.is(Module)

end Scala3CompilerApi
21 changes: 14 additions & 7 deletions plugin/src/main/scala/BetterToStringImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ trait CompilerApi {
def addMethod(clazz: Clazz, method: Method): Clazz
def methodNames(clazz: Clazz): List[String]
def isCaseClass(clazz: Clazz): Boolean
def isObject(clazz: Clazz): Boolean
}

trait BetterToStringImpl[+C <: CompilerApi] {
Expand Down Expand Up @@ -54,8 +55,7 @@ object BetterToStringImpl {
): Clazz = {
val hasToString: Boolean = methodNames(clazz).contains("toString")

val shouldModify =
isCaseClass(clazz) && !isNested && !hasToString
val shouldModify = isCaseClass(clazz) && !isNested && !hasToString

if (shouldModify) overrideToString(clazz, enclosingObject)
else clazz
Expand All @@ -68,6 +68,8 @@ object BetterToStringImpl {
val className = api.className(clazz)
val parentPrefix = enclosingObject.filterNot(api.isPackageOrPackageObject).fold("")(api.enclosingObjectName(_) ++ ".")

val namePart = literalConstant(parentPrefix ++ className)

val paramListParts: List[Tree] = params(clazz).zipWithIndex.flatMap { case (v, index) =>
val commaPrefix = if (index > 0) ", " else ""

Expand All @@ -79,12 +81,17 @@ object BetterToStringImpl {
)
}

val paramParts =
if (api.isObject(clazz)) Nil
else
List(
List(literalConstant("(")),
paramListParts,
List(literalConstant(")"))
).flatten

val parts =
List(
List(literalConstant(parentPrefix ++ className ++ "(")),
paramListParts,
List(literalConstant(")"))
).flatten
namePart :: paramParts

parts.reduceLeft(concat(_, _))
}
Expand Down
1 change: 1 addition & 0 deletions project/plugins.sbt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
addSbtPlugin("com.geirsson" % "sbt-ci-release" % "1.5.7")
addSbtPlugin("io.github.davidgregory084" % "sbt-tpolecat" % "0.1.20")
addSbtPlugin("com.codecommit" % "sbt-github-actions" % "0.12.0")
addSbtPlugin("com.eed3si9n" % "sbt-buildinfo" % "0.10.0")
58 changes: 58 additions & 0 deletions tests/src/test/scala-3/Scala3Tests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import munit.FunSuite

class Scala3Tests extends FunSuite:
test("an enum made of constants should have a normal toString") {
assertEquals(
ScalaVersion.Scala2.toString,
// https://github.com/polyvariant/better-tostring/issues/60
// should be "ScalaVersion.Scala2"
"Scala2"
)
assertEquals(
ScalaVersion.Scala3.toString,
// https://github.com/polyvariant/better-tostring/issues/60
// should be "ScalaVersion.Scala3"
"Scala3"
)
}

test("an enum being an ADT should get a custom toString") {
assertEquals(
User.LoggedIn("admin").toString,
"User.LoggedIn(name = admin)"
)
assertEquals(
User.Unauthorized.toString,
// https://github.com/polyvariant/better-tostring/issues/60
// should be "User.Unauthorized"
"Unauthorized"
)
}

test("an enum with a custom toString should use it") {
assertEquals(
EnumCustomTostring.SimpleCase.toString,
"example"
)

// https://github.com/polyvariant/better-tostring/issues/34
// we aren't there yet - need to be able to find inherited `toString`s first
assertEquals(
EnumCustomTostring.ParameterizedCase("foo").toString,
// Should be "example" because the existing toString should take precedence.
// Update the test when #34 is fixed.
"EnumCustomTostring.ParameterizedCase(value = foo)"
)
}

enum ScalaVersion:
case Scala2, Scala3

enum EnumCustomTostring:
case SimpleCase
case ParameterizedCase(value: String)
override def toString: String = "example"

enum User:
case LoggedIn(name: String)
case Unauthorized
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import munit.FunSuite
import munit.TestOptions
import b2s.buildinfo.BuildInfo

class Tests extends FunSuite {

Expand Down Expand Up @@ -27,6 +29,12 @@ class Tests extends FunSuite {
"***"
)
}
test("Case class with custom toString val should not be overridden") {
assertEquals(
CustomTostringVal("Joe").toString,
"***"
)
}

test("Method with alternate constructors should stringify based on primary constructor") {
assertEquals(
Expand All @@ -37,13 +45,21 @@ class Tests extends FunSuite {
)
}

test("Class nested in an object should include enclosing object's name") {
test("Case class nested in an object should include enclosing object's name") {
assertEquals(
ObjectNestedParent.ObjectNestedClass("Joe").toString,
"ObjectNestedParent.ObjectNestedClass(name = Joe)"
)
}

// https://github.com/polyvariant/better-tostring/issues/59
test(onlyScala3("Case object nested in an object should include enclosing object's name")) {
assertEquals(
ObjectNestedParent.ObjectNestedObject.toString,
"ObjectNestedParent.ObjectNestedObject"
)
}

test("Class nested in a package object should not include package's name") {
assertEquals(
pack.InPackageObject("Joe").toString,
Expand Down Expand Up @@ -74,6 +90,40 @@ class Tests extends FunSuite {
"LocalClass(a)"
)
}

test("Lone case object should use the default toString") {
assertEquals(CaseObject.toString, "CaseObject")
}

test("Case object with toString should not get extra toString") {
assertEquals(
CaseObjectWithToString.toString,
"example"
)
}

test("Case object with toString val should not get extra toString") {
assertEquals(
CaseObjectWithToStringVal.toString,
"example"
)
}

def onlyScala3(name: String) = {
val isScala3 = BuildInfo.scalaVersion.startsWith("3")
if (isScala3) name: TestOptions else name.fail
}

}

case object CaseObject

case object CaseObjectWithToString {
override def toString: String = "example"
}

case object CaseObjectWithToStringVal {
override val toString: String = "example"
}

final case class SimpleCaseClass(name: String, age: Int)
Expand All @@ -83,6 +133,10 @@ final case class CustomTostring(name: String) {
override def toString: String = "***"
}

final case class CustomTostringVal(name: String) {
override val toString: String = "***"
}

final case class HasOtherConstructors(s: String) {
def this(a: Int) = this(a.toString + " beers")
}
Expand All @@ -93,6 +147,7 @@ final class NestedParent() {

object ObjectNestedParent {
case class ObjectNestedClass(name: String)
case object ObjectNestedObject
}

final class DeeplyNestedInClassGrandparent {
Expand Down

0 comments on commit f80f946

Please sign in to comment.