Merge pull request #2830 from dotty-staging/fix-complete-scala

Fix #2809: Invalidate and unlink symbols for artifact classfiles

Author: smarter

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -441,10 +441,17 @@ object SymDenotations {
     final def markAbsent(): Unit =
       myInfo = NoType
 
+    /** Is symbol known to not exist, or potentially not completed yet? */
+    final def unforcedIsAbsent(implicit ctx: Context): Boolean =
+      myInfo == NoType ||
+      (this is (ModuleVal, butNot = Package)) && moduleClass.unforcedIsAbsent
+
     /** Is symbol known to not exist? */
-    final def isAbsent(implicit ctx: Context): Boolean =
+    final def isAbsent(implicit ctx: Context): Boolean = {
+      ensureCompleted()
       myInfo == NoType ||
       (this is (ModuleVal, butNot = Package)) && moduleClass.isAbsent
+    }
 
     /** Is this symbol the root class or its companion object? */
     final def isRoot: Boolean =
@@ -563,7 +570,7 @@ object SymDenotations {
     final def isCoDefinedWith(that: Symbol)(implicit ctx: Context) =
       (this.effectiveOwner == that.effectiveOwner) &&
       (  !(this.effectiveOwner is PackageClass)
-        || this.isAbsent || that.isAbsent
+        || this.unforcedIsAbsent || that.unforcedIsAbsent
         || { // check if they are defined in the same file(or a jar)
            val thisFile = this.symbol.associatedFile
            val thatFile = that.symbol.associatedFile

compiler/src/dotty/tools/dotc/core/Symbols.scala

@@ -185,7 +185,7 @@ trait Symbols { this: Context =>
   val companionMethodFlags = Flags.Synthetic | Flags.Private | Flags.Method
 
   def synthesizeCompanionMethod(name: Name, target: SymDenotation, owner: SymDenotation)(implicit ctx: Context) =
-    if (owner.exists && target.exists && !owner.isAbsent && !target.isAbsent) {
+    if (owner.exists && target.exists && !owner.unforcedIsAbsent && !target.unforcedIsAbsent) {
       val existing = owner.unforcedDecls.lookup(name)
 
       existing.orElse{

compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala

@@ -19,6 +19,10 @@ import scala.util.control.NonFatal
 object ClassfileParser {
   /** Marker trait for unpicklers that can be embedded in classfiles. */
   trait Embedded
+
+  /** Indicate that there is nothing to unpickle and the corresponding symbols can
+    * be invalidated. */
+  object NoEmbedded extends Embedded
 }
 
 class ClassfileParser(
@@ -147,6 +151,15 @@ class ClassfileParser(
 
       setClassInfo(classRoot, classInfo)
       setClassInfo(moduleRoot, staticInfo)
+    } else if (result == Some(NoEmbedded)) {
+      for (sym <- List(moduleRoot.sourceModule.symbol, moduleRoot.symbol, classRoot.symbol)) {
+        classRoot.owner.asClass.delete(sym)
+        if (classRoot.owner == defn.ScalaShadowingPackageClass) {
+          // Symbols in scalaShadowing are also added to scala
+          defn.ScalaPackageClass.delete(sym)
+        }
+        sym.markAbsent()
+      }
     }
 
     // eager load java enum definitions for exhaustivity check of pattern match
@@ -700,6 +713,10 @@ class ClassfileParser(
     }
   }
 
+  // Nothing$ and Null$ were incorrectly emitted with a Scala attribute
+  // instead of ScalaSignature before 2.13.0-M2, see https://github.com/scala/scala/pull/5952
+  private[this] val scalaUnpickleWhitelist = List(tpnme.nothingClass, tpnme.nullClass)
+
   /** Parse inner classes. Expects `in.bp` to point to the superclass entry.
    *  Restores the old `bp`.
    *  @return true iff classfile is from Scala, so no Java info needs to be read.
@@ -760,6 +777,18 @@ class ClassfileParser(
         return unpickleTASTY(in.nextBytes(attrLen))
       }
 
+      if (scan(tpnme.ScalaATTR) && !scalaUnpickleWhitelist.contains(classRoot.name)) {
+        // To understand the situation, it's helpful to know that:
+        // - Scalac emits the `ScalaSig` attribute for classfiles with pickled information
+        // and the `Scala` attribute for everything else.
+        // - Dotty emits the `TASTY` attribute for classfiles with pickled information
+        // and the `Scala` attribute for _every_ classfile.
+        //
+        // Therefore, if the `Scala` attribute is present but the `TASTY`
+        // attribute isn't, this classfile is a compilation artifact.
+        return Some(NoEmbedded)
+      }
+
       if (scan(tpnme.RuntimeAnnotationATTR)) {
         val attrLen = in.nextInt
         val nAnnots = in.nextChar

compiler/src/dotty/tools/dotc/interactive/Interactive.scala

@@ -89,7 +89,7 @@ object Interactive {
     val boundaryCtx = ctx.withOwner(boundary)
     try
       prefix.memberDenots(completionsFilter, (name, buf) =>
-        buf ++= prefix.member(name).altsWith(_.symbol.isAccessibleFrom(prefix)(boundaryCtx))
+        buf ++= prefix.member(name).altsWith(d => !d.isAbsent && d.symbol.isAccessibleFrom(prefix)(boundaryCtx))
       ).map(_.symbol).toList
     catch {
       case ex: TypeError => Nil

compiler/src/dotty/tools/dotc/transform/Pickler.scala

@@ -33,7 +33,7 @@ class Pickler extends Phase {
   /** Drop any elements of this list that are linked module classes of other elements in the list */
   private def dropCompanionModuleClasses(clss: List[ClassSymbol])(implicit ctx: Context): List[ClassSymbol] = {
     val companionModuleClasses =
-      clss.filterNot(_ is Module).map(_.linkedClass).filterNot(_.isAbsent)
+      clss.filterNot(_ is Module).map(_.linkedClass).filterNot(_.unforcedIsAbsent)
     clss.filterNot(companionModuleClasses.contains)
   }
 

compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala

@@ -145,10 +145,7 @@ trait TypeAssigner {
   final def reallyExists(denot: Denotation)(implicit ctx: Context): Boolean = try
     denot match {
       case denot: SymDenotation =>
-        denot.exists && {
-          denot.ensureCompleted()
-          !denot.isAbsent
-        }
+        denot.exists && !denot.isAbsent
       case denot: SingleDenotation =>
         val sym = denot.symbol
         (sym eq NoSymbol) || reallyExists(sym.denot)

tests/neg/classfile-artifacts.scala

@@ -0,0 +1,15 @@
+// For some reason, if the imports are not scoped, only the first import error
+// is reported
+
+class A {
+  import scala.languageFeature$experimental$._ // error
+}
+
+class B {
+  import scala.language$Scala2$._ // error
+}
+
+class C {
+ import scala.languageFeature$._ // error
+}
+