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.

Sign up for GitHub

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 #3339: Disallow accesses to private package members from nested pkgs #3735

Merged
merged 2 commits into from
Jan 4, 2018
+31 −10
Merged

Fix #3339: Disallow accesses to private package members from nested pkgs #3735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cad41d2
Fix #3339: Disallow accesses to private package members from nested pkgs
odersky Jan 2, 2018
894d779
Improve access error messages + test case
odersky Jan 2, 2018
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
9 changes: 4 additions & 5 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -674,10 +674,7 @@ object SymDenotations {
final def isAccessibleFrom(pre: Type, superAccess: Boolean = false, whyNot: StringBuffer = null)(implicit ctx: Context): Boolean = {

/** Are we inside definition of `boundary`? */
def accessWithin(boundary: Symbol) =
ctx.owner.isContainedIn(boundary) &&
(!(this is JavaDefined) || // disregard package nesting for Java
ctx.owner.enclosingPackageClass == boundary.enclosingPackageClass)
def accessWithin(boundary: Symbol) = ctx.owner.isContainedIn(boundary)

/** Are we within definition of linked class of `boundary`? */
def accessWithinLinked(boundary: Symbol) = {
Expand Down Expand Up @@ -733,7 +730,9 @@ object SymDenotations {
( !(this is Local)
|| (owner is ImplClass) // allow private local accesses to impl class members
|| isCorrectThisType(pre)
)
) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This long chain of conditions is getting really hard to read, factor it out into a few vals?

Copy link
Contributor Author

@odersky odersky Jan 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried but it does not seem to help. The key is treating !A || B as an implication, then things get clearer. I was wondering whether we should add an inline helper method implies on Boolean to do this. But it's quite a lot of effort.

(!(this.is(Private) && owner.is(Package)) ||
owner == ctx.owner.enclosingPackageClass)
|| (this is Protected) &&
( superAccess
|| pre.isInstanceOf[ThisType]
Expand Down
21 changes: 16 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,19 +195,30 @@ trait TypeAssigner {
TryDynamicCallType
} else {
val alts = tpe.denot.alternatives.map(_.symbol).filter(_.exists)
var packageAccess = false
val what = alts match {
case Nil =>
name.toString
i"$name cannot be accessed as a member of $pre"
case sym :: Nil =>
if (sym.owner == pre.typeSymbol) sym.show else sym.showLocated
if (sym.owner.is(Package)) {
packageAccess = true
i"${sym.showLocated} cannot be accessed"
}
else {
val symStr = if (sym.owner == pre.typeSymbol) sym.show else sym.showLocated
i"$symStr cannot be accessed as a member of $pre"
}
case _ =>
em"none of the overloaded alternatives named $name"
em"none of the overloaded alternatives named $name can be accessed as members of $pre"
}
val where = if (ctx.owner.exists) s" from ${ctx.owner.enclosingClass}" else ""
val where =
if (!ctx.owner.exists) ""
else if (packageAccess) i" from nested ${ctx.owner.enclosingPackageClass}"
else i" from ${ctx.owner.enclosingClass}"
val whyNot = new StringBuffer
alts foreach (_.isAccessibleFrom(pre, superAccess, whyNot))
if (tpe.isError) tpe
else errorType(ex"$what cannot be accessed as a member of $pre$where.$whyNot", pos)
else errorType(ex"$what$where.$whyNot", pos)
}
}
else ctx.makePackageObjPrefixExplicit(tpe withDenot d)
Expand Down
11 changes: 11 additions & 0 deletions tests/neg/i3339.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package outer {
private class A

package inner {
object Test {
def main(args: Array[String]): Unit = {
println(new A) // error: cannot access
}
}
}
}