scalafmt compilation breaks after upgrade to "Update to Scalafmt 3.8.3" #1675

gergelyfabian · 2025-01-14T13:02:26Z

If I build my example project with rules_scala (in version 190fbee), I get the following errors:

external/io_bazel_rules_scala/scala/scalafmt/scalafmt/ScalafmtWorker.scala:35: error: Symbol 'type scala.meta.inputs.InputException' is missing from the classpath.
This symbol is required by 'class scala.meta.parsers.ParseException'.
Make sure that type InputException is in your classpath and check for conflicting dependencies with `-Ylog-classpath`.
A full rebuild may help if 'ParseException.class' was compiled against an incompatible version of scala.meta.inputs.
      case e @ (_: org.scalafmt.Error | _: scala.meta.parsers.ParseException) => {
                                           ^
external/io_bazel_rules_scala/scala/scalafmt/scalafmt/ScalafmtWorker.scala:35: warning: fruitless type test: a value of type Throwable cannot also be a scala.meta.parsers.ParseException
      case e @ (_: org.scalafmt.Error | _: scala.meta.parsers.ParseException) => {
                                                              ^
external/io_bazel_rules_scala/scala/scalafmt/scalafmt/ScalafmtAdapter.scala:5: error: object sysops is not a member of package org.scalafmt
import org.scalafmt.sysops.FileOps
                    ^
external/io_bazel_rules_scala/scala/scalafmt/scalafmt/ScalafmtAdapter.scala:9: error: not found: value FileOps
        FileOps.readFile(file.toPath())(codec)
        ^
external/io_bazel_rules_scala/scala/scalafmt/scalafmt/ScalafmtAdapter.scala:5: warning: Unused import
import org.scalafmt.sysops.FileOps

How to reproduce:

git clone https://github.com/gergelyfabian/bazel-scala-example
git checkout upgrade-rules-scala-to-scalafmt-changes
bazel build //...

The version just before 190fbee worked ok. Note: in my repo I also upgraded scalafmt to align with the version in rules_scala. Tested the previous version of rules_scala without scalafmt changes.

The text was updated successfully, but these errors were encountered:

gergelyfabian · 2025-01-14T13:04:47Z

Same error for 719f353.

The latest version (6e36591) has a bigger blocker:

ERROR: /home/user/opt/bazel-scala-example/example-lib/BUILD:18:10: While resolving toolchains for target //example-lib:java_test (33bab35): invalid registered toolchain '@io_bazel_rules_scala//testing:scalatest_toolchain_2_13_15': no such target '@@io_bazel_rules_scala//testing:scalatest_toolchain_2_13_15': target 'scalatest_toolchain_2_13_15' not declared in package 'testing' defined by /home/user/.cache/bazel/_bazel_user/64527517dacf7170f7d914889f83481c/external/io_bazel_rules_scala/testing/BUILD

mbland · 2025-01-14T18:56:57Z

I've reproduced this locally, and I believe this is because ~~I inadvertently broke the default dep_providers of the scala_toolchain rule from //scala/scalafmt/toolchain:toolchain.bzl~~ the example project's scalafmt_toolchain doesn't provide enough dep_providers artifacts.

Still working on a confirmation/fix, but the reason this didn't break the build is because all our own tests end up using setup_scala_toolchain() from //scala/scalafmt/toolchain:setup_scalafmt_toolchain.bzl. This implementation already does the right thing, adding all the required deps_providers. But the example project's //tools/jdk package rolls its own using scala_toolchain directly, and currently only provides three deps in its :scalafmt_classpath_provider:

# Scalafmt                                                                      
declare_deps_provider(                                                          
    name = "scalafmt_classpath_provider",
    deps_id = "scalafmt_classpath",
    visibility = ["//visibility:public"],                                       
    deps = [                                                                    
        "@maven//:com_geirsson_metaconfig_core_%s" % scala_binary_suffix,       
        "@maven//:org_scalameta_parsers_%s" % scala_binary_suffix,              
        "@maven//:org_scalameta_scalafmt_core_%s" % scala_binary_suffix,        
    ],                                                                          
)

This may have been sufficient for Scalafmt 3.0.0, but 3.8.3 added significantly more (#1631).

Will reply further after I've tested my hypothesis, but I have to run for a bit.

gergelyfabian · 2025-01-14T19:50:15Z

Interesting... I think I have followed a rules_scala documentation when setting up this in the past...

gergelyfabian · 2025-01-14T20:22:53Z

Nice, I can confirm that this is a fix:

-load("@io_bazel_rules_scala//scala/scalafmt:scalafmt_repositories.bzl", "scalafmt_default_config")
+load("@io_bazel_rules_scala//scala/scalafmt:scalafmt_repositories.bzl", "scalafmt_default_config", "scalafmt_repositories")
 
 scalafmt_default_config()
 
-register_toolchains("//tools/jdk:scalafmt_toolchain")
+scalafmt_repositories()

So, I started using the toolchain made available by rules_scala.

gergelyfabian · 2025-01-14T20:39:36Z

Sorry, I was wrong, when I use the scalafmt_repositories, when I build a "format-test" target, I get:

ERROR: no such package '@@org_scala_lang_scalap_2_12_20//file': BUILD file not found in directory 'file' of external repository @@org_scala_lang_scalap_2_12_20. Add a BUILD file to a directory to mark it as a package.

This is not happening for all targets though...

mbland · 2025-01-14T23:47:31Z

After the Scalatest stanza replacement, I got bazel build //... to succeed with the following patch:

diff --git a/WORKSPACE b/WORKSPACE
index 1df7df2..106c53a 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -41,11 +41,11 @@ http_archive(
     ],
 )
 
-RULES_SCALA_VERSION = "190fbee676632eb67be34b5ee91613391fc8e633"
+RULES_SCALA_VERSION = "a8ae50ef8c6f9b4bf551e9d6ccf0b796dd07539d"
 
 http_archive(
     name = "io_bazel_rules_scala",
-    #integrity = "sha256-SKK6zg+DchyPWe3efuM6p2ly926IzL+IFz4w2oDC6Is=",
+    integrity = "sha256-s+6dL4C28BwS1TSKOe6LgL/rN+wmqzAfVxlSU+tejVE=",
     strip_prefix = "rules_scala-%s" % RULES_SCALA_VERSION,
     #url = "https://github.com/bazelbuild/rules_scala/releases/download/v%s/rules_scala-v%s.tar.gz" % (RULES_SCALA_VERSION, RULES_SCALA_VERSION),
     url = "https://github.com/bazelbuild/rules_scala/archive/{}.zip".format(RULES_SCALA_VERSION),
@@ -56,21 +56,11 @@ load("//tools:scala_version.bzl", "scala_binary_suffix", "scala_binary_version",
 
 scala_config(scala_version = scala_version)
 
-load("@io_bazel_rules_scala//scala:scala.bzl", "rules_scala_setup", "rules_scala_toolchain_deps_repositories")
+load("@io_bazel_rules_scala//scala:toolchains.bzl", "scala_toolchains")
 
-# Loads other rules Rules Scala depends on.
-rules_scala_setup()
-
-rules_scala_toolchain_deps_repositories()
-
-register_toolchains("//tools/jdk:my_scala_toolchain")
-
-# optional: setup ScalaTest toolchain and dependencies
-load("@io_bazel_rules_scala//testing:scalatest.bzl", "scalatest_repositories", "scalatest_toolchain")
-
-scalatest_repositories()
-
-scalatest_toolchain()
+scala_toolchains(
+    testing = True,
+)
 
 load("@io_bazel_rules_scala//scala/scalafmt:scalafmt_repositories.bzl", "scalafmt_default_config", "scalafmt_repositories")
 
@@ -182,4 +172,7 @@ load("@maven//:defs.bzl", "pinned_maven_install")
 
 pinned_maven_install()
 
-register_toolchains("//tools/jdk:java21_toolchain_definition")
+register_toolchains(
+    "//tools/jdk:java21_toolchain_definition",
+    "@io_bazel_rules_scala_toolchains//...:all",
+)
diff --git a/tools/jdk/BUILD.bazel b/tools/jdk/BUILD.bazel
index 67f12d4..bf7a34d 100644
--- a/tools/jdk/BUILD.bazel
+++ b/tools/jdk/BUILD.bazel
@@ -85,9 +85,14 @@ declare_deps_provider(
     deps_id = "scalafmt_classpath",
     visibility = ["//visibility:public"],
     deps = [
-        "@maven//:com_geirsson_metaconfig_core_%s" % scala_binary_suffix,
-        "@maven//:org_scalameta_parsers_%s" % scala_binary_suffix,
-        "@maven//:org_scalameta_scalafmt_core_%s" % scala_binary_suffix,
+        "@maven//:%s_%s" % (dep, scala_binary_suffix)
+        for dep in [
+            "com_geirsson_metaconfig_core",
+            "org_scalameta_parsers",
+            "org_scalameta_scalafmt_core",
+            "org_scalameta_scalafmt_sysops",
+            "org_scalameta_trees",
+        ]
     ],
 )

This seems to be the minimal set of packages required for this particular target. The missing InputException symbol from earlier comes from org_scalameta_trees; removing this dep reproduces the error.

I figured this out by looking at _SCALAFMT_DEPS and _SCALAFMT_DEPS_2_12 from //scala/scalafmt/scalafmt_repositories.bzl in rules_scala and mapping different deps to the rules_jvm_external schema of the example project.

I'm not sure how best to add a test for this, or how to document it for folks wanting to roll their own Scalafmt toolchain. I'll think on it, but I'm open to suggestions.

It's likely that no one will ever rely on the default when defining their own `scalafmt_toolchain`. While investigating bazel-contrib#1675, I realized the `dep_providers` default was set to a nonexistent target. This didn't break our tests because our Scalafmt toolchains are created by `setup_scala_toolchain`, which sets `dep_providers` explicitly. I thought about aliasing the provider generated for the toolchain for `SCALA_VERSION` in `@io_bazel_rules_scala_toolchains//scalafmt`, or generating a new one. I ultimately decided to remove the default, because the `deps_providers` is literally the only attribute. Anyone defining their own `scalafmt_toolchain` will certainly define their own `deps_providers` target(s).

gergelyfabian · 2025-01-15T09:57:28Z

Thank you! Upgraded my example repo's master branch to latest rules_scala, added most of the above fixes, and it works. Left my own scala toolchain for now.

gergelyfabian · 2025-01-15T10:00:15Z

My changes were:

diff --git a/WORKSPACE b/WORKSPACE
index d5e4f10..b08029f 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -41,13 +41,14 @@ http_archive(
     ],
 )
 
-RULES_SCALA_VERSION = "6.6.0"
+RULES_SCALA_VERSION = "a8ae50ef8c6f9b4bf551e9d6ccf0b796dd07539d"
 
 http_archive(
     name = "io_bazel_rules_scala",
-    integrity = "sha256-5zTu+VzybAFxVmvcJNg72Cva+Mp4c77GzpsNUkva8F0=",
+    integrity = "sha256-s+6dL4C28BwS1TSKOe6LgL/rN+wmqzAfVxlSU+tejVE=",
     strip_prefix = "rules_scala-%s" % RULES_SCALA_VERSION,
-    url = "https://github.com/bazelbuild/rules_scala/releases/download/v%s/rules_scala-v%s.tar.gz" % (RULES_SCALA_VERSION, RULES_SCALA_VERSION),
+    #url = "https://github.com/bazelbuild/rules_scala/releases/download/v%s/rules_scala-v%s.tar.gz" % (RULES_SCALA_VERSION, RULES_SCALA_VERSION),
+    url = "https://github.com/bazelbuild/rules_scala/archive/{}.zip".format(RULES_SCALA_VERSION),
 )
 
 load("@io_bazel_rules_scala//:scala_config.bzl", "scala_config")
@@ -55,22 +56,15 @@ load("//tools:scala_version.bzl", "scala_binary_suffix", "scala_binary_version",
 
 scala_config(scala_version = scala_version)
 
-load("@io_bazel_rules_scala//scala:scala.bzl", "rules_scala_setup", "rules_scala_toolchain_deps_repositories")
+load("@io_bazel_rules_scala//scala:toolchains.bzl", "scala_toolchains")
 
-# Loads other rules Rules Scala depends on.
-rules_scala_setup()
-
-rules_scala_toolchain_deps_repositories()
+scala_toolchains(
+    fetch_sources = True,
+    testing = True,
+)
 
 register_toolchains("//tools/jdk:my_scala_toolchain")
 
-# optional: setup ScalaTest toolchain and dependencies
-load("@io_bazel_rules_scala//testing:scalatest.bzl", "scalatest_repositories", "scalatest_toolchain")
-
-scalatest_repositories()
-
-scalatest_toolchain()
-
 load("@io_bazel_rules_scala//scala/scalafmt:scalafmt_repositories.bzl", "scalafmt_default_config", "scalafmt_repositories")
 
 scalafmt_default_config()
@@ -165,7 +159,7 @@ maven_install(
         "org.scala-lang:scala-library:jar:%s" % scala_version,
         "org.scala-lang:scala-reflect:jar:%s" % scala_version,
         "org.scala-lang:scala-compiler:jar:%s" % scala_version,
-        "org.scalameta:scalafmt-core_%s:3.0.0" % scala_binary_version,
+        "org.scalameta:scalafmt-core_%s:3.8.3" % scala_binary_version,
     ],
     # Some useful options that you may want to try:
     fetch_sources = True,
@@ -181,4 +175,7 @@ load("@maven//:defs.bzl", "pinned_maven_install")
 
 pinned_maven_install()
 
-register_toolchains("//tools/jdk:java21_toolchain_definition")
+register_toolchains(
+    "//tools/jdk:java21_toolchain_definition",
+    "@io_bazel_rules_scala_toolchains//...:all",
+)
diff --git a/tools/jdk/BUILD.bazel b/tools/jdk/BUILD.bazel
index 67f12d4..bf7a34d 100644
--- a/tools/jdk/BUILD.bazel
+++ b/tools/jdk/BUILD.bazel
@@ -85,9 +85,14 @@ declare_deps_provider(
     deps_id = "scalafmt_classpath",
     visibility = ["//visibility:public"],
     deps = [
-        "@maven//:com_geirsson_metaconfig_core_%s" % scala_binary_suffix,
-        "@maven//:org_scalameta_parsers_%s" % scala_binary_suffix,
-        "@maven//:org_scalameta_scalafmt_core_%s" % scala_binary_suffix,
+        "@maven//:%s_%s" % (dep, scala_binary_suffix)
+        for dep in [
+            "com_geirsson_metaconfig_core",
+            "org_scalameta_parsers",
+            "org_scalameta_scalafmt_core",
+            "org_scalameta_scalafmt_sysops",
+            "org_scalameta_trees",
+        ]
     ],
 )

The rest is more-or-less obvious, like upgrading dependencies (for Scalafmt).

* Toolchainize //scala/scalafmt:scalafmt_toolchain This is a pretty straightforward and easy update on top of the previous `setup_toolchains` and `scala_toolchains_repo` changes. Part of #1482. * Make test_reproducibility.sh build test/coverage_* Replaces the nonexistent `//test/coverage/...` target with packages that actually exist, rendering the test more meaningful. A spurious failure caused me to run the following to inspect what was happening: ```txt RULES_SCALA_TEST_ONLY=test_build_is_identical ./test_reproducibility.sh ``` This caused me to see that the following command was failing because `//test/coverage/...` didn't exist: ```txt bazel build --collect_code_coverage -- //test/coverage/... ``` * Export `SCALAFMT_TOOLCHAIN_TYPE` constant Reduces string duplication in `//scala/scalafmt/toolchain/*.bzl` files. * Remove scalafmt_toolchain dep_providers default It's likely that no one will ever rely on the default when defining their own `scalafmt_toolchain`. While investigating #1675, I realized the `dep_providers` default was set to a nonexistent target. This didn't break our tests because our Scalafmt toolchains are created by `setup_scala_toolchain`, which sets `dep_providers` explicitly. I thought about aliasing the provider generated for the toolchain for `SCALA_VERSION` in `@io_bazel_rules_scala_toolchains//scalafmt`, or generating a new one. I ultimately decided to remove the default, because the `deps_providers` is literally the only attribute. Anyone defining their own `scalafmt_toolchain` will certainly define their own `deps_providers` target(s). * Update tests for Bazel 6 + Bzlmod Copied the regular expression used to optionally match canonical repo name prefixes in `buildozer` commands from #1622. These never failed when building with Bzlmod and Bazel 7.4.1, but did under Bazel 6.5.0. * Fix `//third_party` tests with `$(rootpath)` Fixes `{strict_deps,unused_dependency_checker}_test` from `//third_party/dependency_analyzer/src/test` under Bazel 8 by updating `$(location)` to `$(rootpath)`. Part of #1652. `//third_party/dependency_analyzer/src/test:strict_deps_test` and `//third_party/dependency_analyzer/src/test:unused_dependency_checker_test` both failed with errors of the form: ```txt StrictDepsTest: - error on indirect dependency target *** FAILED *** (379 milliseconds) nice errors on sequence of strings.this.infos.exists(nice errors on sequence of strings.this.checkErrorContainsMessage(target)) was false expected an error on //commons:Target to appear in errors (with buildozer command)! Errors: List(object apache is not a member of package org) (StrictDepsTest.scala:85) ``` This happened both under `WORKSPACE` and Bzlmod. Copying the test script with the following (with `$ID` being one of `7`, `8`, `7-updated`, or `8-updated`) helped reveal the differences: ```txt cp bazel-bin/third_party/dependency_analyzer/src/test/strict_deps_test \ strict_deps_test-$ID.sh ``` Under Bazel 7, we find the following lines whether using `$(location)` or `$(rootpath)` on the `org.apache.commons` artifact (the other artifacts already use `$(rootpath)`: ```txt -Dscala.library.location=external/io_bazel_rules_scala_scala_library_2_12_20/scala-library-2.12.20.jar -Dscala.reflect.location=external/io_bazel_rules_scala_scala_reflect_2_12_20/scala-reflect-2.12.20.jar -Dguava.jar.location=external/com_google_guava_guava_21_0_with_file/guava-21.0.jar -Dapache.commons.jar.location=external/org_apache_commons_commons_lang_3_5_without_file/commons-lang3-3.5.jar ``` Under Bazel 8, notice that the `external/` prefix has become `../` for the other paths already using `$(rootpath)`, but remains `external/` for the `$(location)` artifact. This breaks the Bazel 8 build: ```txt -Dscala.library.location=../io_bazel_rules_scala_scala_library_2_12_20/scala-library-2.12.20.jar -Dscala.reflect.location=../io_bazel_rules_scala_scala_reflect_2_12_20/scala-reflect-2.12.20.jar -Dguava.jar.location=../com_google_guava_guava_21_0_with_file/guava-21.0.jar -Dapache.commons.jar.location=external/org_apache_commons_commons_lang_3_5_without_file/commons-lang3-3.5.jar ``` Under Bazel 8, using `$(rootpath)` for the `org.apache.commons` artifact converts its `external/`, fixing the Bazel 8 build: ```txt -Dscala.library.location=../io_bazel_rules_scala_scala_library_2_12_20/scala-library-2.12.20.jar -Dscala.reflect.location=../io_bazel_rules_scala_scala_reflect_2_12_20/scala-reflect-2.12.20.jar -Dguava.jar.location=../com_google_guava_guava_21_0_with_file/guava-21.0.jar -Dapache.commons.jar.location=../org_apache_commons_commons_lang_3_5_without_file/commons-lang3-3.5.jar ```

mbland mentioned this issue Jan 14, 2025

build breaks after testing toolchain changes #1676

Closed

gergelyfabian closed this as completed Jan 14, 2025

gergelyfabian reopened this Jan 14, 2025

This was referenced Jan 15, 2025

Toolchainize //scala/scalafmt:scalafmt_toolchain #1678

Merged

Support Bzlmod and add rules_scala to bazel-central-registry #1482

Closed

gergelyfabian closed this as completed Jan 15, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

scalafmt compilation breaks after upgrade to "Update to Scalafmt 3.8.3" #1675

scalafmt compilation breaks after upgrade to "Update to Scalafmt 3.8.3" #1675

gergelyfabian commented Jan 14, 2025

gergelyfabian commented Jan 14, 2025

mbland commented Jan 14, 2025 •

edited

Loading

gergelyfabian commented Jan 14, 2025

gergelyfabian commented Jan 14, 2025

gergelyfabian commented Jan 14, 2025 •

edited

Loading

mbland commented Jan 14, 2025 •

edited

Loading

gergelyfabian commented Jan 15, 2025

gergelyfabian commented Jan 15, 2025 •

edited

Loading

scalafmt compilation breaks after upgrade to "Update to Scalafmt 3.8.3" #1675

scalafmt compilation breaks after upgrade to "Update to Scalafmt 3.8.3" #1675

Comments

gergelyfabian commented Jan 14, 2025

gergelyfabian commented Jan 14, 2025

mbland commented Jan 14, 2025 • edited Loading

gergelyfabian commented Jan 14, 2025

gergelyfabian commented Jan 14, 2025

gergelyfabian commented Jan 14, 2025 • edited Loading

mbland commented Jan 14, 2025 • edited Loading

gergelyfabian commented Jan 15, 2025

gergelyfabian commented Jan 15, 2025 • edited Loading

mbland commented Jan 14, 2025 •

edited

Loading

gergelyfabian commented Jan 14, 2025 •

edited

Loading

mbland commented Jan 14, 2025 •

edited

Loading

gergelyfabian commented Jan 15, 2025 •

edited

Loading