Make IO.Par's applicative commutative

[kubukoz]

Apparently MiMa claims this is binary incompatible, but maybe it's a false positive (the trait is private[effect] so maybe it only matters for implementations), so I submitted it anyway.

Please let me know what you think about making IO.Par's applicative commutative (I checked it before and it passed the laws... apparently)

[alexandru]

This may be problematic. IOInstances may be private[effect] however what's public is object IO which inherits it.

That said I thought that in a relationship like B <: A if you upgrade a function return type from A to B it should still be binary compatible. So I wonder why it doesn't work here.

[alexandru]

Maybe Mima complains because the upgrade is in IOInstances for a non-final method that can be overridden.

[kubukoz]

Maybe... I'll look into it. Is the general idea OK?

[alexandru]

I think so, but we need an answer for the Mima issue.

Not sure what we can do to find out. Maybe do a simulation with such an update attempted on just an object.

[kubukoz]

Alright, finally got around to it. Looks like it is indeed incompatible.

Here's what I did (just to make sure I did the right thing):

IO.scala:

object IO extends IOInstances

trait Animal
class Dog extends Animal

trait IOInstances {
  def instance: Animal = new Dog
}

Usage.scala:

object Usage extends App {
  val a: Animal = IO.instance
  println(a)
}

$ scalac IO.scala
$ scalac Usage.scala
$ scala Usage
< Dog@7fbe847c

Then I changed the line in IOInstances:

-  def instance: Animal = new Dog
+  def instance: Dog = new Dog

Recompiled IO.scala, ran Usage again:

java.lang.NoSuchMethodError: IO$.instance()LAnimal;
	at Usage$.delayedEndpoint$Usage$1(User.scala:2)
	at Usage$delayedInit$body.apply(User.scala:1)
	at scala.Function0.apply$mcV$sp(Function0.scala:39)
	at scala.Function0.apply$mcV$sp$(Function0.scala:39)
	at scala.runtime.AbstractFunction0.apply$mcV$sp(AbstractFunction0.scala:17)
	at scala.App.$anonfun$main$1$adapted(App.scala:80)
	at scala.collection.immutable.List.foreach(List.scala:392)
	at scala.App.main(App.scala:80)
	at scala.App.main$(App.scala:78)
	at Usage$.main(User.scala:1)
	at Usage.main(User.scala)

I suppose we would need to add a new method that returns a more specific type (CommutativeApplicative) and do some prioritization tricks to get it to work.

[kubukoz]

I submitted a workaround in the latest commit, basically moving the instance as explicit to the IOParallelNewtype trait and accessing it by implicits in the appropriately prioritized traits (at least this seems to be working when I check MiMa locally and see if the implicits are accessible)

[codecov-io]

Codecov Report

Merging #472 into master will increase coverage by 0.15%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #472      +/-   ##
==========================================
+ Coverage    89.6%   89.76%   +0.15%     
==========================================
  Files          70       70              
  Lines        2050     2052       +2     
  Branches      196      196              
==========================================
+ Hits         1837     1842       +5     
+ Misses        213      210       -3