Merge pull request #1212 from mikejcurry/non-empty-vector-value-class

Make NonEmptyVector a value class. #1204

build.sbt

@@ -112,7 +112,7 @@ lazy val disciplineDependencies = Seq(
 lazy val testingDependencies = Seq(
   libraryDependencies += "org.typelevel" %%% "catalysts-platform" % "0.0.2",
   libraryDependencies += "org.typelevel" %%% "catalysts-macros" % "0.0.2" % "test",
-  libraryDependencies += "org.scalatest" %%% "scalatest" % "3.0.0-M7" % "test")
+  libraryDependencies += "org.scalatest" %%% "scalatest" % "3.0.0-M8" % "test")
 
 
 /**
@@ -208,7 +208,7 @@ lazy val kernel = crossProject.crossType(CrossType.Pure)
   .jsSettings(commonJsSettings:_*)
   .jvmSettings((commonJvmSettings ++ (mimaPreviousArtifacts := Set("org.typelevel" %% "cats-kernel" % "0.6.0"))):_*)
 
-lazy val kernelJVM = kernel.jvm 
+lazy val kernelJVM = kernel.jvm
 lazy val kernelJS = kernel.js
 
 lazy val kernelLaws = crossProject.crossType(CrossType.Pure)

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

@@ -6,9 +6,13 @@ import scala.collection.immutable.VectorBuilder
 import cats.instances.vector._
 
 /**
- * A data type which represents a non empty Vector.
+ * A data type which represents a `Vector` guaranteed to contain at least one element.
+ * <br/>
+ * Note that the constructor is `private` to prevent accidental construction of an empty
+ * `NonEmptyVector`. However, due to https://issues.scala-lang.org/browse/SI-6601, on
+ * Scala 2.10, this may be bypassed due to a compiler bug.
  */
