Make minor changes to ExtractUsedNames et al

This commit makes minor changes to `ExtractUsednames` in the spirit of
better readability and some microoptimization to leave it the way it was
before (see the rewrite of `add` by a `contains` + `add`).

It also makes some changes to the API: sets some classes to final and
renames `ClassFileManagers` to `ClassFileManager` for consistency with
the rest of the API. In the future, I'm happy to consider a name change,
but for now it's better to stick to the convention of `ClassFileManager`
being acting like a "companion object".

internal/compiler-bridge/src/main/scala/xsbt/ExtractUsedNames.scala

@@ -56,13 +56,13 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
   import global._
   import JavaUtils._
 
-  class NamesUsedInClass {
+  private final class NamesUsedInClass {
     // Default names and other scopes are separated for performance reasons
     val defaultNames: JavaSet[Name] = new JavaSet[global.Name]()
     val scopedNames: JavaMap[Name, EnumSet[UseScope]] = new JavaMap[Name, EnumSet[UseScope]]()
 
     // We have to leave with commas on ends
-    override def toString() = {
+    override def toString(): String = {
       val builder = new StringBuilder(": ")
       defaultNames.foreach { name =>
         builder.append(name.decoded.trim)
@@ -78,63 +78,73 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
     }
   }
 
+  private def DefaultScopes = EnumSet.of(UseScope.Default)
+  private def PatmatScopes = EnumSet.of(UseScope.PatMatTarget)
+
   def extractAndReport(unit: CompilationUnit): Unit = {
     val tree = unit.body
     val traverser = new ExtractUsedNamesTraverser
     traverser.traverse(tree)
+
     val namesUsedAtTopLevel = traverser.namesUsedAtTopLevel
+    val defaultNamesTopLevel = namesUsedAtTopLevel.defaultNames
+    val scopedNamesTopLevel = namesUsedAtTopLevel.scopedNames
 
-    if (!namesUsedAtTopLevel.defaultNames.isEmpty || !namesUsedAtTopLevel.scopedNames.isEmpty) {
+    // Handle names used at top level that cannot be related to an owner
+    if (!defaultNamesTopLevel.isEmpty || !scopedNamesTopLevel.isEmpty) {
       val responsible = firstClassOrModuleDef(tree)
       responsible match {
         case Some(classOrModuleDef) =>
           val sym = classOrModuleDef.symbol
-          val firstClassSymbol = if (sym.isModule) sym.moduleClass else sym
+          val firstClassSymbol = enclOrModuleClass(sym)
           val firstClassName = className(firstClassSymbol)
           val namesInFirstClass = traverser.usedNamesFromClass(firstClassName)
-
-          namesInFirstClass.defaultNames.addAll(namesUsedAtTopLevel.defaultNames)
-          namesUsedAtTopLevel.scopedNames.foreach {
-            (topLevelName, topLevelScopes) =>
-              namesInFirstClass.scopedNames.get(topLevelName) match {
-                case null =>
-                  namesInFirstClass.scopedNames.put(topLevelName, topLevelScopes)
-                  ()
-                case scopes =>
-                  scopes.addAll(topLevelScopes)
-                  ()
-              }
+          val scopedNamesInFirstClass = namesInFirstClass.scopedNames
+
+          namesInFirstClass.defaultNames.addAll(defaultNamesTopLevel)
+          scopedNamesTopLevel.foreach { (topLevelName, topLevelScopes) =>
+            val existingScopes = scopedNamesInFirstClass.get(topLevelName)
+            if (existingScopes == null)
+              scopedNamesInFirstClass.put(topLevelName, topLevelScopes)
+            else existingScopes.addAll(topLevelScopes)
+            ()
           }
 
         case None =>
           reporter.warning(unit.position(0), Feedback.OrphanNames)
       }
     }
 
-    def usedNameDebugMessage: String = {
-      val builder = new StringBuilder(s"The ${unit.source} contains the following used names:\n")
+    debuglog {
+      val msg = s"The ${unit.source} contains the following used names:\n"
+      val builder = new StringBuilder(msg)
       traverser.usedNamesFromClasses.foreach {
         (name, usedNames) =>
-          builder.append(name.toString.trim).append(": ").append(usedNames.toString()).append("\n")
+          builder
+            .append(name.toString.trim)
+            .append(": ")
+            .append(usedNames.toString())
+            .append("\n")
+          ()
       }
       builder.toString()
     }
-    debuglog(usedNameDebugMessage)
 
+    // Handle names circumscribed to classes
     traverser.usedNamesFromClasses.foreach {
       (rawClassName, usedNames) =>
         val className = rawClassName.toString.trim
-        usedNames.defaultNames.foreach {
-          rawUsedName =>
-            val useName = rawUsedName.decoded.trim
-            val useScopes = usedNames.scopedNames.get(rawUsedName) match {
-              case null =>
-                EnumSet.of(UseScope.Default)
-              case scopes =>
-                scopes.add(UseScope.Default)
-                scopes
+        usedNames.defaultNames.foreach { rawUsedName =>
+          val useName = rawUsedName.decoded.trim
+          val existingScopes = usedNames.scopedNames.get(rawUsedName)
+          val useScopes = {
+            if (existingScopes == null) DefaultScopes
+            else {
+              existingScopes.add(UseScope.Default)
+              existingScopes
             }
-            callback.usedName(className, useName, useScopes)
+          }
+          callback.usedName(className, useName, useScopes)
         }
     }
   }
@@ -170,15 +180,14 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
     }
 
     /** Returns mutable set with all names from given class used in current context */
-    def usedNamesFromClass(className: Name): NamesUsedInClass =
-      usedNamesFromClasses.get(className) match {
-        case null =>
-          val newOne = new NamesUsedInClass
-          usedNamesFromClasses.put(className, newOne)
-          newOne
-        case existing =>
-          existing
-      }
+    def usedNamesFromClass(className: Name): NamesUsedInClass = {
+      val names = usedNamesFromClasses.get(className)
+      if (names == null) {
+        val newOne = new NamesUsedInClass
+        usedNamesFromClasses.put(className, newOne)
+        newOne
+      } else names
+    }
 
     /*
      * Some macros appear to contain themselves as original tree.
@@ -201,13 +210,12 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
       override def addDependency(symbol: global.Symbol): Unit = {
         if (!ignoredSymbol(symbol) && symbol.isSealed) {
           val name = symbol.name
-          if (!isEmptyName(name))
-            _currentScopedNamesCache.get(name) match {
-              case null =>
-                _currentScopedNamesCache.put(name, EnumSet.of(UseScope.PatMatTarget))
-              case scopes =>
-                scopes.add(UseScope.PatMatTarget)
-            }
+          if (!isEmptyName(name)) {
+            val existingScopes = _currentScopedNamesCache.get(name)
+            if (existingScopes == null)
+              _currentScopedNamesCache.put(name, PatmatScopes)
+            else existingScopes.add(UseScope.PatMatTarget)
+          }
         }
         ()
       }
@@ -264,8 +272,10 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
       // not what we need
       case t: TypeTree if t.original != null =>
         val original = t.original
-        if (inspectedTypeTrees.add(original))
+        if (!inspectedTypeTrees.contains(original)) {
+          inspectedTypeTrees.add(original)
           original.foreach(traverse)
+        }
 
       case t if t.hasSymbolField =>
         val symbol = t.symbol
@@ -299,12 +309,12 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext
     }
 
     /**
-     * Updates caches for closest non-local class owner
-     * of a tree given `currentOwner`, defined and updated by `Traverser`.
+     * Updates caches for closest non-local class owner of a tree given
+     * `currentOwner`, defined and updated by `Traverser`.
      *
      * This method modifies the state associated with the names variable
-     * `_currentNamesCache` and `_currentScopedNamesCache`, which is composed by `_currentOwner` and
-     * and `_currentNonLocalClass`.
+     * `_currentNamesCache` and `_currentScopedNamesCache`, which are composed
+     * by `_currentOwner` and and `_currentNonLocalClass`.
      *
      * * The used caching strategy works as follows:
      * 1. Do nothing if owners are referentially equal.

internal/compiler-bridge/src/test/scala/xsbt/ExtractUsedNamesSpecification.scala

@@ -237,7 +237,7 @@ class ExtractUsedNamesSpecification extends UnitSpec {
     // Seq and Set is not
     findPatMatUsages(classWithPatMatOfType(s"Seq[Set[$sealedClassName]]")) shouldEqual Set(sealedClassName)
 
-    def inNestedCase(tpe: String = sealedClassName) =
+    def inNestedCase(tpe: String) =
       s"""package client
           |import base._
           |
@@ -247,7 +247,7 @@ class ExtractUsedNamesSpecification extends UnitSpec {
           |  }
           |}""".stripMargin
 
-    findPatMatUsages(inNestedCase()) shouldEqual Set()
+    findPatMatUsages(inNestedCase(sealedClassName)) shouldEqual Set()
 
     val notUsedInPatternMatch =
       s"""package client

internal/compiler-interface/src/main/java/xsbti/UseScope.java

@@ -1,11 +1,17 @@
 package xsbti;
 
-
-import java.util.EnumSet;
-
+/**
+ * Defines the scope in which a name hash was captured.
+ *
+ * The incremental compiler uses [[UseScope]] to determine some Scala semantics
+ * assumed in the presence of a name in a concrete position. For instance,
+ * [[PatMatTarget]] is used for names that appear as the target types of a
+ * pattern match.
+ *
+ * The order of declaration of these is crucial. Don't change it.
+ */
 public enum UseScope {
-    // Order of declaration is crucial
-    // We can support only 6 values with current implementation of mapper
+    // Don't add more than 6 scopes. Otherwise, change `Mapper` implementation.
     Default, Implicit, PatMatTarget
 }
 

internal/zinc-core/src/main/scala/sbt/internal/inc/ClassFileManagers.scala → internal/zinc-core/src/main/scala/sbt/internal/inc/ClassFileManager.scala

@@ -16,7 +16,7 @@ import collection.mutable
 import xsbti.compile.{ DeleteImmediatelyManagerType, IncOptions, TransactionalManagerType }
 import xsbti.compile.ClassFileManager
 
-object ClassFileManagers {
+object ClassFileManager {
 
   private case class WrappedClassFileManager(internal: ClassFileManager, external: Option[ClassFileManager])
     extends ClassFileManager {

internal/zinc-core/src/main/scala/sbt/internal/inc/Incremental.scala

@@ -102,7 +102,7 @@ object Incremental {
   private[inc] def apiDebug(options: IncOptions): Boolean = options.apiDebug || java.lang.Boolean.getBoolean(apiDebugProp)
 
   private[sbt] def prune(invalidatedSrcs: Set[File], previous: CompileAnalysis): Analysis =
-    prune(invalidatedSrcs, previous, ClassFileManagers.deleteImmediately())
+    prune(invalidatedSrcs, previous, ClassFileManager.deleteImmediately())
 
   private[sbt] def prune(invalidatedSrcs: Set[File], previous0: CompileAnalysis, classfileManager: ClassFileManager): Analysis =
     {
@@ -113,7 +113,7 @@ object Incremental {
 
   private[this] def manageClassfiles[T](options: IncOptions)(run: ClassFileManager => T): T =
     {
-      val classfileManager = ClassFileManagers.getClassFileManager(options)
+      val classfileManager = ClassFileManager.getClassFileManager(options)
       val result = try run(classfileManager) catch {
         case e: Throwable =>
           classfileManager.complete(false)

internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalNameHashing.scala

@@ -36,8 +36,8 @@ private final class IncrementalNameHashing(log: sbt.util.Logger, options: IncOpt
     if (SameAPI(a, b))
       None
     else {
-      val aNameHashes = a.nameHashes()
-      val bNameHashes = b.nameHashes()
+      val aNameHashes = a.nameHashes
+      val bNameHashes = b.nameHashes
       val modifiedNames = ModifiedNames.compareTwoNameHashes(aNameHashes, bNameHashes)
       val apiChange = NamesChange(className, modifiedNames)
       Some(apiChange)
@@ -107,14 +107,16 @@ private final class IncrementalNameHashing(log: sbt.util.Logger, options: IncOpt
       if (all.isEmpty) s"Change $change does not affect any class."
       else {
         val byTransitiveInheritance =
-          if (transitiveInheritance.nonEmpty) s"by transitiveInheritance: $transitiveInheritance" else ""
+          if (transitiveInheritance.nonEmpty) s"by transitive inheritance: $transitiveInheritance" else ""
         val byLocalInheritance =
-          if (localInheritance.nonEmpty) s"by localInheritance: $localInheritance" else ""
+          if (localInheritance.nonEmpty) s"by local inheritance: $localInheritance" else ""
         val byMemberRef =
-          if (memberRef.nonEmpty) s"by transitiveInheritance: $memberRef" else ""
+          if (memberRef.nonEmpty) s"by member reference: $memberRef" else ""
 
-        s"""$change invalidates ${all.size} classes due to ${memberRefInvalidator.invalidationReason(change)}
-           |\t$byTransitiveInheritance $byLocalInheritance $byMemberRef
+        s"""Change $change invalidates ${all.size} classes due to ${memberRefInvalidator.invalidationReason(change)}
+           |\t> $byTransitiveInheritance
+           |\t> $byLocalInheritance
+           |\t> $byMemberRef
         """.stripMargin
       }
     }