From bad824c0b16a1c7bc19b9072b4c87228de0756ba Mon Sep 17 00:00:00 2001
From: Grzegorz Kossakowski <grzegorz.kossakowski@gmail.com>
Date: Fri, 18 Dec 2015 14:32:43 -0800
Subject: [PATCH] Do not compute name hashes for private members.

The NameHashing classes assumed that extracted API data structure have
private members filtered out already. That assumption was wrong and
resulted in bug #2324.

We fix the bug by simply reusing the same logic as used by SameAPI class.

Fixes #2324.
---
 compile/api/src/main/scala/xsbt/api/APIUtil.scala     |  9 +++++++++
 compile/api/src/main/scala/xsbt/api/NameHashing.scala |  4 +++-
 compile/api/src/main/scala/xsbt/api/SameAPI.scala     | 11 +++--------
 .../scala/xsbt/api/NameHashingSpecification.scala     |  2 +-
 4 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/compile/api/src/main/scala/xsbt/api/APIUtil.scala b/compile/api/src/main/scala/xsbt/api/APIUtil.scala
index e7c4ac1c8..d8c4e88cc 100644
--- a/compile/api/src/main/scala/xsbt/api/APIUtil.scala
+++ b/compile/api/src/main/scala/xsbt/api/APIUtil.scala
@@ -60,6 +60,15 @@ object APIUtil {
     new Structure(lzy(s.parents), filterDefinitions(s.declared, isModule), filterDefinitions(s.inherited, isModule))
   def filterDefinitions(ds: Array[Definition], isModule: Boolean): Lazy[Array[Definition]] =
     lzy(if (isModule) ds filter Discovery.isMainMethod else Array())
+
+  def isNonPrivate(d: Definition): Boolean = isNonPrivate(d.access)
+  /** Returns false if the `access` is `Private` and qualified, true otherwise.*/
+  def isNonPrivate(access: Access): Boolean =
+    access match {
+      case p: Private if !p.qualifier.isInstanceOf[IdQualifier] => false
+      case _ => true
+    }
+
   private[this] def lzy[T <: AnyRef](t: T): Lazy[T] = SafeLazy.strict(t)
 
   private[this] val emptyType = new EmptyType
diff --git a/compile/api/src/main/scala/xsbt/api/NameHashing.scala b/compile/api/src/main/scala/xsbt/api/NameHashing.scala
index 60221e22f..b958fa48a 100644
--- a/compile/api/src/main/scala/xsbt/api/NameHashing.scala
+++ b/compile/api/src/main/scala/xsbt/api/NameHashing.scala
@@ -78,7 +78,9 @@ class NameHashing {
           visitDefinition(topLevelDef)
       }
     }
-    override def visitDefinition(d: Definition): Unit = {
+    // if the definition is private, we do not visit because we do
+    // not want to include any private members or its children
+    override def visitDefinition(d: Definition): Unit = if (APIUtil.isNonPrivate(d)) {
       val locatedDef = LocatedDefinition(currentLocation, d)
       locatedDefs += locatedDef
       d match {
diff --git a/compile/api/src/main/scala/xsbt/api/SameAPI.scala b/compile/api/src/main/scala/xsbt/api/SameAPI.scala
index ebf03f5cd..1991bed11 100644
--- a/compile/api/src/main/scala/xsbt/api/SameAPI.scala
+++ b/compile/api/src/main/scala/xsbt/api/SameAPI.scala
@@ -89,14 +89,9 @@ object SameAPI {
    * All top-level definitions are always considered: 'private' only means package-private.
    * Other definitions are considered if they are not qualified with 'private[this]' or 'private'.
    */
-  def filterDefinitions(d: Seq[Definition], topLevel: Boolean, includePrivate: Boolean) = if (topLevel || includePrivate) d else d.filter(isNonPrivate)
-  def isNonPrivate(d: Definition): Boolean = isNonPrivate(d.access)
-  /** Returns false if the `access` is `Private` and qualified, true otherwise.*/
-  def isNonPrivate(access: Access): Boolean =
-    access match {
-      case p: Private if !p.qualifier.isInstanceOf[IdQualifier] => false
-      case _ => true
-    }
+  def filterDefinitions(d: Seq[Definition], topLevel: Boolean, includePrivate: Boolean) =
+    if (topLevel || includePrivate) d else d.filter(APIUtil.isNonPrivate)
+
 }
 /**
  * Used to implement API equality.
diff --git a/compile/api/src/test/scala/xsbt/api/NameHashingSpecification.scala b/compile/api/src/test/scala/xsbt/api/NameHashingSpecification.scala
index 4d3423530..4849838b2 100644
--- a/compile/api/src/test/scala/xsbt/api/NameHashingSpecification.scala
+++ b/compile/api/src/test/scala/xsbt/api/NameHashingSpecification.scala
@@ -285,7 +285,7 @@ class NameHashingSpecification extends Specification {
     val nameHashes = nameHashesForClass(classFoo)
     // make sure there's no name hash for the private member "x"
     Seq("Foo") === nameHashes.regularMembers.map(_.name).toSeq
-  }.pendingUntilFixed("The NameHashing calculates name hashes of all members")
+  }
 
   private def assertNameHashEqualForRegularName(name: String, nameHashes1: _internalOnly_NameHashes,
     nameHashes2: _internalOnly_NameHashes) = {