Merge pull request #5724 from jvican/stub-errors-2.12.x

SCP-009: Improve direct dependency experience

5 files changed, 99 insertions, 12 deletions

diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala
index 819887f959..56ad4738d9 100644
--- a/src/compiler/scala/tools/nsc/Global.scala
+++ b/src/compiler/scala/tools/nsc/Global.scala
@@ -86,6 +86,19 @@ 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): Symbol = {
+    val stubSymbol = super.newStubSymbol(owner, name, missingMessage)
+    val stubErrorPosition = {
+      val lastTreeToTyper = analyzer.lastTreeToTyper
+      if (lastTreeToTyper != EmptyTree) lastTreeToTyper.pos else stubSymbol.pos
+    }
+    stubSymbol.setPos(stubErrorPosition)
+  }
+
   // platform specific elements
 
   protected class GlobalPlatform extends {
diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
index 95f34c1719..f146419a73 100644
--- a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
+++ b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
@@ -1061,8 +1061,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))
@@ -1184,7 +1187,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)
     }
   }
diff --git a/src/partest-extras/scala/tools/partest/StubErrorMessageTest.scala b/src/partest-extras/scala/tools/partest/StubErrorMessageTest.scala
new file mode 100644
index 0000000000..f713b79e75
--- /dev/null
+++ b/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))
+    }
+  }
+}
diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
index 9d71136fc5..854849d27c 100644
--- a/src/reflect/scala/reflect/internal/Symbols.scala
+++ b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -193,6 +193,15 @@ 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): Symbol = {
+    name match {
+      case n: TypeName  => 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
@@ -504,9 +513,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  => new StubClassSymbol(this, n, missingMessage)
-      case _            => new StubTermSymbol(this, name.toTermName, missingMessage)
+    def newStubSymbol(name: Name, missingMessage: String): Symbol = {
+      // Invoke the overriden `newStubSymbol` in Global that gives us access to typer
+      Symbols.this.newStubSymbol(this, name, missingMessage)
     }
 
     /** Given a field, construct a term symbol that represents the source construct that gave rise the field */
@@ -3427,7 +3436,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
 
diff --git a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala
index b4152c9b8c..16fbab7103 100644
--- a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala
+++ b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala
@@ -246,14 +246,15 @@ abstract class UnPickler {
             adjust(mirrorThatLoaded(owner).missingHook(owner, name)) orElse {
               // (4) 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
               val stubName = if (tag == EXTref) name else name.toTypeName
-              owner.newStubSymbol(stubName, missingMessage)
+              // The position of the error message is set by `newStubSymbol`
+              NoSymbol.newStubSymbol(stubName, missingMessage)
             }
           }
         }
@@ -696,11 +697,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.
@@ -723,7 +731,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)