Skip to content

Replace @scalafmt_default with toolchains target #1725

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mbland
Copy link
Contributor

@mbland mbland commented Apr 27, 2025

Description

Removes the scalafmt_config() macro and replaces it with the new @rules_scala_toolchains//scalafmt:config target. Part of #1482.

Motivation

The new test/shell/test_dependency_versions.sh test found a problem with the previous implementation. The dev_deps extension in MODULE.bazel generated @scalafmt_default, leaving it invisible to rules_scala when it's not the main module:

ERROR: no such package
  '@@[unknown repo 'scalafmt_default' requested from @@rules_scala~]//':
  The repository
  '@@[unknown repo 'scalafmt_default' requested from @@rules_scala~]'
  could not be resolved:
  No repository visible as '@scalafmt_default'
  from repository '@@rules_scala~'

ERROR: .../tmp/test_dependency_versions/BUILD:52:20:
  every rule of type scalafmt_scala_test
  implicitly depends upon the target
  '@@[unknown repo 'scalafmt_default' requested from @@rules_scala~]//:config',
  but this target could not be found because of:
  no such package
  '@@[unknown repo 'scalafmt_default' requested from @@rules_scala~]//':
  The repository '@@[unknown repo 'scalafmt_default' requested from @@rules_scala~]'
  could not be resolved:
  No repository visible as '@scalafmt_default' from repository '@@rules_scala~'

Documentation for implicit attribute config of rules of type scalafmt_scala_test:
The Scalafmt configuration file.

ERROR: Analysis of target '//:ScalafmtTest' failed;
  build aborted: Analysis failed

The scalafmt_default_config() macro is already gone, and only scala_toolchains() invoked scalafmt_config(), making this a straightforward change.

@mbland mbland requested review from liucijus and simuons as code owners April 27, 2025 02:09
@mbland mbland force-pushed the replace-scalafmt-default-repo branch from e495b34 to ce3234a Compare April 27, 2025 12:20

- Put `.scalafmt.conf` at the root of your workspace
- Pass `.scalafmt.conf` in via `scala_toolchains`:

```py
# MODULE.bazel
scala_deps.toolchains(
scalafmt = True,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we get rid of this flag? I mean scala_deps.scalafmt(default_config = "//path/to/my/custom:scalafmt.conf")
implies scalafmt = True,
For workspace sure we would need to specify boolean flag and pass config as there are no other way to render this in single repository.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that was just a mistake in the docs. Fixed the commit while rebasing on master, and the build is passing.


But speaking of the boolean flag in WORKSPACE, well... The idea from #1722 (comment) of using the module extension to instantiate user defined toolchains is haunting me. I started experimenting with the idea in mbland/bzlmod-toolchains-extension.

In the process, I found a way to make the scalafmt, scala_proto, and twitter_scrooge parameters accept either a boolean (where True implies "use all default settings") or a dict (which implies True). I also updated the semantics of scala/extensions/deps.bzl somewhat, hopefully making them clearer and more consistent. I also changed scala_proto(options) to scala_proto(default_gen_deps) to match setup_scala_proto_toolchains().

I don't know that I'm close yet to making it work for all user defined toolchains, but it should make it easier for users to configure the builtin toolchains more precisely. I'll send a pull request from that new branch soon after this pull request lands.

Removes the `scalafmt_config()` macro and replaces it with the new
`@rules_scala_toolchains//scalafmt:config` target.

The new `test/shell/test_dependency_versions.sh` test found a problem
with the previous implementation. The `dev_deps` extension in
`MODULE.bazel` generated `@scalafmt_default`, leaving it invisible to
`rules_scala` when it's not the main module:

```txt
ERROR: no such package
  '@@[unknown repo 'scalafmt_default' requested from @@rules_scala~]//':
  The repository
  '@@[unknown repo 'scalafmt_default' requested from @@rules_scala~]'
  could not be resolved:
  No repository visible as '@scalafmt_default'
  from repository '@@rules_scala~'

ERROR: .../tmp/test_dependency_versions/BUILD:52:20:
  every rule of type scalafmt_scala_test
  implicitly depends upon the target
  '@@[unknown repo 'scalafmt_default' requested from @@rules_scala~]//:config',
  but this target could not be found because of:
  no such package
  '@@[unknown repo 'scalafmt_default' requested from @@rules_scala~]//':
  The repository '@@[unknown repo 'scalafmt_default' requested from @@rules_scala~]'
  could not be resolved:
  No repository visible as '@scalafmt_default' from repository '@@rules_scala~'

Documentation for implicit attribute config of rules of type scalafmt_scala_test:
The Scalafmt configuration file.

ERROR: Analysis of target '//:ScalafmtTest' failed;
  build aborted: Analysis failed
```

The `scalafmt_default_config()` macro is already gone, and only
`scala_toolchains()` invoked `scalafmt_config()`, making this a
straightforward change.
@mbland mbland force-pushed the replace-scalafmt-default-repo branch from ce3234a to 819449a Compare April 28, 2025 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants