Skip to content

Commit

Permalink
feat: make it possible for lint violations to cause non-zero bazel ex…
Browse files Browse the repository at this point in the history
…it code
  • Loading branch information
alexeagle committed Oct 13, 2023
1 parent d7e751c commit 6182e8e
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 19 deletions.
49 changes: 38 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Run linters under Bazel (EXPERIMENTAL)
# Run linters under Bazel

> It is currently EXPERIMENTAL and pre-release. No support is promised.
> There may be breaking changes, or we may archive and abandon the repository.
Expand All @@ -10,16 +10,7 @@ Features:
- **No changes needed to rulesets**. Works with the Bazel rules you already use.
- **No changes needed to BUILD files**. You don't need to add lint wrapper macros, and lint doesn't appear in `bazel query` output.
Instead, users can lint their existing `*_library` targets.
- Lints can be **presented in various ways**:
- a hard failure, like it would with a `lint_test` rule that fails the build
- as a warning, using whatever reporting method you prefer
- or even as bot code review comments (e.g. with [reviewdog])

How developers use it:

1. (preferred) This ruleset provides an Aspect CLI plugin,
so it can register the missing 'lint' command and users just type `bazel lint //path/to:targets`.
2. Run with vanilla Bazel, by placing a couple commands in a shell script, Makefile, or similar wrapper.
- Lint results can be **presented in various ways**, see below

See it in action:

Expand All @@ -36,6 +27,42 @@ We have a separate project for formatting, see <https://github.com/aspect-build/
[tricorder]: https://static.googleusercontent.com/media/research.google.com/en//pubs/archive/43322.pdf
[reviewdog]: https://github.com/reviewdog/reviewdog

## Ways to present results

### 1. Warnings in the terminal with `bazel lint`

This ruleset provides an Aspect CLI plugin, so it can register the missing 'lint' command.

Users just type `bazel lint //path/to:targets`.

Reports are then written to the terminal.

### 2. Warnings in the terminal with a wrapper

You can use vanilla Bazel rather than Aspect CLI.

Placing a couple commands in a shell script, Makefile, or similar wrapper.

See the `example/lint.sh` file as an example.

### 3. Errors during `bazel build`

By adding `--aspects_parameters=fail_on_violation=true` to the command-line, we pass a parameter
to our linter aspects that cause them to honor the exit code of the lint tool.

This makes the build fail when any lint violations are present.

### 4. Failures during `bazel test`

We haven't implemented this yet, follow https://github.com/aspect-build/rules_lint/issues/11

### 5. Code review comments

You can wire the reports from bazel-out to a tool like [reviewdog].

We're working on a demo with https://aspect.build/workflows that automatically runs `bazel lint` as
part of your CI and reports the results (including suggested fixes) to your GitHub Code Review thread.

## Available linters

- Protocol Buffers
Expand Down
3 changes: 2 additions & 1 deletion docs/buf.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 26 additions & 0 deletions docs/eslint.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions example/lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ if [ "$#" -eq 0 ]; then
fi

# Produce report files
# You can add --aspects_parameters=fail_on_violation=true to make this command fail instead.
bazel build --aspects //:lint.bzl%eslint,//:lint.bzl%buf,//:lint.bzl%flake8 --output_groups=report $@

# Show the results.
Expand Down
14 changes: 10 additions & 4 deletions lint/buf.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@ load("@rules_proto//proto:defs.bzl", "ProtoInfo")
def _short_path(file, _):
return file.path

