Warning: "Calls to parameterless method compose will be easy to mistake for calls to overloads which have a single implicit parameter list"

[saeltz]

Seen in this open-source project that uses Cats. See for example this test run. We're treating warnings as errors in the compiler.

[errpr] Calls to parameterless method compose will be easy to mistake for calls to overloads which have a single implicit parameter list:
[error]   def compose[G[_]](implicit evidence$1: cats.Invariant[G]): cats.Invariant[[α]F[G[α]]]
[error]   def compose[G[_]](implicit evidence$1: cats.Functor[G]): cats.Functor[[α]F[G[α]]]
[error]   def compose[G[_]](implicit evidence$1: cats.Apply[G]): cats.Apply[[α]F[G[α]]]
[error]   def compose[G[_]](implicit evidence$1: cats.Applicative[G]): cats.Applicative[[α]F[G[α]]]
[error]     new Monad[ScynamoDecoder] with SemigroupK[ScynamoDecoder] {

Scala recommends to not have parameterless methods with an implicit parameter list, as seen in Applicative.

The only way for us is to not treat warnings as errors or silence this specific warning. Are there any plans to change this? It's probably a breaking change.

[satorg]

Thank you for the bug report!

Indeed, it looks like a new Lint warning introduced in Scala v2.13.17:

• scala/scala#11033 (initial implementation)
• scala/scala#11120 (further refinements)

The warning is emitted under a new category "lint-overload" and can be disabled at the build level with -Xlint:-overload correspondigly.

Just for better clarity: the cause of the warning is that the parameterless compose method from SemigroupK coexists with a bunch of other compose methods from Monad in your instance implementations. If there was no SemigroupK mixed in, then the warning would not appear either. Prior to v2.13.17 that condition was not checked and reported.

Also I can see that the Cats build itself started printing similar warnings for certain typeclasses and instances. Unfortunately, Cats build still doesn't have fatal warnings enforced, therefore the new warnings sneaked into the build unnoticed.

[satorg]

Perhaps, in your case the simplest yet least ivasive way to work-around it is to suppress the warning on the compose method of the concern specifically:

implicit val catsInstances: Monad[ScynamoDecoder] with SemigroupK[ScynamoDecoder] =
  new Monad[ScynamoDecoder] with SemigroupK[ScynamoDecoder] {
    ...

    @nowarn("cat=lint-overload")
    override def compose[G[_]]: SemigroupK[λ[α => ScynamoDecoder[G[α]]]] = super.compose[G]
  }

It won't make things worse than they already are, yet allowing you to leverage the new lint option in other cases.

[satorg]

Are there any plans to change this? It's probably a breaking change.

Great question! What I can think of is deprecating all the compose methods with context types and add more specific aliases for them based on those type names, e.g.:

@deprecate("use `composeFunctor` instead", "whenever")
def compose[G[_]: Functor]: Functor[λ[α => F[G[α]]]] = composeFunctor

def composeFunctor[G[_]: Functor]: Functor[λ[α => F[G[α]]]] = /* implementation goes here */

However, it won't help in your case, because the original compose, though deprecated, will be there anyway.

In order to address that, we can also make the original methods unresolvable with typeclasses:

@deprecate("use `composeFunctor` instead", "whenever")
def compose[G[_]](G: Functor[G]): Functor[λ[α => F[G[α]]]] = composeFunctor(G)

This should allow to retain binary compatibility, but not source compatibility, unfortunately. And to be honest, I'm not sure if it is possible to retain the source compatibility at all. So it's going to be a tough change anyway.

Now I'm going to pause here and ask other Cats maintainers if they have better ideas for this case.

[satorg]

Another possibility to alleviate that could be if Scala folks could exclude deprecated methods from the "overload" linting:

trait One   { def foo: Whatever2 }
trait Two   { def foo(implicit a: A): Whatever2 }
trait Three { def foo(implicit b: B): Whatever3 }

// Reports that `foo` from `One` can be confused with both `Two#foo` and `Three#foo`.
trait Four extends One with Two with Three

But if we deprecate Two#foo:

trait One   { def foo: Whatever2 }
trait Two   { @deprecated(...) def foo(implicit a: A): Whatever2 }
trait Three { def foo(implicit b: B): Whatever3 }

// Reports that `foo` from `One` can be confused with `Three#foo` only.
trait Four extends One with Two with Three

And if we deprecate both Two#foo and Three#foo:

trait One   { def foo: Whatever2 }
trait Two   { @deprecated(...) def foo(implicit a: A): Whatever2 }
trait Three { @deprecated(...) def foo(implicit b: B): Whatever3 }

// No warnings reported
trait Four extends One with Two with Three

I think we can assume that if the compiler picks a deprecated method due to the implicit resolution routine, the method will be reported as deprecated anyway, so no need to report it as a concerning overload once again.

@som-snytt, would it work, wdyt?

[som-snytt]

I see that project's build is already selective about warnings. I think it's fine to silence the alarm if it's not a problem.

Using -Wconf leaves an "audit trail" in the build file.

-Wconf:any:wv or -Wconf:any:warning-verbose adds the special strings to messages. Note that if you leave it in the build, it should be first not last, since last wins.

The unused import is silenced with: "-Wconf:cat=unused-imports&origin=scala.collection.compat._:s", and for compose: "-Wconf:cat=lint-overload&site=scynamo.DefaultScynamoDecoderInstances.catsInstances:s",

The site is a regex; here is an example that works for all catsInstances. There are backslashes for escaping special regex chars.

  "-Xlint:_",
  "-Wunused:_",
  "-Wconf:any:wv",
  "-Wconf:cat=unused-imports&origin=scala.collection.compat._:s",
  """-Wconf:cat=lint-overload&site=scynamo\..*\.catsInstances:s""",
  "-Wconf:cat=unused-params&site=scynamo.LowestPrioAutoEncoder.autoDerivedScynamoEncoder.evidence\\$5:s",

I'm not sure about the direct question about deprecation. Does anyone pay attention if it says there were 3 deprecations instead of 2?

The purpose of the lint is to avoid head-scratching. If the suspicious API is really benign (because the correct method is always picked, no one is ever confused, etc) then it is safe to "snooze" the alarm.

[satorg]

@som-snytt , thank you for the comprehensive explanation!

I'm not sure about the direct question about deprecation. Does anyone pay attention if it says there were 3 deprecations instead of 2?

My idea there was that it should allow Cats to migrate from ambiguous compose methods to more specific names like composeFunctor, composeApply, etc. without introducing either binary or source breaking changes:

1. Cats deprecates all ambiguous compose methods and introduce their safe aliases (composeFunctor, composeApply, etc)
2. When Scala compiler observes these confusing overloads, it only reports non-deprecated ones. If all of them are deprecated – no warning message is issued.
3. If a library like scynamo makes use of any of those compose methods, it will start receiving the deprecation messages about them, so they would get a chance to migrate to "better" non-confusing method names and stop receiving any warnings whatsoever.

Whithout such support from Scala compiler, even if Cats deprecate compose-s and introduce non-confusing aliases, its clients like scynamo will continue receiving those "lint-overload" warnings even if they migrate to the new method names.

[som-snytt]

@satorg I didn't like the idea of changing method names because of a "fake" problem, but for your use case, I agree that it makes sense to ignore deprecated elements. The purpose of the lint is to give a warning when defining the class. If the user of the class wonders why their overloaded compose isn't working, hopefully they will notice a deprecation.

Also, I wonder if it should only warn when methods are declared and not in mix-in composition. That may weaken the lint too much, or just enough for anonymous classes.

[satorg]

@som-snytt I wouldn't say the problem is "fake". Cats already has compose methods with qualifiers. For example, Functor has compose as well as composeBifunctor and composeContravariant. Therefore switching from unspecified compose methods that can only be resolved by their implicit paratemers (which can be quite confusing) to more specific composeWhat methods could be a move into the right direction, I guess.

So if you think that excluding deprecated methods from the "lint-overload" warnings makes sense and might work, I can create an issue in the Scala repo to give it a broader discussion.

[som-snytt]

@satorg A ticket would be fine. I'm happy to implement it.

[som-snytt]

Ticket as shown at https://github.com/scala/bug/issues/13138 since 2.13.18 may arrive sooner than later.

[satorg]

Oh, thank you very much for taking on this!

[satorg]

For reference: scala/scala#11171