Skip to content

Commit c52c3e5

Browse files
committed
Update to Scalafmt 3.8.3
Part of bazel-contrib#1482. Ensures that all Scalafmt targets succeed under every supported Scala version. 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 ```
1 parent a4104f6 commit c52c3e5

19 files changed

+924
-553
lines changed

.scalafmt.conf

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
1+
runner.dialect = scala213
12
align.openParenCallSite = false
23
align.openParenDefnSite = false
34
continuationIndent.defnSite = 2
45
danglingParentheses.preset = true
56
docstrings.style = Asterisk
67
importSelectors = singleLine
78
maxColumn = 120
8-
verticalMultiline.newlineBeforeImplicitKW = true
99
rewrite.redundantBraces.stringInterpolation = true
1010
rewrite.rules = [
1111
RedundantParens,
1212
PreferCurlyFors,
1313
SortImports
1414
]
15-
unindentTopLevelOperators = false
16-
lineEndings=preserve
15+
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):
@@ -38,15 +38,19 @@ def _validate_scalac_srcjar(srcjar):
3838
def dt_patched_compiler_setup(scala_version, scala_compiler_srcjar = None):
3939
scala_major_version = extract_major_version(scala_version)
4040
scala_minor_version = extract_minor_version(scala_version)
41-
patch = "@io_bazel_rules_scala//dt_patches:dt_compiler_%s.patch" % scala_major_version
41+
patch = Label("//dt_patches:dt_compiler_%s.patch" % scala_major_version)
4242

4343
minor_version = int(scala_minor_version)
4444

4545
if scala_major_version == "2.12":
4646
if minor_version >= 1 and minor_version <= 7:
47-
patch = "@io_bazel_rules_scala//dt_patches:dt_compiler_%s.1.patch" % scala_major_version
47+
patch = Label(
48+
"//dt_patches:dt_compiler_%s.1.patch" % scala_major_version,
49+
)
4850
elif minor_version <= 11:
49-
patch = "@io_bazel_rules_scala//dt_patches:dt_compiler_%s.8.patch" % scala_major_version
51+
patch = Label(
52+
"//dt_patches:dt_compiler_%s.8.patch" % scala_major_version,
53+
)
5054

