Error on unused lets (#1163)

* Error on unused lets

* don't root synthetics

* ban non-binding patterns, add tests

* remove unused code

bench/src/main/scala/org/bykn/bosatsu/TestBench.scala

@@ -65,7 +65,7 @@ gauss$n = range($n).foldLeft(0, add)
     val c = compiled0
     val ev = Evaluation(c._1, Predef.jvmExternals)
     // run the evaluation
-    val _ = ev.evaluateLast(c._2).get._1.value
+    val _ = ev.evaluateMain(c._2).get._1.value
     ()
   }
 
@@ -76,7 +76,7 @@ gauss$n = range($n).foldLeft(0, add)
     val c = compiled1
     val ev = Evaluation(c._1, Predef.jvmExternals)
     // run the evaluation
-    val _ = ev.evaluateLast(c._2).get._1.value
+    val _ = ev.evaluateMain(c._2).get._1.value
     ()
   }
 
@@ -153,7 +153,7 @@ max_pal = match max_pal_opt:
     val c = compiled2
     val ev = Evaluation(c._1, Predef.jvmExternals)
     // run the evaluation
-    val _ = ev.evaluateLast(c._2).get._1.value
+    val _ = ev.evaluateMain(c._2).get._1.value
     ()
   }
 }

core/src/main/resources/bosatsu/predef.bosatsu

