Factory.newBuilder returns same instance

[oxbowlakes]

There is a serious bug in the implicit conversion of a generic companion to a CanBuildFrom . The Factory created from the List companion incorrectly returns the same instance across calls to newBuilder:

To demonstrate/reproduce the Issue

$ cat build.sbt
scalaVersion := "2.12.11"
libraryDependencies += "org.scala-lang.modules" %% "scala-collection-compat" % "2.1.6"
​
$ cat src/main/scala/Test.scala
import scala.collection.compat._
​
object Test extends App {
  val factory: Factory[Int, List[Int]] = List
​
  println(factory.newBuilder eq factory.newBuilder)
​
  println(List(1, 2, 3).to(factory))
  println(List(4, 5, 6).to(factory))
}
​
$ sbt run
...
[info] running Test
true
List(1, 2, 3)
List(1, 2, 3, 4, 5, 6)

​

Why does this happen?

simpleCBF.apply takes a by-name parameter, and relies on the fact that each time the parameter is referenced, this results in a new builder. It's therefore important that the input to the method is not captured in a local value.

Possible fixes

Change (on line 58 of PackageShared.scala)

val builder: m.Builder[A, CC[A]] = fact match

To

def builder: m.Builder[A, CC[A]] = fact match

[SethTisue]

nice catch! @julienrf wdyt about the proposed fix?

[julienrf]

Thanks @oxbowlakes for the investigation! I agree with the fix you proposed. Would you be interested in submitting a pull request?

[oxbowlakes]

Yes, sure. Is there anything I should read first?

[julienrf]

I think you can just apply the fix in a separate branch, and then open a pull request :)

[oxbowlakes]

So I've cloned the repo, created a branch in my clone and am trying to build it (before I've made any changes) and failing completely. The source compiles, but I cannot get the tests to compile. Is there a chat-channel where I can get some help? Cheers!

[julienrf]

sbt compat211/Test/compile works for me, which command did you invoke and what was the error message?

[oxbowlakes]

I ran test - which evidently was not correct! I have now checked that my test compiles and fails, and then passes following the fix

[oxbowlakes]

The PR is here: #338