From d6774a8bc7a0ee02b79a946f85f6dd344eb45b5f Mon Sep 17 00:00:00 2001 From: Jasper Moeys Date: Thu, 31 Jan 2019 17:45:23 +0100 Subject: [PATCH] Case class copy and apply inherit access modifiers from constructor Fixes scala/bug#7884 --- .../tools/nsc/typechecker/Unapplies.scala | 26 +++++++- .../scala/reflect/internal/TreeInfo.scala | 6 ++ .../neg/caseclass_private_constructor.check | 53 +++++++++++++++ .../neg/caseclass_private_constructor.scala | 59 +++++++++++++++++ .../pos/caseclass_private_constructor.scala | 64 +++++++++++++++++++ test/files/pos/t6734.scala | 2 +- test/files/pos/userdefined_apply.scala | 16 ++--- test/files/run/t9425.scala | 4 +- test/files/run/t9546e.scala | 4 +- 9 files changed, 218 insertions(+), 16 deletions(-) create mode 100644 test/files/neg/caseclass_private_constructor.check create mode 100644 test/files/neg/caseclass_private_constructor.scala create mode 100644 test/files/pos/caseclass_private_constructor.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala b/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala index 1729368ab244..0b2e05876086 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala @@ -97,6 +97,9 @@ trait Unapplies extends ast.TreeDSL { } + private def applyShouldInheritAccess(mods: Modifiers) = + currentRun.isScala214 && (mods.hasFlag(PRIVATE) || (!mods.hasFlag(PROTECTED) && mods.hasAccessBoundary)) + /** The module corresponding to a case class; overrides toString to show the module's name */ def caseModuleDef(cdef: ClassDef): ModuleDef = { @@ -104,7 +107,7 @@ trait Unapplies extends ast.TreeDSL { def inheritFromFun = !cdef.mods.hasAbstractFlag && cdef.tparams.isEmpty && (params match { case List(ps) if ps.length <= MaxFunctionArity => true case _ => false - }) + }) && !applyShouldInheritAccess(constrMods(cdef)) def createFun = { def primaries = params.head map (_.tpt) gen.scalaFunctionConstr(primaries, toIdent(cdef), abstractFun = true) @@ -142,9 +145,20 @@ trait Unapplies extends ast.TreeDSL { ) } + + private def constrMods(cdef: ClassDef): Modifiers = treeInfo.firstConstructorMods(cdef.impl.body) + /** The apply method corresponding to a case class */ - def caseModuleApplyMeth(cdef: ClassDef): DefDef = factoryMeth(caseMods, nme.apply, cdef) + def caseModuleApplyMeth(cdef: ClassDef): DefDef = { + val inheritedMods = constrMods(cdef) + val mods = + if (applyShouldInheritAccess(inheritedMods)) + (caseMods | (inheritedMods.flags & PRIVATE)).copy(privateWithin = inheritedMods.privateWithin) + else + caseMods + factoryMeth(mods, nme.apply, cdef) + } /** The unapply method corresponding to a case class */ @@ -257,8 +271,14 @@ trait Unapplies extends ast.TreeDSL { val classTpe = classType(cdef, tparams) val argss = mmap(paramss)(toIdent) val body: Tree = New(classTpe, argss) + val copyMods = + if (currentRun.isScala214) { + val inheritedMods = constrMods(cdef) + Modifiers(SYNTHETIC | (inheritedMods.flags & AccessFlags), inheritedMods.privateWithin) + } + else Modifiers(SYNTHETIC) val copyDefDef = atPos(cdef.pos.focus)( - DefDef(Modifiers(SYNTHETIC), nme.copy, tparams, paramss, TypeTree(), body) + DefDef(copyMods, nme.copy, tparams, paramss, TypeTree(), body) ) Some(copyDefDef) } diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala index 6757ec859f1d..18cb6ada4dad 100644 --- a/src/reflect/scala/reflect/internal/TreeInfo.scala +++ b/src/reflect/scala/reflect/internal/TreeInfo.scala @@ -531,6 +531,12 @@ abstract class TreeInfo { case _ => Nil } + /** The modifiers of the first constructor in `stats`. */ + def firstConstructorMods(stats: List[Tree]): Modifiers = firstConstructor(stats) match { + case DefDef(mods, _, _, _, _, _) => mods + case _ => Modifiers() + } + /** The value definitions marked PRESUPER in this statement sequence */ def preSuperFields(stats: List[Tree]): List[ValDef] = stats collect { case vd: ValDef if isEarlyValDef(vd) => vd } diff --git a/test/files/neg/caseclass_private_constructor.check b/test/files/neg/caseclass_private_constructor.check new file mode 100644 index 000000000000..c6d98dbc89f1 --- /dev/null +++ b/test/files/neg/caseclass_private_constructor.check @@ -0,0 +1,53 @@ +caseclass_private_constructor.scala:6: error: method apply in object A cannot be accessed in object A +error after rewriting to A. +possible cause: maybe a wrong Dynamic method signature? + def a1: A = A(1) // error: apply is private + ^ +caseclass_private_constructor.scala:7: error: method copy in class A cannot be accessed in A + def a2: A = a1.copy(2) // error: copy is private + ^ +caseclass_private_constructor.scala:12: error: method apply in object B cannot be accessed in object B +error after rewriting to B. +possible cause: maybe a wrong Dynamic method signature? + def b1: B = B(1) // error: apply is private + ^ +caseclass_private_constructor.scala:13: error: method copy in class B cannot be accessed in B + def b2: B = b1.copy(2) // error: copy is private + ^ +caseclass_private_constructor.scala:24: error: method apply in object C cannot be accessed in object qualified_private.C +error after rewriting to qualified_private.C. +possible cause: maybe a wrong Dynamic method signature? + def c1: C = C(1) // error: apply is private + ^ +caseclass_private_constructor.scala:25: error: method copy in class C cannot be accessed in qualified_private.C + def c2: C = c1.copy(2) // error: copy is private + ^ +caseclass_private_constructor.scala:27: error: method apply in object D cannot be accessed in object qualified_private.D +error after rewriting to qualified_private.D. +possible cause: maybe a wrong Dynamic method signature? + def d1: D = D(1) // error: apply is private + ^ +caseclass_private_constructor.scala:28: error: method copy in class D cannot be accessed in qualified_private.D + def d2: D = d1.copy(2) // error: copy is private + ^ +caseclass_private_constructor.scala:34: error: method copy in class E cannot be accessed in E + Access to protected method copy not permitted because + enclosing object ETest is not a subclass of + class E where target is defined + def e2: E = e2.copy(2) // error: copy is protected + ^ +caseclass_private_constructor.scala:43: error: method copy in class F cannot be accessed in qualified_protected.F + Access to protected method copy not permitted because + enclosing object QProtTest is not a subclass of + class F in object qualified_protected where target is defined + def f2: F = f2.copy(2) // error: copy is protected + ^ +caseclass_private_constructor.scala:57: error: method copy in class OverrideApply cannot be accessed in OverrideApply + def oa = OverrideApply(42).copy(24) // error: copy is still private + ^ +caseclass_private_constructor.scala:58: error: method apply in object OverrideCopy cannot be accessed in object OverrideCopy +error after rewriting to OverrideCopy. +possible cause: maybe a wrong Dynamic method signature? + def oc = OverrideCopy(42) // error: apply is still private + ^ +12 errors found diff --git a/test/files/neg/caseclass_private_constructor.scala b/test/files/neg/caseclass_private_constructor.scala new file mode 100644 index 000000000000..15f559c4e728 --- /dev/null +++ b/test/files/neg/caseclass_private_constructor.scala @@ -0,0 +1,59 @@ +// scalac: -Xsource:2.14 + +case class A private (i: Int) +object A +object ATest { + def a1: A = A(1) // error: apply is private + def a2: A = a1.copy(2) // error: copy is private +} + +case class B private (i: Int) // no user-defined companion object, should compile +object BTest { + def b1: B = B(1) // error: apply is private + def b2: B = b1.copy(2) // error: copy is private +} + +object qualified_private { + case class C private[qualified_private] (i: Int) + object C + + case class D private[qualified_private] (i: Int) // no user-defined companion object, should compile +} +object QPrivTest { + import qualified_private._ + def c1: C = C(1) // error: apply is private + def c2: C = c1.copy(2) // error: copy is private + + def d1: D = D(1) // error: apply is private + def d2: D = d1.copy(2) // error: copy is private +} + +case class E protected (i: Int) +object ETest { + def e1: E = E(1) + def e2: E = e2.copy(2) // error: copy is protected +} + +object qualified_protected { + case class F protected[qualified_protected] (i: Int) +} +object QProtTest { + import qualified_protected._ + def f1: F = F(1) + def f2: F = f2.copy(2) // error: copy is protected +} + + +case class OverrideApply private (i: Int) +object OverrideApply { + def apply(i: Int): OverrideApply = new OverrideApply(i) +} + +case class OverrideCopy private (i: Int) { + def copy(i: Int = i): OverrideCopy = OverrideCopy(i) +} + +object OverrideTest { + def oa = OverrideApply(42).copy(24) // error: copy is still private + def oc = OverrideCopy(42) // error: apply is still private +} diff --git a/test/files/pos/caseclass_private_constructor.scala b/test/files/pos/caseclass_private_constructor.scala new file mode 100644 index 000000000000..6a361f848b4f --- /dev/null +++ b/test/files/pos/caseclass_private_constructor.scala @@ -0,0 +1,64 @@ +// scalac: -Xsource:2.14 + +case class A private (i: Int) +object A { + def a = A(1).copy(2) // apply and copy are accessible in companion +} + +case class B private (i: Int) { // no user-defined companion object, should compile + def b = B(1).copy(2) // apply and copy are accessible +} + +object qualified_private { + case class A private[qualified_private] (i: Int) + object A { + def a = A(1).copy(2) // apply and copy are accessible in companion + } + + def a = A(1).copy(2) // apply and copy are accessible in qualified_private object + + case class B private[qualified_private] (i: Int) { // no user-defined companion object, should compile + def b = B(1).copy(2) // apply and copy are accessible + } + + def b = B(1).copy(2) // apply and copy are accessible in qualified_private object +} + +case class C protected (i: Int) +class CSub extends C(1) { + def c = copy(2) // copy is accessible in subclass +} +object CTest { + def c = C(1) // apply is public +} + +object qualified_protected { + case class C protected[qualified_protected] (i: Int) + class CSub extends C(1) { + def c = copy(2) // copy is accessible in subclass + } + object CTest { + def c = C(1) // apply is public + def checkExtendsFunction: Int => C = C // companion extends (Int => C) + } + + def c = C(1).copy(2) +} +object CQualifiedTest { + def c = qualified_protected.C(1) // apply is public +} + + +case class OverrideApply private (i: Int) +object OverrideApply { + def apply(i: Int): OverrideApply = new OverrideApply(i) +} + +case class OverrideCopy private (i: Int) { + def copy(i: Int = i): OverrideCopy = OverrideCopy(i) +} + +object OverrideTest { + def oa = OverrideApply(42) // overridden apply is public + def oc(o: OverrideCopy) = o.copy(42) // overridden copy is public +} diff --git a/test/files/pos/t6734.scala b/test/files/pos/t6734.scala index 88932cd2cc5e..8dbef2e62820 100644 --- a/test/files/pos/t6734.scala +++ b/test/files/pos/t6734.scala @@ -6,7 +6,7 @@ package object p package p { import scala.concurrent.Future - case class C private[p] (value: Future[Int]) // private to avoid rewriting C.apply to new C + case class C protected[p] (value: Future[Int]) // protected to avoid rewriting C.apply to new C } package client { diff --git a/test/files/pos/userdefined_apply.scala b/test/files/pos/userdefined_apply.scala index e29f9f514168..d1e335756a02 100644 --- a/test/files/pos/userdefined_apply.scala +++ b/test/files/pos/userdefined_apply.scala @@ -1,27 +1,27 @@ // NOTE: the companion inherits a public apply method from Function1! -case class NeedsCompanion private (x: Int) +case class NeedsCompanion protected (x: Int) object ClashNoSig { // ok private def apply(x: Int) = if (x > 0) new ClashNoSig(x) else ??? } -case class ClashNoSig private (x: Int) +case class ClashNoSig protected (x: Int) object Clash { private def apply(x: Int) = if (x > 0) new Clash(x) else ??? } -case class Clash private (x: Int) +case class Clash protected (x: Int) object ClashSig { private def apply(x: Int): ClashSig = if (x > 0) new ClashSig(x) else ??? } -case class ClashSig private (x: Int) +case class ClashSig protected (x: Int) object ClashOverload { private def apply(x: Int): ClashOverload = if (x > 0) new ClashOverload(x) else apply("") def apply(x: String): ClashOverload = ??? } -case class ClashOverload private (x: Int) +case class ClashOverload protected (x: Int) object NoClashSig { private def apply(x: Boolean): NoClashSig = if (x) NoClashSig(1) else ??? @@ -33,7 +33,7 @@ object NoClashOverload { private def apply(x: Boolean): NoClashOverload = if (x) NoClashOverload(1) else apply("") def apply(x: String): NoClashOverload = ??? } -case class NoClashOverload private (x: Int) +case class NoClashOverload protected (x: Int) @@ -43,7 +43,7 @@ class BaseNCP[T] { } object NoClashPoly extends BaseNCP[Boolean] -case class NoClashPoly private(x: Int) +case class NoClashPoly protected(x: Int) class BaseCP[T] { @@ -51,4 +51,4 @@ class BaseCP[T] { def apply(x: T): ClashPoly = if (???) ClashPoly(1) else ??? } object ClashPoly extends BaseCP[Int] -case class ClashPoly private(x: Int) +case class ClashPoly protected(x: Int) diff --git a/test/files/run/t9425.scala b/test/files/run/t9425.scala index f251cc85798a..fe9c0b5ae70b 100644 --- a/test/files/run/t9425.scala +++ b/test/files/run/t9425.scala @@ -1,8 +1,8 @@ -class C { case class Foo private (x: Int); Foo.apply(0) } +class C { case class Foo protected (x: Int); Foo.apply(0) } object Test { def test(c: C) = {import c.Foo; Foo.apply(0)} def main(args: Array[String]): Unit = { test(new C) - } + } } diff --git a/test/files/run/t9546e.scala b/test/files/run/t9546e.scala index b19d0871aa8b..2352b1551a87 100644 --- a/test/files/run/t9546e.scala +++ b/test/files/run/t9546e.scala @@ -1,5 +1,5 @@ -case class A private (x: Int) -case class B private (x: Int)(y: Int) +case class A protected (x: Int) +case class B protected (x: Int)(y: Int) class C { def f = A(1)