rules_jvm icon indicating copy to clipboard operation
rules_jvm copied to clipboard

FR: Test rule for running google-java-format check

Open davido opened this issue 3 years ago • 3 comments

Currently we are doing this outside of the build, e.g. with this alias:

  $ alias gjf
  alias gjf='git show --diff-filter=AMR --pretty="" --name-only HEAD | grep java$ | xargs -r /home/davido/projects/gerrit/tools/format/google-java-format-1.7 -i'

On Slack @shs96c commented how this test rule could be implemented:

We’ve not got a rule for that yet, but the process of writing one is simple enough:

o Create a java_google_format_test rule (it’s important that it’s a test rule!)

o Hook it into _create_lint_tests (the pattern should be obvious)

o Optionally create a rule that when run will actually do the reformatting

o Create either a config rule or just allow people to set True as the value for the workspace’s lint_setup

Once that’s done, anyone using the java_library, java_binary, java_test or java_export rules from contrib_rules_jvm can opt into the linting framework and have the google java formatter be part of their regular test suites.

Update

Linting can be disabled by adding a no-lint tag to any rule that needs it. And for everything in a build file by using package_lint_config(linters = {}).

It was also pointed out, that there is another PR was uploaded upstream.

davido avatar May 12 '22 09:05 davido

If we can expose the Google Format command as a *_test rule, then this will be easy to add.

shs96c avatar Jun 29 '22 11:06 shs96c

@shs96c

If we can expose the Google Format command as a *_test rule, then this will be easy to add.

I still don't get how bazel test gjf-check would work on the whole source tree across all Bazel packages?

As mentioned in this comment: https://github.com/google/google-java-format/issues/917#issuecomment-2146607683 it would only work for a project with one single source Bazel package, e.g.:

genrule(
    name = "gjf-check-invocation",
    srcs = glob(["src/main/java/**/*.java"]),
    cmd = "find . -name \"*.java\" | xargs $(location :gjf-binary) --dry-run > $@",
    tools = [":gjf-binary"],
    outs = ["check-java-format.log"],
)

sh_test(
    name = "gjf-check",
    srcs = [":gjf-check-invocation"],
    tags = ["oauth"],
)

Gerrit Code Review project has (atm) 168 different Bazel packages:

✔ ~/projects/gerrit [tools_gjf L|✔]
07:37 $ find java javatests -name BUILD | wc -l
#     168

That means gerrit would need 168 gjf-check-package_1, ..., gjf-check-package_168 test rules, right?

Just re-reading your comment:

Once that’s done, anyone using the java_library, java_binary, java_test or java_export rules from contrib_rules_jvm can opt into the linting framework and have the google java formatter be part of their regular test suites.

We would have to delegate java_library invocation to contrib_rules_jvm and this would perform google java formatter as a part of actually java compile, similarly, how Error Prone check is performed during Bazel's own java compiler toolchain.

davido avatar Jun 04 '24 05:06 davido

https://github.com/aspect-build/rules_lint supports google-java-format as a Java formatter, which might meet your needs.

Although I have not personally used rules_lint for formatting Java code (yet!), I have used it to lint Python code and have been quite pleased.

cj81499 avatar Jun 04 '24 16:06 cj81499