Skip to content

Commit

Permalink
SI-8085 Fix BrowserTraverser for package objects
Browse files Browse the repository at this point in the history
A source file like:

    import foo.bar
    package object baz

Is parsed into:

    package <empty> {
       import foo.bar
       package baz {
          object `package`
       }
    }

A special case in Namers compensates by adjusting the owner of
`baz` to be `<root>`, rather than `<empty>`.

This wasn't being accounted for in `BrowserTraverser`, which
underpins `-sourcepath`, and allows the presentation compiler to
load top level symbols from sources outside those passes as
the list of sources to compile.

This bug did not appear in sources like:

    package p1
    package object p2 { ... }

... because the parser does not wrap this in the `package <empty> {}`

This goes some way to explaining why it has gone unnoticed for
so long.
  • Loading branch information
retronym committed Dec 18, 2013
1 parent a12dd9c commit 7e85b59
Show file tree
Hide file tree
Showing 11 changed files with 66 additions and 13 deletions.
6 changes: 4 additions & 2 deletions src/compiler/scala/tools/nsc/symtab/BrowsingLoaders.scala
Expand Up @@ -64,8 +64,10 @@ abstract class BrowsingLoaders extends SymbolLoaders {
addPackagePrefix(pre)
packagePrefix += ("." + name)
case Ident(name) =>
if (packagePrefix.length != 0) packagePrefix += "."
packagePrefix += name
if (name != nme.EMPTY_PACKAGE_NAME) { // mirrors logic in Namers, see createPackageSymbol
if (packagePrefix.length != 0) packagePrefix += "."
packagePrefix += name
}
case _ =>
throw new MalformedInput(pkg.pos.point, "illegal tree node in package prefix: "+pkg)
}
Expand Down
4 changes: 2 additions & 2 deletions test/files/presentation/t8085.check
@@ -1,3 +1,3 @@
reload: NodeScalaSuite.scala
prefixes differ: <empty>.nodescala,nodescala
value always is not a member of object scala.concurrent.Future
open package module: package nodescala
Test OK
1 change: 0 additions & 1 deletion test/files/presentation/t8085/Test.scala
Expand Up @@ -5,7 +5,6 @@ import scala.tools.nsc.interactive.Response
object Test extends InteractiveTest {

override def execute(): Unit = {
// loadSourceAndWaitUntilTypechecked("package.scala") // <-- uncomment this line and the test succeed
val src = loadSourceAndWaitUntilTypechecked("NodeScalaSuite.scala")
checkErrors(src)
}
Expand Down
@@ -1,12 +1,10 @@
package nodescala

import scala.concurrent.Future

class NodeScalaSuite {
Future.always(517)
"".rich

// This is here only to prove that the presentation compiler is instantiated with the
// correct `sourcepath` value (if it wasn't, you would see a `not found: type Foo` in
// the test's output
println(new Foo())
}
}
7 changes: 3 additions & 4 deletions test/files/presentation/t8085/src/nodescala/package.scala
@@ -1,8 +1,7 @@
import scala.concurrent.Future // <-- if you move the import *inside* the package object, then it all works fine!!
import scala.Some // <-- if you move the import *inside* the package object, then it all works fine!!

package object nodescala {
implicit class FutureCompanionOps[T](val f: Future.type) extends AnyVal {
def always[T](value: T): Future[T] = Promise[T].success(value).future
implicit class StringOps(val f: String) {
def rich = 0
}
}

3 changes: 3 additions & 0 deletions test/files/presentation/t8085b.check
@@ -0,0 +1,3 @@
reload: NodeScalaSuite.scala
open package module: package nodescala
Test OK
1 change: 1 addition & 0 deletions test/files/presentation/t8085b.flags
@@ -0,0 +1 @@
-sourcepath src
27 changes: 27 additions & 0 deletions test/files/presentation/t8085b/Test.scala
@@ -0,0 +1,27 @@
import scala.tools.nsc.interactive.tests.InteractiveTest
import scala.reflect.internal.util.SourceFile
import scala.tools.nsc.interactive.Response

object Test extends InteractiveTest {

override def execute(): Unit = {
val src = loadSourceAndWaitUntilTypechecked("NodeScalaSuite.scala")
checkErrors(src)
}

private def loadSourceAndWaitUntilTypechecked(sourceName: String): SourceFile = {
val sourceFile = sourceFiles.find(_.file.name == sourceName).head
askReload(List(sourceFile)).get
askLoadedTyped(sourceFile).get
sourceFile
}

private def checkErrors(source: SourceFile): Unit = compiler.getUnitOf(source) match {
case Some(unit) =>
val problems = unit.problems.toList
if(problems.isEmpty) reporter.println("Test OK")
else problems.foreach(problem => reporter.println(problem.msg))

case None => reporter.println("No compilation unit found for " + source.file.name)
}
}
4 changes: 4 additions & 0 deletions test/files/presentation/t8085b/src/p1/nodescala/Foo.scala
@@ -0,0 +1,4 @@
package p1
package nodescala

class Foo
@@ -0,0 +1,11 @@
package p1
package nodescala

class NodeScalaSuite {
"".rich

// This is here only to prove that the presentation compiler is instantiated with the
// correct `sourcepath` value (if it wasn't, you would see a `not found: type Foo` in
// the test's output
println(new Foo())
}
9 changes: 9 additions & 0 deletions test/files/presentation/t8085b/src/p1/nodescala/package.scala
@@ -0,0 +1,9 @@
import scala.Some // <-- if you move the import *inside* the package object, then it all works fine!!

package p1 {
package object nodescala {
implicit class StringOps(val f: String) {
def rich = 0
}
}
}

0 comments on commit 7e85b59

Please sign in to comment.