diff --git a/core/src/main/scala/org/bykn/bosatsu/ListUtil.scala b/core/src/main/scala/org/bykn/bosatsu/ListUtil.scala index 950204004..c258dfdb7 100644 --- a/core/src/main/scala/org/bykn/bosatsu/ListUtil.scala +++ b/core/src/main/scala/org/bykn/bosatsu/ListUtil.scala @@ -50,4 +50,26 @@ private[bosatsu] object ListUtil { if (bs eq as) nel else NonEmptyList.fromListUnsafe(bs) } + + def distinctByHashSet[A](nel: NonEmptyList[A]): NonEmptyList[A] = { + // This code leverages the scala type ::[A] which is the nonempty + // list in order to avoid allocations building a NonEmptyList + // since a :: tailnel will have to allocate twice vs 1 time. + def revCons(item: ::[A], tail: List[A]): NonEmptyList[A] = + item.tail match { + case nel: ::[A] => revCons(nel, item.head :: tail) + case _: Nil.type => NonEmptyList(item.head, tail) + } + @annotation.tailrec + def loop(prior: Set[A], tail: List[A], front: ::[A]): NonEmptyList[A] = + tail match { + case Nil => revCons(front, Nil) + case head :: next => + if (prior(head)) loop(prior, next, front) + else loop(prior + head, next, ::(head, front)) + } + + val h = nel.head + loop(Set.empty + h, nel.tail, ::(h, Nil)) + } } diff --git a/core/src/main/scala/org/bykn/bosatsu/Package.scala b/core/src/main/scala/org/bykn/bosatsu/Package.scala index aa9604dd1..e7b400702 100644 --- a/core/src/main/scala/org/bykn/bosatsu/Package.scala +++ b/core/src/main/scala/org/bykn/bosatsu/Package.scala @@ -248,11 +248,14 @@ object Package { // here we make a pass to get all the local names val optProg = SourceConverter .toProgram(p, imps.map(i => i.copy(pack = i.pack.name)), stmts) - .leftMap( - _.map( - PackageError.SourceConverterErrorIn(_, p): PackageError - ).toNonEmptyList - ) + .leftMap { scerrs => + scerrs.groupByNem(_.region) + .transform { (region, errs) => + val uniqs = ListUtil.distinctByHashSet(errs.toNonEmptyList) + PackageError.SourceConverterErrorsIn(region, uniqs, p): PackageError + } + .toNonEmptyList + } lazy val typeDefRegions: Map[Type.Const.Defined, Region] = stmts.iterator.collect { case tds: TypeDefinitionStatement => diff --git a/core/src/main/scala/org/bykn/bosatsu/PackageError.scala b/core/src/main/scala/org/bykn/bosatsu/PackageError.scala index 2e199e97a..2a9855b80 100644 --- a/core/src/main/scala/org/bykn/bosatsu/PackageError.scala +++ b/core/src/main/scala/org/bykn/bosatsu/PackageError.scala @@ -561,8 +561,9 @@ object PackageError { } } - case class SourceConverterErrorIn( - err: SourceConverter.Error, + case class SourceConverterErrorsIn( + region: Region, + errs: NonEmptyList[SourceConverter.Error], pack: PackageName ) extends PackageError { def message( @@ -570,16 +571,54 @@ object PackageError { errColor: Colorize ) = { val (lm, _) = sourceMap.getMapSrc(pack) - val msg = { - val context = - lm.showRegion(err.region, 2, errColor) - .getOrElse( - Doc.str(err.region) - ) // we should highlight the whole region - - Doc.text(err.message) + Doc.hardLine + context + val context = + lm.showRegion(region, 2, errColor) + .getOrElse( + Doc.str(region) + ) // we should highlight the whole region + val headDoc = sourceMap.headLine(pack, Some(region)) + + val (missing, notMissing) = errs.toList.partitionMap { + case ma: SourceConverter.MissingArg => Left(ma) + case notMa => Right(notMa) + } + val mdocs = missing.groupBy { ma => (ma.name, ma.syntax) } + .toList + .sortBy { case ((name, _), _) => name } + .map { case ((_, syn), mas) => + val allMissing = mas.map(_.missing) + + val missingDoc = Doc.intercalate(Doc.comma + Doc.space, + allMissing.sorted.map { m => Doc.text(m.asString) }) + + val fieldStr = if (allMissing.lengthCompare(1) == 0) "field" else "fields" + + val hint = + syn match { + case SourceConverter.ConstructorSyntax.Pat(_) => + Doc.line + Doc.text("if you want to ignore those fields, add a ... to signify ignoring missing.") + case _ => + // we can't ignore fields when constructing + Doc.empty + } + (Doc.text(s"missing $fieldStr: ") + missingDoc + Doc.line + Doc.text("in") + + Doc.line + syn.toDoc + hint + ).nested(4) + } + + val mdoc = Doc.intercalate(Doc.hardLine, mdocs) + val notMDoc = Doc.intercalate(Doc.hardLine, notMissing.map { se => Doc.text(se.message) }) + val msg = if (missing.nonEmpty) { + if (notMissing.nonEmpty) { + mdoc + Doc.hardLine + notMDoc + } + else mdoc + } + else { + notMDoc } - val doc = sourceMap.headLine(pack, Some(err.region)) + Doc.hardLine + msg + + val doc = headDoc + Doc.hardLine + msg + Doc.hardLine + context doc.render(80) } diff --git a/core/src/main/scala/org/bykn/bosatsu/Pattern.scala b/core/src/main/scala/org/bykn/bosatsu/Pattern.scala index 58178d0aa..76738f5d5 100644 --- a/core/src/main/scala/org/bykn/bosatsu/Pattern.scala +++ b/core/src/main/scala/org/bykn/bosatsu/Pattern.scala @@ -870,7 +870,7 @@ object Pattern { } ) prefix + - Doc.text(" {") + + Doc.text(" { ") + kvargs + suffix + Doc.text(" }") diff --git a/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala b/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala index 2b7316c19..fe57084c3 100644 --- a/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala +++ b/core/src/test/scala/org/bykn/bosatsu/EvaluationTest.scala @@ -1706,7 +1706,7 @@ x = 1 main = match x: case Foo: 2 -""")) { case te @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case te @ PackageError.SourceConverterErrorsIn(_, _, _) => val msg = te.message(Map.empty, Colorize.None) assert(!msg.contains("Name(")) assert(msg.contains("package B\nunknown constructor Foo")) @@ -1720,7 +1720,7 @@ struct X main = match 1: case X1: 0 -""")) { case te @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case te @ PackageError.SourceConverterErrorsIn(_, _, _) => assert( te.message( Map.empty, @@ -2296,7 +2296,7 @@ get = Pair(first, ...) -> first # missing second first = 1 res = get(Pair { first }) -""")) { case s @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case s @ PackageError.SourceConverterErrorsIn(_, _, _) => s.message(Map.empty, Colorize.None); () } @@ -2311,7 +2311,7 @@ get = Pair(first, ...) -> first first = 1 second = 3 res = get(Pair { first, second, third }) -""")) { case s @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case s @ PackageError.SourceConverterErrorsIn(_, _, _) => s.message(Map.empty, Colorize.None); () } @@ -2323,7 +2323,7 @@ struct Pair(first, second) get = Pair { first } -> first res = get(Pair(1, "two")) -""")) { case s @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case s @ PackageError.SourceConverterErrorsIn(_, _, _) => s.message(Map.empty, Colorize.None); () } @@ -2336,7 +2336,7 @@ struct Pair(first, second) get = Pair(first) -> first res = get(Pair(1, "two")) -""")) { case s @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case s @ PackageError.SourceConverterErrorsIn(_, _, _) => s.message(Map.empty, Colorize.None); () } @@ -2349,7 +2349,7 @@ struct Pair(first, second) get = \Pair { first, sec: _ } -> first res = get(Pair(1, "two")) -""")) { case s @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case s @ PackageError.SourceConverterErrorsIn(_, _, _) => s.message(Map.empty, Colorize.None); () } @@ -2362,7 +2362,7 @@ struct Pair(first, second) get = Pair { first, sec: _, ... } -> first res = get(Pair(1, "two")) -""")) { case s @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case s @ PackageError.SourceConverterErrorsIn(_, _, _) => s.message(Map.empty, Colorize.None); () } @@ -2375,7 +2375,7 @@ struct Pair(first, second) get = Pair(first, _, _) -> first res = get(Pair(1, "two")) -""")) { case s @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case s @ PackageError.SourceConverterErrorsIn(_, _, _) => s.message(Map.empty, Colorize.None); () } @@ -2388,7 +2388,7 @@ struct Pair(first, second) get = Pair(first, _, _, ...) -> first res = get(Pair(1, "two")) -""")) { case s @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case s @ PackageError.SourceConverterErrorsIn(_, _, _) => s.message(Map.empty, Colorize.None); () } } @@ -2613,7 +2613,7 @@ external def foo(x: String) -> List[String] def foo(x): x -""")) { case s @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case s @ PackageError.SourceConverterErrorsIn(_, _, _) => assert( s.message( Map.empty, @@ -2630,7 +2630,7 @@ external def foo(x: String) -> List[String] foo = 1 -""")) { case s @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case s @ PackageError.SourceConverterErrorsIn(_, _, _) => assert( s.message( Map.empty, @@ -2646,7 +2646,7 @@ package A external def foo(x: String) -> List[String] external def foo(x: String) -> List[String] -""")) { case s @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case s @ PackageError.SourceConverterErrorsIn(_, _, _) => assert( s.message( Map.empty, @@ -2686,7 +2686,7 @@ package Err struct Foo[a](a) main = Foo(1, "2") -""")) { case sce @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case sce @ PackageError.SourceConverterErrorsIn(_, _, _) => assert( sce.message( Map.empty, @@ -2702,7 +2702,7 @@ package Err struct Foo[a](a: a, b: b) main = Foo(1, "2") -""")) { case sce @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case sce @ PackageError.SourceConverterErrorsIn(_, _, _) => assert( sce.message( Map.empty, @@ -2718,7 +2718,7 @@ package Err enum Enum[a]: Foo(a) main = Foo(1, "2") -""")) { case sce @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case sce @ PackageError.SourceConverterErrorsIn(_, _, _) => assert( sce.message( Map.empty, @@ -2734,7 +2734,7 @@ package Err enum Enum[a]: Foo(a: a), Bar(a: b) main = Foo(1, "2") -""")) { case sce @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case sce @ PackageError.SourceConverterErrorsIn(_, _, _) => assert( sce.message( Map.empty, @@ -3004,7 +3004,7 @@ struct Foo struct Foo(x) main = Foo(1) -""")) { case sce @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case sce @ PackageError.SourceConverterErrorsIn(_, _, _) => assert( sce.message( Map.empty, @@ -3024,7 +3024,7 @@ enum Bar: Foo struct Foo(x) main = Foo(1) -""")) { case sce @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case sce @ PackageError.SourceConverterErrorsIn(_, _, _) => assert( sce.message( Map.empty, @@ -3132,7 +3132,7 @@ out = match (1,2): case (a, a): a test = Assertion(True, "") -""")) { case sce @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case sce @ PackageError.SourceConverterErrorsIn(_, _, _) => assert( sce.message( Map.empty, @@ -3149,7 +3149,7 @@ out = match [(1,2), (1, 0)]: case _: 0 test = Assertion(True, "") -""")) { case sce @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case sce @ PackageError.SourceConverterErrorsIn(_, _, _) => assert( sce.message( Map.empty, @@ -3198,7 +3198,7 @@ struct Bar(baz: Either[Int, String]) test = Assertion(True, "") -""")) { case sce @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case sce @ PackageError.SourceConverterErrorsIn(_, _, _) => assert( sce.message( Map.empty, @@ -3287,7 +3287,7 @@ def foo[a](a: a) -> a: def and_again[b](x: a): x and_again(again(x)) -""")) { case sce @ PackageError.SourceConverterErrorIn(_, _) => +""")) { case sce @ PackageError.SourceConverterErrorsIn(_, _, _) => assert( sce.message( Map.empty, @@ -3870,7 +3870,7 @@ z = (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, """ evalFail(List(testCode)) { - case kie @ PackageError.SourceConverterErrorIn(_, _) => + case kie @ PackageError.SourceConverterErrorsIn(_, _, _) => val message = kie.message(Map.empty, Colorize.None) assert( message.contains( @@ -3896,7 +3896,7 @@ res = z matches (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, """ evalFail(List(testCode1)) { - case kie @ PackageError.SourceConverterErrorIn(_, _) => + case kie @ PackageError.SourceConverterErrorsIn(_, _, _) => val message = kie.message(Map.empty, Colorize.None) assert( message.contains( diff --git a/core/src/test/scala/org/bykn/bosatsu/ListUtilTest.scala b/core/src/test/scala/org/bykn/bosatsu/ListUtilTest.scala index 2483438b2..9ce5e60c9 100644 --- a/core/src/test/scala/org/bykn/bosatsu/ListUtilTest.scala +++ b/core/src/test/scala/org/bykn/bosatsu/ListUtilTest.scala @@ -94,4 +94,13 @@ class ListUtilTest extends AnyFunSuite { } } } + + test("distinctByHashSet works like List.distinct") { + forAll { (nel: NonEmptyList[Byte] ) => + val asList = nel.toList.distinct + val viaFn = ListUtil.distinctByHashSet(nel).toList + + assert(viaFn == asList) + } + } } diff --git a/core/src/test/scala/org/bykn/bosatsu/ParserTest.scala b/core/src/test/scala/org/bykn/bosatsu/ParserTest.scala index 28e6265dc..4d1929f5d 100644 --- a/core/src/test/scala/org/bykn/bosatsu/ParserTest.scala +++ b/core/src/test/scala/org/bykn/bosatsu/ParserTest.scala @@ -28,7 +28,8 @@ trait ParseFns { ("...(" + s.drop(idx - 20).take(30) + ")...") } - def firstDiff(s1: String, s2: String): String = + @annotation.tailrec + final def firstDiff(s1: String, s2: String): String = if (s1 == s2) "" else if (s1.isEmpty) s2 else if (s2.isEmpty) s1