summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/compiler/scala/tools/nsc/symtab/Symbols.scala4
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/RefChecks.scala86
-rw-r--r--test/files/neg/java-access-neg.check16
-rw-r--r--test/files/neg/java-access-neg/J.java15
-rw-r--r--test/files/neg/java-access-neg/S2.scala61
-rw-r--r--test/files/pos/java-access-pos/J.java15
-rw-r--r--test/files/pos/java-access-pos/S1.scala67
-rw-r--r--test/pending/neg/t3633/test/PackageProtected.java5
8 files changed, 228 insertions, 41 deletions
diff --git a/src/compiler/scala/tools/nsc/symtab/Symbols.scala b/src/compiler/scala/tools/nsc/symtab/Symbols.scala
index 18c34eb036..7092b8272c 100644
--- a/src/compiler/scala/tools/nsc/symtab/Symbols.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Symbols.scala
@@ -670,10 +670,10 @@ trait Symbols extends reflect.generic.Symbols { self: SymbolTable =>
final def resetFlags { rawflags = rawflags & TopLevelCreationFlags }
/** The class or term up to which this symbol is accessible,
- * or RootClass if it is public
+ * or RootClass if it is public.
*/
def accessBoundary(base: Symbol): Symbol = {
- if (hasFlag(PRIVATE) || owner.isTerm) owner
+ if (hasFlag(PRIVATE) || isLocal) owner
else if (hasAccessBoundary && !phase.erasedTypes) privateWithin
else if (hasFlag(PROTECTED)) base
else RootClass
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
index 9fd1c69ef7..bc16d7c377 100644
--- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -254,13 +254,15 @@ abstract class RefChecks extends InfoTransform {
tp1 <:< tp2
}
- /** Check that all conditions for overriding <code>other</code> by
- * <code>member</code> of class <code>clazz</code> are met.
+ /** Check that all conditions for overriding `other` by `member`
+ * of class `clazz` are met.
*/
def checkOverride(clazz: Symbol, member: Symbol, other: Symbol) {
+ def noErrorType = other.tpe != ErrorType && member.tpe != ErrorType
+ def isRootOrNone(sym: Symbol) = sym == RootClass || sym == NoSymbol
def overrideError(msg: String) {
- if (other.tpe != ErrorType && member.tpe != ErrorType) {
+ if (noErrorType) {
val fullmsg =
"overriding "+infoStringWithLocation(other)+";\n "+
infoString(member)+" "+msg+
@@ -274,13 +276,13 @@ abstract class RefChecks extends InfoTransform {
}
def overrideTypeError() {
- if (other.tpe != ErrorType && member.tpe != ErrorType) {
+ if (noErrorType) {
overrideError("has incompatible type")
}
}
def accessFlagsToString(sym: Symbol)
- = flagsToString(sym getFlag (PRIVATE | PROTECTED), if (sym.privateWithin eq NoSymbol) "" else sym.privateWithin.name.toString)
+ = flagsToString(sym getFlag (PRIVATE | PROTECTED), if (!sym.hasAccessBoundary) "" else sym.privateWithin.name.toString)
def overrideAccessError() {
val otherAccess = accessFlagsToString(other)
@@ -319,23 +321,19 @@ abstract class RefChecks extends InfoTransform {
// ^-may be overridden by member with access privileges-v
// m: public | public/protected | public/protected/package-protected-in-same-package-as-o
- if (member hasFlag PRIVATE) // (1.1)
+ if (member.isPrivate) // (1.1)
overrideError("has weaker access privileges; it should not be private")
- @inline def definedIn(sym: Symbol, in: Symbol) = sym != RootClass && sym != NoSymbol && sym.hasTransOwner(in)
-
+ // todo: align accessibility implication checking with isAccessible in Contexts
val ob = other.accessBoundary(member.owner)
val mb = member.accessBoundary(member.owner)
- // println("checking override in "+ clazz +"\n other: "+ infoString(other) +" ab: "+ ob.ownerChain +" flags: "+ accessFlagsToString(other))
- // println(" overriding member: "+ infoString(member) +" ab: "+ mb.ownerChain +" flags: "+ accessFlagsToString(member))
- // todo: align accessibility implication checking with isAccessible in Contexts
- if (!( mb == RootClass // m is public, definitely relaxes o's access restrictions (unless o.isJavaDefined, see below)
- || mb == NoSymbol // AM: what does this check?? accessBoundary does not ever seem to return NoSymbol (unless member.owner were NoSymbol)
- || ((!(other hasFlag PROTECTED) || (member hasFlag PROTECTED)) && definedIn(ob, mb)) // (if o isProtected, so is m) and m relaxes o's access boundary
- )) {
- overrideAccessError()
+ def isOverrideAccessOK = member.isPublic || { // member is public, definitely same or relaxed access
+ (!other.isProtected || member.isProtected) && // if o is protected, so is m
+ (!isRootOrNone(ob) && ob.hasTransOwner(mb)) // m relaxes o's access boundary
}
- else if (other.isClass || other.isModule) {
+ if (!isOverrideAccessOK) {
+ overrideAccessError()
+ } else if (other.isClass || other.isModule) {
overrideError("cannot be used here - classes and objects cannot be overridden");
} else if (!other.isDeferred && (member.isClass || member.isModule)) {
overrideError("cannot be used here - classes and objects can only override abstract types");
@@ -555,24 +553,44 @@ abstract class RefChecks extends InfoTransform {
* (which must be different from `clazz`) whose name and type
* seen as a member of `class.thisType` matches `member`'s.
*/
- def hasMatchingSym(inclazz: Symbol, member: Symbol): Boolean =
- inclazz != clazz && {
- lazy val memberEnclPackageCls = member.enclosingPackageClass
-
- val isVarargs = hasRepeatedParam(member.tpe)
- inclazz.info.nonPrivateDecl(member.name).filter { sym =>
- (!sym.isTerm || {
- val symtpe = clazz.thisType.memberType(sym)
- (member.tpe matches symtpe) || isVarargs && (toJavaRepeatedParam(member.tpe) matches symtpe)
- }) && {
- // http://java.sun.com/docs/books/jls/third_edition/html/names.html#6.6.5:
- // If a public class has a [member] with default access, then this [member] is not accessible to,
- // or inherited by a subclass declared outside this package.
- // (sym is a java member with default access in pkg P) implies (member's enclosing package == P)
- !(inclazz.isJavaDefined && sym.privateWithin == sym.enclosingPackageClass) || memberEnclPackageCls == sym.privateWithin
- }
- } != NoSymbol
+ def hasMatchingSym(inclazz: Symbol, member: Symbol): Boolean = {
+ val isVarargs = hasRepeatedParam(member.tpe)
+ lazy val varargsType = toJavaRepeatedParam(member.tpe)
+
+ def isSignatureMatch(sym: Symbol) = !sym.isTerm || {
+ val symtpe = clazz.thisType memberType sym
+ def matches(tp: Type) = tp matches symtpe
+
+ matches(member.tpe) || (isVarargs && matches(varargsType))
}
+ /** The rules for accessing members which have an access boundary are more
+ * restrictive in java than scala. Since java has no concept of package nesting,
+ * a member with "default" (package-level) access can only be accessed by members
+ * in the exact same package. Example:
+ *
+ * package a.b;
+ * public class JavaClass { void foo() { } }
+ *
+ * The member foo() can be accessed only from members of package a.b, and not
+ * nested packages like a.b.c. In the analogous scala class:
+ *
+ * package a.b
+ * class ScalaClass { private[b] def foo() = () }
+ *
+ * The member IS accessible to classes in package a.b.c. The javaAccessCheck logic
+ * is restricting the set of matching signatures according to the above semantics.
+ */
+ def javaAccessCheck(sym: Symbol) = (
+ !inclazz.isJavaDefined // not a java defined member
+ || !sym.hasAccessBoundary // no access boundary
+ || sym.isProtected // marked protected in java, thus accessible to subclasses
+ || sym.privateWithin == member.enclosingPackageClass // exact package match
+ )
+ def classDecls = inclazz.info.nonPrivateDecl(member.name)
+ def matchingSyms = classDecls filter (sym => isSignatureMatch(sym) && javaAccessCheck(sym))
+
+ (inclazz != clazz) && (matchingSyms != NoSymbol)
+ }
// 4. Check that every defined member with an `override' modifier overrides some other member.
for (member <- clazz.info.decls.toList)
diff --git a/test/files/neg/java-access-neg.check b/test/files/neg/java-access-neg.check
new file mode 100644
index 0000000000..af2812b579
--- /dev/null
+++ b/test/files/neg/java-access-neg.check
@@ -0,0 +1,16 @@
+S2.scala:12: error: method packageAbstract overrides nothing
+ override private[b] def packageAbstract() = () // fail
+ ^
+S2.scala:16: error: method packageConcrete overrides nothing
+ override private[b] def packageConcrete() = () // fail
+ ^
+S2.scala:36: error: method packageConcrete overrides nothing
+ override protected[b] def packageConcrete() = () // fail
+ ^
+S2.scala:47: error: method packageConcrete overrides nothing
+ override private[a] def packageConcrete() = () // fail
+ ^
+S2.scala:58: error: method packageConcrete overrides nothing
+ override def packageConcrete() = () // fail
+ ^
+5 errors found
diff --git a/test/files/neg/java-access-neg/J.java b/test/files/neg/java-access-neg/J.java
new file mode 100644
index 0000000000..b6bc3363a1
--- /dev/null
+++ b/test/files/neg/java-access-neg/J.java
@@ -0,0 +1,15 @@
+package a.b;
+
+public abstract class J {
+ public J() { }
+ J(int x1) { }
+ protected J(int x1, int x2) { }
+
+ abstract void packageAbstract();
+ protected abstract void protectedAbstract();
+ public abstract void publicAbstract();
+
+ void packageConcrete() { return; }
+ protected void protectedConcrete() { return; }
+ public void publicConcrete() { return; }
+}
diff --git a/test/files/neg/java-access-neg/S2.scala b/test/files/neg/java-access-neg/S2.scala
new file mode 100644
index 0000000000..b082bb7174
--- /dev/null
+++ b/test/files/neg/java-access-neg/S2.scala
@@ -0,0 +1,61 @@
+package a.b
+package c
+
+import a.b.J
+
+/** Variations of java-access-pos with us in a nested package.
+ */
+
+/** Declaring "override" all the time.
+ */
+class S1 extends J {
+ override private[b] def packageAbstract() = () // fail
+ override protected[b] def protectedAbstract() = ()
+ override def publicAbstract() = ()
+
+ override private[b] def packageConcrete() = () // fail
+ override protected[b] def protectedConcrete() = ()
+ override def publicConcrete() = ()
+}
+
+/** Implementing abstracts.
+ */
+class S2 extends J {
+ private[b] def packageAbstract() = () // fail
+ protected[b] def protectedAbstract() = ()
+ def publicAbstract() = ()
+}
+
+/** Widening access.
+ */
+class S3 extends J {
+ protected[b] def packageAbstract() = () // fail
+ protected[b] def protectedAbstract() = ()
+ def publicAbstract() = ()
+
+ override protected[b] def packageConcrete() = () // fail
+ override protected[b] def protectedConcrete() = ()
+ override def publicConcrete() = ()
+}
+/** More widening.
+ */
+class S4 extends J {
+ private[a] def packageAbstract() = () // fail
+ protected[a] def protectedAbstract() = ()
+ def publicAbstract() = ()
+
+ override private[a] def packageConcrete() = () // fail
+ override protected[a] def protectedConcrete() = ()
+ override def publicConcrete() = ()
+}
+/** Yet more widening.
+ */
+class S5 extends J {
+ def packageAbstract() = () // fail
+ def protectedAbstract() = ()
+ def publicAbstract() = ()
+
+ override def packageConcrete() = () // fail
+ override def protectedConcrete() = ()
+ override def publicConcrete() = ()
+}
diff --git a/test/files/pos/java-access-pos/J.java b/test/files/pos/java-access-pos/J.java
new file mode 100644
index 0000000000..b6bc3363a1
--- /dev/null
+++ b/test/files/pos/java-access-pos/J.java
@@ -0,0 +1,15 @@
+package a.b;
+
+public abstract class J {
+ public J() { }
+ J(int x1) { }
+ protected J(int x1, int x2) { }
+
+ abstract void packageAbstract();
+ protected abstract void protectedAbstract();
+ public abstract void publicAbstract();
+
+ void packageConcrete() { return; }
+ protected void protectedConcrete() { return; }
+ public void publicConcrete() { return; }
+}
diff --git a/test/files/pos/java-access-pos/S1.scala b/test/files/pos/java-access-pos/S1.scala
new file mode 100644
index 0000000000..10730e3a70
--- /dev/null
+++ b/test/files/pos/java-access-pos/S1.scala
@@ -0,0 +1,67 @@
+package a.b
+
+/** Declaring "override" all the time.
+ */
+class S1 extends J {
+ override private[b] def packageAbstract() = ()
+ override protected[b] def protectedAbstract() = ()
+ override def publicAbstract() = ()
+
+ override private[b] def packageConcrete() = ()
+ override protected[b] def protectedConcrete() = ()
+ override def publicConcrete() = ()
+}
+
+/** Implementing abstracts.
+ */
+class S2 extends J {
+ private[b] def packageAbstract() = ()
+ protected[b] def protectedAbstract() = ()
+ def publicAbstract() = ()
+}
+
+/** Widening access.
+ */
+class S3 extends J {
+ protected[b] def packageAbstract() = ()
+ protected[b] def protectedAbstract() = ()
+ def publicAbstract() = ()
+
+ override protected[b] def packageConcrete() = ()
+ override protected[b] def protectedConcrete() = ()
+ override def publicConcrete() = ()
+}
+/** More widening.
+ */
+class S4 extends J {
+ private[a] def packageAbstract() = ()
+ protected[a] def protectedAbstract() = ()
+ def publicAbstract() = ()
+
+ override private[a] def packageConcrete() = ()
+ override protected[a] def protectedConcrete() = ()
+ override def publicConcrete() = ()
+}
+/** Yet more widening.
+ */
+class S5 extends J {
+ def packageAbstract() = ()
+ def protectedAbstract() = ()
+ def publicAbstract() = ()
+
+ override def packageConcrete() = ()
+ override def protectedConcrete() = ()
+ override def publicConcrete() = ()
+}
+/** Constructors.
+ */
+class S6 extends J(1) {
+ def packageAbstract() = ()
+ def protectedAbstract() = ()
+ def publicAbstract() = ()
+}
+class S7 extends J(1, 2) {
+ def packageAbstract() = ()
+ def protectedAbstract() = ()
+ def publicAbstract() = ()
+} \ No newline at end of file
diff --git a/test/pending/neg/t3633/test/PackageProtected.java b/test/pending/neg/t3633/test/PackageProtected.java
deleted file mode 100644
index f4535a55b4..0000000000
--- a/test/pending/neg/t3633/test/PackageProtected.java
+++ /dev/null
@@ -1,5 +0,0 @@
-package test;
-
-class PackageProtected {
- int foo;
-}
generated by cgit v1.2.3 (git 2.39.1) at 2024-09-04 07:53:28 +0000