Clarify source and binary class name handling.

Introduce ClassName trait that centralizes logic for creating both source and binary names for a class symbol. In addition to using it in Analyzer, Dependency, ExtractDeclaredClasses we also use it in ExtractAPI. We want every component of the incremental compiler to use consitent naming scheme. Add ClassNameSpecification that documents expected behavior of ClassName.
gkossakowski · Jan 30, 2016 · 87fd296 · 87fd296
1 parent ba92ff4
commit 87fd296
Show file tree

Hide file tree

Showing 8 changed files with 106 additions and 23 deletions.
diff --git a/compile/interface/src/main/scala/xsbt/Analyzer.scala b/compile/interface/src/main/scala/xsbt/Analyzer.scala
@@ -30,7 +30,7 @@ final class Analyzer(val global: CallbackGlobal) extends LocateClassFile {
           val sym = iclass.symbol
           def addGenerated(separatorRequired: Boolean): Unit = {
             for (classFile <- outputDirs map (fileForClass(_, sym, separatorRequired)) find (_.exists)) {
-              val srcClassName = className(sym, separatorRequired)
+              val srcClassName = className(sym)
               val binaryClassName = flatclassName(sym, '.', separatorRequired)
               callback.generatedNonLocalClass(sourceFile, classFile, binaryClassName, srcClassName)
             }

diff --git a/compile/interface/src/main/scala/xsbt/ClassName.scala b/compile/interface/src/main/scala/xsbt/ClassName.scala
@@ -0,0 +1,31 @@
+package xsbt
+
+/**
+ * Utility methods for creating (source|binary) class names for a Symbol.
+ */
+trait ClassName extends Compat {
+  val global: CallbackGlobal
+  import global._
+
+  /**
+    * Creates a flat (binary) name for a class symbol `s`.
+    */
+  protected def flatname(s: Symbol, separator: Char): String =
+    atPhase(currentRun.flattenPhase.next) { s fullName separator }
+
+  /**
+    * Create a (source) name for a class symbol `s`.
+    */
+  protected def className(s: Symbol): String = pickledName(s)
+
+  private def pickledName(s: Symbol): String =
+    atPhase(currentRun.picklerPhase) { s.fullName }
+
+  protected def isTopLevelModule(sym: Symbol): Boolean =
+    atPhase(currentRun.picklerPhase.next) {
+      sym.isModuleClass && !sym.isImplClass && !sym.isNestedClass
+    }
+
+  protected def flatclassName(s: Symbol, sep: Char, dollarRequired: Boolean): String =
+    flatname(s, sep) + (if (dollarRequired) "$" else "")
+}
diff --git a/compile/interface/src/main/scala/xsbt/Dependency.scala b/compile/interface/src/main/scala/xsbt/Dependency.scala
@@ -56,7 +56,7 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile {
          * run) or from class file and calls respective callback method.
          */
         def processDependency(context: DependencyContext)(dep: ClassDependency): Unit = {
-          val fromClassName = className(dep.from, dollarRequired = false)
+          val fromClassName = className(dep.from)
           def binaryDependency(file: File, onBinaryClassName: String) =
             callback.binaryDependency(file, onBinaryClassName, fromClassName, sourceFile, context)
           val onSource = dep.to.sourceFile
@@ -73,7 +73,7 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile {
               case None => ()
             }
           } else if (onSource.file != sourceFile) {
-            val onClassName = className(dep.to, dollarRequired = false)
+            val onClassName = className(dep.to)
             callback.classDependency(onClassName, fromClassName, context)
           }
         }

diff --git a/compile/interface/src/main/scala/xsbt/ExtractAPI.scala b/compile/interface/src/main/scala/xsbt/ExtractAPI.scala
@@ -28,7 +28,7 @@ import xsbti.api.{ ClassLike, DefinitionType, PathComponent, SimpleType }
 class ExtractAPI[GlobalType <: CallbackGlobal](val global: GlobalType,
     // Tracks the source file associated with the CompilationUnit currently being processed by the API phase.
     // This is used when recording inheritance dependencies.
-    sourceFile: File) extends Compat {
+    sourceFile: File) extends Compat with ClassName {
 
   import global._
 
@@ -601,7 +601,7 @@ class ExtractAPI[GlobalType <: CallbackGlobal](val global: GlobalType,
 
     new xsbti.api.ClassLike(
       defType, lzy(selfType(in, sym)), lzy(structureWithInherited(viewer(in).memberInfo(sym), sym)), emptyStringArray, typeParameters(in, sym), // look at class symbol
-      c.fullName, getAccess(c), getModifiers(c), annotations(in, c)) // use original symbol (which is a term symbol when `c.isModule`) for `name` and other non-classy stuff
+      className(c), getAccess(c), getModifiers(c), annotations(in, c)) // use original symbol (which is a term symbol when `c.isModule`) for `name` and other non-classy stuff
   }
 
   // TODO: could we restrict ourselves to classes, ignoring the term symbol for modules,

diff --git a/compile/interface/src/main/scala/xsbt/ExtractDeclaredClasses.scala b/compile/interface/src/main/scala/xsbt/ExtractDeclaredClasses.scala
@@ -32,7 +32,7 @@ class ExtractDeclaredClasses[GlobalType <: CallbackGlobal](val global: GlobalTyp
 			case _ => ()
 		}
 
-  private def fullName(s: Symbol): String = className(s, false)
+  private def fullName(s: Symbol): String = className(s)
 	}
 
 }