5155
build_file_content = "\n".join([
5256
"package(default_visibility = [\"//visibility:public\"])",
@@ -146,14 +150,16 @@ def _artifact_ids(scala_version):
146150
"io_bazel_rules_scala_scala_parser_combinators",
147151
"org_scalameta_semanticdb_scalac",
148152
] if scala_version.startswith("2") else [
149-
"io_bazel_rules_scala_scala_library",
153+
"io_bazel_rules_scala_scala_asm",
150154
"io_bazel_rules_scala_scala_compiler",
155+
"io_bazel_rules_scala_scala_compiler_2",
151156
"io_bazel_rules_scala_scala_interfaces",
157+
"io_bazel_rules_scala_scala_library",
158+
"io_bazel_rules_scala_scala_library_2",
159+
"io_bazel_rules_scala_scala_parser_combinators",
160+
"io_bazel_rules_scala_scala_reflect_2",
152161
"io_bazel_rules_scala_scala_tasty_core",
153-
"io_bazel_rules_scala_scala_asm",
154162
"io_bazel_rules_scala_scala_xml",
155-
"io_bazel_rules_scala_scala_parser_combinators",
156-
"io_bazel_rules_scala_scala_library_2",
157163
"org_scala_sbt_compiler_interface",
158164
]
159165

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

+43-21
Original file line numberDiff line numberDiff line change
@@ -14,49 +14,71 @@ def scalafmt_default_config(path = ".scalafmt.conf"):
1414
build.append(" srcs = [\"{}\"],".format(path))
1515
build.append(" visibility = [\"//visibility:public\"],")
1616
build.append(")")
17+
1718
native.new_local_repository(name = "scalafmt_default", build_file_content = "\n".join(build), path = "")
1819

1920
_SCALAFMT_DEPS = [
21+
"com_geirsson_metaconfig_core",
22+
"com_geirsson_metaconfig_typesafe_config",
23+
"com_google_protobuf_protobuf_java",
24+
"com_lihaoyi_fansi",
25+
"com_lihaoyi_fastparse",
26+
"com_lihaoyi_sourcecode",
27+
"com_thesamet_scalapb_lenses",
28+
"com_thesamet_scalapb_scalapb_runtime",
29+
"com_typesafe_config",
30+
"org_scala_lang_modules_scala_collection_compat",
31+
"org_scala_lang_scalap",
2032
"org_scalameta_common",
21-
"org_scalameta_fastparse",
22-
"org_scalameta_fastparse_utils",
2333
"org_scalameta_parsers",
2434
"org_scalameta_scalafmt_core",
2535
"org_scalameta_scalameta",
2636
"org_scalameta_trees",
2737
"org_typelevel_paiges_core",
28-
"com_typesafe_config",
29-
"org_scala_lang_scalap",
30-
"com_thesamet_scalapb_lenses",
31-
"com_thesamet_scalapb_scalapb_runtime",
32-
"com_lihaoyi_fansi",
33-
"com_lihaoyi_fastparse",
34-
"org_scalameta_fastparse_utils",
35-
"org_scala_lang_modules_scala_collection_compat",
38+
]
39+
40+
_SCALAFMT_DEPS_2_11 = [
3641
"com_lihaoyi_pprint",
37-
"com_lihaoyi_sourcecode",
38-
"com_google_protobuf_protobuf_java",
39-
"com_geirsson_metaconfig_core",
40-
"com_geirsson_metaconfig_typesafe_config",
42+
"org_scalameta_fastparse",
43+
"org_scalameta_fastparse_utils",
4144
]
4245

43-
def _artifact_ids(scala_version):
46+
_SCALAFMT_DEPS_2_12 = [
47+
"com_geirsson_metaconfig_pprint",
48+
"org_scalameta_mdoc_parser",
49+
"org_scalameta_scalafmt_config",
50+
"org_scalameta_scalafmt_sysops",
51+
]
52+
53+
def scalafmt_artifact_ids(scala_version):
4454
major_version = extract_major_version(scala_version)
45-
geny = ["com_lihaoyi_geny"] if major_version != "2.11" else []
46-
parallel_collections = ["io_bazel_rules_scala_scala_parallel_collections"] if major_version == "2.13" or major_version.startswith("3") else []
47-
return _SCALAFMT_DEPS + geny + parallel_collections
55+
56+
if major_version == "2.11":
57+
return _SCALAFMT_DEPS + _SCALAFMT_DEPS_2_11
58+
59+
extra_deps = []
60+
61+
if major_version == "2.12":
62+
extra_deps.append("com_github_bigwheel_util_backports")
63+
else:
64+
extra_deps.append("io_bazel_rules_scala_scala_parallel_collections")
65+
66+
return _SCALAFMT_DEPS + _SCALAFMT_DEPS_2_12 + extra_deps
4867

4968
def scalafmt_repositories(
5069
maven_servers = _default_maven_server_urls(),
51-
overriden_artifacts = {}):
70+
overriden_artifacts = {},
71+
bzlmod_enabled = False):
5272
for scala_version in SCALA_VERSIONS:
5373
repositories(
5474
scala_version = scala_version,
55-
for_artifact_ids = _artifact_ids(scala_version),
75+
for_artifact_ids = scalafmt_artifact_ids(scala_version),
5676
maven_servers = maven_servers,
5777
overriden_artifacts = overriden_artifacts,
5878
)
59-
_register_scalafmt_toolchains()
79+
80+
if not bzlmod_enabled:
81+
_register_scalafmt_toolchains()
6082

6183
def _register_scalafmt_toolchains():
6284
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

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
1+
runner.dialect = scala213
12
maxColumn = 40
2-
lineEndings=preserve
3+
lineEndings = preserve
+2-1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
1+
runner.dialect = scala213
12
maxColumn = 40
2-
lineEndings=preserve
3+
lineEndings = preserve
+2-2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
runner.dialect=scala3
1+
runner.dialect = scala3
22
maxColumn = 40
3-
lineEndings=preserve
3+
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.19",
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)