Skip to content

Commit

Permalink
Tweaks to scalafmt rules/script to support bazel test wrappers
Browse files Browse the repository at this point in the history
With these changes, I can use a macro to place a wrapping `sh_test`
target around this script, and change the phase ordering to put
`phase_scalafmt` before the `runfiles` phase, and suddenly it works
as intended!

NB: If this change is acceptable to folks, then it might be worth
changing the default phase placement for `ext_scalafmt` to place
it before `runfiles` by default.

Example use:

In custom bzl file, I have:

```
_enable_format_targets_and_tests_by_default = True

_ext_scalafmt = dicts.add(
    _base_ext_scalafmt,
    {"attrs": dicts.add(
        _base_ext_scalafmt["attrs"],
        {"format": attr.bool(
            default = _enable_format_targets_and_tests_by_default,
            doc = "Enable the check-format and auto-format synthetic run targets (foo.format-test and foo.format respectively)",
        )},
    )},
    {"phase_providers": [
        "@//path/to/some/buildfile:phase_scalafmt",
    ]},
)

scala_library = make_scala_library(_ext_scalafmt)  # etc for other rule types

def maybe_add_format_test(**kwargs):
    if (kwargs.get("format", _enable_format_targets_and_tests_by_default)):
        name = kwargs["name"]
        sh_test(
            name = "{}.format-test.target".format(name),
            srcs = [":{}.format-test".format(name)],
            data = [
                ":{}".format(name),
            ],
            local = True,
        )

def _scalafmt_singleton_implementation(ctx):
    return [
        _ScalaRulePhase(
            custom_phases = [
                ("-", "runfiles", "scalafmt", _phase_scalafmt),
            ],
        ),
    ]

scalafmt_singleton = rule(
    implementation = _scalafmt_singleton_implementation,
)
```

then in `path/to/some/buildfile/BUILD.bazel`, I have:

```
load("//path/to/above/my_rules.bzl", "scalafmt_singleton")

scalafmt_singleton(
    name = "phase_scalafmt",
    visibility = ["//visibility:public"],
)
```
  • Loading branch information
virusdave committed Feb 5, 2022
1 parent f794d14 commit bf25d5f
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 5 deletions.
9 changes: 7 additions & 2 deletions scala/private/phases/phase_scalafmt.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,24 @@ load(

def phase_scalafmt(ctx, p):
if ctx.attr.format:
manifest, files = _build_format(ctx)
manifest, files, srcs = _build_format(ctx)
_formatter(ctx, manifest, files, ctx.file._runner, ctx.outputs.scalafmt_runner)
_formatter(ctx, manifest, files, ctx.file._testrunner, ctx.outputs.scalafmt_testrunner)
# Return a depset containing all the relevant files, so a wrapping `sh_test` can successfully access them.
return struct(runfiles = depset([manifest] + files + srcs))
else:
_write_empty_content(ctx, ctx.outputs.scalafmt_runner)
_write_empty_content(ctx, ctx.outputs.scalafmt_testrunner)
return None

def _build_format(ctx):
files = []
srcs = []
manifest_content = []
for src in ctx.files.srcs:
# only format scala source files, not generated files
if src.path.endswith(_scala_extension) and src.is_source:
srcs.append(src)
file = ctx.actions.declare_file("{}.fmt.output".format(src.short_path))
files.append(file)
ctx.actions.run(
Expand All @@ -40,7 +45,7 @@ def _build_format(ctx):
manifest = ctx.actions.declare_file("format/{}/manifest.txt".format(ctx.label.name))
ctx.actions.write(manifest, "\n".join(manifest_content) + "\n")

return manifest, files
return manifest, files, srcs

def _formatter(ctx, manifest, files, template, output_runner):
ctx.actions.run_shell(
Expand Down
15 changes: 12 additions & 3 deletions scala/scalafmt/private/format-test.template.sh
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
#!/bin/bash -e
WORKSPACE_ROOT="${1:-$BUILD_WORKSPACE_DIRECTORY}"

# Explanation: IF $BUILD_WORKSPACE_DIRECTORY is set to something (as it would be during a
# `bazel run`), then append a trailing `/`. If it's not set (as it wouldn't be during
# a `bazel test` invocation in a wrapping `sh_test` rule), then elide the trailing `/`, and
# instead rely upon a relative path from the test's runtrees. The corresponding change
# to `phase_scalafmt` places the source files into the `runfiles` set, so they'll be symlinked
# correctly in the appropriate relative location.
WORKSPACE_ROOT="${1:-${BUILD_WORKSPACE_DIRECTORY}${BUILD_WORKSPACE_DIRECTORY:+/}}"

RUNPATH="${TEST_SRCDIR-$0.runfiles}"/%workspace%
RUNPATH=(${RUNPATH//bin/ })
RUNPATH="${RUNPATH[0]}"bin

EXIT=0

while read original formatted; do
if [[ ! -z "$original" ]] && [[ ! -z "$formatted" ]]; then
if ! cmp -s "$WORKSPACE_ROOT/$original" "$RUNPATH/$formatted"; then
if ! cmp -s "${WORKSPACE_ROOT}$original" "$RUNPATH/$formatted"; then
echo $original
diff "$WORKSPACE_ROOT/$original" "$RUNPATH/$formatted" || true
diff "${WORKSPACE_ROOT}$original" "$RUNPATH/$formatted" || true
EXIT=1
fi
fi
Expand Down

0 comments on commit bf25d5f

Please sign in to comment.