Merge pull request #74 from mlangc/various-simple-bugfixes

Treat locally defined types properly

Author: misto

org.scala-refactoring.library/src/main/scala/scala/tools/refactoring/analysis/CompilationUnitDependencies.scala

@@ -5,7 +5,9 @@
 package scala.tools.refactoring
 package analysis
 
-trait CompilationUnitDependencies {
+import scala.tools.refactoring.common.CompilerApiExtensions
+
+trait CompilationUnitDependencies extends CompilerApiExtensions {
   // we need to interactive compiler because we work with RangePositions
   this: common.InteractiveScalaCompiler with common.TreeTraverser with common.TreeExtractors with common.PimpedTrees =>
 
@@ -72,19 +74,9 @@ trait CompilationUnitDependencies {
    */
   def neededImports(t: Tree): List[Select] = {
 
-    /**
-     * Check if the definition is also a child of the outer `t`. In that case, we don't need
-     * to add an import because the dependency is to a local definition.
-     */
-    def isLocalDefinition(dependency: Tree) = t.exists {
-      case t: DefTree => dependency.symbol == t.symbol
-      case _ => false
-    }
-
     val deps = dependencies(t)
 
     val neededDependencies = deps.flatMap {
-      case t if isLocalDefinition(t) => None
       case t: Select if !t.pos.isRange => Some(t)
       case t => findDeepestNeededSelect(t)
     }.filter(isImportReallyNeeded).distinct
@@ -100,6 +92,20 @@ trait CompilationUnitDependencies {
    * because they are defined in the same compilation unit.
    */
   def dependencies(t: Tree): List[Select] = {
+    val wholeTree = t
+
+    def qualifierIsEnclosingPackage(t: Select) = {
+      enclosingPackage(wholeTree, t.pos) match {
+        case pkgDef: PackageDef =>
+          t.qualifier.nameString == pkgDef.nameString
+        case _ => false
+      }
+    }
+
+    def isDefinedLocally(t: Tree) = wholeTree.exists {
+      case defTree: DefTree if t.symbol == defTree.symbol => true
+      case _ => false
+    }
 
     val result = new collection.mutable.HashMap[String, Select]
 
@@ -173,7 +179,6 @@ trait CompilationUnitDependencies {
       val language = newTermName("language")
 
       override def traverse(tree: Tree) = tree match {
-
         // Always add the SIP 18 language imports as required until we can handle them properly
         case Import(select @ Select(Ident(nme.scala_), `language`), feature) =>
           feature foreach (selector => addToResult(Select(select, selector.name)))
@@ -240,7 +245,8 @@ trait CompilationUnitDependencies {
           if (!isMethodCallFromExplicitReceiver(t)
               && !isSelectFromInvisibleThis(qual)
               && t.name != nme.WILDCARD
-              && hasStableQualifier(t)) {
+              && hasStableQualifier(t)
+              && !(isDefinedLocally(t) && qualifierIsEnclosingPackage(t))) {
             addToResult(t)
           }
 

org.scala-refactoring.library/src/main/scala/scala/tools/refactoring/common/CompilerApiExtensions.scala

@@ -0,0 +1,58 @@
+package scala.tools.refactoring.common
+
+import scala.tools.nsc.Global
+
+/*
+ * FIXME: This class duplicates functionality from org.scalaide.core.compiler.CompilerApiExtensions.
+ */
+trait CompilerApiExtensions {
+  this: CompilerAccess =>
+  import global._
+
+  /** Locate the smallest tree that encloses position.
+   *
+   *  @param tree The tree in which to search `pos`
+   *  @param pos  The position to look for
+   *  @param p    An additional condition to be satisfied by the resulting tree
+   *  @return     The innermost enclosing tree for which p is true, or `EmptyTree`
+   *              if the position could not be found.
+   */
+  def locateIn(tree: Tree, pos: Position, p: Tree => Boolean = t => true): Tree =
+    new FilteringLocator(pos, p) locateIn tree
+
+  def enclosingPackage(tree: Tree, pos: Position): Tree = {
+    locateIn(tree, pos, _.isInstanceOf[PackageDef])
+  }
+
+  private class FilteringLocator(pos: Position, p: Tree => Boolean) extends Locator(pos) {
+    override def isEligible(t: Tree) = super.isEligible(t) && p(t)
+  }
+
+  /*
+   * For Scala-2.10 (see scala.reflect.internal.Positions.Locator in Scala-2.11).
+   */
+  private class Locator(pos: Position) extends Traverser {
+    var last: Tree = _
+    def locateIn(root: Tree): Tree = {
+      this.last = EmptyTree
+      traverse(root)
+      this.last
+    }
+    protected def isEligible(t: Tree) = !t.pos.isTransparent
+    override def traverse(t: Tree) {
+      t match {
+        case tt : TypeTree if tt.original != null && (tt.pos includes tt.original.pos) =>
+          traverse(tt.original)
+        case _ =>
+          if (t.pos includes pos) {
+            if (isEligible(t)) last = t
+            super.traverse(t)
+          } else t match {
+            case mdef: MemberDef =>
+              traverseTrees(mdef.mods.annotations)
+            case _ =>
+          }
+      }
+    }
+  }
+}

org.scala-refactoring.library/src/test/scala/scala/tools/refactoring/tests/analysis/CompilationUnitDependenciesTest.scala

@@ -834,5 +834,24 @@ class CompilationUnitDependenciesTest extends TestHelper with CompilationUnitDep
         def xx(i: Int) = i
       }
     """)
+
+  @Test
+  def importLocallyDefinedClass() = assertNeededImports(
+    """test.MyType""",
+    """import test.MyType
+       package test {
+          class MyType
+       }
+       class Test(myType: MyType)
+    }""")
+
+  @Test
+  def importLocallyDefniedClassNotNeeded = assertNeededImports(
+    """""",
+    """package test {
+          class MyType
+          class Test(myType: MyType)
+       }
+    }""")
 }