Merge pull request scala#5804 from jvican/stub-errors-2.11.8

Backport 2.11.9: Improve stub error messages (SCP-009 proposal)

Author: adriaanm

src/compiler/scala/tools/nsc/Global.scala

@@ -96,6 +96,20 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
 
   def erasurePhase: Phase = if (currentRun.isDefined) currentRun.erasurePhase else NoPhase
 
+  /* Override `newStubSymbol` defined in `SymbolTable` to provide us access
+   * to the last tree to typer, whose position is the trigger of stub errors. */
+  override def newStubSymbol(owner: Symbol,
+                             name: Name,
+                             missingMessage: String,
+                             isPackage: Boolean = false): Symbol = {
+    val stubSymbol = super.newStubSymbol(owner, name, missingMessage, isPackage)
+    val stubErrorPosition = {
+      val lastTreeToTyper = analyzer.lastTreeToTyper
+      if (lastTreeToTyper != EmptyTree) lastTreeToTyper.pos else stubSymbol.pos
+    }
+    stubSymbol.setPos(stubErrorPosition)
+  }
+
   // platform specific elements
 
   protected class GlobalPlatform extends {

src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala

@@ -366,7 +366,7 @@ abstract class ClassfileParser {
       //   - better owner than `NoSymbol`
       //   - remove eager warning
       val msg = s"Class $name not found - continuing with a stub."
-      if (!settings.isScaladoc) warning(msg)
+      if ((!settings.isScaladoc) && (settings.verbose || settings.developer)) warning(msg)
       return NoSymbol.newStubSymbol(name.toTypeName, msg)
     }
     val completer     = new loaders.ClassfileLoader(file)
@@ -1030,8 +1030,11 @@ abstract class ClassfileParser {
       val sflags      = jflags.toScalaFlags
       val owner       = ownerForFlags(jflags)
       val scope       = getScope(jflags)
-      def newStub(name: Name) =
-        owner.newStubSymbol(name, s"Class file for ${entry.externalName} not found").setFlag(JAVA)
+      def newStub(name: Name) = {
+        val stub = owner.newStubSymbol(name, s"Class file for ${entry.externalName} not found")
+        stub.setPos(owner.pos)
+        stub.setFlag(JAVA)
+      }
 
       val (innerClass, innerModule) = if (file == NoAbstractFile) {
         (newStub(name.toTypeName), newStub(name.toTermName))
@@ -1152,7 +1155,11 @@ abstract class ClassfileParser {
         if (enclosing == clazz) entry.scope lookup name
         else lookupMemberAtTyperPhaseIfPossible(enclosing, name)
       )
-      def newStub = enclosing.newStubSymbol(name, s"Unable to locate class corresponding to inner class entry for $name in owner ${entry.outerName}")
+      def newStub = {
+        enclosing
+          .newStubSymbol(name, s"Unable to locate class corresponding to inner class entry for $name in owner ${entry.outerName}")
+          .setPos(enclosing.pos)
+      }
       member.orElse(newStub)
     }
   }

src/partest-extras/scala/tools/partest/StubErrorMessageTest.scala

@@ -0,0 +1,47 @@
+package scala.tools.partest
+
+trait StubErrorMessageTest extends StoreReporterDirectTest {
+  // Stub to feed to partest, unused
+  def code = throw new Error("Use `userCode` instead of `code`.")
+
+  val classpath = List(sys.props("partest.lib"), testOutput.path)
+    .mkString(sys.props("path.separator"))
+
+  def compileCode(codes: String*) = {
+    val global = newCompiler("-cp", classpath, "-d", testOutput.path)
+    val sourceFiles = newSources(codes: _*)
+    withRun(global)(_ compileSources sourceFiles)
+  }
+
+  def removeClasses(inPackage: String, classNames: Seq[String]): Unit = {
+    val pkg = new File(testOutput.path, inPackage)
+    classNames.foreach { className =>
+      val classFile = new File(pkg, s"$className.class")
+      assert(classFile.exists)
+      assert(classFile.delete())
+    }
+  }
+
+  def removeFromClasspath(): Unit
+  def codeA: String
+  def codeB: String
+  def userCode: String
+  def extraUserCode: String = ""
+
+  def show(): Unit = {
+    compileCode(codeA)
+    assert(filteredInfos.isEmpty, filteredInfos)
+
+    compileCode(codeB)
+    assert(filteredInfos.isEmpty, filteredInfos)
+    removeFromClasspath()
+
+    if (extraUserCode == "") compileCode(userCode)
+    else compileCode(userCode, extraUserCode)
+    import scala.reflect.internal.util.Position
+    filteredInfos.map { report =>
+      print(if (report.severity == storeReporter.ERROR) "error: " else "")
+      println(Position.formatMessage(report.pos, report.msg, true))
+    }
+  }
+}

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

@@ -182,6 +182,17 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
 
   private[reflect] case class SymbolKind(accurate: String, sanitized: String, abbreviation: String)
 
+  protected def newStubSymbol(owner: Symbol,
+                              name: Name,
+                              missingMessage: String,
+                              isPackage: Boolean = false): Symbol = {
+    name match {
+      case n: TypeName  => if (isPackage) new StubPackageClassSymbol(owner, n, missingMessage)
+                           else new StubClassSymbol(owner, n, missingMessage)
+      case _            => new StubTermSymbol(owner, name.toTermName, missingMessage)
+    }
+  }
+
   /** The class for all symbols */
   abstract class Symbol protected[Symbols] (initOwner: Symbol, initPos: Position, initName: Name)
           extends SymbolContextApiImpl
@@ -505,9 +516,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, missingMessage: String, isPackage: Boolean = false): Symbol = name match {
-      case n: TypeName  => if (isPackage) new StubPackageClassSymbol(this, n, missingMessage) else new StubClassSymbol(this, n, missingMessage)
-      case _            => new StubTermSymbol(this, name.toTermName, missingMessage)
+    def newStubSymbol(name: Name, missingMessage: String, isPackage: Boolean = false): Symbol = {
+      // Invoke the overriden `newStubSymbol` in Global that gives us access to typer
+      Symbols.this.newStubSymbol(this, name, missingMessage, isPackage)
     }
 
     /** Given a field, construct a term symbol that represents the source construct that gave rise the field */
@@ -3491,7 +3502,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     private def fail[T](alt: T): T = {
       // Avoid issuing lots of redundant errors
       if (!hasFlag(IS_ERROR)) {
-        globalError(missingMessage)
+        globalError(pos, missingMessage)
         if (settings.debug.value)
           (new Throwable).printStackTrace
 

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

@@ -266,13 +266,15 @@ abstract class UnPickler {
               adjust(mirrorThatLoaded(owner).missingHook(owner, name)) orElse {
                 // (5) Create a stub symbol to defer hard failure a little longer.
                 val advice = moduleAdvice(s"${owner.fullName}.$name")
+                val lazyCompletingSymbol = completingStack.headOption.getOrElse(NoSymbol)
                 val missingMessage =
-                  s"""|missing or invalid dependency detected while loading class file '$filename'.
-                      |Could not access ${name.longString} in ${owner.kindString} ${owner.fullName},
-                      |because it (or its dependencies) are missing. Check your build definition for
-                      |missing or conflicting dependencies. (Re-run with `-Ylog-classpath` to see the problematic classpath.)
+                  s"""|Symbol '${name.nameKind} ${owner.fullName}.$name' is missing from the classpath.
+                      |This symbol is required by '${lazyCompletingSymbol.kindString} ${lazyCompletingSymbol.fullName}'.
+                      |Make sure that ${name.longString} is in your classpath and check for conflicting dependencies with `-Ylog-classpath`.
                       |A full rebuild may help if '$filename' was compiled against an incompatible version of ${owner.fullName}.$advice""".stripMargin
-                owner.newStubSymbol(name, missingMessage)
+                val stubName = if (tag == EXTref) name else name.toTypeName
+                // The position of the error message is set by `newStubSymbol`
+                NoSymbol.newStubSymbol(stubName, missingMessage)
               }
             }
           }
@@ -717,11 +719,18 @@ abstract class UnPickler {
       new TypeError(e.msg)
     }
 
+    /** Keep track of the symbols pending to be initialized.
+      *
+      * Useful for reporting on stub errors and cyclic errors.
+      */
+    private var completingStack = List.empty[Symbol]
+
     /** A lazy type which when completed returns type at index `i`. */
     private class LazyTypeRef(i: Int) extends LazyType with FlagAgnosticCompleter {
       private val definedAtRunId = currentRunId
       private val p = phase
       protected def completeInternal(sym: Symbol) : Unit = try {
+        completingStack = sym :: completingStack
         val tp = at(i, () => readType(sym.isTerm)) // after NMT_TRANSITION, revert `() => readType(sym.isTerm)` to `readType`
 
         // This is a temporary fix allowing to read classes generated by an older, buggy pickler.
@@ -744,7 +753,10 @@ abstract class UnPickler {
       }
       catch {
         case e: MissingRequirementError => throw toTypeError(e)
+      } finally {
+        completingStack = completingStack.tail
       }
+
       override def complete(sym: Symbol) : Unit = {
         completeInternal(sym)
         if (!isCompilerUniverse) markAllCompleted(sym)

test/files/neg/t5148.check

@@ -1,11 +1,7 @@
-error: missing or invalid dependency detected while loading class file 'Imports.class'.
-Could not access type Wrapper in class scala.tools.nsc.interpreter.IMain.Request,
-because it (or its dependencies) are missing. Check your build definition for
-missing or conflicting dependencies. (Re-run with `-Ylog-classpath` to see the problematic classpath.)
-A full rebuild may help if 'Imports.class' was compiled against an incompatible version of scala.tools.nsc.interpreter.IMain.Request.
-error: missing or invalid dependency detected while loading class file 'Imports.class'.
-Could not access type Request in class scala.tools.nsc.interpreter.IMain,
-because it (or its dependencies) are missing. Check your build definition for
-missing or conflicting dependencies. (Re-run with `-Ylog-classpath` to see the problematic classpath.)
-A full rebuild may help if 'Imports.class' was compiled against an incompatible version of scala.tools.nsc.interpreter.IMain.
-two errors found
+t5148.scala:4: error: Symbol 'type <none>.Request.Wrapper' is missing from the classpath.
+This symbol is required by 'value scala.tools.nsc.interpreter.Imports.wrapper'.
+Make sure that type Wrapper is in your classpath and check for conflicting dependencies with `-Ylog-classpath`.
+A full rebuild may help if 'Imports.class' was compiled against an incompatible version of <none>.Request.
+class IMain extends Imports
+            ^
+one error found

test/files/run/StubErrorBInheritsFromA.check

@@ -0,0 +1,6 @@
+error: newSource1.scala:4: Symbol 'type stuberrors.A' is missing from the classpath.
+This symbol is required by 'class stuberrors.B'.
+Make sure that type A is in your classpath and check for conflicting dependencies with `-Ylog-classpath`.
+A full rebuild may help if 'B.class' was compiled against an incompatible version of stuberrors.
+      new B
+          ^

test/files/run/StubErrorBInheritsFromA.scala

@@ -0,0 +1,22 @@
+object Test extends scala.tools.partest.StubErrorMessageTest {
+  def codeA = """
+    package stuberrors
+    class A
+  """
+
+  def codeB = """
+    package stuberrors
+    class B extends A
+  """
+
+  def userCode = """
+    package stuberrors
+    class C {
+      new B
+    }
+  """
+
+  def removeFromClasspath(): Unit = {
+    removeClasses("stuberrors", List("A"))
+  }
+}

test/files/run/StubErrorComplexInnerClass.check

@@ -0,0 +1,6 @@
+error: newSource1.scala:9: Symbol 'type stuberrors.A' is missing from the classpath.
+This symbol is required by 'class stuberrors.B.BB'.
+Make sure that type A is in your classpath and check for conflicting dependencies with `-Ylog-classpath`.
+A full rebuild may help if 'B.class' was compiled against an incompatible version of stuberrors.
+        new b.BB
+            ^

test/files/run/StubErrorComplexInnerClass.scala

@@ -0,0 +1,42 @@
+object Test extends scala.tools.partest.StubErrorMessageTest {
+  def codeA = """
+    package stuberrors
+    class A
+  """
+
+  def codeB = """
+    package stuberrors
+    class B {
+      def foo: String = ???
+
+      // unused and should fail, but not loaded
+      def unsafeFoo: A = ???
+      // used, B.info -> BB.info -> unpickling A -> stub error
+      class BB extends A
+    }
+  """
+
+  def userCode = """
+    package stuberrors
+    class C {
+      def aloha = {
+        val b = new B
+        val d = new extra.D
+        d.foo
+        println(b.foo)
+        new b.BB
+      }
+    }
+  """
+
+  override def extraUserCode = """
+    package extra
+    class D {
+      def foo = "Hello, World"
+    }
+  """.stripMargin
+
+  def removeFromClasspath(): Unit = {
+    removeClasses("stuberrors", List("A"))
+  }
+}