Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix toString for Scala 3 enums with no parameters #181

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ val betterToString =
.settings(
commonSettings,
(publish / skip) := true,
addCommandAlias("generateAll", List("githubWorkflowGenerate", "mergifyWrite", "readmeWrite").mkString(";"))
addCommandAlias("generateAll", List("githubWorkflowGenerate", "mergifyWrite", "readmeWrite", "headerCreateAll").mkString(";"))
)
.aggregate(plugin, tests)
.enablePlugins(NoPublishPlugin)
Expand Down
12 changes: 11 additions & 1 deletion plugin/src/main/scala-2/Scala2CompilerApi.scala
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,24 @@ object Scala2CompilerApi {
case d: ValDef => d.name.toString
}

def isCaseClass(clazz: Clazz): Boolean = clazz.merge.mods.isCase
// No enums in scala 2
def isEnum(clazz: Clazz): Boolean = false

def isViableDefinition(clazz: Clazz): Boolean = {
val isCaseClassOrObject = clazz.merge.mods.isCase

isCaseClassOrObject
}

// Always return true for ModuleDef - apparently ModuleDef doesn't have the module flag...
def isObject(clazz: Clazz): Boolean = clazz.fold(
clazz = _.mods.hasModuleFlag,
obj = _ => true
)

def productPrefixParam: Nothing = sys.error("invalid state: this shouldn't be called in Scala 2")

def report(str: String): Unit = reporter.echo(str)
}

}
31 changes: 31 additions & 0 deletions plugin/src/main/scala-3.1.x/ParentTypes.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright 2020 Polyvariant
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.polyvariant

import dotty.tools.dotc.core.Symbols.ClassSymbol
import dotty.tools.dotc.core.Contexts.Context
import dotty.tools.dotc.core.Types.Type
import dotty.tools.dotc.core.Types.ClassInfo

extension (clazz: ClassSymbol) {

// 3.1.x/3.2.x doesn't have this method, this is the inlined definition from 3.3.x
def parentTypes(using Context): List[Type] = clazz.info match
case classInfo: ClassInfo => classInfo.declaredParents
case _ => Nil

}
31 changes: 31 additions & 0 deletions plugin/src/main/scala-3.2.x/ParentTypes.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright 2020 Polyvariant
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.polyvariant

import dotty.tools.dotc.core.Symbols.ClassSymbol
import dotty.tools.dotc.core.Contexts.Context
import dotty.tools.dotc.core.Types.Type
import dotty.tools.dotc.core.Types.ClassInfo

extension (clazz: ClassSymbol) {

// 3.1.x/3.2.x doesn't have this method, this is the inlined definition from 3.3.x
def parentTypes(using Context): List[Type] = clazz.info match
case classInfo: ClassInfo => classInfo.declaredParents
case _ => Nil

}
17 changes: 14 additions & 3 deletions plugin/src/main/scala-3/Scala3CompilerApi.scala
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,22 @@ object Scala3CompilerApi:
d.name.toString
}

def isCaseClass(clazz: Clazz): Boolean =
// for some reason, this is true for case objects too
clazz.clazz.flags.is(CaseClass)
def isEnum(clazz: Clazz): Boolean =
// the class is an enum if its direct supertypes include scala.reflect.Enum
clazz.clazz.parentTypes.map(_.typeSymbol).contains(Symbols.defn.EnumClass)

def isViableDefinition(clazz: Clazz): Boolean = {
val isCaseClassOrObject = clazz.clazz.flags.is(CaseClass)

isCaseClassOrObject || isEnum(clazz)
}

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

def report(str: String): Unit =
dotty.tools.dotc.report.echo(str)

def productPrefixParam: ParamName = "productPrefix".toTermName

end Scala3CompilerApi
11 changes: 8 additions & 3 deletions plugin/src/main/scala/BetterToStringImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,13 @@ trait CompilerApi {
def createToString(clazz: Clazz, body: Tree): Method
def addMethod(clazz: Clazz, method: Method): Clazz
def methodNames(clazz: Clazz): List[String]
// better name: "is case class or object"
def isCaseClass(clazz: Clazz): Boolean
def isViableDefinition(clazz: Clazz): Boolean
def isEnum(clazz: Clazz): Boolean
def isObject(clazz: Clazz): Boolean
def productPrefixParam: ParamName

// debugging
def report(str: String): Unit
}

trait BetterToStringImpl[+C <: CompilerApi] {
Expand Down Expand Up @@ -73,7 +77,7 @@ object BetterToStringImpl {
// technically, the method found by this can be even something like "def toString(s: String): Unit", but we're ignoring that
val hasToString: Boolean = methodNames(clazz).contains("toString")

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

if (shouldModify) overrideToString(clazz, enclosingObject)
else clazz
Expand Down Expand Up @@ -101,6 +105,7 @@ object BetterToStringImpl {

val paramParts =
if (api.isObject(clazz)) Nil
else if (api.isEnum(clazz)) List(literalConstant("."), selectInThis(clazz, productPrefixParam))
else
List(
List(literalConstant("(")),
Expand Down
12 changes: 3 additions & 9 deletions tests/src/test/scala-3/Scala3Tests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,11 @@ 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"
"ScalaVersion.Scala2"
)
assertEquals(
ScalaVersion.Scala3.toString,
// https://github.com/polyvariant/better-tostring/issues/60
// should be "ScalaVersion.Scala3"
"Scala3"
"ScalaVersion.Scala3"
)
}

Expand All @@ -39,9 +35,7 @@ class Scala3Tests extends FunSuite:
)
assertEquals(
User.Unauthorized.toString,
// https://github.com/polyvariant/better-tostring/issues/60
// should be "User.Unauthorized"
"Unauthorized"
"User.Unauthorized"
)
}

Expand Down