Skip to content

Commit 190fbee

Browse files
authored
Update to Scalafmt 3.8.3 (#1631)
* Update to Scalafmt 3.8.3 Ensures that all Scalafmt targets succeed under every supported Scala version. Part of #1482. Scala 2.11 remains on Scalafmt 2.7.5, since there's not a more recent compatible version. Between 3.0.0 and 3.8.3, Scalafmt's `FileOps` moved packages, `Config.fromHoconFile` moved to `ScalafmtConfig`, and the methods now take a `java.nio.file.Path` parameter. As a result, this required extracting a thin `ScalafmtAdapter` object, with one implementation for Scala 2.11 and Scalafmt 2.7.5, and one for Scalafmt 3.8.3. This change also adds the file path to the `Scalafmt.format()` call, allowing error messages to show the actual file path instead of `<input>`. Removed the `verticalMultiline.newlineBeforeImplicitKW = true` and `unindentTopLevelOperators = false` options from `.scalafmt.conf`. They don't appear to be available in Scalafmt 3.8.3. Also removes some `@io_bazel_rules_scala` references to make the internal implementation less dependendent on that name. This will allow Bzlmod clients to use `rules_scala` in a `bazel_dep()` without setting `repo_name = "io_bazel_rules_scala"`. --- Scalafmt 3.0.0 works with the current default Scala version 2.12.19, but breaks under Scala 2.13.14: ```txt $ bazel test --repo_env=SCALA_VERSION=2.13.14 //test/scalafmt/... ERROR: .../test/scalafmt/BUILD:49:20: ScalaFmt test/scalafmt/test/scalafmt/unformatted/unformatted-test.scala.fmt.output failed: (Exit 1): scalafmt failed: error executing command (from target //test/scalafmt:unformatted-test) bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/scala/scalafmt/scalafmt '--jvm_flag=-Dfile.encoding=UTF-8' ... (remaining 1 argument skipped) java.util.NoSuchElementException: last of empty IndexedSeq at scala.collection.IndexedSeqOps.last(IndexedSeq.scala:110) at scala.collection.IndexedSeqOps.last$(IndexedSeq.scala:105) at scala.meta.tokens.Tokens.last(Tokens.scala:29) at org.scalafmt.internal.Router.$anonfun$getSplitsImpl$90(Router.scala:707) at scala.Option.map(Option.scala:242) at org.scalafmt.internal.Router.getSplitsImpl(Router.scala:706) at org.scalafmt.internal.Router.getSplits(Router.scala:2314) at org.scalafmt.internal.BestFirstSearch.$anonfun$routes$1(BestFirstSearch.scala:38) [ ...snip... ] ``` This matches: - scala/community-build#1680 Which mentions apparent fixes in: - scalameta/scalameta#3235 - scalameta/scalafmt#3581 So the fix was to upgrade the Scalafmt version. That said, I held its Scalameta dependencies to 4.9.9 per the following link, even though 4.10.2 is out now. - https://mvnrepository.com/artifact/org.scalameta/scalafmt-core_2.13/3.8.3 --- This change updates Scalameta to version 4.9.9 because between 4.9.9 and 4.10.2, Scalafmt breaks Scala 3 file formatting by unindenting some code that it shouldn't. Or, our usage of it breaks somehow; I can't find any open or closed issues in the Scalameta project that matches what happes in rules_scala. (Perhaps I can file one eventually.) - https://github.com/scalameta/scalafmt/issues The solution was to keep the Scalameta dependencies at 4.9.9. FWIW, this was one of the most time consuming bugs to pinpoint and rectify in the entire Bzlmodification process. This was the failing command inside `test_cross_version`: ```txt $ bazel run //scalafmt:unformatted-test3.format-test INFO: Analyzed target //scalafmt:unformatted-test3.format-test (0 packages loaded, 0 targets configured). INFO: From ScalaFmt scalafmt/scalafmt/unformatted/unformatted-test3.scala.fmt.output: ``` The `test_cross_version/scalafmt/unformatted/unformatted-test3.scala` file formatted by this test target looks like this: ```scala import org.scalatest.flatspec._ class Test extends AnyFlatSpec: "Test" should "be formatted" in { assert(true) } ``` I hacked `ScalaWorker.format()` to print the `code` variable, and could see that after the first `Scalafmt.format()` pass, the code looks like this: ```scala import org.scalatest.flatspec._ class Test extends AnyFlatSpec: "Test" should "be formatted" in { assert(true) } ``` Since the result doesn't match the original code, it tries to call `Scalafmt.format()` again on this "formatted" code with the incorrect indentation. That's when we get the following, which doesn't look anything like the original file: ```txt Unable to format file due to bug in scalafmt scalafmt/unformatted/unformatted-test3.scala:3: error: [dialect scala3 [with overrides]] `;` expected but `:` found class Test extends AnyFlatSpec: ^ ``` --- As it turns out, bumping to com.google.protobuf:protobuf-java:4.28.2 alone breaks the bump to Scalafmt 3.8.3. Then bumping to rules_proto 6.0.2, with the separate protobuf v21.7, fixes it, presumably by recompiling `protoc`. The in-between breakage happened in the `test_cross_build` project: ```txt $ bazel build //scalafmt:formatted-binary2 INFO: Analyzed target //scalafmt:formatted-binary2 (4 packages loaded, 64 targets configured). ERROR: .../test_cross_build/scalafmt/BUILD:59:22: ScalaFmt scalafmt/scalafmt/formatted/formatted-binary2.scala.fmt.output failed: Worker process did not return a WorkResponse: ---8<---8<--- Start of log, file at .../bazel-workers/worker-3-ScalaFmt.log ---8<---8<--- Exception in thread "main" java.lang.NoSuchMethodError: 'void com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest.makeExtensionsImmutable()' at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest.<init>(WorkerProtocol.java:1029) at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest.<init>(WorkerProtocol.java:922) at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest$1.parsePartialFrom(WorkerProtocol.java:2482) at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest$1.parsePartialFrom(WorkerProtocol.java:2476) at com.google.protobuf.AbstractParser.parsePartialFrom(AbstractParser.java:192) at com.google.protobuf.AbstractParser.parsePartialDelimitedFrom(AbstractParser.java:232) at com.google.protobuf.AbstractParser.parseDelimitedFrom(AbstractParser.java:244) at com.google.protobuf.AbstractParser.parseDelimitedFrom(AbstractParser.java:249) at com.google.protobuf.AbstractParser.parseDelimitedFrom(AbstractParser.java:25) at com.google.protobuf.GeneratedMessage.parseDelimitedWithIOException(GeneratedMessage.java:367) at com.google.devtools.build.lib.worker.WorkerProtocol$WorkRequest.parseDelimitedFrom(WorkerProtocol.java:1438) at io.bazel.rulesscala.worker.Worker.persistentWorkerMain(Worker.java:81) at io.bazel.rulesscala.worker.Worker.workerMain(Worker.java:49) at io.bazel.rules_scala.scalafmt.ScalafmtWorker$.main(ScalafmtWorker.scala:12) at io.bazel.rules_scala.scalafmt.ScalafmtWorker.main(ScalafmtWorker.scala) ---8<---8<--- End of log ---8<---8<--- Target //scalafmt:formatted-binary2 failed to build ``` * Add attributes to .scalafmt.conf files Per suggestions from @WojciechMazur in #1631. * Bump Scala 2 libs after #1636 Brings all the Scalafmt 3.8.3 updates from #1631 up to date with the Scala version bumps from #1636.
1 parent ca27760 commit 190fbee

19 files changed

+928
-553
lines changed

.scalafmt.conf

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
1+
version = "3.8.3"
2+
runner.dialect = scala213
13
align.openParenCallSite = false
24
align.openParenDefnSite = false
35
continuationIndent.defnSite = 2
46
danglingParentheses.preset = true
57
docstrings.style = Asterisk
68
importSelectors = singleLine
79
maxColumn = 120
8-
verticalMultiline.newlineBeforeImplicitKW = true
910
rewrite.redundantBraces.stringInterpolation = true
1011
rewrite.rules = [
1112
RedundantParens,
1213
PreferCurlyFors,
1314
SortImports
1415
]
15-
unindentTopLevelOperators = false
16-
lineEndings=preserve
16+
indentOperator.topLevelOnly = true
17+
lineEndings = preserve

scala/private/macros/scala_repositories.bzl

+15-9
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
21
load(
3-
"@io_bazel_rules_scala//scala:scala_cross_version.bzl",
2+
"//scala:scala_cross_version.bzl",
43
"extract_major_version",
54
"extract_minor_version",
65
"version_suffix",
76
_default_maven_server_urls = "default_maven_server_urls",
87
)
98
load("//third_party/repositories:repositories.bzl", "repositories")
9+
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
1010
load("@io_bazel_rules_scala_config//:config.bzl", "SCALA_VERSIONS")
1111

1212
def _dt_patched_compiler_impl(rctx):
@@ -69,15 +69,19 @@ def _validate_scalac_srcjar(srcjar):
6969
def dt_patched_compiler_setup(scala_version, scala_compiler_srcjar = None):
7070
scala_major_version = extract_major_version(scala_version)
7171
scala_minor_version = extract_minor_version(scala_version)
72-
patch = "@io_bazel_rules_scala//dt_patches:dt_compiler_%s.patch" % scala_major_version
72+
patch = Label("//dt_patches:dt_compiler_%s.patch" % scala_major_version)
7373

7474
minor_version = int(scala_minor_version)
7575

7676
if scala_major_version == "2.12":
7777
if minor_version >= 1 and minor_version <= 7:
78-
patch = "@io_bazel_rules_scala//dt_patches:dt_compiler_%s.1.patch" % scala_major_version
78+
patch = Label(
79+
"//dt_patches:dt_compiler_%s.1.patch" % scala_major_version,
80+
)
7981
elif minor_version <= 11:
80-
patch = "@io_bazel_rules_scala//dt_patches:dt_compiler_%s.8.patch" % scala_major_version
82+
patch = Label(
83+
"//dt_patches:dt_compiler_%s.8.patch" % scala_major_version,
84+
)
8185

8286
build_file_content = "\n".join([
8387
"package(default_visibility = [\"//visibility:public\"])",
@@ -182,14 +186,16 @@ def _artifact_ids(scala_version):
182186
"io_bazel_rules_scala_scala_parser_combinators",
183187
"org_scalameta_semanticdb_scalac",
184188
] if scala_version.startswith("2") else [
185-
"io_bazel_rules_scala_scala_library",
189+
"io_bazel_rules_scala_scala_asm",
186190
"io_bazel_rules_scala_scala_compiler",
191+
"io_bazel_rules_scala_scala_compiler_2",
187192
"io_bazel_rules_scala_scala_interfaces",
193+
"io_bazel_rules_scala_scala_library",
194+
"io_bazel_rules_scala_scala_library_2",
195+
"io_bazel_rules_scala_scala_parser_combinators",
196+
"io_bazel_rules_scala_scala_reflect_2",
188197
"io_bazel_rules_scala_scala_tasty_core",
189-
"io_bazel_rules_scala_scala_asm",
190198
"io_bazel_rules_scala_scala_xml",
191-
"io_bazel_rules_scala_scala_parser_combinators",
192-
"io_bazel_rules_scala_scala_library_2",
193199
"org_scala_sbt_compiler_interface",
194200
]
195201

scala/scalafmt/BUILD

+8-7
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
1+
load("//scala/scalafmt/toolchain:setup_scalafmt_toolchain.bzl", "setup_scalafmt_toolchains")
2+
load("//scala/scalafmt/toolchain:toolchain.bzl", "export_scalafmt_deps")
3+
load("//scala/scalafmt:phase_scalafmt_ext.bzl", "scalafmt_singleton")
14
load("//scala:scala.bzl", "scala_binary")
25
load("//scala:scala_cross_version.bzl", "version_suffix")
3-
load("//scala/scalafmt/toolchain:toolchain.bzl", "export_scalafmt_deps")
4-
load("//scala/scalafmt/toolchain:setup_scalafmt_toolchain.bzl", "setup_scalafmt_toolchains")
6+
load("//scala:scala_cross_version_select.bzl", "select_for_scala_version")
57
load("@io_bazel_rules_scala_config//:config.bzl", "SCALA_VERSION")
6-
load(
7-
"//scala/scalafmt:phase_scalafmt_ext.bzl",
8-
"scalafmt_singleton",
9-
)
108

119
filegroup(
1210
name = "runner",
@@ -22,7 +20,10 @@ filegroup(
2220

2321
scala_binary(
2422
name = "scalafmt",
25-
srcs = ["scalafmt/ScalafmtWorker.scala"],
23+
srcs = ["scalafmt/ScalafmtWorker.scala"] + select_for_scala_version(
24+
before_2_12_0 = ["scalafmt/ScalafmtAdapter-2_11.scala"],
25+
since_2_12_0 = ["scalafmt/ScalafmtAdapter.scala"],
26+
),
2627
main_class = "io.bazel.rules_scala.scalafmt.ScalafmtWorker",
2728
visibility = ["//visibility:public"],
2829
deps = [
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package io.bazel.rules_scala.scalafmt
2+
3+
import java.io.File
4+
import org.scalafmt.config.Config
5+
import org.scalafmt.config.ScalafmtConfig
6+
import org.scalafmt.util.FileOps
7+
8+
object ScalafmtAdapter {
9+
def readFile(file: File)(implicit codec: scala.io.Codec): String =
10+
FileOps.readFile(file)(codec)
11+
12+
def parseConfigFile(configFile: File): ScalafmtConfig =
13+
Config.fromHoconFile(configFile).get
14+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package io.bazel.rules_scala.scalafmt
2+
3+
import java.io.File
4+
import org.scalafmt.config.ScalafmtConfig
5+
import org.scalafmt.sysops.FileOps
6+
7+
object ScalafmtAdapter {
8+
def readFile(file: File)(implicit codec: scala.io.Codec): String =
9+
FileOps.readFile(file.toPath())(codec)
10+
11+
def parseConfigFile(configFile: File): ScalafmtConfig =
12+
ScalafmtConfig.fromHoconFile(configFile.toPath()).get
13+
}

scala/scalafmt/scalafmt/ScalafmtWorker.scala

+7-5
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ import io.bazel.rulesscala.worker.Worker
44
import java.io.File
55
import java.nio.file.Files
66
import org.scalafmt.Scalafmt
7-
import org.scalafmt.config.Config
8-
import org.scalafmt.util.FileOps
97
import scala.annotation.tailrec
108
import scala.io.Codec
119

@@ -18,12 +16,16 @@ object ScalafmtWorker extends Worker.Interface {
1816
val argFile = args.map{x => new File(x)}
1917
val namespace = argName.zip(argFile).toMap
2018

21-
val source = FileOps.readFile(namespace.getOrElse("input", new File("")))(Codec.UTF8)
19+
val sourceFile = namespace.getOrElse("input", new File(""))
20+
val source = ScalafmtAdapter.readFile(sourceFile)(Codec.UTF8)
21+
22+
val configFile = namespace.getOrElse("config", new File(""))
23+
val config = ScalafmtAdapter.parseConfigFile(configFile)
2224

23-
val config = Config.fromHoconFile(namespace.getOrElse("config", new File(""))).get
2425
@tailrec
2526
def format(code: String): String = {
26-
val formatted = Scalafmt.format(code, config).get
27+
val filePath = sourceFile.getPath()
28+
val formatted = Scalafmt.format(code, config, Set.empty, filePath).get
2729
if (code == formatted) code else format(formatted)
2830
}
2931

scala/scalafmt/scalafmt_repositories.bzl

+42-21
Original file line numberDiff line numberDiff line change
@@ -30,46 +30,67 @@ def scalafmt_default_config(path = ".scalafmt.conf", **kwargs):
3030
scalafmt_config(name = "scalafmt_default", path = "//:" + path, **kwargs)
3131

3232
_SCALAFMT_DEPS = [
33+
"com_geirsson_metaconfig_core",
34+
"com_geirsson_metaconfig_typesafe_config",
35+
"com_google_protobuf_protobuf_java",
36+
"com_lihaoyi_fansi",
37+
"com_lihaoyi_fastparse",
38+
"com_lihaoyi_sourcecode",
39+
"com_thesamet_scalapb_lenses",
40+
"com_thesamet_scalapb_scalapb_runtime",
41+
"com_typesafe_config",
42+
"org_scala_lang_modules_scala_collection_compat",
43+
"org_scala_lang_scalap",
3344
"org_scalameta_common",
34-
"org_scalameta_fastparse",
35-
"org_scalameta_fastparse_utils",
3645
"org_scalameta_parsers",
3746
"org_scalameta_scalafmt_core",
3847
"org_scalameta_scalameta",
3948
"org_scalameta_trees",
4049
"org_typelevel_paiges_core",
41-
"com_typesafe_config",
42-
"org_scala_lang_scalap",
43-
"com_thesamet_scalapb_lenses",
44-
"com_thesamet_scalapb_scalapb_runtime",
45-
"com_lihaoyi_fansi",
46-
"com_lihaoyi_fastparse",
47-
"org_scalameta_fastparse_utils",
48-
"org_scala_lang_modules_scala_collection_compat",
50+
]
51+
52+
_SCALAFMT_DEPS_2_11 = [
4953
"com_lihaoyi_pprint",
50-
"com_lihaoyi_sourcecode",
51-
"com_google_protobuf_protobuf_java",
52-
"com_geirsson_metaconfig_core",
53-
"com_geirsson_metaconfig_typesafe_config",
54+
"org_scalameta_fastparse",
55+
"org_scalameta_fastparse_utils",
5456
]
5557

56-
def _artifact_ids(scala_version):
58+
_SCALAFMT_DEPS_2_12 = [
59+
"com_geirsson_metaconfig_pprint",
60+
"org_scalameta_mdoc_parser",
61+
"org_scalameta_scalafmt_config",
62+
"org_scalameta_scalafmt_sysops",
63+
]
64+
65+
def scalafmt_artifact_ids(scala_version):
5766
major_version = extract_major_version(scala_version)
58-
geny = ["com_lihaoyi_geny"] if major_version != "2.11" else []
59-
parallel_collections = ["io_bazel_rules_scala_scala_parallel_collections"] if major_version == "2.13" or major_version.startswith("3") else []
60-
return _SCALAFMT_DEPS + geny + parallel_collections
67+
68+
if major_version == "2.11":
69+
return _SCALAFMT_DEPS + _SCALAFMT_DEPS_2_11
70+
71+
extra_deps = []
72+
73+
if major_version == "2.12":
74+
extra_deps.append("com_github_bigwheel_util_backports")
75+
else:
76+
extra_deps.append("io_bazel_rules_scala_scala_parallel_collections")
77+
78+
return _SCALAFMT_DEPS + _SCALAFMT_DEPS_2_12 + extra_deps
6179

6280
def scalafmt_repositories(
6381
maven_servers = _default_maven_server_urls(),
64-
overriden_artifacts = {}):
82+
overriden_artifacts = {},
83+
bzlmod_enabled = False):
6584
for scala_version in SCALA_VERSIONS:
6685
repositories(
6786
scala_version = scala_version,
68-
for_artifact_ids = _artifact_ids(scala_version),
87+
for_artifact_ids = scalafmt_artifact_ids(scala_version),
6988
maven_servers = maven_servers,
7089
overriden_artifacts = overriden_artifacts,
7190
)
72-
_register_scalafmt_toolchains()
91+
92+
if not bzlmod_enabled:
93+
_register_scalafmt_toolchains()
7394

7495
def _register_scalafmt_toolchains():
7596
for scala_version in SCALA_VERSIONS:

scala/scalafmt/toolchain/setup_scalafmt_toolchain.bzl

+12-13
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,9 @@
11
load("//scala/scalafmt/toolchain:toolchain.bzl", "scalafmt_toolchain")
2+
load("//scala/scalafmt:scalafmt_repositories.bzl", "scalafmt_artifact_ids")
23
load("//scala:providers.bzl", "declare_deps_provider")
34
load("//scala:scala_cross_version.bzl", "version_suffix")
45
load("@io_bazel_rules_scala_config//:config.bzl", "SCALA_VERSIONS")
56

6-
_SCALAFMT_DEPS = [
7-
"@com_geirsson_metaconfig_core",
8-
"@org_scalameta_common",
9-
"@org_scalameta_parsers",
10-
"@org_scalameta_scalafmt_core",
11-
"@org_scalameta_scalameta",
12-
"@org_scalameta_trees",
13-
"@org_scala_lang_modules_scala_collection_compat",
14-
]
15-
167
def setup_scalafmt_toolchain(
178
name,
189
scalafmt_classpath,
@@ -32,9 +23,14 @@ def setup_scalafmt_toolchain(
3223
)
3324
native.toolchain(
3425
name = name,
35-
target_settings = ["@io_bazel_rules_scala_config//:scala_version" + version_suffix(scala_version)],
26+
target_settings = [
27+
"@io_bazel_rules_scala_config//:scala_version" +
28+
version_suffix(scala_version),
29+
],
3630
toolchain = ":%s_impl" % name,
37-
toolchain_type = "//scala/scalafmt/toolchain:scalafmt_toolchain_type",
31+
toolchain_type = Label(
32+
"//scala/scalafmt/toolchain:scalafmt_toolchain_type",
33+
),
3834
visibility = visibility,
3935
)
4036

@@ -47,4 +43,7 @@ def setup_scalafmt_toolchains():
4743
)
4844

4945
def _deps(scala_version):
50-
return [dep + version_suffix(scala_version) for dep in _SCALAFMT_DEPS]
46+
return [
47+
"@" + artifact_id + version_suffix(scala_version)
48+
for artifact_id in scalafmt_artifact_ids(scala_version)
49+
]

test/scalafmt/.scalafmt.conf

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
1+
version = "3.8.3"
2+
runner.dialect = scala213
13
maxColumn = 40
2-
lineEndings=preserve
4+
lineEndings = preserve
+3-1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
1+
version = "3.8.3"
2+
runner.dialect = scala213
13
maxColumn = 40
2-
lineEndings=preserve
4+
lineEndings = preserve
+3-2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1-
runner.dialect=scala3
1+
version = "3.8.3"
2+
runner.dialect = scala3
23
maxColumn = 40
3-
lineEndings=preserve
4+
lineEndings = preserve

test_cross_build/scalafmt/BUILD

+2-2
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ scalafmt_scala_binary(
5656
scala_version = "2.12.20",
5757
)
5858

59-
scalafmt_scala_library(
59+
scalafmt_scala_binary(
6060
name = "formatted-binary2",
6161
srcs = ["formatted/formatted-binary2.scala"],
6262
config = ":scala2-conf",
@@ -74,7 +74,7 @@ scalafmt_scala_binary(
7474
scala_version = "3.2.2",
7575
)
7676

77-
scalafmt_scala_library(
77+
scalafmt_scala_binary(
7878
name = "formatted-binary3",
7979
srcs = ["formatted/formatted-binary3.scala"],
8080
config = ":scala3-conf",

0 commit comments

Comments
 (0)