Replace buildFromAny with buildFromNothing

[joroKr21]

Instead of generalizing to Any we need to specialize to Nothing.
All other cases are covered by the default BuildFrom.buildFromIterableOps.

Also fixes Intellij highlighting issue.

See scala/bug#12350
Workaround for scala/bug#12104
Followup to #4582

[adamgfraser]

@joroKr21 Are you having an issue here? I think that both definitions are sound, though they say slightly different things. At least on previous versions of Scala 2.13 this was necessary for a CanBuildFrom to be in scope for certain operations. I haven't heard of any other problems with this to date.

[joroKr21]

@adamgfraser I go have a minor issue actually - Intellij also thinks the instances are ambiguous so it breaks highlighting.

The reason is that there is overlap. Nothing doesn't work when the element type is generic because of the peculiarity that in Scala 2 Nothing is used internally to signal that an implicit was not found. The way to fix it is to provide an instance with the element type specialized to Nothing. All other cases are covered by the default instance.

[adamgfraser]

If we're going to change this can we just delete it entirely instead of deprecating it?

[joroKr21]

Wouldn't that be binary incompatible?

[adamgfraser]

Binary compatibility checks seem to be okay.

[joroKr21]

Oh no - it loos like the MiMa checks job has been broken for some time 😱
https://circleci.com/api/v1.1/project/github/zio/zio/107145/output/104/0?file=true&allocation-id=605246e5efc352258c281098-0-build%2F701E1D91

[joroKr21]

Or was that on purpose?

[joroKr21]

@adamgfraser oh I see why - we check binary compatibility against 1.0.1 but this was added in 1.0.4-1: https://github.com/zio/zio/releases/tag/v1.0.4-1

Wow, I never really thought about this, but if you don't bump the version for MiMa that means you could accidentally delete something that you introduced in a later version. Amazing 🤯

[joroKr21]

Should I bump the version in MimaSettings?

[adamgfraser]

Yes we should.

[joroKr21]

That makes me think there should be some process or automation to bump this version on release

[adamgfraser]

Yes. Not sure what the best way is to do it but that would definitely be good.

[adamgfraser]

@joroKr21 That's a good reason! I'm good with this change as long as the tests pass. Would prefer to just delete the old instance if possible.

[joroKr21]

I will also try to compile with -Xsource:3 and see if there is any difference.

[joroKr21]

I will also try to compile with -Xsource:3 and see if there is any difference.

Indeed compiling core-tests with -Xsource:3 fails on master and works with this PR

[adamgfraser]

So how is it possible that it fails if we use X-source:3 but succeeds when we actually compile it on Scala 3?

[joroKr21]

So how is it possible that it fails if we use X-source:3 but succeeds when we actually compile it on Scala 3?

I think BuildFromCompat is only present on Scala 2.13 right?

[adamgfraser]

No the 2.13+ files are included on Scala 3. We need them because Scala 3 uses the standard library including the collections library from Scala 2.13 so we couldn't compile without them. That's part of why I am so confused by this.

[joroKr21]

No the 2.13+ files are included on Scala 3. We need them because Scala 3 uses the standard library including the collections library from Scala 2.13 so we couldn't compile without them. That's part of why I am so confused by this.

AFAIR in dotty the package object doesn't contribute to implicit scope, so maybe that's the reason.

[adamgfraser]

But it still has the change in implicit resolution for contravariant types that was supposed to be what triggered this issue, right?

[joroKr21]

But it still has the change in implicit resolution for contravariant types that was supposed to be what triggered this issue, right?

Yes it does, but if that instance is not in implicit scope it just won't be considered.
And if you import it it takes precedence.

[adamgfraser]

But the instance isn't in the Dotty package, right? It is in the Scala collections which has got to be in implicit scope or everything else with CanBuildFrom would break.

[joroKr21]

The buildFromAny instance is in the zio package object which is the package of zio.BuildFrom

[adamgfraser]

Right but that's true on Scala 3 as well. So why would it compile on Scala 3 but not on Scala 2 with implicit resolution rules from Scala 3?

[joroKr21]

AFAIR in dotty the package object doesn't contribute to implicit scope, so maybe that's the reason.

It's change number 3. here: https://dotty.epfl.ch/docs/reference/changed-features/implicit-resolution.html

[joroKr21]

3. Package prefixes no longer contribute to the implicit search scope of a type. Example:

  package p

  given a: A = A()

  object o:
     given b: B = B()
     type C

Both a and b are visible as implicits at the point of the definition of type C. However, a reference to p.o.C outside of package p will have only b in its implicit search scope but not a.

I'm a bit confused - so does it include package objects or not?
Only top-level definitions? Only the closest package?

[adamgfraser]

I don't know. I guess what I take from this is that this -XSource:3 flag back ports some changes from Scala 3 but not others so conceptually it makes sense that something could compile on both Scala 2 and Scala 3 but not on Scala 2 with some subset of Scala 3 rules.

[adamgfraser]

Kind of raises an interesting question for library authors of whether we need to have this in our build matrix too. As we're seeing it is essentially a different version with different semantics of what compiles or not in some cases. If we did run against this in CI we would have caught before the PR adding this got merged.

[joroKr21]

Indeed that's a good question. I don't think it's possible to port everything from Scala 3. On the other hand -Xsource:3 is useful if you plan to migrate to Scala 3 but can't yet - it would also show you deprecated features you are using.

[adamgfraser]

Thank you for your work on this! 🙏