Hi, I’m Thanh from the lichess.org team. We’re trying to upgrade our main server (Lichess’s Lila) to Scala 3.4.0. And we have too many deprecation warnings. Even though the compiler offers to rewrite most of them for us, it doesn’t always do a good job. There are too many places where the compiler produced faulty code or the rewritten code is not in the way we like it (adding back stick for infix functions for example). To be fair, Lila is a big code base, and using many “exotic” style/pattern of coding. But we hope to make it more idiomatic.
My question is can we control which rewrite rules that compiler uses when we have (“-rewrite”, “-source:future-migration”)? I’d like to use individual rewrite rule to test which one works. Because as I observe some of the rewrite is good like replace with with &, remove trailing underscore _, but not rewrite with fewer braces (we would want that if it’s not broken) or infix operator (because the back stick is ugly ).
In order to make the upgrade process smoother, we used scalafmt’s AvoidInfix in this pr but this was not enough. We need to manually fix the code as scalafmt crashes on many places. It required us to fix some code manually before we can run it again. But even when it ran successfully for the whole projects there are still many deprecated infix function calls left.
Do you urgently need some new 3.4 features? It might take a bit longer for the rewriting to get better. There will be more changes to the syntax on Next soon
This is annoying a lot of people, but I’m afraid we’ll all have to live with it…
No, We don’t. It’s just our tendency to live on the edge, I remember that we ran 3.3.0-RC* in production for months before its final release.
I believe the recommendation is stay on LTS line if you’re libraries and move on if you’re applications and you don’t mind fast pace of changes. We definitely don’t afraid the changing pace, We want to move on as soon as possible to use the most advance compiler and to avoid any dramatically change later on.
I would rather discourage this combination, under -source:future or -source:future-migration you would not get warnings about changed implicit resolutions, but just a plain error about missing implicit instead. See 9. The following change is currently enabled in -source future: on Changes in Implicit Resolution
Compilation errors then prevent applying -rewrite changes.
If there isn’t anything required from -source:future in your codebase then downgrade to -source:3.4-migration during the upgrade
One more note: Lila is tested weekly in the Open Community Build, you might be affected by the implicit resolution changes, as this warning suggests Weekly build A #40 · VirtusLab/community-build3@d1c72d2 · GitHub otherwise, there should be no other issues with upgrading to 3.4.0 or even latest nightly (We force -source:3.X-migration whenever possible, as these stuff is typically non critical, and there less then dozen of projects that actually require -source:future right now)
For the infix rewrites, I believe there are better options than ScalaFmt:
If the methods in question are defined in your project, and you prefer the infix notation, you might want to declare them with an infix modifier.
Use the compiler’s -rewrite option, that one has no issues I know of. Generally, migration to new versions is driven by the compiler team, so it might be that other tools that we don’t control lag behind.
The new infix rules have been the biggest pain point by far in my codebase, in part because there’s no way to allow clean infix Java (or Scala libraries which you don’t personally control), and sometimes infix Java makes code far easier to read (if it’s math-like, for instance). I just run with -Wconf:msg=is not declared infix:s (dots instead of spaces if I’m using scala-cli header comments).
I also make use of whitespace to highlight parallel constructions, which rewrite messes up. For instance (real examples are more complex):
def later(t: Double): Instant = myInstant + t
def later(t: Duration): Instant = myInstant plus t
Backticks mess this up, and if it’s not at the end of the line. parens also mess this up. I’m really not motivated to make a bunch of manual fixes so that my code is harder to read.
If the lichess codebase already has a lot of infix, and infix makes understanding clearer, as is the case with my code, I’d suggest considering a similar approach–just suppress the warnings, and if you feel like it, gradually evolve towards less or no infix.
If you need the automatic rewrite to fix other things, perhaps generating a diff of the full rewrite, then selecting only those diffs that didn’t just add backticks, and applying only the reduced diff, is the way to go.
Just to check that it wasn’t hard, I auto-rewrote everything in my codebase in a new branch, did a git diff > my.patch, used the repl to load and select patches that weren’t just adding backticks, wrote it out again, and did git apply new.patch in another new branch. No surprises. So that approach seems straightforward enough. Not a full solution, but it’s a “rewrite everything but infix” that works.
Just want to say your Lichess project is awesome. (Sorry for violating the forum rules a little, but just pressing “like” does not cover it.) It’s amazing!
Oh dear, sorry! I stopped paying attention to the thread. Anyway, it sounds like the rewrite was successful enough!
Just for reference, here’s a runnable command-line patch editor (if you want it to not ignore whitespace, edit out the trims in the collect statements). Run with scala-cli CutPatch.scala -- in.patch out.patch.
//> using scala 3.4.0
//> using dep com.github.ichoran::kse3-eio:0.2.11
//> using mainClass kerrr.cutpatch.Main
//> using options -Wconf:msg=is.not.declared.infix:s
package kerrr.cutpatch
import kse.basics.{given, _}
import kse.basics.intervals.{given, _}
import kse.flow.{given, _}
import kse.eio.{given, _}
object Main {
def differsByBackticks(has: String, hasnt: String): Boolean =
var i = 0
var j = 0
var result = true
while i < has.length && j < hasnt.length && result do
if has(i) == hasnt(j) then
i += 1
j += 1
else if has(i) == '`' then
i += 1
else
result = false
result && i == has.length && j == hasnt.length
def justAddsBackticks(lines: Array[String]): Boolean =
val pos = lines.collect{ case s if s startsWith "+ " => s.drop(2).trim }
val neg = lines.collect{ case s if s startsWith "- " => s.drop(2).trim }
(pos zip neg).forall{ case (p, n) => differsByBackticks(p, n) } && pos.length == neg.length
def run(args: Array[String]): Unit Or Err = Err.Or:
if args.length != 2 then Err.break("Specify patch file as input and target file as output")
val source = args(0).path
val target = args(1).path
if target.exists then Err.break(s"For safety, please use a target that doesn't already exist")
val content = source.slurp.?
val diffs = content.where(_ startsWith "diff")
val atats = content.where(_ startsWith "@@")
val pts = (diffs ++ atats ++ Array(content.length)).sorted
val intervals = pts.selectOp(1 to End){ (p, i) => Iv(pts(i-1), p) }.
filter(iv =>
iv.length > 0 && ((content(iv.i0) startsWith "diff") || !justAddsBackticks(content.select(iv)))
)
val newContent = intervals.breakable.copyOp{ (iv, i) =>
shortcut.skipIf:
content(iv.i0).startsWith("diff") && (i+1 >= intervals.length || content(intervals(i+1).i0).startsWith("diff"))
content.select(iv)
}
println(s"Writing ${newContent.count(_.head startsWith "@@")} of ${atats.length} edits\nfrom ${source.real}\n to ${target.real}")
newContent.iterator.flatten.createAt(target).?
def main(args: Array[String]): Unit =
run(args).foreachAlt{ e =>
println("Error:")
println(e)
sys.exit(1)
}
}
One could make it dependency-free with a few tweaks to array operations and such.
).
(Sorry for violating the