def buf_lint_action(ctx, buf_toolchain, target, report):
def buf_lint_action(ctx, buf_toolchain, target, report, use_exit_code = False):
"""Runs the buf lint tool as a Bazel action.
Args:
ctx: Rule OR Aspect context
buf_toolchain: provides the buf-lint tool
target: the proto_library target to run on
report: output file to generate
use_exit_code: whether the protoc process exiting non-zero will be a build failure
"""
config = json.encode({
"input_config": "" if ctx.file._config == None else ctx.file._config.short_path,
Expand Down Expand Up @@ -61,15 +62,19 @@ def buf_lint_action(ctx, buf_toolchain, target, report):
], transitive = [deps]),
outputs = [report],
command = """\
{protoc} $@ 2>{report} || true
""".format(protoc = ctx.executable._protoc.path, report = report.path),
{protoc} $@ 2>{report} {exit_zero}
""".format(
protoc = ctx.executable._protoc.path,
report = report.path,
exit_zero = "" if use_exit_code else "|| true",
),
arguments = [args],
)

def _buf_lint_aspect_impl(target, ctx):
if ctx.rule.kind in ["proto_library"]:
report = ctx.actions.declare_file(target.label.name + ".buf-report.txt")
buf_lint_action(ctx, ctx.toolchains[ctx.attr._buf_toolchain], target, report)
buf_lint_action(ctx, ctx.toolchains[ctx.attr._buf_toolchain], target, report, ctx.attr.fail_on_violation)
results = depset([report])
else:
results = depset()
Expand All @@ -89,6 +94,7 @@ def buf_lint_aspect(config, toolchain = "@rules_buf//tools/protoc-gen-buf-lint:t
implementation = _buf_lint_aspect_impl,
attr_aspects = ["deps"],
attrs = {
"fail_on_violation": attr.bool(),
"_buf_toolchain": attr.string(
default = toolchain,
),
Expand Down
5 changes: 3 additions & 2 deletions lint/eslint.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ eslint = eslint_aspect(
load("@aspect_bazel_lib//lib:copy_to_bin.bzl", "copy_files_to_bin_actions")
load("@aspect_rules_js//js:libs.bzl", "js_lib_helpers")

def _eslint_action(ctx, executable, srcs, report, use_exit_code = False):
def eslint_action(ctx, executable, srcs, report, use_exit_code = False):
"""Create a Bazel Action that spawns an eslint process.
Adapter for wrapping Bazel around
Expand Down Expand Up @@ -73,7 +73,7 @@ def _eslint_action(ctx, executable, srcs, report, use_exit_code = False):
def _eslint_aspect_impl(target, ctx):
if ctx.rule.kind in ["ts_project_rule"]:
report = ctx.actions.declare_file(target.label.name + ".eslint-report.txt")
_eslint_action(ctx, ctx.executable, ctx.rule.files.srcs, report)
eslint_action(ctx, ctx.executable, ctx.rule.files.srcs, report, ctx.attr.fail_on_violation)
results = depset([report])
else:
results = depset()
Expand All @@ -97,6 +97,7 @@ def eslint_aspect(binary, config):
return aspect(
implementation = _eslint_aspect_impl,
attrs = {
"fail_on_violation": attr.bool(),
"_eslint": attr.label(
default = binary,
executable = True,
Expand Down
3 changes: 2 additions & 1 deletion lint/flake8.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def flake8_action(ctx, executable, srcs, config, report, use_exit_code = False):
def _flake8_aspect_impl(target, ctx):
if ctx.rule.kind in ["py_library"]:
report = ctx.actions.declare_file(target.label.name + ".flake8-report.txt")
flake8_action(ctx, ctx.executable._flake8, ctx.rule.files.srcs, ctx.file._config_file, report)
flake8_action(ctx, ctx.executable._flake8, ctx.rule.files.srcs, ctx.file._config_file, report, ctx.attr.fail_on_violation)
results = depset([report])
else:
results = depset()
Expand Down Expand Up @@ -79,6 +79,7 @@ def flake8_aspect(binary, config):
# Needed for linters that need semantic information like transitive type declarations.
# attr_aspects = ["deps"],
attrs = {
"fail_on_violation": attr.bool(),
"_flake8": attr.label(
default = binary,
executable = True,
Expand Down

0 comments on commit 6182e8e

Please sign in to comment.