Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/suppress warnings in kernel and kernel-laws #4614

Merged
merged 2 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ trait MonoidLaws[A] extends SemigroupLaws[A] {
def repeat0(x: A): IsEq[A] =
S.combineN(x, 0) <-> S.empty

def collect0(x: A): IsEq[A] =
def collect0: IsEq[A] =
S.combineAll(Nil) <-> S.empty

def combineAll(xs: Vector[A]): IsEq[A] =
Expand All @@ -44,6 +44,8 @@ trait MonoidLaws[A] extends SemigroupLaws[A] {
def isId(x: A, eqv: Eq[A]): IsEq[Boolean] =
eqv.eqv(x, S.empty) <-> S.isEmpty(x)(eqv)

@deprecated("use `collect0` without parameters", "2.12.1")
def collect0(x: A): IsEq[A] = collect0
}

object MonoidLaws {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
package cats.kernel
package laws

import cats.kernel.Order

trait OrderLaws[A] extends PartialOrderLaws[A] {

implicit override def E: Order[A]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,16 @@ import org.scalacheck.Prop.forAll
trait LowerBoundedTests[A] extends PartialOrderTests[A] {
def laws: LowerBoundedLaws[A]

def lowerBounded(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqOA: Eq[Option[A]], eqA: Eq[A]): RuleSet =
def lowerBounded(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A]): RuleSet =
new DefaultRuleSet(
"lowerBounded",
Some(partialOrder),
"bound is less than or equals" -> forAll(laws.boundLteqv _)
)

@deprecated("use `lowerBounded` without `Eq` parameters", "2.12.1")
def lowerBounded(arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqOA: Eq[Option[A]], eqA: Eq[A]): RuleSet =
lowerBounded(arbA, arbF)
}

object LowerBoundedTests {
Expand All @@ -47,12 +51,16 @@ object LowerBoundedTests {
trait UpperBoundedTests[A] extends PartialOrderTests[A] {
def laws: UpperBoundedLaws[A]

def upperBounded(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqOA: Eq[Option[A]], eqA: Eq[A]): RuleSet =
def upperBounded(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A]): RuleSet =
new DefaultRuleSet(
"upperBounded",
Some(partialOrder),
"bound is greater than or equals" -> forAll(laws.boundGteqv _)
)

@deprecated("use `upperBounded` without `Eq` parameters", "2.12.1")
def upperBounded(arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqOA: Eq[Option[A]], eqA: Eq[A]): RuleSet =
upperBounded(arbA, arbF)
}

object UpperBoundedTests {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,40 +32,41 @@ trait PartialNextTests[A] extends PartialOrderTests[A] {

def laws: PartialNextLaws[A]

def partialNext(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqOA: Eq[Option[A]], eqA: Eq[A]): RuleSet =
def partialNext(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A]): RuleSet =
new DefaultRuleSet(
"partialNext",
Some(partialOrder),
"next(a) > a" -> forAll(laws.nextOrderWeak _),
"forall a, b. if a < b. next(a) <= b" -> forAll(laws.nextOrderStrong _)
)

@deprecated("use `partialNext` without `Eq` parameters", "2.12.1")
def partialNext(arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqOA: Eq[Option[A]], eqA: Eq[A]): RuleSet =
partialNext(arbA, arbF)
}

trait PartialPreviousTests[A] extends PartialOrderTests[A] {

def laws: PartialPreviousLaws[A]

def partialPrevious(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqOA: Eq[Option[A]], eqA: Eq[A]): RuleSet =
def partialPrevious(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A]): RuleSet =
new DefaultRuleSet(
"partialPrevious",
Some(partialOrder),
"next(a) > a" -> forAll(laws.previousOrderWeak _),
"forall a, b. if a < b. next(a) <= b" -> forAll(laws.previousOrderStrong _)
)

@deprecated("use `partialPrevious` without `Eq` parameters", "2.12.1")
def partialPrevious(arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqOA: Eq[Option[A]], eqA: Eq[A]): RuleSet =
partialPrevious(arbA, arbF)
}

trait BoundedEnumerableTests[A] extends OrderTests[A] with PartialNextTests[A] with PartialPreviousTests[A] {

def laws: BoundedEnumerableLaws[A]

def boundedEnumerable(implicit
arbA: Arbitrary[A],
arbF: Arbitrary[A => A],
eqOA: Eq[Option[A]],
eqA: Eq[A]
): RuleSet =
def boundedEnumerable(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqOA: Eq[Option[A]]): RuleSet =
new RuleSet {
val name: String = "boundedEnumerable"
val bases: Seq[(String, RuleSet)] = Nil
Expand All @@ -78,6 +79,14 @@ trait BoundedEnumerableTests[A] extends OrderTests[A] with PartialNextTests[A] w
)
}

@deprecated("use `boundedEnumerable` without `Eq[A]` parameter", "2.12.1")
def boundedEnumerable(
arbA: Arbitrary[A],
arbF: Arbitrary[A => A],
eqOA: Eq[Option[A]],
eqA: Eq[A]
): RuleSet =
boundedEnumerable(arbA, arbF, eqOA)
}

object BoundedEnumerableTests {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,16 @@ trait HashTests[A] extends EqTests[A] {

def laws: HashLaws[A]

def hash(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqA: Eq[A], hashA: Hashing[A]): RuleSet =
def hash(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A]): RuleSet =
new DefaultRuleSet(
"hash",
Some(eqv),
"hash compatibility" -> forAll(laws.hashCompatibility _)
)

@deprecated("use `hash` without `Hashing` parameter", "2.12.1")
def hash(arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqA: Eq[A], hashA: Hashing[A]): RuleSet =
hash(arbA, arbF)
}

object HashTests {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ package discipline

import cats.kernel.instances.boolean._
import org.scalacheck.Arbitrary
import org.scalacheck.Prop
import org.scalacheck.Prop.forAll

trait MonoidTests[A] extends SemigroupTests[A] {
Expand All @@ -39,7 +40,7 @@ trait MonoidTests[A] extends SemigroupTests[A] {
"left identity" -> forAll(laws.leftIdentity _),
"right identity" -> forAll(laws.rightIdentity _),
"combine all" -> forAll(laws.combineAll _),
"collect0" -> forAll(laws.collect0 _),
"collect0" -> (laws.collect0: Prop),
"is id" -> forAll((a: A) => laws.isId(a, eqA)),
"repeat0" -> forAll(laws.repeat0 _)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ trait OrderTests[A] extends PartialOrderTests[A] {

def laws: OrderLaws[A]

def order(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqOA: Eq[Option[A]], eqA: Eq[A]): RuleSet =
def order(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A]): RuleSet =
new DefaultRuleSet(
"order",
Some(partialOrder),
Expand All @@ -42,6 +42,9 @@ trait OrderTests[A] extends PartialOrderTests[A] {
"min" -> forAll(laws.min _)
)

@deprecated("use `order` without `Eq` parameters", "2.12.1")
def order(arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqOA: Eq[Option[A]], eqA: Eq[A]): RuleSet =
order(arbA, arbF)
}

object OrderTests {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ trait PartialOrderTests[A] extends EqTests[A] {

def laws: PartialOrderLaws[A]

def partialOrder(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqOA: Eq[Option[A]], eqA: Eq[A]): RuleSet =
def partialOrder(implicit arbA: Arbitrary[A], arbF: Arbitrary[A => A]): RuleSet =
new DefaultRuleSet(
"partialOrder",
Some(eqv),
Expand All @@ -48,6 +48,9 @@ trait PartialOrderTests[A] extends EqTests[A] {
"pmin" -> forAll(laws.pmin _)
)

@deprecated("use `partialOrder` without `Eq` parameters", "2.12.1")
def partialOrder(arbA: Arbitrary[A], arbF: Arbitrary[A => A], eqOA: Eq[Option[A]], eqA: Eq[A]): RuleSet =
partialOrder(arbA, arbF)
}

object PartialOrderTests {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,19 @@ package compat
* limitations under the License.
*/

import scala.annotation.nowarn

private[kernel] class HashCompat {
// Adapted from scala.util.hashing.MurmurHash#productHash.
private[kernel] def product1HashWithPrefix(_1Hash: Int, prefix: String): Int = {
private[kernel] def product1HashWithPrefix(_1Hash: Int, @nowarn("cat=unused") prefix: String): Int = {
import scala.util.hashing.MurmurHash3._
var h = productSeed
h = mix(h, _1Hash)
finalizeHash(h, 1)
}

// Adapted from scala.util.hashing.MurmurHash#productHash.
private[cats] def product2HashWithPrefix(_1Hash: Int, _2Hash: Int, prefix: String): Int = {
private[cats] def product2HashWithPrefix(_1Hash: Int, _2Hash: Int, @nowarn("cat=unused") prefix: String): Int = {
import scala.util.hashing.MurmurHash3._
var h = productSeed
h = mix(h, _1Hash)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,15 @@
package cats.kernel
package instances

import org.typelevel.scalaccompat.annotation.unused

import scala.collection.mutable

import compat.scalaVersionSpecific._

@suppressUnusedImportWarningForScalaVersionSpecific
trait MapInstances extends MapInstances1 {
implicit def catsKernelStdHashForMap[K: Hash, V: Hash]: Hash[Map[K, V]] =
implicit def catsKernelStdHashForMap[K, V](implicit @unused K: Hash[K], V: Hash[V]): Hash[Map[K, V]] =
new MapHash[K, V]

implicit def catsKernelStdCommutativeMonoidForMap[K, V: CommutativeSemigroup]: CommutativeMonoid[Map[K, V]] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@
package cats.kernel
package instances

import compat.scalaVersionSpecific._

import scala.annotation.nowarn
import scala.collection.immutable.Seq

import compat.scalaVersionSpecific._

@suppressUnusedImportWarningForScalaVersionSpecific
trait SeqInstances extends SeqInstances1 {
implicit def catsKernelStdOrderForSeq[A: Order]: Order[Seq[A]] =
Expand Down Expand Up @@ -83,7 +83,9 @@ class SeqMonoid[A] extends Monoid[Seq[A]] {
}

object SeqMonoid {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we want to not just deprecate SeqMonoid but also make it private [cats]:

@deprecated("Use SeqMonoid.apply, which does not allocate a new instance", "2.9.0")
class SeqMonoid[A] extends Monoid[Seq[A]] {

Looks like SeqMonoid is not supposed to be user-facing, so it should be a fairly safe change.
It would be out of scope of this PR, of course, but if it makes sense, I can do it here, because it is still related to the warnings cause.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd be a source breaking change for whoever used it that way. Even though they shouldn't use it, it's nice to make it very clear how to fix it. I don't tend to like to remove deprecations until keeping them causes pain.

Copy link
Contributor Author

@satorg satorg Jun 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel the best strategy here would be to provide a scalafix rule for that. But it would definitely come out of the scope of this PR.

@nowarn("msg=deprecated")
@nowarn("cat=deprecation")
private[this] val singleton: Monoid[Seq[Any]] = new SeqMonoid[Any]

@nowarn("cat=deprecation")
def apply[A]: SeqMonoid[A] = singleton.asInstanceOf[SeqMonoid[A]]
}