Finish up work on NonEmptyList

This continues work started by @WarFox in typelevel#1120 (see [this comment](typelevel#1120 (comment))).

At this point, I have left `OneAnd` in place. However, I think that
after merging this we may want to delete it. In practice it's pretty
awkward to use and sometimes prevents performant operations. See also typelevel#1089.

core/src/main/scala/cats/Traverse.scala

@@ -96,9 +96,9 @@ import simulacrum.typeclass
    * scala> import cats.implicits._
    * scala> val x: List[ValidatedNel[String, Int]] = List(Validated.valid(1), Validated.invalid("a"), Validated.invalid("b")).map(_.toValidatedNel)
    * scala> x.sequenceU
-   * res0: cats.data.ValidatedNel[String,List[Int]] = Invalid(OneAnd(a,List(b)))
+   * res0: cats.data.ValidatedNel[String,List[Int]] = Invalid(NonEmptyList(a, b))
    * scala> x.sequence[ValidatedNel[String, ?], Int]
-   * res1: cats.data.ValidatedNel[String,List[Int]] = Invalid(OneAnd(a,List(b)))
+   * res1: cats.data.ValidatedNel[String,List[Int]] = Invalid(NonEmptyList(a, b))
    * }}}
    */
   def sequenceU[GA](fga: F[GA])(implicit U: Unapply[Applicative, GA]): U.M[F[U.A]] =

core/src/main/scala/cats/data/NonEmptyList.scala

@@ -1,6 +1,12 @@
 package cats
 package data
 
+import cats.instances.list._
+import cats.syntax.eq._
+
+import scala.annotation.tailrec
+import scala.collection.mutable.ListBuffer
+
 /**
  * A data type which represents a non empty list of A, with
  * single element (head) and optional structure (tail).
@@ -12,6 +18,18 @@ final case class NonEmptyList[A](head: A, tail: List[A]) {
    */
   def toList: List[A] = head :: tail
 
+  /**
+   *  Applies f to all the elements of the structure
+   */
+  def map[B](f: A => B): NonEmptyList[B] =
+    NonEmptyList(f(head), tail.map(f))
+
+  def ++(l: List[A]): NonEmptyList[A] =
+    NonEmptyList(head, tail ++ l)
+
+  def flatMap[B](f: A => NonEmptyList[B]): NonEmptyList[B] =
+    f(head) ++ tail.flatMap(f andThen (_.toList))
+
   /**
    * remove elements not matching the predicate
    */
@@ -21,7 +39,7 @@ final case class NonEmptyList[A](head: A, tail: List[A]) {
   /**
    * Append another NonEmptyList
    */
-  def combine(other: NonEmptyList[A]): NonEmptyList[A] =
+  def concat(other: NonEmptyList[A]): NonEmptyList[A] =
     NonEmptyList(head, tail ::: other.toList)
 
   /**
@@ -40,7 +58,7 @@ final case class NonEmptyList[A](head: A, tail: List[A]) {
    * Check whether all elements satisfy the predicate
    */
   def forall(p: A => Boolean): Boolean =
-    p(head) && tail.exists(p)
+    p(head) && tail.forall(p)
 
   /**
    * Left-associative fold on the structure using f.
@@ -52,110 +70,124 @@ final case class NonEmptyList[A](head: A, tail: List[A]) {
    * Right-associative fold on the structure using f.
    */
   def foldRight[B](lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] =
-    toList.foldRight(lb)(f)
+    Foldable[List].foldRight(toList, lb)(f)
 
   /**
-   *  Applies f to all the elements of the structure
-   *  TODO It would be nice to have variance on this particular method def map[AA <: A, B](f: AA => B): NonEmptyList[B] so that you can pass a function for a supertype of A into map. @yilinwei
-   * https://github.com/typelevel/cats/pull/1120#discussion-diff-66881573
+   * Left-associative reduce using f.
    */
-  def map[B](f: A => B): NonEmptyList[B] =
-    NonEmptyList(f(head), tail.map(f))
-}
+  def reduceLeft(f: (A, A) => A): A =
+    tail.foldLeft(head)(f)
+
+  def coflatMap[B](f: NonEmptyList[A] => B): NonEmptyList[B] = {
+    @tailrec def consume(as: List[A], buf: ListBuffer[B]): List[B] =
+      as match {
+        case Nil => buf.toList
+        case a :: as => consume(as, buf += f(NonEmptyList(a, as)))
+      }
+    NonEmptyList(f(this), consume(this.tail, ListBuffer.empty))
+  }
 
