typelevel / typelevel/cats

Deprecate nonInheritedOps and ops objects?

Open
#3,330 3 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

question
Dominant language
Scala
Stars
5.5k
Forks
1.2k
Avg merge
2d 10h
Merged PRs (30d)
5

Description

Update: now that we've switched to the Scalafix-powered version of Simulacrum (see #3424), we can easily deprecate these definitions, and I personally think we should do this in Cats 2.2.0. If anyone has objections please raise them here.


Cats currently has three ways of importing syntax methods for particular type classes. You can use the subpackage in syntax (this is the standard approach to à la carte imports):

scala> import cats.syntax.functor._
import cats.syntax.functor._

scala> cats.Eval.now(List(0)).widen[Seq[Int]]
res1: cats.Eval[Seq[Int]] = Now(List(0))

You could also use the nonInheritedOps object in the type class companion, which does exactly the same thing:

scala> import cats.Functor.nonInheritedOps._
import cats.Functor.nonInheritedOps._

scala> cats.Eval.now(List(0)).widen[Seq[Int]]
res0: cats.Eval[Seq[Int]] = Now(List(0))

Or you can use the ops object in the companion of any descendant type class:

scala> import cats.Monad.ops._
import cats.Monad.ops._

scala> cats.Eval.now(List(0)).widen[Seq[Int]]
res0: cats.Eval[Seq[Int]] = Now(List(0))

While this last is at least different from the standard cats.syntax.abc._ approach, and maybe potentially useful, I've never actually seen anyone use it, or recommend it, and it has a serious downside—if you import ops._ from multiple type class companions, you lose the syntax methods for the intersection of their ancestors:

scala> import cats.instances.all._, cats.Traverse.ops._, cats.Alternative.ops._
import cats.instances.all._
import cats.Traverse.ops._
import cats.Alternative.ops._

scala> List(1) <+> List(2) <+> List(3)
res0: List[Int] = List(1, 2, 3)

scala> List(Some(1), None, Some(2)).sequence
res1: Option[List[Int]] = None

scala> List(1, 2, 3).fmap(_ + 1)
           ^
       error: type mismatch;
        found   : List[Int]
        required: ?{def fmap: ?}
       Note that implicit conversions are not applicable because they are ambiguous:
        both method toAllTraverseOps in object ops of type [F[_], C](target: F[C])(implicit tc: cats.Traverse[F])cats.Traverse.AllOps[F,C]{type TypeClassType = cats.Traverse[F]}
        and method toAllAlternativeOps in object ops of type [F[_], C](target: F[C])(implicit tc: cats.Alternative[F])cats.Alternative.AllOps[F,C]{type TypeClassType = cats.Alternative[F]}
        are possible conversion functions from List[Int] to ?{def fmap: ?}

Or even worse:

scala> List(1, 2, 3).as(0)
                     ^
       error: value as is not a member of List[Int]
       did you mean last, map, or max?

…which is probably why nobody uses or recommends these.

So nonInheritedOps is redundant, and ops makes it really easy to shoot yourself in the foot. I think we should get rid of both of them in Cats 3, and deprecate them now.

These objects are generated by Simulacrum, and I'm not entirely sure it's possible to output deprecated methods using Scala 2's macro annotations (?), but if we move to the Scalafix approach this would be pretty easy.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the Scalafix-powered Simulacrum change in #3424 and the generated nonInheritedOps and ops definitions described here. Determine how both definitions can be deprecated while preserving the standard syntax imports, then verify that the deprecation approach matches the intended Cats 2.2.0 and Cats 3 migration plan.

Written by the indexing model from the issue text.

Assessment

Tech stack
scala
Domain
tooling
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.