Merge pull request #1607 from retronym/ticket/6640

SI-6640 Better reporting of deficient classpaths.

src/compiler/scala/tools/nsc/reporters/Reporter.scala

@@ -20,9 +20,15 @@ abstract class Reporter {
   class Severity(val id: Int) extends severity.Value {
     var count: Int = 0
   }
-  val INFO    = new Severity(0)
-  val WARNING = new Severity(1)
-  val ERROR   = new Severity(2)
+  val INFO    = new Severity(0) {
+    override def toString: String = "INFO"
+  }
+  val WARNING = new Severity(1) {
+    override def toString: String = "WARNING"
+  }
+  val ERROR   = new Severity(2) {
+    override def toString: String = "ERROR"
+  }
 
   /** Whether very long lines can be truncated.  This exists so important
    *  debugging information (like printing the classpath) is not rendered

src/partest/scala/tools/partest/DirectTest.scala

@@ -8,6 +8,7 @@ package scala.tools.partest
 import scala.tools.nsc._
 import io.Directory
 import util.{BatchSourceFile, CommandLineParser}
+import reporters.{Reporter, ConsoleReporter}
 
 /** A class for testing code which is embedded as a string.
  *  It allows for more complete control over settings, compiler
@@ -38,9 +39,12 @@ abstract class DirectTest extends App {
   // new compiler
   def newCompiler(args: String*): Global = {
     val settings = newSettings((CommandLineParser tokenize ("-d \"" + testOutput.path + "\" " + extraSettings)) ++ args.toList)
-    if (settings.Yrangepos.value) new Global(settings) with interactive.RangePositions
-    else new Global(settings)
+    if (settings.Yrangepos.value) new Global(settings, reporter(settings)) with interactive.RangePositions
+    else new Global(settings, reporter(settings))
   }
+
+  def reporter(settings: Settings): Reporter = new ConsoleReporter(settings)
+
   def newSources(sourceCodes: String*) = sourceCodes.toList.zipWithIndex map {
     case (src, idx) => new BatchSourceFile("newSource" + (idx + 1), src)
   }

src/partest/scala/tools/partest/StoreReporterDirectTest.scala

@@ -0,0 +1,15 @@
+package scala.tools.partest
+
+import scala.tools.nsc.Settings
+import scala.tools.nsc.reporters.StoreReporter
+import scala.collection.mutable
+
+trait StoreReporterDirectTest extends DirectTest {
+  lazy val storeReporter: StoreReporter = new scala.tools.nsc.reporters.StoreReporter()
+
+  /** Discards all but the first message issued at a given position. */
+  def filteredInfos: Seq[storeReporter.Info] = storeReporter.infos.groupBy(_.pos).map(_._2.head).toList
+
+  /** Hook into [[scala.tools.partest.DirectTest]] to install the custom reporter */
+  override def reporter(settings: Settings) = storeReporter
+}

src/reflect/scala/reflect/internal/Symbols.scala

@@ -423,9 +423,9 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
      *  failure to the point when that name is used for something, which is
      *  often to the point of never.
      */
-    def newStubSymbol(name: Name): Symbol = name match {
-      case n: TypeName  => new StubClassSymbol(this, n)
-      case _            => new StubTermSymbol(this, name.toTermName)
+    def newStubSymbol(name: Name, missingMessage: String): Symbol = name match {
+      case n: TypeName  => new StubClassSymbol(this, n, missingMessage)
+      case _            => new StubTermSymbol(this, name.toTermName, missingMessage)
     }
 
     @deprecated("Use the other signature", "2.10.0")
@@ -1344,6 +1344,9 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
       }
     }
 
