Skip to content

Ignore private constructors #778

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

Merged
merged 10 commits into from
Dec 11, 2023
Merged
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
19 changes: 16 additions & 3 deletions core/src/main/scala/com/typesafe/tools/mima/core/MemberInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ sealed abstract class MemberInfo(val owner: ClassInfo, val bytecodeName: String,
final var isDeprecated: Boolean = false
final var signature: Signature = Signature.none // Includes generics. 'descriptor' is the erased version.
final var scopedPrivate: Boolean = false
final var classPrivate: Boolean = false

def nonAccessible: Boolean

final def fullName: String = s"${owner.formattedFullName}.$decodedName"
final def abstractPrefix = if (isDeferred && !owner.isTrait) "abstract " else ""
final def scopedPrivatePrefix = if (scopedPrivate) "private[..] " else ""
final def scopedPrivatePrefix = "private[..] "
final def classPrivatePrefix = "private "
final def staticPrefix: String = if (isStatic) "static " else ""
final def tpe: Type = owner.owner.definitions.fromDescriptor(descriptor)
final def hasSyntheticName: Boolean = decodedName.contains('$')
Expand Down Expand Up @@ -44,7 +46,16 @@ private[mima] final class MethodInfo(owner: ClassInfo, bytecodeName: String, fla
def shortMethodString: String = {
val prefix = if (hasSyntheticName) if (isExtensionMethod) "extension " else "synthetic " else ""
val deprecated = if (isDeprecated) "deprecated " else ""
s"${scopedPrivatePrefix}${abstractPrefix}$prefix${deprecated}${staticPrefix}method $decodedName$tpe"

val privatePrefix = if (isScopedPrivate) {
scopedPrivatePrefix
} else if (isClassPrivate) {
classPrivatePrefix
} else {
""
}

s"${privatePrefix}${abstractPrefix}$prefix${deprecated}${staticPrefix}method $decodedName$tpe"
}

lazy val paramsCount: Int = {
Expand All @@ -67,10 +78,12 @@ private[mima] final class MethodInfo(owner: ClassInfo, bytecodeName: String, fla
decodedName.substring(0, i + 1).endsWith("$extension")
}
def nonAccessible: Boolean = {
!isPublic || isScopedPrivate || isSynthetic ||
!isPublic || isScopedPrivate || isClassPrivate || isSynthetic ||
(hasSyntheticName && !(isExtensionMethod || isDefaultGetter || isTraitInit))
}
def isScopedPrivate: Boolean = scopedPrivate

def isClassPrivate: Boolean = classPrivate

override def toString = s"def $bytecodeName: $descriptor"
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.typesafe.tools.mima.core

import PickleFormat._
import com.typesafe.tools.mima.core.PickleFormat.*

object MimaUnpickler {
def unpickleClass(buf: PickleBuffer, clazz: ClassInfo, path: String): Unit = {
Expand Down Expand Up @@ -174,7 +174,6 @@ object MimaUnpickler {
def doMethods(clazz: ClassInfo, methods: List[SymbolInfo]) = {
methods.iterator
.filter(!_.isParam)
.filter(_.name.value != CONSTRUCTOR) // TODO support package private constructors
.toSeq.groupBy(_.name).foreach { case (name, pickleMethods) =>
doMethodOverloads(clazz, name, pickleMethods)
}
Expand All @@ -192,9 +191,10 @@ object MimaUnpickler {
// then implementing the rules of erasure, so that you can then match the pickle
// types with the erased types. Meanwhile we'll just ignore them, worst case users
// need to add a filter like they have for years.
if (pickleMethods.size == bytecodeMethods.size && pickleMethods.exists(_.isScopedPrivate)) {
if (pickleMethods.size == bytecodeMethods.size && pickleMethods.exists(m => m.isScopedPrivate || m.isClassPrivate)) {
bytecodeMethods.zip(pickleMethods).foreach { case (bytecodeMeth, pickleMeth) =>
bytecodeMeth.scopedPrivate = pickleMeth.isScopedPrivate
bytecodeMeth.classPrivate = pickleMeth.isClassPrivate
}
}
}
Expand Down Expand Up @@ -262,6 +262,7 @@ object MimaUnpickler {
def hasFlag(flag: Long): Boolean = (flags & flag) != 0L
def isModuleOrModuleClass = hasFlag(Flags.MODULE_PKL)
def isParam = hasFlag(Flags.PARAM)
def isClassPrivate = hasFlag(Flags.PRIVATE)
}
val NoSymbol: SymbolInfo = SymbolInfo(NONEsym, nme.NoSymbol, null, 0, false)

Expand Down Expand Up @@ -289,6 +290,7 @@ object MimaUnpickler {
}

object Flags {
final val PRIVATE = 1L << 2
final val MODULE_PKL = 1L << 10
final val PARAM = 1L << 13
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ object TastyUnpickler {
val names = readNames(in)
val tree = unpickleTree(getTreeReader(in, names), names)

copyPrivateWithin(tree, clazz.owner)
copyPrivateFlags(tree, clazz.owner)
copyAnnotations(tree, clazz.owner)
}

Expand Down Expand Up @@ -73,7 +73,7 @@ object TastyUnpickler {
}
}.traverse(tree)

def copyPrivateWithin(tree: Tree, pkgInfo: PackageInfo): Unit = new ClassTraverser(pkgInfo) {
def copyPrivateFlags(tree: Tree, pkgInfo: PackageInfo): Unit = new ClassTraverser(pkgInfo) {
override def forEachClass(clsDef: ClsDef, cls: ClassInfo): Unit =
clsDef.privateWithin.foreach(_ => cls.module._scopedPrivate = true)

Expand All @@ -85,17 +85,21 @@ object TastyUnpickler {
def doMethods(tmpl: Template) = {
val clazz = currentClass
(tmpl.fields ::: tmpl.meths).iterator
.filter(_.name != Name.Constructor) // TODO support package private constructors
.toSeq.groupBy(_.name).foreach { case (name, pickleMethods) =>
doMethodOverloads(clazz, name, pickleMethods)
}
}

def doMethodOverloads(clazz: ClassInfo, name: Name, pickleMethods: Seq[TermMemberDef]) = {
val bytecodeMethods = clazz.methods.get(name.source).filter(!_.isBridge).toList
if (pickleMethods.size == bytecodeMethods.size && pickleMethods.exists(_.privateWithin.isDefined)) {
bytecodeMethods.zip(pickleMethods).foreach { case (bytecodeMeth, pickleMeth) =>
bytecodeMeth.scopedPrivate = pickleMeth.privateWithin.isDefined

if (pickleMethods.size == bytecodeMethods.size) {
if (pickleMethods.exists(t => t.privateWithin.isDefined || t.classPrivate)) {
bytecodeMethods.zip(pickleMethods).foreach { case (bytecodeMeth, pickleMeth) =>

bytecodeMeth.scopedPrivate = pickleMeth.privateWithin.isDefined
bytecodeMeth.classPrivate = pickleMeth.classPrivate
}
}
}
}
Expand Down Expand Up @@ -144,8 +148,8 @@ object TastyUnpickler {
val name = readName()
skipTree(readByte()) // type
if (!nothingButMods(end)) skipTree(readByte()) // rhs
val (privateWithin, annots) = readMods(end)
ValDef(name, privateWithin, annots)
val (privateWithin, classPrivate, annots) = readMods(end)
ValDef(name, privateWithin, classPrivate, annots)
}

def readDefDef() = {
Expand All @@ -156,8 +160,8 @@ object TastyUnpickler {
while (nextByte == TYPEPARAM || nextByte == PARAM || nextByte == EMPTYCLAUSE || nextByte == SPLITCLAUSE) skipTree(readByte()) // params
skipTree(readByte()) // returnType
if (!nothingButMods(end)) skipTree(readByte()) // rhs
val (privateWithin, annots) = readMods(end)
DefDef(name, privateWithin, annots)
val (privateWithin, classPrivate, annots) = readMods(end)
DefDef(name, privateWithin, classPrivate, annots)
}

def readTemplate(): Template = {
Expand Down Expand Up @@ -188,25 +192,27 @@ object TastyUnpickler {
def readClassDef(name: Name, end: Addr) = {
// NameRef Template Modifier* -- modifiers class name template
val template = readTemplate()
val (privateWithin, annots) = readMods(end)
val (privateWithin, classPrivate, annots) = readMods(end)
ClsDef(name.toTypeName, template, privateWithin, annots)
}

def readTypeMemberDef(end: Addr) = { goto(end); UnknownTree(TYPEDEF) } // NameRef type_Term Modifier* -- modifiers type name (= type | bounds)

def readMods(end: Addr): (Option[Type], List[Annot]) = {
def readMods(end: Addr): (Option[Type], Boolean, List[Annot]) = {
// PRIVATEqualified qualifier_Type -- private[qualifier]
// PROTECTEDqualified qualifier_Type -- protected[qualifier]
var privateWithin = Option.empty[Type]
var classPrivate = false
val annots = new ListBuffer[Annot]
doUntil(end)(readByte() match {
case ANNOTATION => annots += readAnnot()
case PRIVATEqualified => privateWithin = Some(readType())
case PROTECTEDqualified => privateWithin = Some(readType())
case PRIVATE => classPrivate = true
case tag if isModifierTag(tag) => skipTree(tag)
case tag => assert(false, s"illegal modifier tag ${astTagToString(tag)} at ${currentAddr.index - 1}, end = $end")
})
(privateWithin, annots.toList)
(privateWithin, classPrivate, annots.toList)
}

def readTypeDef() = {
Expand Down Expand Up @@ -271,11 +277,11 @@ object TastyUnpickler {
def show = s"${showXs(annots, end = " ")}${showPrivateWithin(privateWithin)}class $name$template"
}
final case class Template(classes: List[ClsDef], fields: List[ValDef], meths: List[DefDef]) extends Tree { def show = s"${(classes ::: meths).map("\n " + _).mkString}" }
final case class ValDef(name: Name, privateWithin: Option[Type], annots: List[Annot] = Nil) extends TermMemberDef
final case class DefDef(name: Name, privateWithin: Option[Type], annots: List[Annot] = Nil) extends TermMemberDef
final case class ValDef(name: Name, privateWithin: Option[Type], classPrivate: Boolean, annots: List[Annot] = Nil) extends TermMemberDef
final case class DefDef(name: Name, privateWithin: Option[Type], classPrivate: Boolean, annots: List[Annot] = Nil) extends TermMemberDef

sealed trait TermMemberDef extends Tree {
def name: Name; def privateWithin: Option[Type]; def annots: List[Annot]
def name: Name; def privateWithin: Option[Type]; def classPrivate: Boolean; def annots: List[Annot]
def show = s"${showXs(annots, end = " ")}${showPrivateWithin(privateWithin)}def $name"
}

Expand Down Expand Up @@ -311,9 +317,11 @@ object TastyUnpickler {
def traversePkg(pkg: Pkg) = { val Pkg(path, trees) = pkg; traverse(path); trees.foreach(traverse) }
def traverseClsDef(clsDef: ClsDef) = { val ClsDef(name, tmpl, privateWithin, annots) = clsDef; traverseName(name); traverseTemplate(tmpl); traversePrivateWithin(privateWithin); annots.foreach(traverse) }
def traverseTemplate(tmpl: Template) = { val Template(classes, fields, meths) = tmpl; classes.foreach(traverse); fields.foreach(traverse); meths.foreach(traverse) }
def traverseValDef(valDef: ValDef) = { val ValDef(name, privateWithin, annots) = valDef; traverseName(name); traversePrivateWithin(privateWithin); annots.foreach(traverse) }
def traverseDefDef(defDef: DefDef) = { val DefDef(name, privateWithin, annots) = defDef; traverseName(name); traversePrivateWithin(privateWithin); annots.foreach(traverse) }
def traverseValDef(valDef: ValDef) = { val ValDef(name, privateWithin, _, annots) = valDef; traverseName(name); traversePrivateWithin(privateWithin); annots.foreach(traverse) }
def traverseDefDef(defDef: DefDef) = { val DefDef(name, privateWithin, _, annots) = defDef; traverseName(name); traversePrivateWithin(privateWithin); annots.foreach(traverse) }
def traversePrivateWithin(privateWithin: Option[Type]) = { privateWithin.foreach(traverseType) }

//def traverseClassPrivate(classPrivate: Boolean) = { privateWithin.foreach(traverseType) }
def traverseAnnot(annot: Annot) = { val Annot(tycon, fullAnnotation) = annot; traverse(tycon); traverse(fullAnnotation) }

def traversePath(path: Path) = path match {
Expand Down Expand Up @@ -552,10 +560,10 @@ object TastyUnpickler {
}

final case class Header(
val header: (Int, Int, Int, Int),
val version: (Int, Int, Int),
val toolingVersion: String,
val uuid: UUID,
header: (Int, Int, Int, Int),
version: (Int, Int, Int),
toolingVersion: String,
uuid: UUID,
)

def readHeader(in: TastyReader): Header = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,26 @@
package com.typesafe.tools.mima.lib

import java.io.{ ByteArrayOutputStream, PrintStream }
import java.net.{ URI, URLClassLoader }
import javax.tools._
import com.typesafe.tools.mima.core.ClassPath

import java.io.{ByteArrayOutputStream, PrintStream}
import java.net.{URI, URLClassLoader}
import java.nio.charset.StandardCharsets
import java.nio.file.Files
import javax.tools._
import scala.annotation.tailrec
import scala.collection.JavaConverters._
import scala.collection.mutable
import scala.reflect.internal.util.BatchSourceFile
import scala.reflect.io.{ Directory, Path, PlainFile }
import scala.util.{ Failure, Success, Try }

import com.typesafe.tools.mima.core.ClassPath
import scala.reflect.io.{Directory, Path, PlainFile}
import scala.util.{Failure, Success, Try}

final class TestCase(val baseDir: Directory, val scalaCompiler: ScalaCompiler, val javaCompiler: JavaCompiler) {
def name = baseDir.name
def scalaBinaryVersion = if (scalaCompiler.isScala3) "3" else scalaCompiler.version.take(4)
def scalaJars = scalaCompiler.jars

def skip: Boolean = (baseDir / s"skip-${scalaBinaryVersion}.txt").exists

val srcV1 = (baseDir / "v1").toDirectory
val srcV2 = (baseDir / "v2").toDirectory
val srcApp = (baseDir / "app").toDirectory
Expand All @@ -44,8 +47,14 @@ final class TestCase(val baseDir: Directory, val scalaCompiler: ScalaCompiler, v
if (sourceFiles.forall(_.isJava)) return Success(())
val bootcp = ClassPath.join(scalaJars.map(_.getPath))
val cpOpt = if (cp.isEmpty) Nil else List("-classpath", ClassPath.join(cp.map(_.path)))
val optsFile = baseDir / s"scalac-options-${scalaBinaryVersion}.txt"
val testOpts = if (optsFile.exists) {
Files.readAllLines(optsFile.jfile.toPath, StandardCharsets.UTF_8).asScala.filterNot(_.trim.startsWith("#")).toList
} else {
List.empty
}
val paths = sourceFiles.map(_.path)
val args = "-bootclasspath" :: bootcp :: cpOpt ::: "-d" :: s"$out" :: paths
val args = "-bootclasspath" :: bootcp :: testOpts ::: cpOpt ::: "-d" :: s"$out" :: paths
scalaCompiler.compile(args)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ object UnitTests {
} yield ()

def runTestCaseIf(testCase: TestCase, direction: Direction): Try[Unit] = {
if ((testCase.baseDir / direction.oracleFile).exists)
if ((testCase.baseDir / direction.oracleFile).exists && !testCase.skip)
runTestCase1(testCase, direction)
else
Success(())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object App {
def main(args: Array[String]): Unit = {
println(foo.Foo())
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-Xsource:3
-Wconf:cat=scala3-migration:s
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Code does not compile on 2.11 because unapply cannot be redefined
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Code does not compile on 2.11 because unapply cannot be redefined
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package foo

case class Foo private (a: String)

object Foo {
def apply() = new Foo("a")

private def unapply(f: Foo) = f
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package foo

case class Foo private (a: String, b: String) {}
object Foo {
def apply() = new Foo("a", "b")
def apply(a: String) = new Foo(a, "b")

private def unapply(f: Foo) = f
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object App {
def main(args: Array[String]): Unit = {
println(foo.Foo())
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package foo

class Foo private (val a: String)

object Foo {
def apply() = new Foo("a")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package foo

class Foo private (val a: String, val b: String) {}
object Foo {
def apply() = new Foo("a", "b")
def apply(a: String) = new Foo(a, "b")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object App {
def main(args: Array[String]): Unit = {
println(foo.Foo.Bar())
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package foo

object Foo {
class Bar private[foo](a: String) {}

object Bar {
def apply() = new Bar("a")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package foo

object Foo {
class Bar private[foo](a: Int, val b: String) {}

object Bar {
def apply() = new Bar(123, "b")
def apply(a: String) = new Bar(a.toInt, "b")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object App {
def main(args: Array[String]): Unit = {
println(foo.Foo())
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package foo

class Foo private[foo](a: String) {}

object Foo {
def apply() = new Foo("a")
}
Loading