Fixes NPE in bindWithFilter (#134)

* Change MonadFilterLaws to use a gen to test bindWithFilter in order to generate all possible values * Fix bindWithFilter by filtering first, then binding (thanks to @nomisRev) Co-authored-by: danieh <daniel.montoya@47deg.com>
arrow-kt · May 29, 2020 · 5ae4cfa · 5ae4cfa
1 parent e9d2350
commit 5ae4cfa
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 11 deletions.
diff --git a/arrow-core-data/src/main/kotlin/arrow/typeclasses/MonadFilterContinuation.kt b/arrow-core-data/src/main/kotlin/arrow/typeclasses/MonadFilterContinuation.kt
@@ -19,8 +19,10 @@ interface MonadFilterSyntax<F> : MonadSyntax<F> {
   suspend fun <B> Kind<F, B>.bindWithFilter(f: (B) -> Boolean): B
 }
 
-open class MonadFilterContinuation<F, A>(val MF: MonadFilter<F>, override val context: CoroutineContext = EmptyCoroutineContext) :
-  MonadContinuation<F, A>(MF), MonadFilterSyntax<F> {
+open class MonadFilterContinuation<F, A>(
+  val MF: MonadFilter<F>,
+  override val context: CoroutineContext = EmptyCoroutineContext
+) : MonadContinuation<F, A>(MF), MonadFilterSyntax<F> {
 
   override fun resumeWith(result: Result<Kind<F, A>>) {
     result.fold({ super.resumeWith(result) }, {
@@ -43,8 +45,8 @@ open class MonadFilterContinuation<F, A>(val MF: MonadFilter<F>, override val co
    * Binds only if the given predicate matches the inner value otherwise binds into the Monad `empty()` value
    * on `MonadFilter` instances
    */
-  override suspend fun <B> Kind<F, B>.bindWithFilter(f: (B) -> Boolean): B {
-    val b: B = this.bind()
-    return if (f(b)) b else MF.empty<B>().bind()
-  }
+  override suspend fun <B> Kind<F, B>.bindWithFilter(f: (B) -> Boolean): B =
+    MF.run {
+      this@bindWithFilter.filter(f)
+    }.bind()
 }
diff --git a/arrow-core-test/src/main/kotlin/arrow/core/test/laws/MonadFilterLaws.kt b/arrow-core-test/src/main/kotlin/arrow/core/test/laws/MonadFilterLaws.kt
@@ -29,7 +29,7 @@ object MonadFilterLaws {
       Law("MonadFilter Laws: Right Empty") { MF.monadFilterRightEmpty(GEN, EQ) },
       Law("MonadFilter Laws: Consistency") { MF.monadFilterConsistency(GEN, EQ) },
       Law("MonadFilter Laws: Comprehension Guards") { MF.monadFilterEmptyComprehensions(EQ) },
-      Law("MonadFilter Laws: Comprehension bindWithFilter Guards") { MF.monadFilterBindWithFilterComprehensions(EQ) })
+      Law("MonadFilter Laws: Comprehension bindWithFilter Guards") { MF.monadFilterBindWithFilterComprehensions(GEN, EQ) })
   }
 
   fun <F> laws(
@@ -76,11 +76,11 @@ object MonadFilterLaws {
       }.equalUnderTheLaw(if (!guard) empty() else just(n), EQ)
     }
 
-  fun <F> MonadFilter<F>.monadFilterBindWithFilterComprehensions(EQ: Eq<Kind<F, Int>>): Unit =
-    forAll(Gen.bool(), Gen.int()) { guard: Boolean, n: Int ->
+  fun <F, A> MonadFilter<F>.monadFilterBindWithFilterComprehensions(G: Gen<Kind<F, A>>, EQ: Eq<Kind<F, A>>): Unit =
+    forAll(Gen.bool(), G) { guard: Boolean, fa: Kind<F, A> ->
       fx.monadFilter {
-        val x = just(n).bindWithFilter { _ -> guard }
+        val x = fa.bindWithFilter { guard }
         x
-      }.equalUnderTheLaw(if (!guard) empty() else just(n), EQ)
+      }.equalUnderTheLaw(if (!guard) empty() else fa, EQ)
     }
 }