From cde5db30faba9f466239704abf4c50576ac703eb Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Fri, 12 Mar 2021 12:48:26 +0100 Subject: [PATCH] check private[this] members in override checking --- .../tools/nsc/transform/OverridingPairs.scala | 2 +- .../tools/partest/ScaladocModelTest.scala | 10 ++--- .../scala/reflect/internal/Symbols.scala | 6 +-- test/files/neg/t12349.check | 37 ++++++++++++++++--- test/files/neg/t12349/t12349b.scala | 6 +-- test/files/neg/t12349/t12349c.scala | 4 +- test/files/neg/t4762.check | 6 ++- test/files/neg/t9334.check | 6 +++ test/files/neg/t9334.scala | 6 +++ 9 files changed, 62 insertions(+), 21 deletions(-) create mode 100644 test/files/neg/t9334.check create mode 100644 test/files/neg/t9334.scala diff --git a/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala b/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala index 6387ddde49d7..b1930b201737 100644 --- a/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala +++ b/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala @@ -37,7 +37,7 @@ abstract class OverridingPairs extends SymbolPairs { * including bridges. But it may be refined in subclasses. */ override protected def exclude(sym: Symbol) = ( - sym.isPrivateLocal + (sym.isPrivateLocal && sym.isParamAccessor) || sym.isArtifact || sym.isConstructor || (sym.isPrivate && sym.owner != base) // Privates aren't inherited. Needed for pos/t7475a.scala diff --git a/src/partest/scala/tools/partest/ScaladocModelTest.scala b/src/partest/scala/tools/partest/ScaladocModelTest.scala index 487c962a298a..ec158f9cfd60 100644 --- a/src/partest/scala/tools/partest/ScaladocModelTest.scala +++ b/src/partest/scala/tools/partest/ScaladocModelTest.scala @@ -85,15 +85,15 @@ abstract class ScaladocModelTest extends DirectTest { System.setErr(prevErr) } - private[this] var settings: doc.Settings = null + private[this] var docSettings: doc.Settings = null // create a new scaladoc compiler def newDocFactory: DocFactory = { - settings = new doc.Settings(_ => ()) - settings.scaladocQuietRun = true // yaay, no more "model contains X documentable templates"! + docSettings = new doc.Settings(_ => ()) + docSettings.scaladocQuietRun = true // yaay, no more "model contains X documentable templates"! val args = extraSettings + " " + scaladocSettings - new ScalaDoc.Command((CommandLineParser tokenize (args)), settings) // side-effecting, I think - val docFact = new DocFactory(new ConsoleReporter(settings), settings) + new ScalaDoc.Command((CommandLineParser tokenize (args)), docSettings) // side-effecting, I think + val docFact = new DocFactory(new ConsoleReporter(docSettings), docSettings) docFact } diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index a144fe6e8c63..9a16166b1f69 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -3508,7 +3508,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => */ class ModuleClassSymbol protected[Symbols] (owner: Symbol, pos: Position, name: TypeName) extends ClassSymbol(owner, pos, name) { - private[this] var module: Symbol = _ + private[this] var moduleSymbol: Symbol = _ private[this] var typeOfThisCache: Type = _ private[this] var typeOfThisPeriod = NoPeriod @@ -3541,8 +3541,8 @@ trait Symbols extends api.Symbols { self: SymbolTable => implicitMembersCacheValue } // The null check seems to be necessary for the reifier. - override def sourceModule = if (module ne null) module else companionModule - override def sourceModule_=(module: Symbol): Unit = { this.module = module } + override def sourceModule = if (moduleSymbol ne null) moduleSymbol else companionModule + override def sourceModule_=(module: Symbol): Unit = { this.moduleSymbol = module } } class PackageObjectClassSymbol protected[Symbols] (owner0: Symbol, pos0: Position) diff --git a/test/files/neg/t12349.check b/test/files/neg/t12349.check index 2c7426ad6a95..ed6d1b26451d 100644 --- a/test/files/neg/t12349.check +++ b/test/files/neg/t12349.check @@ -33,6 +33,11 @@ def a8(): Unit (defined in class t12349a) override should be public protected[this] override def a8(): Unit = println("Inner12349b#a8()") // weaker access privileges ^ +t12349b.scala:14: error: weaker access privileges in overriding +def a9(): Unit (defined in class t12349a) + override should not be private + private[this] override def a9(): Unit = println("Inner12349b#a9()") // weaker access privileges + ^ t12349b.scala:18: error: weaker access privileges in overriding protected[package t12349] def b3(): Unit (defined in class t12349a) override should not be private @@ -48,6 +53,11 @@ protected[package t12349] def b7(): Unit (defined in class t12349a) override should at least be protected[t12349] private[t12349] override def b7(): Unit = println("Inner12349b#b7()") // weaker access privileges ^ +t12349b.scala:24: error: weaker access privileges in overriding +protected[package t12349] def b9(): Unit (defined in class t12349a) + override should not be private + private[this] override def b9(): Unit = println("Inner12349b#b9()") // weaker access privileges + ^ t12349b.scala:27: error: weaker access privileges in overriding private[package t12349] def c2(): Unit (defined in class t12349a) override should at least be private[t12349] @@ -73,6 +83,11 @@ private[package t12349] def c8(): Unit (defined in class t12349a) override should at least be private[t12349] protected[this] override def c8(): Unit = println("Inner12349b#c8()") // weaker access privileges ^ +t12349b.scala:34: error: weaker access privileges in overriding +private[package t12349] def c9(): Unit (defined in class t12349a) + override should not be private + private[this] override def c9(): Unit = println("Inner12349b#c9()") // weaker access privileges + ^ t12349b.scala:36: error: method d1 overrides nothing override def d1(): Unit = println("Inner12349b#d1()") // overrides nothing ^ @@ -135,6 +150,11 @@ def a8(): Unit (defined in class t12349a) override should be public protected[this] override def a8(): Unit = println("Inner12349c#a8()") // weaker access privileges ^ +t12349c.scala:18: error: weaker access privileges in overriding +def a9(): Unit (defined in class t12349a) + override should not be private + private[this] override def a9(): Unit = println("Inner12349c#a9()") // weaker access privileges + ^ t12349c.scala:22: error: weaker access privileges in overriding protected[package t12349] def b3(): Unit (defined in class t12349a) override should not be private @@ -150,6 +170,11 @@ protected[package t12349] def b7(): Unit (defined in class t12349a) override should at least be protected[t12349] private[pkg] override def b7(): Unit = println("Inner12349c#b7()") // weaker access privileges ^ +t12349c.scala:28: error: weaker access privileges in overriding +protected[package t12349] def b9(): Unit (defined in class t12349a) + override should not be private + private[this] override def b9(): Unit = println("Inner12349c#b9()") // weaker access privileges + ^ t12349c.scala:31: error: weaker access privileges in overriding private[package t12349] def c2(): Unit (defined in class t12349a) override should at least be private[t12349] @@ -185,14 +210,14 @@ private[package t12349] def c8(): Unit (defined in class t12349a) override should at least be private[t12349] protected[this] override def c8(): Unit = println("Inner12349c#c8()") // weaker access privileges ^ +t12349c.scala:38: error: weaker access privileges in overriding +private[package t12349] def c9(): Unit (defined in class t12349a) + override should not be private + private[this] override def c9(): Unit = println("Inner12349c#c9()") // overrides nothing (invisible) + ^ t12349c.scala:30: error: method c1 overrides nothing override def c1(): Unit = println("Inner12349c#c1()") // overrides nothing (invisible) ^ -t12349c.scala:38: error: method c9 overrides nothing. -Note: the super classes of class Inner12349c contain the following, non final members named c9: -private[package t12349] def c9(): Unit - private[this] override def c9(): Unit = println("Inner12349c#c9()") // overrides nothing (invisible) - ^ t12349c.scala:40: error: method d1 overrides nothing override def d1(): Unit = println("Inner12349c#d1()") // overrides nothing ^ @@ -220,4 +245,4 @@ t12349c.scala:47: error: method d8 overrides nothing t12349c.scala:48: error: method d9 overrides nothing private[this] override def d9(): Unit = println("Inner12349c#d9()") // overrides nothing ^ -52 errors +57 errors diff --git a/test/files/neg/t12349/t12349b.scala b/test/files/neg/t12349/t12349b.scala index 19079a3eb003..38b3309779b3 100644 --- a/test/files/neg/t12349/t12349b.scala +++ b/test/files/neg/t12349/t12349b.scala @@ -11,7 +11,7 @@ object t12349b { protected[t12349] override def a6(): Unit = println("Inner12349b#a6()") // weaker access privileges private[t12349] override def a7(): Unit = println("Inner12349b#a7()") // weaker access privileges protected[this] override def a8(): Unit = println("Inner12349b#a8()") // weaker access privileges - private[this] override def a9(): Unit = println("Inner12349b#a9()") // [#9334] + private[this] override def a9(): Unit = println("Inner12349b#a9()") // weaker access privileges override def b1(): Unit = println("Inner12349b#b1()") protected override def b2(): Unit = println("Inner12349b#b2()") @@ -21,7 +21,7 @@ object t12349b { protected[t12349] override def b6(): Unit = println("Inner12349b#b6()") private[t12349] override def b7(): Unit = println("Inner12349b#b7()") // weaker access privileges protected[this] override def b8(): Unit = println("Inner12349b#b8()") // [#12349] - not fixed by PR #9525 - private[this] override def b9(): Unit = println("Inner12349b#b9()") // [#9334] + private[this] override def b9(): Unit = println("Inner12349b#b9()") // weaker access privileges override def c1(): Unit = println("Inner12349b#c1()") protected override def c2(): Unit = println("Inner12349b#c2()") // weaker access privileges @@ -31,7 +31,7 @@ object t12349b { protected[t12349] override def c6(): Unit = println("Inner12349b#c6()") private[t12349] override def c7(): Unit = println("Inner12349b#c7()") protected[this] override def c8(): Unit = println("Inner12349b#c8()") // weaker access privileges - private[this] override def c9(): Unit = println("Inner12349b#c9()") // [#9334] + private[this] override def c9(): Unit = println("Inner12349b#c9()") // weaker access privileges override def d1(): Unit = println("Inner12349b#d1()") // overrides nothing protected override def d2(): Unit = println("Inner12349b#d2()") // overrides nothing diff --git a/test/files/neg/t12349/t12349c.scala b/test/files/neg/t12349/t12349c.scala index 3ad062d33472..942991a22430 100644 --- a/test/files/neg/t12349/t12349c.scala +++ b/test/files/neg/t12349/t12349c.scala @@ -15,7 +15,7 @@ package pkg { protected[pkg] override def a6(): Unit = println("Inner12349c#a6()") // weaker access privileges private[pkg] override def a7(): Unit = println("Inner12349c#a7()") // weaker access privileges protected[this] override def a8(): Unit = println("Inner12349c#a8()") // weaker access privileges - private[this] override def a9(): Unit = println("Inner12349c#a9()") // [#9334] + private[this] override def a9(): Unit = println("Inner12349c#a9()") // weaker access privileges override def b1(): Unit = println("Inner12349c#b1()") protected override def b2(): Unit = println("Inner12349c#b2()") @@ -25,7 +25,7 @@ package pkg { protected[pkg] override def b6(): Unit = println("Inner12349c#b6()") private[pkg] override def b7(): Unit = println("Inner12349c#b7()") // weaker access privileges protected[this] override def b8(): Unit = println("Inner12349c#b8()") // [#12349] - not fixed by PR #9525 - private[this] override def b9(): Unit = println("Inner12349c#b9()") // [#9334] + private[this] override def b9(): Unit = println("Inner12349c#b9()") // weaker access privileges override def c1(): Unit = println("Inner12349c#c1()") // overrides nothing (invisible) protected override def c2(): Unit = println("Inner12349c#c2()") // weaker access privileges diff --git a/test/files/neg/t4762.check b/test/files/neg/t4762.check index bd1c9ebff690..aa7bdcec39eb 100644 --- a/test/files/neg/t4762.check +++ b/test/files/neg/t4762.check @@ -4,6 +4,10 @@ t4762.scala:17: warning: private[this] value x in class B shadows mutable x inhe t4762.scala:50: warning: private[this] value x in class Derived shadows mutable x inherited from class Base. Changes to x will not be visible within class Derived - you may want to give them distinct names. class Derived( x : Int ) extends Base( x ) { override def toString = x.toString } ^ -error: No warnings can be incurred under -Werror. +t4762.scala:13: error: weaker access privileges in overriding +val y: Int (defined in class A) + override should not be private + private[this] def y: Int = 99 + ^ 2 warnings 1 error diff --git a/test/files/neg/t9334.check b/test/files/neg/t9334.check new file mode 100644 index 000000000000..e5fe6ef6d0ed --- /dev/null +++ b/test/files/neg/t9334.check @@ -0,0 +1,6 @@ +t9334.scala:5: error: weaker access privileges in overriding +def aaa: Int (defined in class A) + override should not be private + private[this] def aaa: Int = 42 + ^ +1 error diff --git a/test/files/neg/t9334.scala b/test/files/neg/t9334.scala new file mode 100644 index 000000000000..c8838e855db2 --- /dev/null +++ b/test/files/neg/t9334.scala @@ -0,0 +1,6 @@ +class A { + def aaa: Int = 10 +} +class B extends A { + private[this] def aaa: Int = 42 +}