Fix ensure

modules/core/shared/src/main/scala/io/circe/Decoder.scala

@@ -135,25 +135,52 @@ trait Decoder[A] extends Serializable { self =>
   /**
    * Build a new instance that fails if the condition does not hold for the
    * result.
+   *
+   * Note that in the case of chained calls to this method, only the first
+   * failure will be returned.
    */
   final def ensure(pred: A => Boolean, message: => String): Decoder[A] = new Decoder[A] {
     final def apply(c: HCursor): Decoder.Result[A] = self(c) match {
-      case l @ Left(_) => l
-      case Right(a)    => if (pred(a)) Right(a) else Left(DecodingFailure(message, c.history))
+      case r @ Right(a) => if (pred(a)) r else Left(DecodingFailure(message, c.history))
+      case l @ Left(_)  => l
     }
 
     override def decodeAccumulating(c: HCursor): AccumulatingDecoder.Result[A] = self.decodeAccumulating(c) match {
-      case v @ Valid(a) => if (pred(a)) v else Validated.invalidNel(DecodingFailure(message, c.history))
-      case Invalid(nel) =>
-        Validated.invalid(
-          AccumulatingDecoder.failureNelInstance.combine(nel, NonEmptyList(DecodingFailure(message, c.history), Nil))
-        )
+      case v @ Valid(a)   => if (pred(a)) v else Validated.invalidNel(DecodingFailure(message, c.history))
+      case i @ Invalid(_) => i
     }
   }
 
   /**
    * Build a new instance that fails with one or more errors if the condition
-   * does not hold for the input.
+   * does not hold for the result.
+   *
+   * If the result of the function applied to the decoded value is the empty
+   * list, the new decoder will succeed with that value.
+   */
+  final def ensure(errors: A => List[String]): Decoder[A] = new Decoder[A] {
+    final def apply(c: HCursor): Decoder.Result[A] = self(c) match {
+      case r @ Right(a) =>
+        errors(a) match {
+          case Nil          => r
+          case message :: _ => Left(DecodingFailure(message, c.history))
+        }
+      case l @ Left(_) => l
+    }
+
+    override def decodeAccumulating(c: HCursor): AccumulatingDecoder.Result[A] = self.decodeAccumulating(c) match {
+      case v @ Valid(a) =>
+        errors(a).map(DecodingFailure(_, c.history)) match {
+          case Nil    => v
+          case h :: t => Validated.invalid(NonEmptyList(h, t))
+        }
+      case i @ Invalid(_) => i
+    }
+  }
+
+  /**
+   * Build a new instance that fails if the condition does not hold for the
+   * input.
    *
    * Note that this condition is checked before decoding with the current
    * decoder, and if it does not hold, decoding does not continue. This means

modules/tests/shared/src/test/scala/io/circe/DecoderSuite.scala

@@ -322,6 +322,9 @@ class DecoderSuite extends CirceSuite with LargeNumberDecoderTests with TableDri
     assert(decoder.apply(friday.hcursor).isEmpty)
   }
 
+  val isPositive: Int => Boolean = _ > 0
+  val isOdd: Int => Boolean = _ % 2 != 0
+
   "ensure" should "fail appropriately on an invalid result" in forAll { (i: Int) =>
     val message = "Not positive!"
 
@@ -331,29 +334,57 @@ class DecoderSuite extends CirceSuite with LargeNumberDecoderTests with TableDri
     assert(decodePositiveInt.decodeJson(Json.fromInt(i)) === expected)
   }
 
-  it should "fail appropriately on an invalid result in error-accumulation mode" in forAll { (i: Int) =>
+  it should "only include the first failure when chained, even in error-accumulation mode" in forAll { (i: Int) =>
+    val positiveMessage = "Not positive!"
+    val oddMessage = "Not odd!"
+
+    val badDecodePositiveOddInt: Decoder[Int] =
+      Decoder[Int].ensure(isPositive, positiveMessage).ensure(isOdd, oddMessage)
+
+    val expected = if (isPositive(i)) {
+      if (isOdd(i)) {
+        Validated.valid(i)
+      } else {
+        Validated.invalidNel(DecodingFailure(oddMessage, Nil))
+      }
+    } else {
+      Validated.invalidNel(DecodingFailure(positiveMessage, Nil))
+    }
+
+    assert(badDecodePositiveOddInt.decodeAccumulating(Json.fromInt(i).hcursor) === expected)
+  }
+
+  it should "not include failures it hasn't checked for" in {
+    val decodePositiveInt: Decoder[Int] =
+      Decoder[Int].ensure(isPositive, "Not positive!")
+
+    val expected = Validated.invalidNel(DecodingFailure("Int", Nil))
+
+    assert(decodePositiveInt.decodeAccumulating(Json.Null.hcursor) === expected)
+  }
+
+  it should "include all given failures in error-accumulation mode" in forAll { (i: Int) =>
     val positiveMessage = "Not positive!"
     val oddMessage = "Not odd!"
 
     val decodePositiveOddInt: Decoder[Int] =
-      Decoder[Int].ensure(_ > 0, positiveMessage).ensure(_ % 2 == 1, oddMessage)
+      Decoder[Int].ensure(
+        i =>
+          (if (isPositive(i)) Nil else List(positiveMessage)) ++
+            (if (isOdd(i)) Nil else List(oddMessage))
+      )
 
-    val expected = if (i > 0) {
-      if (i % 2 == 1) {
+    val expected = if (isPositive(i)) {
+      if (isOdd(i)) {
         Validated.valid(i)
       } else {
         Validated.invalidNel(DecodingFailure(oddMessage, Nil))
       }
     } else {
-      if (i % 2 == 1) {
+      if (isOdd(i)) {
         Validated.invalidNel(DecodingFailure(positiveMessage, Nil))
       } else {
-        Validated.invalid(
-          NonEmptyList.of(
-            DecodingFailure(positiveMessage, Nil),
-            DecodingFailure(oddMessage, Nil)
-          )
-        )
+        Validated.invalid(NonEmptyList.of(DecodingFailure(positiveMessage, Nil), DecodingFailure(oddMessage, Nil)))
       }
     }