Scala3 style imports

[satorg]

Removes runner.dialectOverride.allowStarWildcardImport = false from .scalafmt.conf then applies scalafmtAll.

UPD.: also re-enables "-Xsource:3" for algebraCore and algebraLaws.

[satorg]

Looks like it is failing because in #4478 the "-Xsource:3" option was disabled for algebraCore and algebraLaws with the following commit message:

Workaround binary-compatibility issues

🤔

[satorg]

Hi @armanbilge,
Do you remember by chance what was the issue with "-Xsource:3" in #4478? I re-enabled it in this PR and all the checks seem passing. Do you think it is safe to leave "-Xsource:3" enabled then?
Thanks!

[armanbilge]

Huh, that surprises me. I do have a vague recollection of the issue, I believe it related to some methods that were lacking an explicit type, and under -Xsource:3 the type inference works differently, so this would break binary compatibility. And adding an explicit type is impossible, because the binary-compatible explicit type is different for Scala 2 and Scala 3.

If CI is happy, it's probably okay now (?) but it would be really good to understand why ...

[satorg]

Yes, it is puzzling quite a bit.. I'm going to convert this Draft to a complete PR and let it stay that way for a while.
Just in order to collect more thoughts and opinions on that and allow some time for pondering all the implications.

[satorg]

@danicheg @armanbilge does it make sense to proceed with this, wdyt? I personally feel, yes, it does, since it could be yet another step towards Scala3 syntax migration, but I wouldn't like to run ahead of the train anyway.

Just to keep things connected: there's another PR (yet draft) in the same direction: #4673 by @xuwei-k, which I'd really love to have merged once it's ready.

[danicheg]

I think this requires an accepted tactic among the team. Currently, we have a coherent Scala 2 codebase with few provisos. Personally, I'm in favour of keeping that order of things as long as possible. Cats is a Scala 2 project cross-compiling to Scala 3. I don't think braiding the syntaxes of these two languages would make sense. But that's an interesting approach, indeed. What if they invented a strategy to turn Scala 2 syntax into Scala 3 one day?

[satorg]

What if they invented a strategy to turn Scala 2 syntax into Scala 3 one day?

Hmm.. I don't know. For now it looks like Scala3 is moving in the opposite direction – it keeps gradually deprecating Scala2 style syntaxes from version to version.

My point is that if Cats already has -Xsource:3 enabled by default and rewrite.scala3.convertToNewSyntax = true configured in .scalafmt.conf, then why to hold on imports? Imo, recent versions of Scala2 were equipped with -Xsource:3 for the sake of gradual migration rather than do-everything-at-once approach.

[armanbilge]

I think this is the right direction. It seems to me that a lot of the syntax changes from 2->3 are addressing various "warts" (see also #4673). I think it's great that these improvements are being backported to Scala 2.

My only hesitation with this PR is trying to understand what was breaking bincompat previously, and why it no longer is, see #4677 (comment). I guess it shouldn't be a blocker, I would just feel better knowing why 😅

[joroKr21]

Could be about different type inference when overriding a method from the superclass

[johnynek]

my view is that users shouldn't care about this and the future is scala 3, so I'm +1.

[satorg]

Updated: resolved conflicts (here).

Where do we stand on this one? Is there anything I can do to clear the concerns?

---

I wonder if Typelevel has org-wide policies on embracing Scala3 syntax and such.
And if it doesn't, perhaps it would be helpful get one in order to make all the projects lined up from that perspective...

[satorg]

@johnynek , thank you.
(your comment appeared while I was working on my previous one, so they look a little out of order)

[satorg]

@armanbilge , I finally got myself to bisect that funky issue with the disappearing binary compatibility issues.
Honestly, the result is quite surprising to me.

When -Xsource:3 is enabled, Mima keeps reporting binary compatibility issues until #4600 is merged.
With that PR merged, Mima doesn't see any issues anymore.
A previous PR merge (which is #4593) still triggers Mima.

Here is the earliest commit that includes both #4600 and -Xsource:3

032e939

Let me know if it somehow clears your concerns (it doesn't clear mine though, I rather feel confused).

[satorg]

@armanbilge, I'd like to propose an opinionated solution to the above conundrum: what if we merge this and publish the next time not as a release but rather a pre-release version (somewhat RC1 or even M1)? Then we can let stay it for a while that way to allow other projects to test it for possible binary incompatibilities. Does it look like a good plan?

[armanbilge]

@satorg thanks for continuing to work on this! And I really appreciate your diligence raising the question of binary-compatibility in #4677 (comment).

My personal opinion is that there is no urgency to merge this PR, as it is an entirely internal-facing change (perhaps I am missing something?). Meanwhile, we know that unfortunately MiMa cannot always catch binary-breaking changes (e.g. see #4702 (comment)). So when I weigh the value of this change against the risk, I think it makes more sense to hold until we understand what is happening.

---

Let me know if it somehow clears your concerns (it doesn't clear mine though, I rather feel confused).

Your investigation also increases my concerns and confusion. I have not had a chance yet to do my own investigation, based on your results.

---

publish the next time not as a release but rather a pre-release version (somewhat RC1 or even M1)? Then we can let stay it for a while that way to allow other projects to test it for possible binary incompatibilities

This could certainly help us discovery binary-compatibilities, but I don't think it's an effective way to make ourselves confident that no incompatibilities exist. As we know, the ecosystem runs deep, and not all projects will try the milestone / release candidate.

---

In any case, I don't intend to be the sole person to hold this PR up, this is just my opinion 🙂

[satorg]

@armanbilge ,

I think it makes more sense to hold until we understand what is happening.

Totally makes sense and I'd like that too, but in order to proceed with that I might use some help on what else I could look into. Do you have any ideas by chance?

[armanbilge]

A previous PR merge (which is #4593) still triggers Mima.

Are you sure? I added back -Xsource:3 on dfd9559 and was unable to get any MiMa issues.

[satorg]

Hi @armanbilge , you're right:

Are you sure?

I am not :) Honestly, I don't completely understand how come I ended up with the previous commits range. Perhaps it is because I tried to avoid full rebuilding every time (it runs kinda slow on my 6-year old laptop 🤷 )

This time, however, I took the hard path: I was executing git clean -xdf before every single mimaReportBinaryIssues (because "this is the way" ©️). Thereby I got the following results:

• Base commit 512ecc2 (pr Update to sbt-typelevel 0.5.0 #4478)
  with -Xsource:3 enabled triggers Mima errors for Scala 2.13 only. Scala 2.12 and 3 both are good.
  The errors keep occurring for all the following merges until
• Commit f7f5eda (pr Update scala-library, scala-reflect to 2.13.13 #4568).

So, it looks like the upgrade to Scala 2.13.13 fixes the issue. I guess, this phrase from the release notes can be relevant:

When upgrading to 2.13.13, existing users of -Xsource:3 should explicitly consider switching to -Xsource:3-cross. Some behaviors of -Xsource:3 have changed, e.g. in result type inference for inherited methods; see new doc page for details

Let me know if it makes sense please.
I wonder if we should give a shot to -Xsource:3-cross instead of just -Xsource:3 🤔

[armanbilge]

@satorg great, thank you so much, that all makes sense now! We're good to move forward now 🚀

I wonder if we should give a shot to -Xsource:3-cross instead of just -Xsource:3 🤔

Possibly. To be honest, I'm confused by the naming, but a brief glance at the documentation page suggests that it's more aggressive 🤔 anyway, it's definitely something we can investigate for a new major version of sbt-typelevel.

[satorg]

Thanks everyone who participated in reviewing this PR! I really appreciate your feedback and support!