Router: use template to break before extends
#3581
Conversation
In older versions of scalameta, `extends` was not part of template, and we had to identify the tree just preceding it. Also, make sure to handle cases when the template is empty (in that case there's no reason to break before a non-existent `extends`).
|
@kitbellew do you think it would be appropriate for us to say something about this in the Scala 2.13.12 release notes? and if so, can you suggest a wording? |
|
@SethTisue oh, what did I miss? how's this related to that scala release? |
|
#3580 was reported against a nightly build of Scala 2.13.12, after scala/scala#10392 was merged. I'm asking if we have a situation where users who upgrade Scala 2.13.12 will also need to upgrade their Scalafmt version in order to format their code. (It's possible I have some misconception.) |
|
@SethTisue I think the problem was because scalafmt was rebuilt and re-tested (and failed) with the new scala version, as part of that community build. but the released version of scalafmt hadn't been built with that version (and the released version of scalafmt hadn't failed the same tests), so it doesn't seem like it would have real-world consequences. @tgodzik what do you think? |
|
Yeah, I agree that this shouldn't be an issue. The |
As it turns out, we're using a very old version of Scalafmt that breaks completely: ```txt ERROR: rules_scala/test/scalafmt/BUILD:43:20: ScalaFmt test/scalafmt/test/scalafmt/formatted/formatted-test.scala.fmt.output failed: (Exit 1): scalafmt failed: error executing ScalaFmt command (from target //test/scalafmt:formatted-test) bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/scala/scalafmt/scalafmt '--jvm_flag=-Dfile.encoding=UTF-8' ... (remaining 1 argument skipped) java.util.NoSuchElementException: last of empty IndexedSeq ``` This matches: - scala/community-build#1680 Which mentions apparent fixes in: - scalameta/scalameta#3235 - scalameta/scalafmt#3581 So the fix is to update Scalafmt, but given how we don't use rules_jvm_external, that means a lot of manual updates to third_party/repositories/scala_*.bzl. There doesn't appear to be a way to automate this; there's no indication that the most recent update was automated in any way: - bazelbuild#1543 So I'll plow through all the JARs and make the necessary changes: - https://mvnrepository.com/artifact/org.scalameta/scalafmt-core I can't update scala_2_11, since there's not a more recent compatible version. But the changes to scala_2_12 and scala_2_13 should be similar, and the changes to 2_13 should apply to 3_{1,2,3,4} as well. I had to hack .scalafmt.conf and ScalafmtWorker.scala a bit. This may make it incompatible with 2.11.
In older versions of scalameta,
extendswas not part of template, and we had to identify the tree just preceding it.Also, make sure to handle cases when the template is empty (in that case there's no reason to break before a non-existent
extends).Fixes #3580.