@@ -46,6 +46,7 @@ export (
   Dict,
   add,
   add_key,
+  build_List,
   char_to_String,
   cmp_Int,
   concat,
@@ -56,6 +57,7 @@ export (
   eq_Int,
   flat_map_List,
   foldLeft,
+  foldr_List,
   gcd_Int,
   get_key,
   int_loop,

core/src/main/scala/org/bykn/bosatsu/Evaluation.scala

@@ -74,13 +74,6 @@ case class Evaluation[T](pm: PackageMap.Typed[T], externals: Externals) {
       }
     )
 
-  def evaluateLast(p: PackageName): Option[(Eval[Value], Type)] =
-    for {
-      pack <- pm.toMap.get(p)
-      (name, _, tpe) <- pack.program.lets.lastOption
-      value <- evaluate(p).get(name)
-    } yield (value, tpe.getType)
-
   // TODO: this only works for lets, not externals
   def evaluateName(
       p: PackageName,
@@ -104,6 +97,13 @@ case class Evaluation[T](pm: PackageMap.Typed[T], externals: Externals) {
       value <- evaluate(p).get(name)
     } yield value
 
+  def evaluateMain(p: PackageName): Option[(Eval[Value], Type)] =
+    for {
+      pack <- pm.toMap.get(p)
+      (name, _, te) <- Package.mainValue(pack)
+      value <- evaluate(p).get(name)
+    } yield (value, te.getType)
+
   /* TODO: this is useful for debugging, but we should probably test it and write a parser for the
    * list syntax
 

core/src/main/scala/org/bykn/bosatsu/Identifier.scala

@@ -128,4 +128,7 @@ object Identifier {
 
   implicit def ordering[A <: Identifier]: Ordering[A] =
     order[A].toOrdering
+
+  def synthetic(name: String): Bindable =
+    Name("_" + name)
 }

core/src/main/scala/org/bykn/bosatsu/Indented.scala

@@ -4,8 +4,6 @@ import org.typelevel.paiges.{Doc, Document}
 
 import cats.parse.{Parser => P}
 
-import cats.implicits._
-
 case class Indented[T](spaces: Int, value: T) {
   require(spaces > 0, s"need non-empty indentation: $spaces")
 }

core/src/main/scala/org/bykn/bosatsu/MainModule.scala

@@ -521,13 +521,13 @@ abstract class MainModule[IO[_]](implicit
             ps: List[(Path, PackageName)]
         ): IO[(PackageName, Option[Bindable])] =
           ps.collectFirst { case (path, pn) if path == mainFile => pn } match {
+            case Some(p) => moduleIOMonad.pure((p, None))
             case None =>
               moduleIOMonad.raiseError(
                 new Exception(
                   s"could not find file $mainFile in parsed sources"
                 )
               )
-            case Some(p) => moduleIOMonad.pure((p, None))
           }
       }
 
@@ -1018,16 +1018,14 @@ abstract class MainModule[IO[_]](implicit
       def runEval: IO[(Evaluation[Any], Output.EvaluationResult)] = withEC {
         implicit ec =>
           for {
-            pn <- inputs.packMap(this, List(mainPackage), errColor)
-            (packs, names) = pn
-            mainPackageNameValue <- mainPackage.getMain(names)
-            (mainPackageName, value) = mainPackageNameValue
+            (packs, names) <- inputs.packMap(this, List(mainPackage), errColor)
+            (mainPackageName, value) <- mainPackage.getMain(names)
             out <-
               if (packs.toMap.contains(mainPackageName)) {
                 val ev = Evaluation(packs, Predef.jvmExternals)
 
                 val res = value match {
-                  case None        => ev.evaluateLast(mainPackageName)
+                  case None        => ev.evaluateMain(mainPackageName)
                   case Some(ident) => ev.evaluateName(mainPackageName, ident)
                 }
 

core/src/main/scala/org/bykn/bosatsu/Package.scala

@@ -97,6 +97,11 @@ object Package {
       te.getType == Type.TestType
     }.lastOption
 
+  def mainValue[A](
+      tp: Typed[A]
+  ): Option[(Identifier.Bindable, RecursionKind, TypedExpr[A])] =
+    tp.program.lets.lastOption
+
   /** Discard any top level values that are not referenced, exported, the final
     * test value, or the final expression
     *

core/src/main/scala/org/bykn/bosatsu/PackageCustoms.scala

@@ -1,6 +1,5 @@
 package org.bykn.bosatsu
 
-import cats.Monad
 import cats.data.{
   Chain,
   NonEmptyList,
@@ -16,16 +15,18 @@ import scala.collection.immutable.SortedSet
 import cats.syntax.all._
 import org.bykn.bosatsu.Referant.Constructor
 import org.bykn.bosatsu.Referant.DefinedT
-import org.bykn.bosatsu.TypedExpr.Match
+import org.bykn.bosatsu.Identifier.Bindable
+import org.bykn.bosatsu.graph.Dag
 
 /** This checks the imports and exports of compiled packages and makes sure they
   * are valid
   */
 object PackageCustoms {
-  def apply[A](
+  def apply[A: HasRegion](
       pack: Package.Typed[A]
   ): ValidatedNec[PackageError, Package.Typed[A]] =
     checkValuesHaveExportedTypes(pack.name, pack.exports) *>
+      noUselessBinds(pack) *>
       allImportsAreUsed(pack)
 
   private def removeUnused[A](
@@ -59,6 +60,16 @@ object PackageCustoms {
         pack.copy(imports = i)
     }
 
+  private type VSet = Set[(PackageName, Identifier)]
+  private type VState[X] = State[VSet, X]
+
+  private def usedGlobals[A](pack: Package.Typed[A]): Set[(PackageName, Identifier)] = {
+    val usedValuesSt: VState[Unit] =
+      pack.program.lets.traverse_ { case (_, _, te) => TypedExpr.usedGlobals(te) }
+
+    usedValuesSt.runS(Set.empty).value
+  }
+
   private def allImportsAreUsed[A](
       pack: Package.Typed[A]
   ): ValidatedNec[PackageError, Package.Typed[A]] = {
@@ -94,34 +105,7 @@ object PackageCustoms {
 
     if (impValues.isEmpty && impTypes.isEmpty) Validated.valid(pack)
     else {
-      type VSet = Set[(PackageName, Identifier)]
-      type VState[X] = State[VSet, X]
-      val usedValuesSt: VState[Unit] =
-        pack.program.lets.traverse_ { case (_, _, te) =>
-          te.traverseUp {
-            case g @ TypedExpr.Global(p, n, _, _) =>
-              State(s => (s + ((p, n)), g))
-            case m @ Match(_, branches, _) =>
-              branches
-                .traverse_ { case (pat, _) =>
-                  pat
-                    .traverseStruct[
-                      VState,
-                      (PackageName, Identifier.Constructor)
-                    ] { (n, parts) =>
-                      State.modify[VSet](_ + n) *>
-                        parts.map { inner =>
-                          Pattern.PositionalStruct(n, inner)
-                        }
-                    }
-                    .void
-                }
-                .as(m)
-            case te => Monad[VState].pure(te)
-          }
-        }
-
-      val usedValues = usedValuesSt.runS(Set.empty).value
+      val usedValues = usedGlobals(pack)
 
       val usedTypes: Set[Type.Const] =
         pack.program.lets.iterator
@@ -224,4 +208,65 @@ object PackageCustoms {
       case Some(nel) => Validated.invalid(nel)
     }
   }
-}
+
+  private def noUselessBinds[A: HasRegion](pack: Package.Typed[A]): ValidatedNec[PackageError, Unit] = {
+    type Node = Either[pack.exports.type, Bindable]
+    implicit val ordNode: Ordering[Node] =
+      new Ordering[Node] {
+        def compare(x: Node, y: Node): Int =
+          x match {
+            case Right(bx) =>
+              y match {
+                case Left(_) => 1
+                case Right(by) =>
+                  Ordering[Identifier].compare(bx, by)
+              }
+            case Left(_) => 
+              y match {
+                case Left(_) => 0
+                case Right(_) => -1
+              }
+          }
+      }
+
+    val exports: Node = Left(pack.exports)
+    val roots: List[Node] =
+      (exports ::
+        Package.testValue(pack).map { case (b, _, _) => Right(b) }.toList :::
+        Package.mainValue(pack).map { case (b, _, _) => Right(b) }.toList).distinct
+
+    val bindMap: Map[Bindable, TypedExpr[A]] =
+      pack.program.lets.iterator.map { case (b, _, te) => (b, te) }.toMap
+
+    def internalDeps(te: TypedExpr[A]): Set[Bindable] =
+      TypedExpr.usedGlobals(te).runS(Set.empty).value.collect {
+        case (pn, i: Identifier.Bindable) if pn == pack.name => i
+      }
+
+    def depsOf(n: Node): Iterable[Node] =
+      n match {
+        case Left(_) => pack.exports.flatMap {
+          case ExportedName.Binding(n, _) => Right(n) :: Nil
+          case _ => Nil
+        }
+        case Right(value) =>
+          bindMap.get(value) match {
+            case None => Nil
+            case Some(te) => internalDeps(te).map(Right(_))
+          }
+      }
+    val canReach: SortedSet[Node] = Dag.transitiveSet(roots)(depsOf _)
+
+    val unused = pack.program.lets.filter {
+      case (bn, _, _) => !canReach.contains(Right(bn))
+    }
+
+    NonEmptyList.fromList(unused) match {
+      case None => Validated.unit
+      case Some(value) => 
+        Validated.invalidNec(PackageError.UnusedLets(pack.name, value.map { case (b, r, te) =>
+          (b, r, te, HasRegion.region(te))  
+        }))
+    }
+  }
+}

core/src/main/scala/org/bykn/bosatsu/PackageError.scala

@@ -886,4 +886,18 @@ object PackageError {
       ) + di + Doc.hardLine).render(80)
     }
   }
+
+  case class UnusedLets(
+      inPack: PackageName,
+      unusedLets: NonEmptyList[(Identifier.Bindable, RecursionKind, TypedExpr[Any], Region)]
+  ) extends PackageError {
+    def message(
+        sourceMap: Map[PackageName, (LocationMap, String)],
+        errColor: Colorize
+    ) =
+      UnusedLetError(
+        inPack,
+        unusedLets.map { case (b, _, _, r) => (b, r)}
+      ).message(sourceMap, errColor)
+  }
 }

core/src/main/scala/org/bykn/bosatsu/PackageMap.scala

@@ -426,6 +426,8 @@ object PackageMap {
                   // We have a result, which we can continue to check
                   val pack = Package(nm, imps, exports, program)
                   val res = (fte, pack)
+                  // 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) =>

core/src/main/scala/org/bykn/bosatsu/SourceConverter.scala

@@ -1216,13 +1216,9 @@ final class SourceConverter(
   private lazy val localTypeEnv: Result[TypeEnv[Any]] =
     toTypeEnv.map(p => importedTypeEnv ++ TypeEnv.fromParsed(p))
 
-  private def anonNameStrings(): Iterator[String] =
-    rankn.Type.allBinders.iterator
-      .map(_.name)
-
   private def unusedNames(allNames: Bindable => Boolean): Iterator[Bindable] =
-    anonNameStrings()
-      .map(Identifier.Name(_))
+    rankn.Type.allBinders.iterator
+      .map { b => Identifier.synthetic(b.name) }
       .filterNot(allNames)
 
   /** Externals are not permitted to be shadowed at the top level
@@ -1405,6 +1401,12 @@ final class SourceConverter(
             }
       }
 
+    val noBinds: Result[Unit] = stmts.parTraverse_ {
+      case Bind(BindingStatement(b, d, _)) if b.names.isEmpty =>
+        SourceConverter.partial(SourceConverter.NonBindingPattern(b, d), ())
+      case _ => SourceConverter.successUnit
+    }
+
     val flatIn: List[(Bindable, RecursionKind, Flattened)] =
       SourceConverter.makeLetsUnique(flatList) { (bind, _) =>
         // rename all but the last item
@@ -1461,6 +1463,7 @@ final class SourceConverter(
           case (b, _, Right((_, d))) => Right(Right((b, d)))
         }
 
+    noBinds.parProductR(
     parFold(Set.empty[Bindable], withEx) { case (topBound, stmt) =>
       stmt match {
         case Right(Right((nm, decl))) =>
@@ -1513,8 +1516,7 @@ final class SourceConverter(
         case Left(ExternalDef(n, _, _)) =>
           (topBound + n, success(Nil))
       }
-    }(SourceConverter.parallelIor)
-      .map(_.flatten)
+    }(SourceConverter.parallelIor)).map(_.flatten)
   }
 
   def toProgram(
@@ -1936,4 +1938,10 @@ object SourceConverter {
           .render(80)
       }
   }
+
+  final case class NonBindingPattern(pattern: Pattern.Parsed, bound: Declaration) extends Error {
+    def message =
+      (Document[Pattern.Parsed].document(pattern) + Doc.text(" does not bind any names.")).render(80)
+    def region = bound.region
+  }
 }