-private[data] sealed trait NonEmptyListInstances extends NonEmptyListLowPriority2 {
+  def ===(o: NonEmptyList[A])(implicit A: Eq[A]): Boolean =
+    (this.head === o.head) && this.tail === o.tail
 
+  def show(implicit A: Show[A]): String =
+    toList.iterator.map(A.show).mkString("NonEmptyList(", ", ", ")")
+
+  override def toString: String = s"NonEmpty$toList"
+}
+
+object NonEmptyList extends NonEmptyListInstances {
   def apply[A](head: A, tail: A*): NonEmptyList[A] = NonEmptyList(head, tail.toList)
 
-  implicit def catsDataEqForNonEmptyList[A](implicit A: Eq[A]): Eq[NonEmptyList[A]] =
-    new Eq[NonEmptyList[A]] {
-      def eqv(x: NonEmptyList[A], y: NonEmptyList[A]): Boolean = x === y
-    }
+  def fromList[A](l: List[A]): Option[NonEmptyList[A]] =
+    if (l.isEmpty) None else Some(NonEmptyList(l.head, l.tail))
 
-  implicit def catsDataShowForNonEmptyList[A](implicit A: Show[A]): Show[NonEmptyList[A]] =
-    Show.show[NonEmptyList[A]](_.show)
+  def fromListUnsafe[A](l: List[A]): NonEmptyList[A] =
+    if (l.nonEmpty) NonEmptyList(l.head, l.tail)
+    else throw new IllegalArgumentException("Cannot create NonEmptyList from empty list")
+}
+
+private[data] sealed trait NonEmptyListInstances extends NonEmptyListInstances0 {
+
+  implicit val catsDataInstancesForNonEmptyList: SemigroupK[NonEmptyList] with Reducible[NonEmptyList]
+      with Comonad[NonEmptyList] with Traverse[NonEmptyList] with MonadRec[NonEmptyList] =
+    new NonEmptyReducible[NonEmptyList, List] with SemigroupK[NonEmptyList]
+        with Comonad[NonEmptyList] with Traverse[NonEmptyList] with MonadRec[NonEmptyList] {
 
-  implicit def catsDataSemigroupKForNonEmptyList[A]: SemigroupK[NonEmptyList[?]] =
-    new SemigroupK[NonEmptyList[?]] {
       def combineK[A](a: NonEmptyList[A], b: NonEmptyList[A]): NonEmptyList[A] =
-        a combine b
-    }
+        a concat b
 
-  implicit def catsDataSemigroupForNonEmptyList[A]: Semigroup[NonEmptyList[A]] =
-    catsDataSemigroupKForNonEmptyList[F].algebra
+      override def split[A](fa: NonEmptyList[A]): (A, List[A]) = (fa.head, fa.tail)
 
-  implicit def catsDataReducibleForNonEmptyList[A]: Reducible[NonEmptyList[?]] =
-    new NonEmptyReducible[NonEmptyList[?]] {
-      override def split[A](fa: NonEmptyList[A]): (A, F[A]) = (fa.head, fa.tail)
-    }
+      override def reduceLeft[A](fa: NonEmptyList[A])(f: (A, A) => A): A =
+        fa.reduceLeft(f)
 
-  implicit def catsDataMonadForNonEmptyList[A]: Monad[NonEmptyList[?]] =
-    new Monad[NonEmptyList[?]] {
-      override def map[A, B](fa: NonEmptyList[F, A])(f: A => B): NonEmptyList[F, B] =
+      override def map[A, B](fa: NonEmptyList[A])(f: A => B): NonEmptyList[B] =
         fa map f
 
-      def pure[A](x: A): NonEmptyList[F, A] =
-        NonEmptyList(x, Nil)
-
-      // TODO Could we move this method (and other type class methods) to NonEmptyList and then reference them in the instances? I think that will make the scaladoc for NonEmptyList a bit nicer. @non
-      def flatMap[A, B](fa: NonEmptyList[F, A])(f: A => NonEmptyList[F, B]): NonEmptyList[F, B] = {
-        val end = monad.flatMap(fa.tail) { a =>
-          val fa = f(a)
-          monad.combineK(monad.pure(fa.head), fa.tail)
-        }
-        val fst = f(fa.head)
-        NonEmptyList(fst.head, monad.combineK(fst.tail, end))
-        // TODO @yilinwei https://github.com/typelevel/cats/pull/1120#discussion_r66882139
-        // val xs = f(head) ++ tail.flatMap(f.andThen(_.toList))
-        // NonEmptyList(xs.head, xs.tail)
-      }
-    }
-}
+      def pure[A](x: A): NonEmptyList[A] =
+        NonEmptyList(x, List.empty)
+
+      def flatMap[A, B](fa: NonEmptyList[A])(f: A => NonEmptyList[B]): NonEmptyList[B] =
+        fa flatMap f
 
-trait NonEmptyListLowPriority0 {
-  implicit val nelComonad: Comonad[NonEmptyList[List, ?]] =
-    new Comonad[NonEmptyList[List, ?]] {
-      def coflatMap[A, B](fa: NonEmptyList[List, A])(f: NonEmptyList[List, A] => B): NonEmptyList[List, B] = {
-        @tailrec def consume(as: List[A], buf: ListBuffer[B]): List[B] =
-          as match {
-            case Nil => buf.toList
-            case a :: as => consume(as, buf += f(NonEmptyList(a, as)))
+      def coflatMap[A, B](fa: NonEmptyList[A])(f: NonEmptyList[A] => B): NonEmptyList[B] =
+        fa coflatMap f
+
+      def extract[A](fa: NonEmptyList[A]): A = fa.head
+
+      def traverse[G[_], A, B](fa: NonEmptyList[A])(f: (A) => G[B])(implicit G: Applicative[G]): G[NonEmptyList[B]] =
+        G.map2Eval(f(fa.head), Always(Traverse[List].traverse(fa.tail)(f)))(NonEmptyList(_, _)).value
+
+      override def foldLeft[A, B](fa: NonEmptyList[A], b: B)(f: (B, A) => B): B =
+        fa.foldLeft(b)(f)
+
+      override def foldRight[A, B](fa: NonEmptyList[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] =
+        fa.foldRight(lb)(f)
+
+      def tailRecM[A, B](a: A)(f: A => NonEmptyList[A Xor B]): NonEmptyList[B] = {
+        val buf = new ListBuffer[B]
+        @tailrec def go(v: NonEmptyList[A Xor B]): Unit = v.head match {
+            case Xor.Right(b) =>
+            buf += b
+            NonEmptyList.fromList(v.tail) match {
+              case Some(t) => go(t)
+              case None => ()
+            }
+          case Xor.Left(a) => go(f(a) ++ v.tail)
           }
-        NonEmptyList(f(fa), consume(fa.tail, ListBuffer.empty))
+        go(f(a))
+        NonEmptyList.fromListUnsafe(buf.result())
       }
 
-      def extract[A](fa: NonEmptyList[List, A]): A =
-        fa.head
+      override def forall[A](fa: NonEmptyList[A])(p: A => Boolean): Boolean =
+        fa forall p
 
-      def map[A, B](fa: NonEmptyList[List, A])(f: A => B): NonEmptyList[List, B] =
-        fa map f
-    }
-}
+      override def exists[A](fa: NonEmptyList[A])(p: A => Boolean): Boolean =
+        fa exists p
 
-trait NonEmptyListLowPriority1 extends NonEmptyListLowPriority0 {
-  implicit def catsDataFunctorForNonEmptyList[F[_]](implicit F: Functor[F]): Functor[NonEmptyList[F, ?]] =
-    new Functor[NonEmptyList[F, ?]] {
-      def map[A, B](fa: NonEmptyList[F, A])(f: A => B): NonEmptyList[F, B] =
-        fa map f
+      override def toList[A](fa: NonEmptyList[A]): List[A] = fa.toList
     }
 
+  implicit def catsDataShowForNonEmptyList[A](implicit A: Show[A]): Show[NonEmptyList[A]] =
+    Show.show[NonEmptyList[A]](_.show)
+
+  implicit def catsDataSemigroupForNonEmptyList[A]: Semigroup[NonEmptyList[A]] =
+    SemigroupK[NonEmptyList].algebra[A]
+
+  implicit def catsDataOrderForNonEmptyList[A:Order]: Order[NonEmptyList[A]] =
+    Order.by(_.toList)
 }
 
-trait NonEmptyListLowPriority2 extends NonEmptyListLowPriority1 {
-  implicit def catsDataTraverseForNonEmptyList[F[_]](implicit F: Traverse[F]): Traverse[NonEmptyList[F, ?]] =
-    new Traverse[NonEmptyList[F, ?]] {
-      def traverse[G[_], A, B](fa: NonEmptyList[F, A])(f: (A) => G[B])(implicit G: Applicative[G]): G[NonEmptyList[F, B]] = {
-        G.map2Eval(f(fa.head), Always(F.traverse(fa.tail)(f)))(NonEmptyList(_, _)).value
-      }
+private[data] sealed trait NonEmptyListInstances0 extends NonEmptyListInstances1 {
+  implicit def catsDataPartialOrderForNonEmptyList[A:PartialOrder]: PartialOrder[NonEmptyList[A]] =
+    PartialOrder.by(_.toList)
+}
 
-      def foldLeft[A, B](fa: NonEmptyList[F, A], b: B)(f: (B, A) => B): B = {
-        fa.foldLeft(b)(f)
-      }
+private[data] sealed trait NonEmptyListInstances1 {
 
-      def foldRight[A, B](fa: NonEmptyList[F, A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = {
-        fa.foldRight(lb)(f)
-      }
+  implicit def catsDataEqForNonEmptyList[A](implicit A: Eq[A]): Eq[NonEmptyList[A]] =
+    new Eq[NonEmptyList[A]] {
+      def eqv(x: NonEmptyList[A], y: NonEmptyList[A]): Boolean = x === y
     }
 }
-
-object NonEmptyList extends NonEmptyListInstances

core/src/main/scala/cats/data/OneAnd.scala

@@ -2,8 +2,8 @@ package cats
 package data
 
 import scala.annotation.tailrec
-import scala.collection.mutable.ListBuffer
-import cats.instances.list._
+import scala.collection.mutable.Builder
+import cats.instances.stream._
 
 /**
  * A data type which represents a single element (head) and some other
@@ -140,21 +140,22 @@ private[data] sealed trait OneAndInstances extends OneAndLowPriority2 {
 }
 
 private[data] trait OneAndLowPriority0 {
-  implicit val catsDataComonadForOneAnd: Comonad[OneAnd[List, ?]] =
-    new Comonad[OneAnd[List, ?]] {
-      def coflatMap[A, B](fa: OneAnd[List, A])(f: OneAnd[List, A] => B): OneAnd[List, B] = {
-        @tailrec def consume(as: List[A], buf: ListBuffer[B]): List[B] =
-          as match {
-            case Nil => buf.toList
-            case a :: as => consume(as, buf += f(OneAnd(a, as)))
+  implicit val catsDataComonadForNonEmptyStream: Comonad[OneAnd[Stream, ?]] =
+    new Comonad[OneAnd[Stream, ?]] {
+      def coflatMap[A, B](fa: OneAnd[Stream, A])(f: OneAnd[Stream, A] => B): OneAnd[Stream, B] = {
+        @tailrec def consume(as: Stream[A], buf: Builder[B, Stream[B]]): Stream[B] =
+          if (as.isEmpty) buf.result
+          else {
+            val tail = as.tail
+            consume(tail, buf += f(OneAnd(as.head, tail)))
           }
-        OneAnd(f(fa), consume(fa.tail, ListBuffer.empty))
+        OneAnd(f(fa), consume(fa.tail, Stream.newBuilder))
       }
 
-      def extract[A](fa: OneAnd[List, A]): A =
+      def extract[A](fa: OneAnd[Stream, A]): A =
         fa.head
 
-      def map[A, B](fa: OneAnd[List, A])(f: A => B): OneAnd[List, B] =
+      def map[A, B](fa: OneAnd[Stream, A])(f: A => B): OneAnd[Stream, B] =
         fa map f
     }
 }

core/src/main/scala/cats/data/XorT.scala

@@ -173,7 +173,7 @@ final case class XorT[F[_], A, B](value: F[A Xor B]) {
    * scala> val v2: Validated[NonEmptyList[Error], Int] = Validated.Invalid(NonEmptyList("error 2"))
    * scala> val xort: XorT[Option, Error, Int] = XorT(Some(Xor.left("error 3")))
    * scala> xort.withValidated { v3 => (v1 |@| v2 |@| v3.leftMap(NonEmptyList(_))).map{ case (i, j, k) => i + j + k } }
-   * res0: XorT[Option, NonEmptyList[Error], Int] = XorT(Some(Left(OneAnd(error 1,List(error 2, error 3)))))
+   * res0: XorT[Option, NonEmptyList[Error], Int] = XorT(Some(Left(NonEmptyList(error 1, error 2, error 3))))
    * }}}
    */
   def withValidated[AA, BB](f: Validated[A, B] => Validated[AA, BB])(implicit F: Functor[F]): XorT[F, AA, BB] =

core/src/main/scala/cats/syntax/option.scala

@@ -103,7 +103,7 @@ final class OptionOps[A](val oa: Option[A]) extends AnyVal {
    *
    * scala> val error1: Option[String] = Some("error!")
    * scala> error1.toInvalidNel(3)
-   * res0: ValidatedNel[String, Int] = Invalid(OneAnd(error!,List()))
+   * res0: ValidatedNel[String, Int] = Invalid(NonEmptyList(error!))
    *
    * scala> val error2: Option[String] = None
    * scala> error2.toInvalidNel(3)
@@ -149,7 +149,7 @@ final class OptionOps[A](val oa: Option[A]) extends AnyVal {
    *
    * scala> val result2: Option[Int] = None
    * scala> result2.toValidNel("error!")
-   * res1: ValidatedNel[String, Int] = Invalid(OneAnd(error!,List()))
+   * res1: ValidatedNel[String, Int] = Invalid(NonEmptyList(error!))
    * }}}
    */
   def toValidNel[B](b: => B): ValidatedNel[B, A] = oa.fold[ValidatedNel[B, A]](Validated.invalidNel(b))(Validated.Valid(_))

laws/src/main/scala/cats/laws/discipline/Arbitrary.scala

@@ -15,7 +15,13 @@ object arbitrary extends ArbitraryInstances0 {
   // in the context of Int => Boolean. Once scalacheck supports better Function1 arbitrary
   // instances this can be removed.
   implicit def catsLawsArbitraryForIntToBool: Arbitrary[(Int) => Boolean] =
-    Arbitrary(getArbitrary[Int].map(x => (input) => input < x))
+    Arbitrary(
+      getArbitrary[Int].map(x =>
+       new Function1[Int, Boolean] {
+         def apply(i: Int): Boolean = i < x
+
+         override def toString = s"<function testing whether input is less than $x>"
+       }))
 
   implicit def catsLawsArbitraryForConst[A, B](implicit A: Arbitrary[A]): Arbitrary[Const[A, B]] =
     Arbitrary(A.arbitrary.map(Const[A, B]))
@@ -26,6 +32,9 @@ object arbitrary extends ArbitraryInstances0 {
   implicit def catsLawsArbitraryForNonEmptyVector[A](implicit A: Arbitrary[A]): Arbitrary[NonEmptyVector[A]] =
     Arbitrary(implicitly[Arbitrary[Vector[A]]].arbitrary.flatMap(fa => A.arbitrary.map(a => NonEmptyVector(a, fa))))
 
+  implicit def catsLawsArbitraryForNonEmptyList[A](implicit A: Arbitrary[A]): Arbitrary[NonEmptyList[A]] =
+    Arbitrary(implicitly[Arbitrary[List[A]]].arbitrary.flatMap(fa => A.arbitrary.map(a => NonEmptyList(a, fa))))
+
   implicit def catsLawsArbitraryForXor[A, B](implicit A: Arbitrary[A], B: Arbitrary[B]): Arbitrary[A Xor B] =
     Arbitrary(Gen.oneOf(A.arbitrary.map(Xor.left), B.arbitrary.map(Xor.right)))
 

tests/src/test/scala/cats/tests/CokleisliTests.scala

@@ -39,12 +39,6 @@ class CokleisliTests extends SlowCatsSuite {
     // Ceremony to help scalac to do the right thing, see also #267.
     type CokleisliNEL[A, B] = Cokleisli[NonEmptyList, A, B]
 
-    implicit def ev0[A: Arbitrary, B: Arbitrary]: Arbitrary[CokleisliNEL[A, B]] =
-      catsLawsArbitraryForCokleisli
-
-    implicit def ev1[A: Arbitrary, B: Eq]: Eq[CokleisliNEL[A, B]] =
-      cokleisliEq[NonEmptyList, A, B](catsLawsArbitraryForOneAnd, Eq[B])
-
     checkAll("Cokleisli[NonEmptyList, Int, Int]", ArrowTests[CokleisliNEL].arrow[Int, Int, Int, Int, Int, Int])
     checkAll("Arrow[Cokleisli[NonEmptyList, ?, ?]]", SerializableTests.serializable(Arrow[CokleisliNEL]))
   }
@@ -53,12 +47,6 @@ class CokleisliTests extends SlowCatsSuite {
     // More ceremony, see above
     type CokleisliNELE[A] = Cokleisli[NonEmptyList, A, A]
 
-    implicit def ev0[A: Arbitrary]: Arbitrary[CokleisliNELE[A]] =
-      catsLawsArbitraryForCokleisli[NonEmptyList, A, A]
-
-    implicit def ev1[A: Eq](implicit arb: Arbitrary[A]): Eq[CokleisliNELE[A]] =
-      cokleisliEqE[NonEmptyList, A](catsLawsArbitraryForOneAnd, Eq[A])
-
     {
       implicit val cokleisliMonoidK = Cokleisli.catsDataMonoidKForCokleisli[NonEmptyList]
       checkAll("Cokleisli[NonEmptyList, Int, Int]", MonoidKTests[CokleisliNELE].monoidK[Int])

tests/src/test/scala/cats/tests/ConstTests.scala

@@ -6,7 +6,7 @@ import cats.kernel.laws.{GroupLaws, OrderLaws}
 import cats.data.{Const, NonEmptyList}
 import cats.functor.Contravariant
 import cats.laws.discipline._
-import cats.laws.discipline.arbitrary.{catsLawsArbitraryForConst, catsLawsArbitraryForOneAnd}
+import cats.laws.discipline.arbitrary.{catsLawsArbitraryForConst, catsLawsArbitraryForNonEmptyList}
 
 class ConstTests extends CatsSuite {