From 99afa56b8bc4d6369876be971284d71572ce112f Mon Sep 17 00:00:00 2001
From: "P. Oscar Boykin" <johnynek@users.noreply.github.com>
Date: Sat, 11 May 2024 13:46:26 -1000
Subject: [PATCH] Improve duplicate imports (#1196)

---
 .../org/bykn/bosatsu/TypedExprToProto.scala   |  19 ++-
 .../org/bykn/bosatsu/TestProtoType.scala      |   6 +-
 .../scala/org/bykn/bosatsu/Evaluation.scala   |  12 +-
 .../main/scala/org/bykn/bosatsu/Import.scala  |  93 +-----------
 .../scala/org/bykn/bosatsu/ImportMap.scala    |  74 ++++++++++
 .../scala/org/bykn/bosatsu/ImportedName.scala |  64 +++++++++
 .../scala/org/bykn/bosatsu/MainModule.scala   |   2 +-
 .../bykn/bosatsu/MatchlessFromTypedExpr.scala |   2 +-
 .../scala/org/bykn/bosatsu/NameKind.scala     |  12 +-
 .../main/scala/org/bykn/bosatsu/Package.scala |  54 ++++---
 .../org/bykn/bosatsu/PackageCustoms.scala     |  11 +-
 .../scala/org/bykn/bosatsu/PackageError.scala |  66 ++++++---
 .../scala/org/bykn/bosatsu/PackageMap.scala   | 123 +++++++++++-----
 .../org/bykn/bosatsu/EvaluationTest.scala     | 136 +++++++++++++-----
 .../src/test/scala/org/bykn/bosatsu/Gen.scala |   6 +-
 .../org/bykn/bosatsu/MatchlessTests.scala     |   8 +-
 .../scala/org/bykn/bosatsu/Regressions.scala  |   2 +-
 .../scala/org/bykn/bosatsu/TestUtils.scala    |   4 +-
 18 files changed, 457 insertions(+), 237 deletions(-)
 create mode 100644 core/src/main/scala/org/bykn/bosatsu/ImportMap.scala
 create mode 100644 core/src/main/scala/org/bykn/bosatsu/ImportedName.scala

diff --git a/cli/src/main/scala/org/bykn/bosatsu/TypedExprToProto.scala b/cli/src/main/scala/org/bykn/bosatsu/TypedExprToProto.scala
index 3e66c8909..9ebc8a9f4 100644
--- a/cli/src/main/scala/org/bykn/bosatsu/TypedExprToProto.scala
+++ b/cli/src/main/scala/org/bykn/bosatsu/TypedExprToProto.scala
@@ -1552,7 +1552,7 @@ object ProtoConverter {
     // the Int is in index in the list of definedTypes:
     val allDts
         : SortedMap[(PackageName, TypeName), (DefinedType[Kind.Arg], Int)] =
-      cpack.program.types.definedTypes.mapWithIndex((dt, idx) => (dt, idx))
+      cpack.types.definedTypes.mapWithIndex((dt, idx) => (dt, idx))
     val dtVect: Vector[DefinedType[Kind.Arg]] =
       allDts.values.iterator.map(_._1).toVector
     val tab =
@@ -1560,10 +1560,9 @@ object ProtoConverter {
         nmId <- getId(cpack.name.asString)
         imps <- cpack.imports.traverse(importToProto(allDts, _))
         exps <- cpack.exports.traverse(expNameToProto(allDts, _))
-        prog = cpack.program
-        lets <- prog.lets.traverse(letToProto)
-        exdefs <- prog.externalDefs.traverse { nm =>
-          extDefToProto(nm, prog.types.getValue(cpack.name, nm))
+        lets <- cpack.lets.traverse(letToProto)
+        exdefs <- cpack.externalDefs.traverse { nm =>
+          extDefToProto(nm, cpack.types.getValue(cpack.name, nm))
         }
         dts <- dtVect.traverse(definedTypeToProto)
       } yield { (ss: SerState) =>
@@ -1774,7 +1773,7 @@ object ProtoConverter {
               case Left((_, dt)) =>
                 dt.toDefinedType(tc)
               case Right(comp) =>
-                comp.program.types.toDefinedType(tc)
+                comp.types.toDefinedType(tc)
             }
 
             res.flatMap {
@@ -1816,13 +1815,19 @@ object ProtoConverter {
               imps <- pack.imports.toList.traverse(
                 importsFromProto(_, loadIface, loadDT)
               )
+              impMap <- ReaderT.liftF(
+                ImportMap.fromImports(imps)((_, _) => ImportMap.Unify.Error) match {
+                  case (Nil, im) => Success(im)
+                  case (nel, _) =>
+                    Failure(new Exception(s"duplicated imports in package: $nel"))
+                })
               exps <- pack.exports.toList.traverse(
                 exportedNameFromProto(loadDT, _)
               )
               lets <- pack.lets.toList.traverse(letsFromProto)
               eds <- pack.externalDefs.toList.traverse(externalDefsFromProto)
               prog <- buildProgram(packageName, lets, eds)
-            } yield Package(packageName, imps, exps, prog)
+            } yield Package(packageName, imps, exps, (prog, impMap))
 
           // build up the decoding state by decoding the tables in order
           val tab1 = Scoped.run(
diff --git a/cli/src/test/scala/org/bykn/bosatsu/TestProtoType.scala b/cli/src/test/scala/org/bykn/bosatsu/TestProtoType.scala
index 0dbd95ff1..68a70cc2a 100644
--- a/cli/src/test/scala/org/bykn/bosatsu/TestProtoType.scala
+++ b/cli/src/test/scala/org/bykn/bosatsu/TestProtoType.scala
@@ -46,12 +46,12 @@ class TestProtoType extends AnyFunSuite with ParTest {
         .map(_._2)
         .getOrElse(0)
 
-    val context = 100
+    val context = 1000
     assert(
       Eq[A].eqv(a, orig),
       s"${a.toString.drop(diffIdx - context / 2).take(context)} != ${orig.toString.drop(diffIdx - context / 2).take(context)}"
     )
-    // assert(Eq[A].eqv(a, orig), s"$a\n\n!=\n\n$orig")
+    //assert(Eq[A].eqv(a, orig), s"$a\n\n!=\n\n$orig")
   }
 
   def testWithTempFile(fn: Path => IO[Unit]): Unit = {
@@ -59,7 +59,7 @@ class TestProtoType extends AnyFunSuite with ParTest {
       val f = File.createTempFile("proto_test", ".proto")
       f.toPath
     }) { path =>
-      IO {
+      IO.blocking {
         val _ = path.toFile.delete
         ()
       }
diff --git a/core/src/main/scala/org/bykn/bosatsu/Evaluation.scala b/core/src/main/scala/org/bykn/bosatsu/Evaluation.scala
index 7a2368c49..fc40e1868 100644
--- a/core/src/main/scala/org/bykn/bosatsu/Evaluation.scala
+++ b/core/src/main/scala/org/bykn/bosatsu/Evaluation.scala
@@ -16,9 +16,9 @@ case class Evaluation[T](pm: PackageMap.Typed[T], externals: Externals) {
     MMap.empty
 
   private def externalEnv(p: Package.Typed[T]): Map[Identifier, Eval[Value]] = {
-    val externalNames = p.program.externalDefs
+    val externalNames = p.externalDefs
     externalNames.iterator.map { n =>
-      val tpe = p.program.types.getValue(p.name, n) match {
+      val tpe = p.types.getValue(p.name, n) match {
         case Some(t) => t
         case None    =>
           // $COVERAGE-OFF$
@@ -70,7 +70,7 @@ case class Evaluation[T](pm: PackageMap.Typed[T], externals: Externals) {
     envCache.getOrElseUpdate(
       packName, {
         val pack = pm.toMap(packName)
-        externalEnv(pack) ++ evalLets(packName, pack.program.lets)
+        externalEnv(pack) ++ evalLets(packName, pack.lets)
       }
     )
 
@@ -81,7 +81,7 @@ case class Evaluation[T](pm: PackageMap.Typed[T], externals: Externals) {
   ): Option[(Eval[Value], Type)] =
     for {
       pack <- pm.toMap.get(p)
-      (_, _, tpe) <- pack.program.lets.filter { case (n, _, _) =>
+      (_, _, tpe) <- pack.lets.filter { case (n, _, _) =>
         n == name
       }.lastOption
       value <- evaluate(p).get(name)
@@ -133,7 +133,7 @@ case class Evaluation[T](pm: PackageMap.Typed[T], externals: Externals) {
   val valueToJson: ValueToJson = ValueToJson { case Type.Const.Defined(pn, t) =>
     for {
       pack <- pm.toMap.get(pn)
-      dt <- pack.program.types.getType(pn, t)
+      dt <- pack.types.getType(pn, t)
     } yield dt
   }
 
@@ -143,7 +143,7 @@ case class Evaluation[T](pm: PackageMap.Typed[T], externals: Externals) {
   val valueToDoc: ValueToDoc = ValueToDoc { case Type.Const.Defined(pn, t) =>
     for {
       pack <- pm.toMap.get(pn)
-      dt <- pack.program.types.getType(pn, t)
+      dt <- pack.types.getType(pn, t)
     } yield dt
   }
 }
diff --git a/core/src/main/scala/org/bykn/bosatsu/Import.scala b/core/src/main/scala/org/bykn/bosatsu/Import.scala
index 8a11e3f6c..41536d168 100644
--- a/core/src/main/scala/org/bykn/bosatsu/Import.scala
+++ b/core/src/main/scala/org/bykn/bosatsu/Import.scala
@@ -1,71 +1,12 @@
 package org.bykn.bosatsu
 
-import cats.{Foldable, Functor}
+import cats.Foldable
 import cats.data.NonEmptyList
-import cats.implicits._
 import cats.parse.{Parser => P}
+import cats.syntax.all._
 import org.typelevel.paiges.{Doc, Document}
-
 import Parser.{spaces, Combinators}
 
-sealed abstract class ImportedName[+T] {
-  def originalName: Identifier
-  def localName: Identifier
-  def tag: T
-  def isRenamed: Boolean = originalName != localName
-  def withTag[U](tag: U): ImportedName[U]
-
-  def map[U](fn: T => U): ImportedName[U] =
-    this match {
-      case ImportedName.OriginalName(n, t) =>
-        ImportedName.OriginalName(n, fn(t))
-      case ImportedName.Renamed(o, l, t) =>
-        ImportedName.Renamed(o, l, fn(t))
-    }
-
-  def traverse[F[_], U](
-      fn: T => F[U]
-  )(implicit F: Functor[F]): F[ImportedName[U]] =
-    this match {
-      case ImportedName.OriginalName(n, t) =>
-        F.map(fn(t))(ImportedName.OriginalName(n, _))
-      case ImportedName.Renamed(o, l, t) =>
-        F.map(fn(t))(ImportedName.Renamed(o, l, _))
-    }
-}
-
-object ImportedName {
-  case class OriginalName[T](originalName: Identifier, tag: T)
-      extends ImportedName[T] {
-    def localName = originalName
-    def withTag[U](tag: U): ImportedName[U] = copy(tag = tag)
-  }
-  case class Renamed[T](originalName: Identifier, localName: Identifier, tag: T)
-      extends ImportedName[T] {
-    def withTag[U](tag: U): ImportedName[U] = copy(tag = tag)
-  }
-
-  implicit val document: Document[ImportedName[Unit]] =
-    Document.instance[ImportedName[Unit]] {
-      case ImportedName.OriginalName(nm, _) => Document[Identifier].document(nm)
-      case ImportedName.Renamed(from, to, _) =>
-        Document[Identifier].document(from) + Doc.text(" as ") +
-          Document[Identifier].document(to)
-    }
-
-  val parser: P[ImportedName[Unit]] = {
-    def basedOn(of: P[Identifier]): P[ImportedName[Unit]] =
-      (of ~ (spaces.soft *> P.string("as") *> spaces *> of).?)
-        .map {
-          case (from, Some(to)) => ImportedName.Renamed(from, to, ())
-          case (orig, None)     => ImportedName.OriginalName(orig, ())
-        }
-
-    basedOn(Identifier.bindableParser)
-      .orElse(basedOn(Identifier.consParser))
-  }
-}
-
 case class Import[A, B](pack: A, items: NonEmptyList[ImportedName[B]]) {
   def resolveToGlobal: Map[Identifier, (A, Identifier)] =
     items.foldLeft(Map.empty[Identifier, (A, Identifier)]) {
@@ -121,33 +62,3 @@ object Import {
     }
   }
 }
-
-/** There are all the distinct imported names and the original ImportedName
-  */
-case class ImportMap[A, B](toMap: Map[Identifier, (A, ImportedName[B])]) {
-  def apply(name: Identifier): Option[(A, ImportedName[B])] =
-    toMap.get(name)
-
-  def +(that: (A, ImportedName[B])): ImportMap[A, B] =
-    ImportMap(toMap.updated(that._2.localName, that))
-}
-
-object ImportMap {
-  def empty[A, B]: ImportMap[A, B] = ImportMap(Map.empty)
-  // Return the list of collisions in local names along with a map
-  // with the last name overwriting the import
-  def fromImports[A, B](
-      is: List[Import[A, B]]
-  ): (List[(A, ImportedName[B])], ImportMap[A, B]) =
-    is.iterator
-      .flatMap { case Import(p, is) => is.toList.iterator.map((p, _)) }
-      .foldLeft((List.empty[(A, ImportedName[B])], ImportMap.empty[A, B])) {
-        case ((dups, imap), pim @ (_, im)) =>
-          val dups1 = imap(im.localName) match {
-            case Some(nm) => nm :: dups
-            case None     => dups
-          }
-
-          (dups1, imap + pim)
-      }
-}
diff --git a/core/src/main/scala/org/bykn/bosatsu/ImportMap.scala b/core/src/main/scala/org/bykn/bosatsu/ImportMap.scala
new file mode 100644
index 000000000..37333b30d
--- /dev/null
+++ b/core/src/main/scala/org/bykn/bosatsu/ImportMap.scala
@@ -0,0 +1,74 @@
+package org.bykn.bosatsu
+
+import cats.{Applicative, Functor, Order, Parallel}
+import scala.collection.immutable.SortedMap
+
+import cats.syntax.all._
+
+/** There are all the distinct imported names and the original ImportedName
+  */
+case class ImportMap[A, B](toMap: SortedMap[Identifier, (A, ImportedName[B])]) {
+  def apply(name: Identifier): Option[(A, ImportedName[B])] =
+    toMap.get(name)
+
+  def +(that: (A, ImportedName[B])): ImportMap[A, B] =
+    ImportMap(toMap.updated(that._2.localName, that))
+
+  def toList(implicit ord: Order[A]): List[Import[A, B]] =
+    toMap.iterator
+      .map { case (_, ab) => ab }
+      .toList
+      .groupByNel(_._1)
+      .iterator
+      .map { case (a, bs) =>
+        Import(a, bs.map(_._2))       
+      }
+      .toList
+
+  def traverse[F[_]: Applicative, C, D](fn: (A, ImportedName[B]) => F[(C, ImportedName[D])]): F[ImportMap[C, D]] =
+    toMap.traverse[F, (C, ImportedName[D])] { case (a, ib) => fn(a, ib) }
+      .map(ImportMap(_))
+
+  def parTraverse[F[_]: Parallel: Functor, C, D](fn: (A, ImportedName[B]) => F[(C, ImportedName[D])]): F[ImportMap[C, D]] =
+    toMap.parTraverse[F, (C, ImportedName[D])] { case (a, ib) => fn(a, ib) }
+      .map(ImportMap(_))
+}
+
+object ImportMap {
+  def empty[A, B]: ImportMap[A, B] = ImportMap(SortedMap.empty)
+
+  sealed abstract class Unify
+  object Unify {
+    case object Error extends Unify
+    case object Left extends Unify
+    case object Right extends Unify
+  }
+  // Return the list of collisions in local names along with a map
+  // with the last name overwriting the import
+  def fromImports[A, B](
+      is: List[Import[A, B]]
+  )(unify: ((A, ImportedName[B]), (A, ImportedName[B])) => Unify): (List[(A, ImportedName[B])], ImportMap[A, B]) =
+    is.iterator
+      .flatMap { case Import(p, is) => is.toList.iterator.map((p, _)) }
+      .foldLeft((List.empty[(A, ImportedName[B])], ImportMap.empty[A, B])) {
+        case (old @ (dups, imap), pim @ (_, im)) =>
+          val (dups1, imap1) = imap(im.localName) match {
+            case Some(nm) =>
+              unify(nm, pim) match {
+                case Unify.Error =>
+                  // pim and nm are a collision, add both
+                  (pim :: nm :: dups, imap + pim)
+                case Unify.Left => old
+                case Unify.Right => (dups, imap + pim)
+              }
+            case None     => (dups, imap + pim)
+          }
+
+          (dups1.reverse.distinct, imap1)
+      }
+
+  // This is only safe after verifying there are not collisions
+  // which has been done on compiled packages
+  def fromImportsUnsafe[A, B](is: List[Import[A, B]]): ImportMap[A, B] =
+    fromImports(is)((a, b) => sys.error(s"collision in $a and $b: $is"))._2
+}
\ No newline at end of file
diff --git a/core/src/main/scala/org/bykn/bosatsu/ImportedName.scala b/core/src/main/scala/org/bykn/bosatsu/ImportedName.scala
new file mode 100644
index 000000000..90883869c
--- /dev/null
+++ b/core/src/main/scala/org/bykn/bosatsu/ImportedName.scala
@@ -0,0 +1,64 @@
+package org.bykn.bosatsu
+
+import cats.Functor
+import cats.parse.{Parser => P}
+import org.typelevel.paiges.{Doc, Document}
+import Parser.spaces
+
+sealed abstract class ImportedName[+T] {
+  def originalName: Identifier
+  def localName: Identifier
+  def tag: T
+  def isRenamed: Boolean = originalName != localName
+  def withTag[U](tag: U): ImportedName[U]
+
+  def map[U](fn: T => U): ImportedName[U] =
+    this match {
+      case ImportedName.OriginalName(n, t) =>
+        ImportedName.OriginalName(n, fn(t))
+      case ImportedName.Renamed(o, l, t) =>
+        ImportedName.Renamed(o, l, fn(t))
+    }
+
+  def traverse[F[_], U](
+      fn: T => F[U]
+  )(implicit F: Functor[F]): F[ImportedName[U]] =
+    this match {
+      case ImportedName.OriginalName(n, t) =>
+        F.map(fn(t))(ImportedName.OriginalName(n, _))
+      case ImportedName.Renamed(o, l, t) =>
+        F.map(fn(t))(ImportedName.Renamed(o, l, _))
+    }
+}
+
+object ImportedName {
+  case class OriginalName[T](originalName: Identifier, tag: T)
+      extends ImportedName[T] {
+    def localName = originalName
+    def withTag[U](tag: U): ImportedName[U] = copy(tag = tag)
+  }
+  case class Renamed[T](originalName: Identifier, localName: Identifier, tag: T)
+      extends ImportedName[T] {
+    def withTag[U](tag: U): ImportedName[U] = copy(tag = tag)
+  }
+
+  implicit val document: Document[ImportedName[Unit]] =
+    Document.instance[ImportedName[Unit]] {
+      case ImportedName.OriginalName(nm, _) => Document[Identifier].document(nm)
+      case ImportedName.Renamed(from, to, _) =>
+        Document[Identifier].document(from) + Doc.text(" as ") +
+          Document[Identifier].document(to)
+    }
+
+  val parser: P[ImportedName[Unit]] = {
+    def basedOn(of: P[Identifier]): P[ImportedName[Unit]] =
+      (of ~ (spaces.soft *> P.string("as") *> spaces *> of).?)
+        .map {
+          case (from, Some(to)) => ImportedName.Renamed(from, to, ())
+          case (orig, None)     => ImportedName.OriginalName(orig, ())
+        }
+
+    basedOn(Identifier.bindableParser)
+      .orElse(basedOn(Identifier.consParser))
+  }
+}
diff --git a/core/src/main/scala/org/bykn/bosatsu/MainModule.scala b/core/src/main/scala/org/bykn/bosatsu/MainModule.scala
index a11a83b0f..2f89babf4 100644
--- a/core/src/main/scala/org/bykn/bosatsu/MainModule.scala
+++ b/core/src/main/scala/org/bykn/bosatsu/MainModule.scala
@@ -654,7 +654,7 @@ abstract class MainModule[IO[_]](implicit
               ).get
 
               val evalMap = pm.toMap.iterator.flatMap { case (n, p) =>
-                val optEval = p.program.lets.findLast { case (_, _, te) =>
+                val optEval = p.lets.findLast { case (_, _, te) =>
                   typeEvalMap.contains(te.getType)
                 }
                 optEval.map { case (b, _, te) =>
diff --git a/core/src/main/scala/org/bykn/bosatsu/MatchlessFromTypedExpr.scala b/core/src/main/scala/org/bykn/bosatsu/MatchlessFromTypedExpr.scala
index dd269e719..a467cecbf 100644
--- a/core/src/main/scala/org/bykn/bosatsu/MatchlessFromTypedExpr.scala
+++ b/core/src/main/scala/org/bykn/bosatsu/MatchlessFromTypedExpr.scala
@@ -17,7 +17,7 @@ object MatchlessFromTypedExpr {
     val allItemsList = pm.toMap.toList
       .traverse[Par.F, (PackageName, List[(Bindable, Matchless.Expr)])] {
         case (pname, pack) =>
-          val lets = pack.program.lets
+          val lets = pack.lets
 
           Par.start {
             val exprs: List[(Bindable, Matchless.Expr)] =
diff --git a/core/src/main/scala/org/bykn/bosatsu/NameKind.scala b/core/src/main/scala/org/bykn/bosatsu/NameKind.scala
index 9932cdc9c..c6134d592 100644
--- a/core/src/main/scala/org/bykn/bosatsu/NameKind.scala
+++ b/core/src/main/scala/org/bykn/bosatsu/NameKind.scala
@@ -23,16 +23,14 @@ object NameKind {
       defType: rankn.Type
   ) extends NameKind[T]
 
-  def externals[T](from: Package.Typed[T]): Iterable[ExternalDef[T]] = {
-    val prog = from.program
-    prog.externalDefs.to(LazyList).map { n =>
+  def externals[T](from: Package.Typed[T]): Iterable[ExternalDef[T]] =
+    from.externalDefs.to(LazyList).map { n =>
       // The type could be an import, so we need to check for the type
       // in the TypeEnv
       val pn = from.name
-      val tpe = prog.types.getExternalValue(pn, n).get
+      val tpe = from.types.getExternalValue(pn, n).get
       ExternalDef[T](pn, n, tpe)
     }
-  }
 
   def apply[T](
       from: Package.Typed[T],
@@ -42,12 +40,12 @@ object NameKind {
 
     def getLet: Option[NameKind[T]] =
       item.toBindable.flatMap { b =>
-        prog.getLet(b).map { case (rec, d) => Let[T](b, rec, d) }
+        prog._1.getLet(b).map { case (rec, d) => Let[T](b, rec, d) }
       }
 
     def getConstructor: Option[NameKind[T]] =
       item.toConstructor.flatMap { cn =>
-        prog.types
+        prog._1.types
           .getConstructor(from.name, cn)
           .map { case (dt, cfn) =>
             Constructor(cn, cfn.args, dt, dt.fnTypeOf(cfn))
diff --git a/core/src/main/scala/org/bykn/bosatsu/Package.scala b/core/src/main/scala/org/bykn/bosatsu/Package.scala
index 707c44553..b25510920 100644
--- a/core/src/main/scala/org/bykn/bosatsu/Package.scala
+++ b/core/src/main/scala/org/bykn/bosatsu/Package.scala
@@ -1,6 +1,6 @@
 package org.bykn.bosatsu
 
-import cats.{Functor, Parallel}
+import cats.{Functor, Order, Parallel}
 import cats.data.{Ior, ValidatedNel, Validated, NonEmptyList}
 import cats.implicits._
 import cats.parse.{Parser0 => P0, Parser => P}
@@ -39,12 +39,6 @@ final case class Package[A, B, C, +D](
       case _ => false
     }
 
-  // TODO, this isn't great
-  private lazy val importMap: ImportMap[A, B] =
-    ImportMap.fromImports(imports)._2
-
-  def localImport(n: Identifier): Option[(A, ImportedName[B])] = importMap(n)
-
   def withImport(i: Import[A, B]): Package[A, B, C, D] =
     copy(imports = i :: imports)
 
@@ -72,7 +66,11 @@ object Package {
     FixPackage[Unit, Unit, (List[Statement], ImportMap[PackageName, Unit])]
   type Typed[T] = Package[Interface, NonEmptyList[Referant[Kind.Arg]], Referant[
     Kind.Arg
-  ], Program[TypeEnv[Kind.Arg], TypedExpr[T], Any]]
+  ], (
+    Program[TypeEnv[Kind.Arg], TypedExpr[T], Any],
+    ImportMap[Interface, NonEmptyList[Referant[Kind.Arg]]]
+    )
+  ]
   type Inferred = Typed[Declaration]
 
   type Header =
@@ -81,10 +79,11 @@ object Package {
   val typedFunctor: Functor[Typed] =
     new Functor[Typed] {
       def map[A, B](fa: Typed[A])(fn: A => B): Typed[B] = {
-        val mapLet = fa.program.lets.map { case (n, r, te) =>
+        val mapLet = fa.lets.map { case (n, r, te) =>
           (n, r, Functor[TypedExpr].map(te)(fn))
         }
-        fa.copy(program = fa.program.copy(lets = mapLet))
+        val (prog, imap) = fa.program
+        fa.copy(program = (prog.copy(lets = mapLet), imap))
       }
     }
 
@@ -93,14 +92,14 @@ object Package {
   def testValue[A](
       tp: Typed[A]
   ): Option[(Identifier.Bindable, RecursionKind, TypedExpr[A])] =
-    tp.program.lets.filter { case (_, _, te) =>
+    tp.lets.filter { case (_, _, te) =>
       te.getType == Type.TestType
     }.lastOption
 
   def mainValue[A](
       tp: Typed[A]
   ): Option[(Identifier.Bindable, RecursionKind, TypedExpr[A])] =
-    tp.program.lets.lastOption
+    tp.lets.lastOption
 
   /** Discard any top level values that are not referenced, exported, the final
     * test value, or the final expression
@@ -110,13 +109,13 @@ object Package {
   def discardUnused[A](tp: Typed[A]): Typed[A] = {
     val pinned: Set[Identifier] =
       tp.exports.iterator.map(_.name).toSet ++
-        tp.program.lets.lastOption.map(_._1) ++
+        tp.lets.lastOption.map(_._1) ++
         testValue(tp).map(_._1)
 
     def topLevels(s: Set[(PackageName, Identifier)]): Set[Identifier] =
       s.collect { case (p, i) if p === tp.name => i }
 
-    val letWithGlobals = tp.program.lets.map { case tup @ (_, _, te) =>
+    val letWithGlobals = tp.lets.map { case tup @ (_, _, te) =>
       (tup, topLevels(te.globals))
     }
 
@@ -136,7 +135,7 @@ object Package {
     val reachedLets = letWithGlobals.collect {
       case (tup @ (bn, _, _), _) if reached(bn) => tup
     }
-    tp.copy(program = tp.program.copy(lets = reachedLets))
+    tp.copy(program = (tp.program._1.copy(lets = reachedLets), tp.program._2))
   }
 
   def fix[A, B, C](p: PackageF[A, B, C]): FixPackage[A, B, C] =
@@ -155,7 +154,7 @@ object Package {
     inferred.mapProgram(_ => ()).replaceImports(Nil)
 
   def setProgramFrom[A, B](t: Typed[A], newFrom: B): Typed[A] =
-    t.copy(program = t.program.copy(from = newFrom))
+    t.copy(program = (t.program._1.copy(from = newFrom), t.program._2))
 
   implicit val document
       : Document[Package[PackageName, Unit, Unit, List[Statement]]] =
@@ -358,7 +357,7 @@ object Package {
               (fullTypeEnv, Program(typeEnv, lets, extDefs, stmts))
             }
             .left
-            .map(PackageError.TypeErrorIn(_, p))
+            .map(PackageError.TypeErrorIn(_, p, lets, theseExternals))
 
           val checkUnusedLets =
             lets
@@ -468,7 +467,7 @@ object Package {
           val tpes = Doc.text("types: ") + Doc
             .intercalate(
               Doc.comma + Doc.line,
-              pack.program.types.definedTypes.toList.map { case (_, t) =>
+              pack.types.definedTypes.toList.map { case (_, t) =>
                 Doc.text(t.name.ident.sourceCodeRepr)
               }
             )
@@ -478,7 +477,7 @@ object Package {
           val eqDoc = Doc.text(" = ")
           val exprs = Doc.intercalate(
             Doc.hardLine + Doc.hardLine,
-            pack.program.lets.map { case (n, _, te) =>
+            pack.lets.map { case (n, _, te) =>
               Doc.text(n.sourceCodeRepr) + eqDoc + te.repr
             }
           )
@@ -510,4 +509,21 @@ object Package {
           Doc.intercalate(Doc.hardLine, all)
         }.nested(4)
     }
+
+  implicit class TypedMethods[A](private val pack: Typed[A]) extends AnyVal {
+    def lets: List[(Identifier.Bindable, RecursionKind, TypedExpr[A])] =
+      pack.program._1.lets
+
+    def types: TypeEnv[Kind.Arg] =
+      pack.program._1.types
+
+    def externalDefs: List[Identifier.Bindable] =
+      pack.program._1.externalDefs
+
+    def localImport(n: Identifier): Option[(Package.Interface, ImportedName[NonEmptyList[Referant[Kind.Arg]]])] =
+      pack.program._2(n)
+  }
+
+  def orderByName[A, B, C, D]: Order[Package[A, B, C, D]] =
+    Order.by[Package[A, B, C, D], PackageName](_.name)
 }
diff --git a/core/src/main/scala/org/bykn/bosatsu/PackageCustoms.scala b/core/src/main/scala/org/bykn/bosatsu/PackageCustoms.scala
index bb654303a..ea1f0beff 100644
--- a/core/src/main/scala/org/bykn/bosatsu/PackageCustoms.scala
+++ b/core/src/main/scala/org/bykn/bosatsu/PackageCustoms.scala
@@ -57,7 +57,8 @@ object PackageCustoms {
           }
         }
 
-        pack.copy(imports = i)
+        val cleanImap = ImportMap.fromImportsUnsafe(i)
+        pack.copy(imports = i, program = (pack.program._1, cleanImap))
     }
 
   private type VSet = Set[(PackageName, Identifier)]
@@ -67,7 +68,7 @@ object PackageCustoms {
       pack: Package.Typed[A]
   ): Set[(PackageName, Identifier)] = {
     val usedValuesSt: VState[Unit] =
-      pack.program.lets.traverse_ { case (_, _, te) =>
+      pack.lets.traverse_ { case (_, _, te) =>
         TypedExpr.usedGlobals(te)
       }
 
@@ -112,7 +113,7 @@ object PackageCustoms {
       val usedValues = usedGlobals(pack)
 
       val usedTypes: Set[Type.Const] =
-        pack.program.lets.iterator
+        pack.program._1.lets.iterator
           .flatMap(
             _._3.allTypes.flatMap(Type.constantsOf(_))
           )
@@ -245,7 +246,7 @@ object PackageCustoms {
           .toList).distinct
 
     val bindMap: Map[Bindable, TypedExpr[A]] =
-      pack.program.lets.iterator.map { case (b, _, te) => (b, te) }.toMap
+      pack.program._1.lets.iterator.map { case (b, _, te) => (b, te) }.toMap
 
     def internalDeps(te: TypedExpr[A]): Set[Bindable] =
       TypedExpr.usedGlobals(te).runS(Set.empty).value.collect {
@@ -267,7 +268,7 @@ object PackageCustoms {
       }
     val canReach: SortedSet[Node] = Dag.transitiveSet(roots)(depsOf _)
 
-    val unused = pack.program.lets.filter { case (bn, _, _) =>
+    val unused = pack.lets.filter { case (bn, _, _) =>
       !canReach.contains(Right(bn))
     }
 
diff --git a/core/src/main/scala/org/bykn/bosatsu/PackageError.scala b/core/src/main/scala/org/bykn/bosatsu/PackageError.scala
index 92ef69c54..c6251a735 100644
--- a/core/src/main/scala/org/bykn/bosatsu/PackageError.scala
+++ b/core/src/main/scala/org/bykn/bosatsu/PackageError.scala
@@ -27,22 +27,31 @@ object PackageError {
       ident: Identifier,
       existing: Iterable[(Identifier, A)],
       count: Int
-  ): List[(Identifier, A)] =
-    existing.iterator
+  ): List[(Identifier, A)] = {
+    val istr = ident.asString
+    val prefixes =
+      existing.iterator.filter { case (i, _) =>
+        i.asString.startsWith(istr)
+      }
+      .toList
+
+    val close = existing.iterator
       .map { case (i, a) =>
-        val d = EditDistance.string(ident.asString, i.asString)
+        val d = EditDistance.string(istr, i.asString)
         (i, d, a)
       }
       .filter { case (i, d, _) =>
         // don't show things that require total edits
-        (d < ident.asString.length) && (d < i.asString.length)
+        (d < istr.length) && (d < i.asString.length)
       }
       .toList
       .sortBy { case (_, d, _) => d }
-      .distinct
       .take(count)
       .map { case (i, _, a) => (i, a) }
 
+      (prefixes.sortBy(_._1) ::: close).distinct
+    }
+
   private val emptyLocMap = LocationMap("")
 
   type SourceMap = Map[PackageName, (LocationMap, String)]
@@ -131,21 +140,30 @@ object PackageError {
   }
 
   case class DuplicatedImport(
+      inPackage: PackageName,
       duplicates: NonEmptyList[(PackageName, ImportedName[Unit])]
   ) extends PackageError {
     def message(
         sourceMap: Map[PackageName, (LocationMap, String)],
         errColor: Colorize
-    ) =
+    ) = {
+      val (_, sourceName) = sourceMap.getMapSrc(inPackage)
+      val first = s"duplicate import in $sourceName package ${inPackage.asString}"
+
       duplicates
         .sortBy(_._2.localName)
         .toList
         .iterator
         .map { case (pack, imp) =>
-          val (_, sourceName) = sourceMap.getMapSrc(pack)
-          s"duplicate import in $sourceName package ${pack.asString} imports ${imp.originalName.sourceCodeRepr} as ${imp.localName.sourceCodeRepr}"
+          if (imp.isRenamed) {
+            s"\tfrom ${pack.asString} import ${imp.originalName.sourceCodeRepr} as ${imp.localName.sourceCodeRepr}" 
+          }
+          else {
+            s"\tfrom ${pack.asString} import ${imp.originalName.sourceCodeRepr}" 
+          }
         }
-        .mkString("\n")
+        .mkString(first + "\n", "\n", "\n")
+      }
   }
 
   // We could check if we forgot to export the name in the package and give that error
@@ -168,9 +186,13 @@ object PackageError {
         case Some(_) =>
           s"in $sourceName package: ${ipname.asString} has ${iname.originalName.sourceCodeRepr} but it is not exported. Add to exports"
         case None =>
-          val near = nearest(iname.originalName, letMap, 3)
+          val cands = nearest(iname.originalName, letMap, 3)
             .map { case (n, _) => n.sourceCodeRepr }
-            .mkString(" Nearest: ", ", ", "")
+
+          val near =
+            if (cands.nonEmpty) cands.mkString(" Nearest: ", ", ", "")
+            else ""
+
           s"in $sourceName package: ${ipname.asString} does not have name ${iname.originalName.sourceCodeRepr}.$near"
       }
     }
@@ -190,13 +212,13 @@ object PackageError {
 
       val exportMap = exportNames.map(e => (e, ())).toMap
 
+      val candidates =
+        nearest(iname.originalName, exportMap, 3)
+          .map(ident => Doc.text(ident._1.sourceCodeRepr))
+
       val near = Doc.text(" Nearest: ") +
         (Doc
-          .intercalate(
-            Doc.text(",") + Doc.line,
-            nearest(iname.originalName, exportMap, 3)
-              .map(ident => Doc.text(ident._1.sourceCodeRepr))
-          )
+          .intercalate(Doc.text(",") + Doc.line, candidates)
           .nested(4)
           .grouped)
 
@@ -238,7 +260,9 @@ object PackageError {
         .mkString(", ")
   }
 
-  case class TypeErrorIn(tpeErr: Infer.Error, pack: PackageName)
+  case class TypeErrorIn(tpeErr: Infer.Error, pack: PackageName,
+    lets: List[(Identifier.Bindable, RecursionKind, Expr[Declaration])],
+    externals: Map[Identifier.Bindable, (Type, Region)])
       extends PackageError {
     def message(
         sourceMap: Map[PackageName, (LocationMap, String)],
@@ -292,14 +316,18 @@ object PackageError {
           case Infer.Error.VarNotInScope((_, name), scope, region) =>
             val ctx =
               lm.showRegion(region, 2, errColor).getOrElse(Doc.str(region))
+
+            val names = (scope.map { case ((_, n), _) => n }.toList ::: lets.map(_._1)).distinct
+
             val candidates: List[String] =
-              nearest(name, scope.map { case ((_, n), _) => (n, ()) }, 3)
-                .map { case (n, _) => n.asString }
+              nearest(name, names.map((_, ())), 3)
+               .map { case (n, _) => n.asString }
 
             val cmessage =
               if (candidates.nonEmpty)
                 candidates.mkString("\nClosest: ", ", ", ".\n")
               else ""
+
             val qname = "\"" + name.sourceCodeRepr + "\""
             (
               Doc.text("name ") + Doc.text(qname) + Doc.text(" unknown.") + Doc
diff --git a/core/src/main/scala/org/bykn/bosatsu/PackageMap.scala b/core/src/main/scala/org/bykn/bosatsu/PackageMap.scala
index 4ae918888..87eefdc90 100644
--- a/core/src/main/scala/org/bykn/bosatsu/PackageMap.scala
+++ b/core/src/main/scala/org/bykn/bosatsu/PackageMap.scala
@@ -32,22 +32,22 @@ case class PackageMap[A, B, C, +D](
     packs.foldLeft(this: PackageMap[A, B, C, D1])(_ + _)
 
   def getDataRepr(implicit
-      ev: D <:< Program[TypeEnv[Any], Any, Any]
+      ev: Package[A, B, C, D] <:< Package.Typed[Any]
   ): (PackageName, Constructor) => Option[DataRepr] = { (pname, cons) =>
     toMap
       .get(pname)
       .flatMap { pack =>
-        ev(pack.program).types
+        ev(pack).types
           .getConstructor(pname, cons)
           .map(_._1.dataRepr(cons))
       }
   }
 
   def allExternals(implicit
-      ev: D <:< Program[TypeEnv[Any], Any, Any]
+      ev: Package[A, B, C, D] <:< Package.Typed[Any]
   ): Map[PackageName, List[Identifier.Bindable]] =
     toMap.iterator.map { case (name, pack) =>
-      (name, ev(pack.program).externalDefs)
+      (name, ev(pack).externalDefs)
     }.toMap
 }
 
@@ -75,11 +75,12 @@ object PackageMap {
     Package.Interface,
     NonEmptyList[Referant[Kind.Arg]],
     Referant[Kind.Arg],
-    Program[
+    (Program[
       TypeEnv[Kind.Arg],
       TypedExpr[T],
       Any
-    ]
+    ],
+      ImportMap[Package.Interface, NonEmptyList[Referant[Kind.Arg]]])
   ]
 
   type SourceMap = Map[PackageName, (LocationMap, String)]
@@ -153,7 +154,7 @@ object PackageMap {
               A
             ]]) =>
               deps
-                .traverse { i =>
+                .parTraverse { i =>
                   i.pack match {
                     case Right(pack) =>
                       rec(pack)
@@ -180,7 +181,7 @@ object PackageMap {
     type M = SortedMap[PackageName, PackageFix]
     val r
         : ReaderT[Either[NonEmptyList[PackageError], *], List[PackageName], M] =
-      map.toMap.traverse(step)
+      map.toMap.parTraverse(step)
 
     // we start with no imports on
     val m: Either[NonEmptyList[PackageError], M] = r.run(Nil)
@@ -230,11 +231,45 @@ object PackageMap {
         ]
     ) = {
 
-      val (errs0, imap) = ImportMap.fromImports(p.imports)
+      val (errs0, imap) = ImportMap.fromImports(p.imports) { case ((p1, i1), (p2, i2)) => 
+        val leftPredef = p1 === PackageName.PredefName
+        val rightPredef = p2 === PackageName.PredefName
+
+        if (leftPredef) {
+          if (rightPredef) {
+            // Both are predef, if one is renamed, choose that, else error
+            val r1 = i1.isRenamed
+            val r2 = i2.isRenamed
+            if (r1 && !r2) ImportMap.Unify.Left
+            else if (!r1 && r2) ImportMap.Unify.Right
+            else if ((i1 == i2) && !r1) {
+              // explicitly importing from predef is allowed.
+              // choose one, doesn't matter which they are the same
+              ImportMap.Unify.Left
+            }
+            else {
+              // Both are renamed... this isn't allowed
+              ImportMap.Unify.Error
+            }
+          }
+          else {
+            // Predef is replaced by non-predef
+            ImportMap.Unify.Right
+          }
+        }
+        else if (rightPredef) {
+          // Predef is replaced by non-predef
+          ImportMap.Unify.Left
+        }
+        else {
+          // neither are Predef, so we error
+          ImportMap.Unify.Error
+        }
+      }
       val errs =
         NonEmptyList
           .fromList(errs0)
-          .map(PackageError.DuplicatedImport)
+          .map(PackageError.DuplicatedImport(p.name, _))
 
       (errs, p.mapProgram((_, imap)))
     }
@@ -313,9 +348,26 @@ object PackageMap {
       Memoize.memoizeDagFuture[ResolvedU, Ior[NonEmptyList[
         PackageError
       ], (TypeEnv[Kind.Arg], Package.Inferred)]] {
-        // TODO, we ignore importMap here, we only check earlier we don't
-        // have duplicate imports
-        case (Package(nm, imports, exports, (stmt, _)), recurse) =>
+        case (Package(nm, imports, exports, (stmt, imps)), recurse) =>
+
+          val nameToRes = imports.iterator.map { i => 
+            val resolved: Package.Resolved = i.pack
+            val name = FixType.unfix(resolved) match {
+              case Left(iface) => iface.name
+              case Right(pack) => pack.name
+            }
+            
+            (name, resolved)
+          }
+          .toMap
+
+          def resolvedImports: ImportMap[Package.Resolved, Unit] = 
+            imps.traverse[cats.Id, Package.Resolved, Unit] { (p, i) =>
+              // the Map.apply below should be safe because the imps
+              // are aligned with imports
+              (nameToRes(p), i)
+            }
+
           def getImport[A, B](
               packF: Package.Inferred,
               exMap: Map[Identifier, NonEmptyList[ExportedName[A]]],
@@ -328,7 +380,7 @@ object PackageMap {
                     PackageError.UnknownImportName(
                       nm,
                       packF.name,
-                      packF.program.lets.iterator.map { case (n, _, _) =>
+                      packF.program._1.lets.iterator.map { case (n, _, _) =>
                         (n: Identifier, ())
                       }.toMap,
                       i,
@@ -371,12 +423,12 @@ object PackageMap {
            * type can have the same name as a constructor. After this step, each
            * distinct object has its own entry in the list
            */
-          type ImpRes =
-            Import[Package.Interface, NonEmptyList[Referant[Kind.Arg]]]
+          type IName = NonEmptyList[Referant[Kind.Arg]]
+
           def stepImport(
-              imp: Import[Package.Resolved, Unit]
-          ): FutVal[ImpRes] = {
-            val Import(fixpack, items) = imp
+              fixpack: Package.Resolved,
+              item: ImportedName[Unit]
+          ): FutVal[(Package.Interface, ImportedName[IName])] = {
             Package.unfix(fixpack) match {
               case Right(p) =>
                 /*
@@ -386,9 +438,8 @@ object PackageMap {
                   .flatMap { case (_, packF) =>
                     val packInterface = Package.interfaceOf(packF)
                     val exMap = packF.exports.groupByNel(_.name)
-                    val ior = items
-                      .parTraverse(getImport(packF, exMap, _))
-                      .map(Import(packInterface, _))
+                    val ior = getImport(packF, exMap, item)
+                      .map((packInterface, _))
                     IorT.fromIor(ior)
                   }
               case Left(iface) =>
@@ -397,40 +448,42 @@ object PackageMap {
                  */
                 val exMap = iface.exports.groupByNel(_.name)
                 // this is very fast and does not need to be done in a thread
-                val ior = items
-                  .parTraverse(getImportIface(iface, exMap, _))
-                  .map(Import(iface, _))
+                val ior =
+                  getImportIface(iface, exMap, item)
+                  .map((iface, _))
                 IorT.fromIor(ior)
             }
           }
 
-          val inferImports: FutVal[List[ImpRes]] =
-            imports.parTraverse(stepImport(_))
+          val inferImports: FutVal[ImportMap[Package.Interface, NonEmptyList[Referant[Kind.Arg]]]] =
+            resolvedImports.parTraverse(stepImport(_, _))
 
           val inferBody =
             inferImports
-              .flatMap { imps =>
+              .flatMap { impMap =>
                 // run this in a thread
+                val ilist = impMap.toList(Package.orderByName)
                 IorT(
                   Par.start(
-                    Package.inferBodyUnopt(nm, imps, stmt).map((imps, _))
+                    Package.inferBodyUnopt(nm, ilist, stmt)
+                      .map((ilist, impMap, _))
                   )
                 )
               }
 
           inferBody.flatMap {
-            case (imps, (fte, program @ Program(types, lets, _, _))) =>
+            case (ilist, imap, (fte, program @ Program(types, lets, _, _))) =>
               val ior = ExportedName
                 .buildExports(nm, exports, types, lets) match {
                 case Validated.Valid(exports) =>
                   // We have a result, which we can continue to check
-                  val pack = Package(nm, imps, exports, program)
-                  val res = (fte, pack)
+                  val pack = Package(nm, ilist, exports, (program, imap))
                   // We have to check the "customs" before any normalization
                   // or optimization
                   PackageCustoms(pack) match {
                     case Validated.Valid(p1) => Ior.right((fte, p1))
                     case Validated.Invalid(errs) =>
+                      val res = (fte, pack)
                       Ior.both(errs.toNonEmptyList, res)
                   }
                 case Validated.Invalid(badPackages) =>
@@ -456,11 +509,11 @@ object PackageMap {
           ior.traverse { case (fte, pack) =>
             Par.start {
               val optPack = pack.copy(program =
-                TypedExprNormalization.normalizeProgram(
+                (TypedExprNormalization.normalizeProgram(
                   pack.name,
                   fte,
-                  pack.program
-                )
+                  pack.program._1
+                ), pack.program._2)
               )
               Package.discardUnused(optPack)
             }
diff --git a/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala b/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala
index 17517b4ba..9c015508c 100644
--- a/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala
+++ b/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala
@@ -234,7 +234,7 @@ def go(x):
   y
 
 main = go(IntCase(42))
-""")) { case PackageError.TypeErrorIn(_, _) => () }
+""")) { case _: PackageError.TypeErrorIn => () }
 
     val errPack = """
 package Err
@@ -250,7 +250,7 @@ main = go(IntCase(42))
 """
     val packs =
       Map((PackageName.parts("Err"), (LocationMap(errPack), "Err.bosatsu")))
-    evalFail(List(errPack)) { case te @ PackageError.TypeErrorIn(_, _) =>
+    evalFail(List(errPack)) { case te: PackageError.TypeErrorIn =>
       val msg = te.message(packs, Colorize.None)
       assert(
         msg.contains(
@@ -667,7 +667,7 @@ main = if True:
   1
 else:
   "1"
-""")) { case PackageError.TypeErrorIn(_, _) => () }
+""")) { case _: PackageError.TypeErrorIn => () }
   }
 
   test(
@@ -741,7 +741,7 @@ struct W(wf: f[a, b] -> a -> b)
 def apply(w):
   W(fn) = w
   fn(w)
-""")) { case err @ PackageError.TypeErrorIn(_, _) =>
+""")) { case err: PackageError.TypeErrorIn =>
       val message = err.message(Map.empty, Colorize.None)
       assert(message.contains("illegal recursive type or function"))
       ()
@@ -877,7 +877,7 @@ def getValue(v):
     case IsInt(i, _): i
 
 main = getValue(int)
-""")) { case PackageError.TypeErrorIn(_, _) => () }
+""")) { case _: PackageError.TypeErrorIn => () }
 
   }
 
@@ -890,7 +890,7 @@ def plus(x: a, y):
   x.add(y)
 
 main = plus(1, 2)
-""")) { case PackageError.TypeErrorIn(_, _) => () }
+""")) { case _: PackageError.TypeErrorIn => () }
   }
 
   test("unused let fails compilation") {
@@ -1715,7 +1715,7 @@ main = a""")) { case PackageError.UnknownImportPackage(_, _) => () }
     evalFail(List("""
 package B
 
-main = a""")) { case te @ PackageError.TypeErrorIn(_, _) =>
+main = a""")) { case te: PackageError.TypeErrorIn =>
       val msg = te.message(Map.empty, Colorize.None)
       assert(!msg.contains("Name("))
       assert(msg.contains("package B\nname \"a\" unknown"))
@@ -1909,7 +1909,7 @@ def fn(x, y):
     case x: x
 
 main = fn(0, 1, 2)
-""")) { case te @ PackageError.TypeErrorIn(_, _) =>
+""")) { case te: PackageError.TypeErrorIn =>
       assert(
         te.message(Map.empty, Colorize.None)
           .contains("does not match function with 3 arguments at:")
@@ -1928,7 +1928,7 @@ def fn(x):
 
 main = fn([1, 2])
 """
-    evalFail(code1571 :: Nil) { case te @ PackageError.TypeErrorIn(_, _) =>
+    evalFail(code1571 :: Nil) { case te: PackageError.TypeErrorIn =>
       // Make sure we point at the function directly
       assert(code1571.substring(67, 69) == "fn")
       assert(
@@ -2769,24 +2769,6 @@ main = Foo(1, "2")
     }
   }
 
-  test("test duplicate import message") {
-    evalFail(List("""
-package Err
-
-from Bosatsu/Predef import foldLeft
-
-main = 1
-""")) { case sce @ PackageError.DuplicatedImport(_) =>
-      assert(
-        sce.message(
-          Map.empty,
-          Colorize.None
-        ) == "duplicate import in <unknown source> package Bosatsu/Predef imports foldLeft as foldLeft"
-      )
-      ()
-    }
-  }
-
   test("test duplicate package message") {
     val pack =
       """
@@ -3341,7 +3323,7 @@ struct RecordGetter[shape, t](
 def get[shape](sh: shape[RecordValue], RecordGetter(getter): RecordGetter[shape, t]) -> t:
   RecordValue(result) = sh.getter()
   result
-""")) { case PackageError.TypeErrorIn(_, _) => () }
+""")) { case _: PackageError.TypeErrorIn => () }
   }
 
   test("test quicklook example") {
@@ -3462,7 +3444,7 @@ struct Id(a)
 # this code could run if we ignored kinds
 def makeFoo(v: Int): Foo(Id(v))
 
-""")) { case kie @ PackageError.TypeErrorIn(_, _) =>
+""")) { case kie: PackageError.TypeErrorIn =>
       assert(
         kie.message(Map.empty, Colorize.None) ==
           """in file: <unknown source>, package Foo
@@ -3485,7 +3467,7 @@ struct Id(a)
 # this code could run if we ignored kinds
 def makeFoo(v: Int) -> Foo[Id, Int]: Foo(Id(v))
 
-""")) { case kie @ PackageError.TypeErrorIn(_, _) =>
+""")) { case kie: PackageError.TypeErrorIn =>
       assert(
         kie.message(Map.empty, Colorize.None) ==
           """in file: <unknown source>, package Foo
@@ -3512,7 +3494,7 @@ def quick_sort0(cmp, left, right):
         # we accidentally omit bigger below
         bigs = quick_sort0(cmp, tail)
         [*smalls, *bigs]
-""")) { case kie @ PackageError.TypeErrorIn(_, _) =>
+""")) { case kie: PackageError.TypeErrorIn =>
       assert(kie.message(Map.empty, Colorize.None) == """in file: <unknown source>, package QS
 type error: expected type Bosatsu/Predef::Fn3[(?13, ?9) -> Bosatsu/Predef::Comparison]
 Region(403,414)
@@ -3536,7 +3518,7 @@ def toInt(n: N, acc: Int) -> Int:
     case S(n): toInt(n, "foo")
 
 """
-    evalFail(List(testCode)) { case kie @ PackageError.TypeErrorIn(_, _) =>
+    evalFail(List(testCode)) { case kie: PackageError.TypeErrorIn =>
       val message = kie.message(Map.empty, Colorize.None)
       assert(message.contains("Region(122,127)"))
       val badRegion = testCode.substring(122, 127)
@@ -3757,7 +3739,7 @@ x: Int = "1"
 y: String = 1
 
 """
-    evalFail(List(testCode)) { case kie @ PackageError.TypeErrorIn(_, _) =>
+    evalFail(List(testCode)) { case kie: PackageError.TypeErrorIn =>
       val message = kie.message(Map.empty, Colorize.None)
       assert(message.contains("Region(30,33)"))
       assert(testCode.substring(30, 33) == "\"1\"")
@@ -3778,7 +3760,7 @@ z = (
 )
 
 """
-    evalFail(List(testCode)) { case kie @ PackageError.TypeErrorIn(_, _) =>
+    evalFail(List(testCode)) { case kie: PackageError.TypeErrorIn =>
       val message = kie.message(Map.empty, Colorize.None)
       assert(message.contains("Region(38,41)"))
       assert(testCode.substring(38, 41) == "\"1\"")
@@ -4020,4 +4002,90 @@ def fn(x):
 test = Assertion(fn(False), "")
 """), "Foo", 1)
   }
+
+  test("test duplicate import error messages") {
+    val testCode = List("""
+package P1
+export foo
+
+foo = 1
+
+""", """
+package P2
+export foo
+
+foo = 2
+""", """
+package P3
+
+from P1 import foo
+from P2 import foo
+
+main = foo
+""")
+
+    evalFail(testCode) {
+      case kie @ PackageError.DuplicatedImport(_, _) =>
+        val message = kie.message(Map.empty, Colorize.None)
+        assert(message == "duplicate import in <unknown source> package P3\n\tfrom P1 import foo\n\tfrom P2 import foo\n")
+        ()
+    }
+
+    // explicit predefs are allowed
+    runBosatsuTest(List("""
+package P
+
+from Bosatsu/Predef import foldLeft
+
+main = Assertion(True, "")
+"""), "P", 1)
+    // explicit predefs renamed are allowed
+    runBosatsuTest(List("""
+package P
+
+from Bosatsu/Predef import foldLeft as fl
+
+res = [].fl(True, (_, _) -> False)
+
+main = Assertion(res, "")
+"""), "P", 1)
+    evalFail(List("""
+package P
+
+from Bosatsu/Predef import concat as fl
+from Bosatsu/Predef import foldLeft as fl
+
+res = [].fl(True, (_, _) -> False)
+
+main = Assertion(res, "")
+    """)) {
+      case kie @ PackageError.DuplicatedImport(_, _) =>
+        val message = kie.message(Map.empty, Colorize.None)
+        assert(message == "duplicate import in <unknown source> package P\n\tfrom Bosatsu/Predef import concat as fl\n\tfrom Bosatsu/Predef import foldLeft as fl\n")
+        ()
+    }
+  }
+
+  test("we always suggest names which share a prefix with an unknown name") {
+    val testCode = List("""
+package P1
+
+fofoooooooo = 1
+
+ofof = 2
+
+main = fof
+""")
+
+    evalFail(testCode) {
+      case kie: PackageError.TypeErrorIn =>
+        val message = kie.message(Map.empty, Colorize.None)
+        assert(message == """in file: <unknown source>, package P1
+name "fof" unknown.
+Closest: fofoooooooo, ofof.
+
+Region(47,50)""")
+        ()
+    }
+  }
 }
diff --git a/core/src/test/scala/org/bykn/bosatsu/Gen.scala b/core/src/test/scala/org/bykn/bosatsu/Gen.scala
index 57cc5be5c..ef069e8e5 100644
--- a/core/src/test/scala/org/bykn/bosatsu/Gen.scala
+++ b/core/src/test/scala/org/bykn/bosatsu/Gen.scala
@@ -1676,10 +1676,12 @@ object Generators {
       pn <- packageNameGen
       // we can't reuse package names
       if !existing.contains(pn)
-      imps <- genImports
+      imps0 <- genImports
+      impMap = ImportMap.fromImports(imps0)((_, _) => ImportMap.Unify.Error)._2
+      imps = impMap.toList(Package.orderByName)
       prog <- genProg(pn, imps)
       exps <- genExports(pn, prog)
-    } yield Package(pn, imps, exps, prog)
+    } yield Package(pn, imps, exps, (prog, impMap))
   }
 
   def genPackagesSt[A](
diff --git a/core/src/test/scala/org/bykn/bosatsu/MatchlessTests.scala b/core/src/test/scala/org/bykn/bosatsu/MatchlessTests.scala
index 642821978..deb8a7356 100644
--- a/core/src/test/scala/org/bykn/bosatsu/MatchlessTests.scala
+++ b/core/src/test/scala/org/bykn/bosatsu/MatchlessTests.scala
@@ -38,15 +38,15 @@ class MatchlessTest extends AnyFunSuite {
     Generators
       .genPackage(Gen.const(()), 5)
       .flatMap { (m: Map[PackageName, Package.Typed[Unit]]) =>
-        val candidates = m.filter { case (_, t) => t.program.lets.nonEmpty }
+        val candidates = m.filter { case (_, t) => t.lets.nonEmpty }
 
         if (candidates.isEmpty) genInputs
         else
           for {
             packName <- Gen.oneOf(candidates.keys.toSeq)
-            prog = m(packName).program
-            (b, r, t) <- Gen.oneOf(prog.lets)
-            fn = fnFromTypeEnv(prog.types)
+            pack = m(packName)
+            (b, r, t) <- Gen.oneOf(pack.lets)
+            fn = fnFromTypeEnv(pack.types)
           } yield (b, r, t, fn)
       }
 
diff --git a/core/src/test/scala/org/bykn/bosatsu/Regressions.scala b/core/src/test/scala/org/bykn/bosatsu/Regressions.scala
index 936bbd957..fc6d31534 100644
--- a/core/src/test/scala/org/bykn/bosatsu/Regressions.scala
+++ b/core/src/test/scala/org/bykn/bosatsu/Regressions.scala
@@ -23,6 +23,6 @@ def applyFields(fields, row):
 
 hlist = (Field("a", x -> "a"), Some((Field("b", x -> "b"), None)))
 main = applyFields(hlist, 1)
-""")) { case PackageError.TypeErrorIn(_, _) => () }
+""")) { case _: PackageError.TypeErrorIn => () }
   }
 }
diff --git a/core/src/test/scala/org/bykn/bosatsu/TestUtils.scala b/core/src/test/scala/org/bykn/bosatsu/TestUtils.scala
index b79d772ad..d71bb4d13 100644
--- a/core/src/test/scala/org/bykn/bosatsu/TestUtils.scala
+++ b/core/src/test/scala/org/bykn/bosatsu/TestUtils.scala
@@ -227,8 +227,8 @@ object TestUtils {
 
       case Validated.Invalid(errs) =>
         val tes = errs.toList
-          .collect { case PackageError.TypeErrorIn(te, _) =>
-            te.toString
+          .collect { case te: PackageError.TypeErrorIn =>
+            te.tpeErr.toString
           }
           .mkString("\n")
         fail(tes + "\n" + errs.toString)