-final case class NonEmptyVector[A] private (toVector: Vector[A]) {
+final class NonEmptyVector[A] private (val toVector: Vector[A]) extends AnyVal {
 
   /** Gets the element at the index, if it exists */
   def get(i: Int): Option[A] =
@@ -19,14 +23,14 @@ final case class NonEmptyVector[A] private (toVector: Vector[A]) {
 
   /** Updates the element at the index, if it exists */
   def updated(i: Int, a: A): Option[NonEmptyVector[A]] =
-    if (toVector.isDefinedAt(i)) Some(NonEmptyVector(toVector.updated(i, a))) else None
+    if (toVector.isDefinedAt(i)) Some(new NonEmptyVector(toVector.updated(i, a))) else None
 
   /**
    * Updates the element at the index, or throws an `IndexOutOfBoundsException`
    * if none exists (if `i` does not satisfy `0 <= i < length`).
    */
   def updatedUnsafe(i: Int, a: A):
-      NonEmptyVector[A] = NonEmptyVector(toVector.updated(i, a))
+      NonEmptyVector[A] = new NonEmptyVector(toVector.updated(i, a))
 
   def head: A = toVector.head
 
@@ -38,27 +42,22 @@ final case class NonEmptyVector[A] private (toVector: Vector[A]) {
   def filter(f: A => Boolean): Vector[A] = toVector.filter(f)
 
   /**
-   * Append another NonEmptyVector to this
+   * Alias for [[concat]]
    */
-  def concat(other: NonEmptyVector[A]): NonEmptyVector[A] = NonEmptyVector(toVector ++ other.toVector)
-
-  /**
-   * Alias for concat
-   */
-  def ++(other: NonEmptyVector[A]): NonEmptyVector[A] = concat(other)
+  def ++(other: Vector[A]): NonEmptyVector[A] = concat(other)
 
   /**
-   * Append another Vector to this
+   * Append another `Vector` to this, producing a new `NonEmptyVector`.
    */
-  def concat(other: Vector[A]): NonEmptyVector[A] = NonEmptyVector(toVector ++ other)
+  def concat(other: Vector[A]): NonEmptyVector[A] = new NonEmptyVector(toVector ++ other)
 
   /**
-   * Alias for concat
+   * Append another `NonEmptyVector` to this, producing a new `NonEmptyVector`.
    */
-  def ++(other: Vector[A]): NonEmptyVector[A] = concat(other)
+  def concatNEV(other: NonEmptyVector[A]): NonEmptyVector[A] = new NonEmptyVector(toVector ++ other.toVector)
 
   /**
-   * find the first element matching the predicate, if one exists
+   * Find the first element matching the predicate, if one exists
    */
   def find(f: A => Boolean): Option[A] = toVector.find(f)
 
@@ -88,13 +87,13 @@ final case class NonEmptyVector[A] private (toVector: Vector[A]) {
     * Applies f to all the elements
     */
   def map[B](f: A => B): NonEmptyVector[B] =
-    NonEmptyVector(toVector.map(f))
+    new NonEmptyVector(toVector.map(f))
 
   /**
     *  Applies f to all elements and combines the result
     */
   def flatMap[B](f: A => NonEmptyVector[B]): NonEmptyVector[B] =
-    NonEmptyVector(toVector.flatMap(a => f(a).toVector))
+    new NonEmptyVector(toVector.flatMap(a => f(a).toVector))
 
   /**
     * Left-associative reduce using f.
@@ -129,6 +128,8 @@ final case class NonEmptyVector[A] private (toVector: Vector[A]) {
     s"NonEmpty${Show[Vector[A]].show(toVector)}"
 
   def length: Int = toVector.length
+
+  override def toString: String = s"NonEmpty${toVector.toString}"
 }
 
 private[data] sealed trait NonEmptyVectorInstances {
@@ -139,7 +140,7 @@ private[data] sealed trait NonEmptyVectorInstances {
         with Comonad[NonEmptyVector] with Traverse[NonEmptyVector] with MonadRec[NonEmptyVector] {
 
       def combineK[A](a: NonEmptyVector[A], b: NonEmptyVector[A]): NonEmptyVector[A] =
-        a concat b
+        a concatNEV b
 
       override def split[A](fa: NonEmptyVector[A]): (A, Vector[A]) = (fa.head, fa.tail)
 
@@ -213,19 +214,19 @@ private[data] sealed trait NonEmptyVectorInstances {
 object NonEmptyVector extends NonEmptyVectorInstances {
 
   def apply[A](head: A, tail: Vector[A]): NonEmptyVector[A] =
-    NonEmptyVector(head +: tail)
+    new NonEmptyVector(head +: tail)
 
   def apply[A](head: A, tail: A*): NonEmptyVector[A] = {
     val buf = Vector.newBuilder[A]
     buf += head
     tail.foreach(buf += _)
-    NonEmptyVector(buf.result)
+    new NonEmptyVector(buf.result)
   }
 
   def fromVector[A](vector: Vector[A]): Option[NonEmptyVector[A]] =
     if (vector.isEmpty) None else Some(new NonEmptyVector(vector))
 
   def fromVectorUnsafe[A](vector: Vector[A]): NonEmptyVector[A] =
-    if (vector.nonEmpty) NonEmptyVector(vector)
+    if (vector.nonEmpty) new NonEmptyVector(vector)
     else throw new IllegalArgumentException("Cannot create NonEmptyVector from empty vector")
 }

tests/src/test/scala/cats/tests/NonEmptyVectorTests.scala

@@ -1,12 +1,16 @@
 package cats
 package tests
 
+import catalysts.Platform
+
 import cats.kernel.laws.{GroupLaws, OrderLaws}
 
 import cats.data.NonEmptyVector
 import cats.laws.discipline.{ComonadTests, SemigroupKTests, FoldableTests, SerializableTests, TraverseTests, ReducibleTests, MonadRecTests}
 import cats.laws.discipline.arbitrary._
 
+import scala.util.Properties
+
 class NonEmptyVectorTests extends CatsSuite {
   // Lots of collections here.. telling ScalaCheck to calm down a bit
   implicit override val generatorDrivenConfig: PropertyCheckConfiguration =
@@ -55,7 +59,7 @@ class NonEmptyVectorTests extends CatsSuite {
       nonEmptyVector.size should === (nonEmptyVector.toList.size.toLong)
     }
   }
- 
+
 
   test("Show is not empty and is formatted as expected") {
     forAll { (nonEmptyVector: NonEmptyVector[Int]) =>
@@ -169,9 +173,9 @@ class NonEmptyVectorTests extends CatsSuite {
     }
   }
 
-  test("++ NonEmptyVector is consistent with concat") {
+  test("++ Vector is consistent with concatNEV") {
     forAll { (nonEmptyVector: NonEmptyVector[Int], other: NonEmptyVector[Int]) =>
-      nonEmptyVector ++ other should === (nonEmptyVector.concat(other))
+      nonEmptyVector ++ other.toVector should === (nonEmptyVector.concatNEV(other))
     }
   }
 
@@ -218,6 +222,42 @@ class NonEmptyVectorTests extends CatsSuite {
       }
     }
   }
+
+  test("NonEmptyVector#hashCode consistent with Vector#hashCode") {
+    forAll { (nonEmptyVector: NonEmptyVector[Int]) =>
+      nonEmptyVector.hashCode should === (nonEmptyVector.toVector.hashCode)
+    }
+  }
+
+  test("NonEmptyVector#equals consistent with Vector#equals") {
+    forAll { (lhs: NonEmptyVector[Int], rhs: NonEmptyVector[Int]) =>
+      lhs.equals(rhs) should === (lhs.toVector.equals(rhs.toVector))
+    }
+  }
+
+  test("NonEmptyVector#toString produces correct output") {
+    forAll { (nonEmptyVector: NonEmptyVector[Int]) =>
+      nonEmptyVector.toString should === (s"NonEmpty${nonEmptyVector.toVector.toString}")
+    }
+    NonEmptyVector(1, Vector.empty).toString should === ("NonEmptyVector(1)")
+    NonEmptyVector(1, Vector.empty).toVector.toString should === ("Vector(1)")
+  }
+
+  test("Cannot create a new NonEmptyVector from constructor") {
+    if(Platform.isJvm) {
+      if (!Properties.versionNumberString.startsWith("2.10")) {
+        // A bug in scala 2.10 allows private constructors to be accessed.
+        // We should still ensure that on scala 2.11 and up we cannot construct the
+        // object directly. see: https://issues.scala-lang.org/browse/SI-6601
+        "val bad: NonEmptyVector[Int] = new NonEmptyVector(Vector(1))" shouldNot compile
+      }
+    }
+  }
+
+  test("Cannot create a new NonEmptyVector from apply with an empty vector") {
+    "val bad: NonEmptyVector[Int] = NonEmptyVector(Vector(1))" shouldNot compile
+  }
+
 }
 
 class ReducibleNonEmptyVectorCheck extends ReducibleCheck[NonEmptyVector]("NonEmptyVector") {