bazelbuild / rules_scala

Scala rules for Bazel
Apache License 2.0
363 stars 278 forks source link

Update dependencyanalyzer3 for Scala 3.5 #1628

Closed mbland closed 1 month ago

mbland commented 1 month ago

Description

Avoids a future Scala 3.5 compatibility issue under Bzlmod. Part of #1482.

Motivation

PR #1604 added Scala 3.5 suppport. I updated my Bzlmod working branch to include those changes, and updated scala_3_5.bzl with dependencies I'd added or updated from scala_3_4.bzl. ./test_examples.sh then failed with the following error, which I recreated in the local examples/scala3 repo:

$ bazel build --repo_env=SCALA_VERSION=3.5.0 //...

ERROR: .../external/rules_scala~/third_party/dependency_analyzer/src/main/BUILD:4:39:
  scala @@rules_scala~//third_party/dependency_analyzer/src/main:dependency_analyzer
  failed: (Exit 1): scalac_bootstrap failed:
  error executing Scalac command
  (from target @@rules_scala~//third_party/dependency_analyzer/src/main:dependency_analyzer)
  bazel-out/.../bin/external/rules_scala~/src/java/io/bazel/rulesscala/scalac/scalac_bootstrap
    ... (remaining 1 argument skipped)

-- [E164] Declaration Error:
    external/rules_scala~/third_party/dependency_analyzer/src/main/io/bazel/rulesscala/dependencyanalyzer3/DependencyAnalyzer.scala:21:6
21 |  def init(options: List[String]): List[PluginPhase] =
   |      ^
   |error overriding method init in trait StandardPlugin of type (options: List[String]): List[dotty.tools.dotc.plugins.PluginPhase];
   |  method init of type (options: List[String]): List[dotty.tools.dotc.plugins.PluginPhase] needs `override` modifier

This may be because I bumped io_bazel_rules_scala_scala_library_2 from org.scala-lang:scala-library:2.13.12 to 2.13.14 in my Bzlmod branch. That's my guess based on information about the dotty packages from the following files:

It's interesting that Scala 3.{1,2,3,4} all passed with that scala-library version bump, but without this change, but I can't explain why.

WojciechMazur commented 1 month ago

It started to fail in Scala 3.5 because in this minor release we've added a def initialize(settings: List[String])(using Context) to replace def init(settings: List[String]). By having an additional context parameter we use reporter to correctly report warnings/errors, eg. when parsing passed settings. It was introduced in https://github.com/scala/scala3/pull/20330 Because of this change def init needs to use override modifier explicitly. It's a good practice to have it also when overriding abstract methods. Probably an additional @nowarn("deprecation") annotation should be added to the init method, because now it would report that we should use initialize instead.

mbland commented 1 month ago

@WojciechMazur Thanks for the explanation! Not having one was bugging me. Now this loop feels closed.