From 8bcce7c4070f0a84e1b53d8f7bb789bf1789cb7b Mon Sep 17 00:00:00 2001 From: Patrick Oscar Boykin Date: Sun, 17 Dec 2023 09:52:48 -1000 Subject: [PATCH] Improve KindFormula.Error reporting --- .../scala/org/bykn/bosatsu/KindFormula.scala | 25 +++++++++----- .../scala/org/bykn/bosatsu/PackageError.scala | 34 ++++++------------- .../main/scala/org/bykn/bosatsu/Shape.scala | 5 ++- .../org/bykn/bosatsu/EvaluationTest.scala | 15 ++++++++ 4 files changed, 47 insertions(+), 32 deletions(-) diff --git a/core/src/main/scala/org/bykn/bosatsu/KindFormula.scala b/core/src/main/scala/org/bykn/bosatsu/KindFormula.scala index 49e7dbde5..1384043ce 100644 --- a/core/src/main/scala/org/bykn/bosatsu/KindFormula.scala +++ b/core/src/main/scala/org/bykn/bosatsu/KindFormula.scala @@ -33,15 +33,22 @@ object KindFormula { case object Type extends KindFormula case class Cons(arg: Arg, result: KindFormula) extends KindFormula - sealed abstract class Error + sealed abstract class Error { + def dt: DefinedType[Option[Kind.Arg]] + } object Error { case class Unsatisfiable( + dte: DefinedType[Either[KnownShape, Kind.Arg]], constraints: LongMap[NonEmptyList[Constraint]], existing: LongMap[Variance], unknowns: SortedSet[Long] - ) extends Error + ) extends Error { + def dt: DefinedType[Option[Kind.Arg]] = dte.map(_.toOption) + } - case class FromShapeError(shapeError: Shape.Error) extends Error + case class FromShapeError(shapeError: Shape.Error) extends Error { + def dt = shapeError.dt + } } sealed abstract class Sat @@ -291,7 +298,7 @@ object KindFormula { // we have allocated all variance variables now, we can see how many varCount <- state.nextId topo = Impl.combineTopo(varCount, constraints) - maybeRes = Impl.go(constraints, dirs, topo).map { vars => + maybeRes = Impl.go(dt, constraints, dirs, topo).map { vars => dtFormula.map(Impl.unformula(_, vars)) } } yield maybeRes).run.value @@ -354,6 +361,7 @@ object KindFormula { // invariant: all subgraph values must be valid keys in the result // we can process the subgraph list in parallel. Those are all the indepentent next values def allSolutionChunk( + dt: DefinedType[Either[Shape.KnownShape, Kind.Arg]], cons: Cons, existing: LongMap[Variance], directions: LongMap[Direction], @@ -414,22 +422,24 @@ object KindFormula { NonEmptyLazyList.fromLazyList(validVariances) match { case Some(nel) => Validated.valid(nel) case None => - Validated.invalidNec(Error.Unsatisfiable(cons, existing, subgraph)) + Validated.invalidNec(Error.Unsatisfiable(dt, cons, existing, subgraph)) } } def allSolutions( + dt: DefinedType[Either[Shape.KnownShape, Kind.Arg]], cons: Cons, existing: LongMap[Variance], directions: LongMap[Direction], subgraph: NonEmptyList[SortedSet[Long]] ): EitherNec[Error, NonEmptyLazyList[LongMap[Variance]]] = subgraph - .traverse(allSolutionChunk(cons, existing, directions, _)) + .traverse(allSolutionChunk(dt, cons, existing, directions, _)) .map(KindFormula.mergeCrossProduct(_)) .toEither def go[F[_]: Foldable]( + dt: DefinedType[Either[Shape.KnownShape, Kind.Arg]], cons: Cons, directions: LongMap[Direction], topo: F[NonEmptyList[SortedSet[Long]]] @@ -437,7 +447,7 @@ object KindFormula { topo.foldM(NonEmptyLazyList(LongMap.empty[Variance])) { (sols, subgraph) => // if there is at least one good solution, we can keep going - val next = sols.map(allSolutions(cons, _, directions, subgraph)) + val next = sols.map(allSolutions(dt, cons, _, directions, subgraph)) val nextPaths: LazyList[LongMap[Variance]] = next.toLazyList.flatMap { case Right(ll) => ll.toLazyList @@ -855,7 +865,6 @@ object KindFormula { ) } yield () } - } } diff --git a/core/src/main/scala/org/bykn/bosatsu/PackageError.scala b/core/src/main/scala/org/bykn/bosatsu/PackageError.scala index f0303013b..c0dad0ad1 100644 --- a/core/src/main/scala/org/bykn/bosatsu/PackageError.scala +++ b/core/src/main/scala/org/bykn/bosatsu/PackageError.scala @@ -534,26 +534,26 @@ object PackageError { case class KindInferenceError(pack: PackageName, kindError: KindFormula.Error, regions: Map[Type.Const.Defined, Region]) extends PackageError { def message(sourceMap: Map[PackageName, (LocationMap, String)], errColor: Colorize) = { val (lm, _) = sourceMap.getMapSrc(pack) + val region = regions(kindError.dt.toTypeConst) + val ctx = lm.showRegion(region, 2, errColor) + .getOrElse(Doc.str(region)) // we should highlight the whole region kindError match { - case KindFormula.Error.Unsatisfiable(_, _, _) => - val prefix = sourceMap.headLine(pack, None) - (prefix + Doc.text(s": $kindError")).render(80) + case KindFormula.Error.Unsatisfiable(_, _, _, _) => + val prefix = sourceMap.headLine(pack, Some(region)) + // TODO: would be good to give a more precise problem, e.g. which + // type parameters are the problem. + val message = Doc.text("could not solve for valid variances") + (prefix + Doc.hardLine + message + Doc.hardLine + ctx).render(80) case KindFormula.Error.FromShapeError(se) => se match { - case Shape.UnificationError(dt, cons, left, right) => - val region = regions(dt.toTypeConst) + case Shape.UnificationError(_, cons, left, right) => val prefix = sourceMap.headLine(pack, Some(region)) - val ctx = lm.showRegion(region, 2, errColor) - .getOrElse(Doc.str(region)) // we should highlight the whole region (prefix + Doc.hardLine + Doc.text("shape error: expected ") + Shape.shapeDoc(left) + Doc.text(" and ") + Shape.shapeDoc(right) + Doc.text(s" to match in the constructor ${cons.name.sourceCodeRepr}") + Doc.hardLine + Doc.hardLine + ctx).render(80) - case Shape.ShapeMismatch(dt, cons, outer, tyApp, right) => + case Shape.ShapeMismatch(_, cons, outer, tyApp, right) => val tmap = showTypes(pack, outer :: tyApp :: Nil) - val region = regions(dt.toTypeConst) val prefix = sourceMap.headLine(pack, Some(region)) - val ctx = lm.showRegion(region, 2, errColor) - .getOrElse(Doc.str(region)) // we should highlight the whole region val typeDoc = if (outer != tyApp) (tmap(outer) + Doc.text(" at application ") + tmap(tyApp)) else tmap(outer) @@ -564,16 +564,12 @@ object PackageError { Doc.hardLine + Doc.hardLine + ctx).render(80) case Shape.FinishFailure(dt, left, right) => - val region = regions(dt.toTypeConst) val tdoc = showTypes(pack, dt.toTypeTyConst :: Nil)(dt.toTypeTyConst) val prefix = sourceMap.headLine(pack, Some(region)) val message = Doc.text("in type ") + tdoc + Doc.text(" could not unify shapes: ") + Shape.shapeDoc(left) + Doc.text(" and ") + Shape.shapeDoc(right) - val ctx = lm.showRegion(region, 2, errColor) - .getOrElse(Doc.str(region)) // we should highlight the whole region (prefix + Doc.hardLine + message + Doc.hardLine + ctx).render(80) case Shape.ShapeLoop(dt, tpe, _) => - val region = regions(dt.toTypeConst) val tpe2 = tpe match { case Left(ap) => ap case Right(v) => Type.TyVar(v) @@ -583,11 +579,8 @@ object PackageError { val prefix = sourceMap.headLine(pack, Some(region)) val message = Doc.text("in type ") + tdocs(dt.toTypeTyConst) + Doc.text(" cyclic dependency encountered in ") + tdocs(tpe2) - val ctx = lm.showRegion(region, 2, errColor) - .getOrElse(Doc.str(region)) // we should highlight the whole region (prefix + Doc.hardLine + message + Doc.hardLine + ctx).render(80) case Shape.UnboundVar(dt, cfn, v) => - val region = regions(dt.toTypeConst) val tpe2 = Type.TyVar(v) val tdocs = showTypes(pack, dt.toTypeTyConst :: tpe2 :: Nil) @@ -598,11 +591,8 @@ object PackageError { val message = Doc.text("in type ") + tdocs(dt.toTypeTyConst) + Doc.text(" unbound type variable ") + tdocs(tpe2) + cfnMsg - val ctx = lm.showRegion(region, 2, errColor) - .getOrElse(Doc.str(region)) // we should highlight the whole region (prefix + Doc.hardLine + message + Doc.hardLine + ctx).render(80) case Shape.UnknownConst(dt, cfn, c) => - val region = regions(dt.toTypeConst) val tpe2 = Type.TyConst(c) val tdocs = showTypes(pack, dt.toTypeTyConst :: tpe2 :: Nil) @@ -613,8 +603,6 @@ object PackageError { val message = Doc.text("in type ") + tdocs(dt.toTypeTyConst) + Doc.text(" unknown type ") + tdocs(tpe2) + cfnMsg - val ctx = lm.showRegion(region, 2, errColor) - .getOrElse(Doc.str(region)) // we should highlight the whole region (prefix + Doc.hardLine + message + Doc.hardLine + ctx).render(80) } } diff --git a/core/src/main/scala/org/bykn/bosatsu/Shape.scala b/core/src/main/scala/org/bykn/bosatsu/Shape.scala index bb0d871cd..a4de39953 100644 --- a/core/src/main/scala/org/bykn/bosatsu/Shape.scala +++ b/core/src/main/scala/org/bykn/bosatsu/Shape.scala @@ -67,7 +67,10 @@ object Shape { Doc.text("kind(") + tdoc + Doc.char(')') } - sealed abstract class Error + sealed abstract class Error { + def dt: DefinedType[Option[Kind.Arg]] + } + case class ShapeLoop( dt: DefinedType[Option[Kind.Arg]], bound: Either[rankn.Type.TyApply, rankn.Type.Var.Bound], diff --git a/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala b/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala index 57c60613d..20b9a9c22 100644 --- a/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala +++ b/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala @@ -3270,4 +3270,19 @@ res = z matches (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, () } } + + test("kind errors show the region of the type") { + val testCode = """ +package ErrorCheck + +struct Foo[a: -*](get: a) + +""" + evalFail(List(testCode)) { case kie@PackageError.KindInferenceError(_, _, _) => + val message = kie.message(Map.empty, Colorize.None) + assert(message.contains("Region(21,46)")) + assert(testCode.substring(21, 46) == "struct Foo[a: -*](get: a)") + () + } + } }