diff --git a/compile/interface/src/main/scala/xsbt/LocateClassFile.scala b/compile/interface/src/main/scala/xsbt/LocateClassFile.scala
@@ -11,7 +11,7 @@ import java.io.File
 /**
  * Contains utility methods for looking up class files corresponding to Symbols.
  */
-abstract class LocateClassFile extends Compat {
+abstract class LocateClassFile extends Compat with ClassName {
   val global: CallbackGlobal
   import global._
 
@@ -33,22 +33,6 @@ abstract class LocateClassFile extends Compat {
           None
       }
     }
-  private def flatname(s: Symbol, separator: Char) =
-    atPhase(currentRun.flattenPhase.next) { s fullName separator }
-
-  private def pickledName(s: Symbol): String =
-    atPhase(currentRun.picklerPhase) { s.fullName }
-
-  protected def isTopLevelModule(sym: Symbol): Boolean =
-    atPhase(currentRun.picklerPhase.next) {
-      sym.isModuleClass && !sym.isImplClass && !sym.isNestedClass
-    }
-
-  protected def className(s: Symbol, dollarRequired: Boolean): String =
-    pickledName(s) + (if (dollarRequired) "$" else "")
-
-  protected def flatclassName(s: Symbol, sep: Char, dollarRequired: Boolean): String =
-    flatname(s, sep) + (if (dollarRequired) "$" else "")
 
   protected def fileForClass(outputDirectory: File, s: Symbol, separatorRequired: Boolean): File =
     new File(outputDirectory, flatclassName(s, File.separatorChar, separatorRequired) + ".class")

diff --git a/compile/interface/src/test/scala/xsbt/ClassNameSpecification.scala b/compile/interface/src/test/scala/xsbt/ClassNameSpecification.scala
@@ -0,0 +1,63 @@
+package xsbt
+
+import org.junit.runner.RunWith
+import xsbti.TestCallback.ExtractedClassDependencies
+import xsbti.api.ClassLike
+import xsbti.api.Def
+import xsbt.api.SameAPI
+import org.specs2.mutable.Specification
+import org.specs2.runner.JUnitRunner
+
+@RunWith(classOf[JUnitRunner])
+class ClassNameSpecification extends Specification {
+
+  "Binary names for top level object" in {
+    val src = "object A"
+
+    val compilerForTesting = new ScalaCompilerForUnitTesting(nameHashing = true)
+    val binaryClassNames = compilerForTesting.extractBinaryClassNamesFromSrc(src)
+
+    binaryClassNames === Set("A" -> "A", "A" -> "A$")
+  }
+
+  "Binary names for top level companion object" in {
+    val src = "class A; object A"
+
+    val compilerForTesting = new ScalaCompilerForUnitTesting(nameHashing = true)
+    val binaryClassNames = compilerForTesting.extractBinaryClassNamesFromSrc(src)
+
+    binaryClassNames === Set("A" -> "A", "A" -> "A$")
+  }
+
+  "Binary names for nested object" in {
+    val src =
+      """|object A {
+         |  object C {
+         |    object D
+         |  }
+         |}
+         |class B {
+         |  object E
+         |}
+      """.stripMargin
+
+    val compilerForTesting = new ScalaCompilerForUnitTesting(nameHashing = true)
+    val binaryClassNames = compilerForTesting.extractBinaryClassNamesFromSrc(src)
+
+    binaryClassNames === Set("A" -> "A$", "A" -> "A", "A.C" -> "A$C$", "A.C.D" -> "A$C$D$",
+      "B" -> "B", "B.E" -> "B$E$")
+  }
+
+  "Binary names for trait" in {
+    val src =
+      """|trait A
+      """.stripMargin
+
+    val compilerForTesting = new ScalaCompilerForUnitTesting(nameHashing = true)
+    val binaryClassNames = compilerForTesting.extractBinaryClassNamesFromSrc(src)
+
+    // we do not track $impl classes because nobody can depend on them directly
+    binaryClassNames === Set("A" -> "A")
+  }
+
+}
diff --git a/compile/interface/src/test/scala/xsbt/ScalaCompilerForUnitTesting.scala b/compile/interface/src/test/scala/xsbt/ScalaCompilerForUnitTesting.scala
@@ -40,6 +40,11 @@ class ScalaCompilerForUnitTesting(nameHashing: Boolean = true) {
     analysisCallback.declaredClasses(tempSrcFile).toSet
   }
 
+  def extractBinaryClassNamesFromSrc(src: String): Set[(String, String)] = {
+    val (Seq(tempSrcFile), analysisCallback) = compileSrcs(src)
+    analysisCallback.classNames(tempSrcFile).toSet
+  }
+
   /**
    * Extract used names from src provided as the second argument.
    *