Fix #199 by distinguishing term imports, type imports, and term+type …

…imports

repl/src/main/scala/ammonite/repl/Util.scala

@@ -75,7 +75,14 @@ case class Evaluated(wrapper: String,
 
 case class ImportData(fromName: String,
                       toName: String,
-                      prefix: String)
+                      prefix: String,
+                      importType: ImportData.ImportType)
+object ImportData{
+  case class ImportType(name: String)
+  val Type = ImportType("Type")
+  val Term = ImportType("Term")
+  val TermType = ImportType("TermType")
+}
 
 /**
  * Encapsulates a read-write cell that can be passed around

repl/src/main/scala/ammonite/repl/interp/AmmonitePlugin.scala

@@ -33,12 +33,12 @@ object AmmonitePlugin{
 
     def decode(t: g.Tree) = {
       val sym = t.symbol
-      (sym.decodedName, sym.decodedName, "")
+      (sym.isType, sym.decodedName, sym.decodedName, "")
     }
 
     val stats = unit.body.children.last.asInstanceOf[g.ModuleDef].impl.body
     val symbols = stats.filter(x => !Option(x.symbol).exists(_.isPrivate))
-                       .foldLeft(List.empty[(String, String, String)]){
+                       .foldLeft(List.empty[(Boolean, String, String, String)]){
       // These are all the ways we want to import names from previous
       // executions into the current one. Most are straightforward, except
       // `import` statements for which we make use of the typechecker to
@@ -54,7 +54,7 @@ object AmmonitePlugin{
         val prefix = rec(expr).reverseMap(x => Parsers.backtickWrap(x.decoded)).mkString(".")
         val renamings =
           for(t @ g.ImportSelector(name, _, rename, _) <- selectors) yield {
-            Option(rename).map(name.decoded -> _.decoded)
+            Option(rename).map(x => name.decoded -> (name.isTypeName, x.decoded))
           }
         val renameMap = renamings.flatten.map(_.swap).toMap
         val info = new g.analyzer.ImportInfo(t, 0)
@@ -67,7 +67,9 @@ object AmmonitePlugin{
           if sym.toString != "package class-use"
           if sym.toString != "object package-info"
           if sym.toString != "class package-info"
-        } yield sym.decodedName
+        } yield {
+          (sym.isType, sym.decodedName)
+        }
 
         val syms = for{
           // For some reason `info.allImportedSymbols` does not show imported
@@ -77,9 +79,9 @@ object AmmonitePlugin{
           //
           // As opposed to via import scala.reflect.macros._.
           // Thus we need to combine allImportedSymbols with the renameMap
-          sym <- symNames.toList ++ renameMap.keys
+          (isType, sym) <- symNames.toList ++ renameMap.keys
         } yield {
-          (renameMap.getOrElse(sym, sym), sym, prefix)
+          (isType, renameMap.getOrElse((isType, sym), sym), sym, prefix)
         }
         syms ::: ctx
       case (ctx, t @ g.DefDef(_, _, _, _, _, _))  => decode(t) :: ctx
@@ -90,21 +92,34 @@ object AmmonitePlugin{
       case (ctx, t) => ctx
     }
 
-    output(
-      for {
-        (fromName, toName, importString) <- symbols
-        //              _ = println(fromName + "\t"+ toName)
+    val grouped =
+      symbols.distinct
+             .groupBy{case (a, b, c, d) => (b, c, d) }
+             .mapValues(_.map(_._1))
+
+//    pprint.log(grouped, "grouped")
+    val open = for {
+      ((fromName, toName, importString), items) <- grouped
+      //              _ = println(fromName + "\t"+ toName)
 
-        // Probably synthetic
-        if !fromName.contains("$")
-        if fromName != "<init>"
-        if fromName != "<clinit>"
-        if fromName != "$main"
-        // Don't care about this
-        if fromName != "toString"
-        // Behaves weird in 2.10.x, better to just ignore.
-        if fromName != "_"
-      } yield ImportData(fromName, toName, importString)
-    )
+      // Probably synthetic
+      if !fromName.contains("$")
+      if fromName != "<init>"
+      if fromName != "<clinit>"
+      if fromName != "$main"
+      // Don't care about this
+      if fromName != "toString"
+      // Behaves weird in 2.10.x, better to just ignore.
+      if fromName != "_"
+    } yield {
+      val importType = items match{
+        case Seq(true) => ImportData.Type
+        case Seq(false) => ImportData.Term
+        case Seq(_, _) => ImportData.TermType
+      }
+      ImportData(fromName, toName, importString, importType)
+    }
+//    pprint.log(open, "open")
+    output(open.toVector)
   }
 }

repl/src/main/scala/ammonite/repl/interp/ClassLoaders.scala

@@ -20,13 +20,19 @@ object Frame{
     //
     // At the end of the day we re-reverse the trimmed list and return it.
     val importData = importss.flatten
-    val stomped = mutable.Set.empty[String]
+    val stompedTypes = mutable.Set.empty[String]
+    val stompedTerms = mutable.Set.empty[String]
     val out = mutable.Buffer.empty[ImportData]
     for(data <- importData.reverseIterator){
-      if(!stomped(data.toName)) {
+      val stomped = data.importType match{
+        case ImportData.Term => Seq(stompedTerms)
+        case ImportData.Type => Seq(stompedTypes)
+        case ImportData.TermType => Seq(stompedTerms, stompedTypes)
+      }
+      if (!stomped.exists(_(data.toName))){
         out.append(data)
-        stomped.add(data.toName)
-        stomped.remove(data.prefix)
+        stomped.foreach(_.add(data.toName))
+        stompedTerms.remove(data.prefix)
       }
     }
     out.reverse

repl/src/main/scala/ammonite/repl/interp/Evaluator.scala

@@ -170,10 +170,15 @@ object Evaluator{
       val grouped = mutable.Buffer[mutable.Buffer[ImportData]]()
       for(data <- importData){
         if (grouped.isEmpty) grouped.append(mutable.Buffer(data))
-        else if (grouped.last.last.prefix == data.prefix) grouped.last.append(data)
-        else grouped.append(mutable.Buffer(data))
+        else {
+          val last = grouped.last.last
+          // Group type imports differently from term imports, in case we
+          // import the same thing but rename it differently
+          if (last.prefix == data.prefix) grouped.last.append(data)
+          else grouped.append(mutable.Buffer(data))
+        }
       }
-
+//      pprint.log(grouped)
       // Stringify everything
       val out = for(group <- grouped) yield {
         val printedGroup = for(item <- group) yield{
@@ -183,6 +188,7 @@ object Evaluator{
         "import " + group.head.prefix + ".{\n  " + printedGroup.mkString(",\n  ") + "\n}\n"
       }
       val res = out.mkString
+
       Timer("importBlock 1")
       res
     }

repl/src/test/scala/ammonite/repl/session/ImportTests.scala

@@ -120,7 +120,7 @@ object ImportTests extends TestSuite{
         """)
       }
       'shadowPrefix{
-        check.session("""
+        * - check.session("""
           @ object importing_issue {
           @   object scala {
           @     def evilThing = ???
@@ -133,20 +133,19 @@ object ImportTests extends TestSuite{
 
           @ import importing_issue._
 
-          @ Duration.Inf
+          @ "foo" // Make sure this still works
+          res4: String = "foo"
 
-          @ "foo"
-          res5: String = "foo"
+          @ Duration.Inf // This fails due to a scala compiler bug
+          error: Compilation Failed
         """)
-      }
-      'shadowPrefix2{
-        // This failed kinda non-deterministically in 0.5.2; it depended
+        // This failed kinda non-deterministically in 0.5.2 (#248); it depended
         // on what ordering the imports were generated in, which depended
         // on the ordering of the `scala.Map` they were stored in, which
         // is arbitrary. Choosing different identifiers for `foo` `bar` and
         // `baz` affected this ordering and whether or not it fail.
         // Nevertheless, here's a test case that used to fail, but now doesn't
-        check.session("""
+        * - check.session("""
           @ object baz { val foo = 1 }
 
           @ object foo { val bar = 2 }
@@ -158,6 +157,71 @@ object ImportTests extends TestSuite{
           @ bar
           res4: Int = 2
         """)
+
+      }
+
+      'typeTermSeparation{
+        // Make sure that you can have a term and a type of the same name
+        // coming from different places and they don't stomp over each other
+        // (#199) and both are accessible.
+        * - check.session("""
+          @ val Foo = 1
+
+          @ type Foo = Int
+
+          @ Foo
+          res2: Int = 1
+
+          @ 2: Foo
+          res3: Foo = 2
+        """)
+
+        * - check.session("""
+          @ object pkg1{ val Order = "lolz" }
+
+          @ object pkg2{ type Order[+T] = Seq[T] }
+
+          @ import pkg1._
+
+          @ Order
+          res3: String = "lolz"
+
+          @ import pkg2._
+
+          @ Seq(1): Order[Int]
+          res5: Order[Int] = List(1)
+
+          @ Seq(Order): Order[String]
+          res6: Order[String] = List("lolz")
+        """)
+
+        // Even though you can import the same-named type and term from different
+        // places and have it work, if you import them both from the same place,
+        // a single type or term of the same name will stomp over both of them.
+        //
+        // This is basically impossible to avoid in Scala unless we want to
+        // generate forwarder objects to import from which is very difficult
+        // (e.g. we would need to generate forwarder-methods for arbitrarily
+        // complex method signatures) or generate ever-more-nested wrapper
+        // objects for imports to make the later imports take priority (which
+        // results in a quadratic number of class files)
+        //
+        // This is sufficiently edge-casey that I'm gonna call this a wontfix
+        * - check.session("""
+          @ object bar { val foo = 1; type foo = Int }
+
+          @ object baz { val foo = 2 }
+
+          @ import bar.foo
+
+          @ import baz.foo
+
+          @ foo
+          res4: Int = 2
+
+          @ 1: foo
+          error: Compilation Failed
+        """)
       }
     }
   }