+    /** Raises a `MissingRequirementError` if this symbol is a `StubSymbol` */
+    def failIfStub() {}
+
     /** Initialize the symbol */
     final def initialize: this.type = {
       if (!isInitialized) info
@@ -3100,14 +3103,18 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     )
   }
   trait StubSymbol extends Symbol {
-    protected def stubWarning = {
-      val from = if (associatedFile == null) "" else s" - referenced from ${associatedFile.canonicalPath}"
-      s"$kindString $nameString$locationString$from (a classfile may be missing)"
-    }
+    protected def missingMessage: String
+
+    /** Fail the stub by throwing a [[scala.reflect.internal.MissingRequirementError]]. */
+    override final def failIfStub() = {MissingRequirementError.signal(missingMessage)} //
+
+    /** Fail the stub by reporting an error to the reporter, setting the IS_ERROR flag
+      * on this symbol, and returning the dummy value `alt`.
+      */
     private def fail[T](alt: T): T = {
       // Avoid issuing lots of redundant errors
       if (!hasFlag(IS_ERROR)) {
-        globalError(s"bad symbolic reference to " + stubWarning)
+        globalError(missingMessage)
         if (settings.debug.value)
           (new Throwable).printStackTrace
 
@@ -3124,12 +3131,10 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     override def rawInfo         = fail(NoType)
     override def companionSymbol = fail(NoSymbol)
 
-    locally {
-      debugwarn("creating stub symbol for " + stubWarning)
-    }
+    debugwarn("creating stub symbol to defer error: " + missingMessage)
   }
-  class StubClassSymbol(owner0: Symbol, name0: TypeName) extends ClassSymbol(owner0, owner0.pos, name0) with StubSymbol
-  class StubTermSymbol(owner0: Symbol, name0: TermName) extends TermSymbol(owner0, owner0.pos, name0) with StubSymbol
+  class StubClassSymbol(owner0: Symbol, name0: TypeName, protected val missingMessage: String) extends ClassSymbol(owner0, owner0.pos, name0) with StubSymbol
+  class StubTermSymbol(owner0: Symbol, name0: TermName, protected val missingMessage: String) extends TermSymbol(owner0, owner0.pos, name0) with StubSymbol
 
   trait FreeSymbol extends Symbol {
     def origin: String

src/reflect/scala/reflect/internal/Types.scala

@@ -1385,7 +1385,12 @@ trait Types extends api.Types { self: SymbolTable =>
   /** A class for this-types of the form <sym>.this.type
    */
   abstract case class ThisType(sym: Symbol) extends SingletonType with ThisTypeApi {
-    assert(sym.isClass, sym)
+    if (!sym.isClass) {
+      // SI-6640 allow StubSymbols to reveal what's missing from the classpath before we trip the assertion.
+      sym.failIfStub()
+      assert(false, sym)
+    }
+
     //assert(sym.isClass && !sym.isModuleClass || sym.isRoot, sym)
     override def isTrivial: Boolean = sym.isPackageClass
     override def isNotNull = true

src/reflect/scala/reflect/internal/pickling/UnPickler.scala

@@ -20,7 +20,7 @@ import scala.annotation.switch
 /** @author Martin Odersky
  *  @version 1.0
  */
-abstract class UnPickler /*extends scala.reflect.generic.UnPickler*/ {
+abstract class UnPickler {
   val global: SymbolTable
   import global._
 
@@ -233,7 +233,12 @@ abstract class UnPickler /*extends scala.reflect.generic.UnPickler*/ {
               // (4) Call the mirror's "missing" hook.
               adjust(mirrorThatLoaded(owner).missingHook(owner, name)) orElse {
                 // (5) Create a stub symbol to defer hard failure a little longer.
-                owner.newStubSymbol(name)
+                val missingMessage =
+                  s"""|bad symbolic reference. A signature in $filename refers to ${name.longString}
+                      |in ${owner.kindString} ${owner.fullName} which is not available.
+                      |It may be completely missing from the current classpath, or the version on
+                      |the classpath might be incompatible with the version used when compiling $filename.""".stripMargin
+                owner.newStubSymbol(name, missingMessage)
               }
             }
           }
@@ -827,11 +832,6 @@ abstract class UnPickler /*extends scala.reflect.generic.UnPickler*/ {
     protected def errorBadSignature(msg: String) =
       throw new RuntimeException("malformed Scala signature of " + classRoot.name + " at " + readIndex + "; " + msg)
 
-    protected def errorMissingRequirement(name: Name, owner: Symbol): Symbol =
-      mirrorThatLoaded(owner).missingHook(owner, name) orElse MissingRequirementError.signal(
-        s"bad reference while unpickling $filename: ${name.longString} not found in ${owner.tpe.widen}"
-      )
-
     def inferMethodAlternative(fun: Tree, argtpes: List[Type], restpe: Type) {} // can't do it; need a compiler for that.
 
     def newLazyTypeRef(i: Int): LazyType = new LazyTypeRef(i)

test/files/neg/t5148.check

@@ -1,3 +1,9 @@
-error: bad symbolic reference to value global in class IMain - referenced from t5148.scala (a classfile may be missing)
-error: bad symbolic reference to value memberHandlers in class IMain - referenced from t5148.scala (a classfile may be missing)
+error: bad symbolic reference. A signature in Imports.class refers to term global
+in class scala.tools.nsc.interpreter.IMain which is not available.
+It may be completely missing from the current classpath, or the version on
+the classpath might be incompatible with the version used when compiling Imports.class.
+error: bad symbolic reference. A signature in Imports.class refers to term memberHandlers
+in class scala.tools.nsc.interpreter.IMain which is not available.
+It may be completely missing from the current classpath, or the version on
+the classpath might be incompatible with the version used when compiling Imports.class.
 two errors found

test/files/run/t6440.check

@@ -1 +1,4 @@
-Stream((), ?)
+pos: source-newSource1,line-9,offset=109 bad symbolic reference. A signature in U.class refers to term pack1
+in package <root> which is not available.
+It may be completely missing from the current classpath, or the version on
+the classpath might be incompatible with the version used when compiling U.class. ERROR

test/files/run/t6440.scala

@@ -1,7 +1,48 @@
-object Test {
+import scala.tools.partest._
+import java.io.File
 
-  def main(args: Array[String]): Unit = {
-    println(Stream.continually(()).filterNot(_ => false).take(2))
+object Test extends StoreReporterDirectTest {
+  def code = ???
+
+  def compileCode(code: String) = {
+    val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator")
+    compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(code)
   }
 
+  def library1 = """
+    package pack1
+    trait T
+  """
+
+  def library2 = """
+    package pack2
+    trait U extends pack1.T
+  """
+
+  def app = """
+    package pack3
+    object X {
+      trait U
+    }
+    import X._
+    import pack2._
+
+    trait V extends U
+  """
+
+  def show(): Unit = {
+    Seq(library1, library2) foreach compileCode
+    assert(filteredInfos.isEmpty, filteredInfos)
+
+    // blow away the entire package
+    val pack1 = new File(testOutput.path, "pack1")
+    val tClass = new File(pack1, "T.class")
+    assert(tClass.exists)
+    assert(tClass.delete())
+    assert(pack1.delete())
+
+    // bad symbolic reference error expected (but no stack trace!)
+    compileCode(app)
+    println(filteredInfos.mkString("\n"))
+  }
 }

test/files/run/t6440b.check

@@ -0,0 +1,4 @@
+pos: NoPosition bad symbolic reference. A signature in U.class refers to type T
+in package pack1 which is not available.
+It may be completely missing from the current classpath, or the version on
+the classpath might be incompatible with the version used when compiling U.class. ERROR

test/files/run/t6440b.scala

@@ -0,0 +1,61 @@
+import scala.tools.partest._
+import java.io.File
+
+object Test extends StoreReporterDirectTest {
+  def code = ???
+
+  def compileCode(code: String) = {
+    val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator")
+    compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(code)
+  }
+
+  def library1 = """
+    package pack1
+    trait T
+    class U {
+      def t = new T {}
+      def one = 1
+    }
+  """
+
+  def library2 = """
+    package pack2
+    object V {
+      def u = new pack1.U
+    }
+    """
+
+  def app1 = """
+    package pack3
+    object Test {
+      pack2.V.u.one // okay
+    }
+  """
+
+  def app2 = """
+    package pack3
+    object Test {
+      pack2.V.u.t // we have to fail if T.class is misisng
+    }
+  """
+
+  def show(): Unit = {
+    compileCode(library1)
+    val pack1 = new File(testOutput.path, "pack1")
+    val tClass = new File(pack1, "T.class")
+    assert(tClass.exists)
+    assert(tClass.delete())
+
+    // allowed to compile, no direct reference to `T`
+    compileCode(library2)
+    assert(filteredInfos.isEmpty, filteredInfos)
+
+    // allowed to compile, no direct reference to `T`
+    compileCode(app1)
+    assert(filteredInfos.isEmpty, filteredInfos)
+
+    // bad symbolic reference error expected (but no stack trace!)
+    compileCode(app2)
+    println(filteredInfos.mkString("\n"))
+  }
+}

test/files/run/typetags_without_scala_reflect_typetag_lookup.check

@@ -1,3 +1,2 @@
-newSource1:9: error: could not find implicit value for evidence parameter of type reflect.runtime.package.universe.TypeTag[Int]
-      Library.foo[Int]
-                 ^
+
+pos: source-newSource1,line-9,offset=466 could not find implicit value for evidence parameter of type reflect.runtime.package.universe.TypeTag[Int] ERROR

test/files/run/typetags_without_scala_reflect_typetag_lookup.scala

@@ -1,6 +1,6 @@
 import scala.tools.partest._
 
-object Test extends DirectTest {
+object Test extends StoreReporterDirectTest {
   def code = ???
 
   def library = """
@@ -32,14 +32,12 @@ object Test extends DirectTest {
   }
 
   def show(): Unit = {
-    val prevErr = System.err
-    val baos = new java.io.ByteArrayOutputStream();
-    System.setErr(new java.io.PrintStream(baos));
     compileLibrary();
+    println(filteredInfos.mkString("\n"))
+    storeReporter.infos.clear()
     compileApp();
-    // we should get bad symbolic reference errors, because we're trying to call a method that can't be unpickled
+    // we should get bad symbolic reference errors, because we're trying to use an implicit that can't be unpickled
     // but we don't know the number of these errors and their order, so I just ignore them all
-    baos.toString.split("\n") filter (!_.startsWith("error: bad symbolic reference")) foreach println
-    System.setErr(prevErr)
+    println(filteredInfos.filterNot(_.msg.contains("bad symbolic reference")).mkString("\n"))
   }
-}
+}

test/files/run/typetags_without_scala_reflect_typetag_manifest_interop.check

@@ -1,3 +1,2 @@
-newSource1:9: error: No Manifest available for App.this.T.
-      manifest[T]
-              ^
+
+pos: source-newSource1,line-9,offset=479 No Manifest available for App.this.T. ERROR

test/files/run/typetags_without_scala_reflect_typetag_manifest_interop.scala

@@ -1,6 +1,7 @@
 import scala.tools.partest._
+import scala.tools.nsc.Settings
 
-object Test extends DirectTest {
+object Test extends StoreReporterDirectTest {
   def code = ???
 
   def library = """
@@ -29,18 +30,18 @@ object Test extends DirectTest {
   """
   def compileApp() = {
     val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator")
+    val global = newCompiler("-cp", classpath, "-d", testOutput.path)
     compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(app)
+    //global.reporter.ERROR.foreach(println)
   }
 
   def show(): Unit = {
-    val prevErr = System.err
-    val baos = new java.io.ByteArrayOutputStream();
-    System.setErr(new java.io.PrintStream(baos));
     compileLibrary();
+    println(filteredInfos.mkString("\n"))
+    storeReporter.infos.clear()
     compileApp();
     // we should get bad symbolic reference errors, because we're trying to use an implicit that can't be unpickled
     // but we don't know the number of these errors and their order, so I just ignore them all
-    baos.toString.split("\n") filter (!_.startsWith("error: bad symbolic reference")) foreach println
-    System.setErr(prevErr)
+    println(filteredInfos.filterNot (_.msg.contains("bad symbolic reference")).mkString("\n"))
   }
-}
+}