Make whenA/unlessA syntax by-name lazy

[armanbilge]

Fixes #3687, supersedes #3899.

These changes are based on #3899 but binary and source compatible without introducing any new methods.

Note that replicateA also becomes by-name in these changes. This is unnecessary and possibly a minor de-optimization, but I don't see any way around it. Note that the usual package-private tricks don't work well with syntax e.g. see scala/bug#12578.

Edit: although, I suppose there's an argument that replicateA should be made by-name on the typeclass itself for when n = 0, but that's another can of worms :)

[satorg]

Note that replicateA also becomes by-name in these changes. This is unnecessary and possibly a minor de-optimization, but I don't see any way around it.

Maybe just keep both ApplicativeOps and ApplicativeByNameOps available (with their implicit conversions)? In that case ApplicativeByNameOps would receive whenA and unlessA while ApplicativeOps would keep replicateA. Apparently, the old whenA and unlessA should be made inaccessible by making them either package local or even protected.

[armanbilge]

Apparently, the old whenA and unlessA should be made inaccessible by making them either package local or even protected.

I'm afraid not, see scala/bug#12578 and linked issues for details.

[error] /workspace/cats/tests/shared/src/test/scala/cats/tests/ApplicativeSuite.scala:68:20: method whenA in class ApplicativeOps cannot be accessed in cats.syntax.ApplicativeOps[Option,Unit]
[error]  Access to protected method whenA not permitted because
[error]  enclosing class ApplicativeSuite in package tests is not a subclass of
[error]  class ApplicativeOps in package syntax where target is defined
[error]     Option(i += 1).whenA(false)
[error]                    ^
[error] /workspace/cats/tests/shared/src/test/scala/cats/tests/ApplicativeSuite.scala:72:20: method unlessA in class ApplicativeOps cannot be accessed in cats.syntax.ApplicativeOps[Option,Unit]
[error]  Access to protected method unlessA not permitted because
[error]  enclosing class ApplicativeSuite in package tests is not a subclass of
[error]  class ApplicativeOps in package syntax where target is defined
[error] Error occurred in an application involving default arguments.
[error]     Option(j += 1).unlessA(true)

[satorg]

Right.. Seems it is not going to be that easy :)
But still.. If we put the implicit conversion for the ApplicativeOps into a lower-level lookup scope, then it might work:

object outer {
  object syntax extends SyntaxLowLevel {
    implicit class Ops2(x: String) { def bar: Int = 2 }
  }
  trait SyntaxLowLevel {
    implicit class Ops1(x: String) {
      private[outer] def bar: Int = 1
      def car: String = "I'm also here"
    }
  }
}

object test extends App {
  import outer.syntax._
  val fooBar = "foo".bar
  val fooCar = "foo".car
  println(s"$fooBar.$fooCar")
}

https://scastie.scala-lang.org/satorg/Lhe4QuVMRqyaHtg8WRtzwQ/8
(modified this example: scala/bug#11787)

[satorg]

I mean, I wonder if this trick might be working:

trait ApplicativeSyntax extends ApplicativeSyntaxLowerLevel {
  implicit final def catsSyntaxApplicativeId[A](a: A): ApplicativeIdOps[A] =
    new ApplicativeIdOps[A](a)

  implicit final def catsSyntaxApplicativeByName[F[_], A](fa: => F[A]): ApplicativeByNameOps[F, A] =
    new ApplicativeByNameOps[F, A](() => fa)

  // Need to keep this for the bin-compat purposes I guess.
  @deprecated("Use bin-compat version", "2.8.0")
  final def catsSyntaxApplicative[F[_], A](fa: F[A]): ApplicativeOps[F, A] =
    new ApplicativeOps[F, A](fa)
}

sealed trait ApplicativeSyntaxLowerLevel {
  implicit final def catsSyntaxApplicativeBinCompat[F[_], A](fa: F[A]): ApplicativeOps[F, A] =
    new ApplicativeOps[F, A](fa)
}

..or something like this. I haven't actually tested it though...

[armanbilge]

Huh, interesting. Actually I had tried that prioritization trick too, and it didn't work. But I do see your scastie there 🤔

Edit: I gave it a try in f32fec9 but doesn't seem to work.

[satorg]

Yes, I see – it does not indeed.. The idea # 3 then – what if we do it in this way:

trait ApplicativeSyntax {
  implicit final def catsSyntaxApplicativeId[A](a: A): ApplicativeIdOps[A] =
    new ApplicativeIdOps[A](a)

  // Brand-new implicit solely for by-name params
  implicit final def catsSyntaxApplicativeByName[F[_], A](fa: => F[A]): ApplicativeByNameOps[F, A] =
    new ApplicativeByNameOps[F, A](() => fa)

  // Brand-new implicit entirely for by-value params
  implicit final def catsSyntaxApplicativeByValue[F[_], A](fa: F[A]): ApplicativeByValueOps[F, A] =
    new ApplicativeByValueOps[F, A](fa)

  // as usual – preserved for bin-compat, but not used
  @deprecated("Use by-name version", "2.8.0")
  final def catsSyntaxApplicative[F[_], A](fa: F[A]): ApplicativeOps[F, A] =
    new ApplicativeOps[F, A](fa)
}

final class ApplicativeIdOps[A](private val a: A) extends AnyVal {
  def pure[F[_]](implicit F: Applicative[F]): F[A] = F.pure(a)
}

@deprecated("Use by-name version", "2.8.0") // as usual – preserved for bin-compat, not used
final class ApplicativeOps[F[_], A](private val fa: F[A]) extends AnyVal {
  def replicateA(n: Int)(implicit F: Applicative[F]): F[List[A]] = F.replicateA(n, fa)
  def unlessA(cond: Boolean)(implicit F: Applicative[F]): F[Unit] = F.unlessA(cond)(fa)
  def whenA(cond: Boolean)(implicit F: Applicative[F]): F[Unit] = F.whenA(cond)(fa)
}

final class ApplicativeByNameOps[F[_], A](private val fa: () => F[A]) extends AnyVal {
  def unlessA(cond: Boolean)(implicit F: Applicative[F]): F[Unit] = F.unlessA(cond)(fa())
  def whenA(cond: Boolean)(implicit F: Applicative[F]): F[Unit] = F.whenA(cond)(fa())
}

final class ApplicativeByValueOps[F[_], A](private val fa: F[A]) extends AnyVal {
  def replicateA(n: Int)(implicit F: Applicative[F]): F[List[A]] = F.replicateA(n, fa)
}

In this case the by-name and by-value syntax methods are completely separated from each other.
So the de-prioritization trick should not be necessary.

[armanbilge]

Ah, yes of course, that should work! Let me fix that, thank you :)

[satorg]

Cool, thank you!