From 96be99961074e411c96e89f000f417bb7b64f431 Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Wed, 7 Oct 2026 15:25:12 -0700 Subject: [PATCH] Add support for linting generated sources --- .bazelignore | 2 +- CHANGELOG.md | 6 ++ README.md | 27 +++++- examples/check_glob/BUILD.bazel | 8 +- examples/check_glob/MODULE.bazel | 1 + examples/optional_attributes/BUILD.bazel | 8 +- examples/optional_attributes/MODULE.bazel | 1 + shellcheck/internal/rules.bzl | 103 +++++++++++++++------- shellcheck/settings/BUILD.bazel | 8 +- tests/BUILD.bazel | 64 ++++++++++++++ tests/follow_source/bin/BUILD.bazel | 12 ++- tests/follow_source/lib1/BUILD.bazel | 12 ++- tests/follow_source/lib2/BUILD.bazel | 12 ++- 13 files changed, 203 insertions(+), 61 deletions(-) diff --git a/.bazelignore b/.bazelignore index 2f4ee11..8c157ec 100644 --- a/.bazelignore +++ b/.bazelignore @@ -1,7 +1,7 @@ examples/check_glob/bazel-bin examples/check_glob/bazel-out examples/check_glob/bazel-testlogs -examples/check_glob/bazel-optional_attributes +examples/check_glob/bazel-check_glob examples/optional_attributes/bazel-bin examples/optional_attributes/bazel-out diff --git a/CHANGELOG.md b/CHANGELOG.md index bc62355..d6cffb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 This document is maintaining changes since the last released version (0.1.1) +## Unreleased + +* Add a `targets` attribute to {obj}`shellcheck_test` for linting `rules_shell` targets and their transitive `deps`. +* Add `//shellcheck/settings:check_generated` and a matching tri-state `check_generated` attribute on + {obj}`shellcheck_test` for linting generated sources. + ## v0.6.1 * Add `--external-sources` support to {obj}`shellcheck_aspect`. diff --git a/README.md b/README.md index fbbe8aa..7c9ad2a 100644 --- a/README.md +++ b/README.md @@ -12,20 +12,33 @@ Then `shellcheck` can be accessed by running: bazel run @rules_shellcheck//:shellcheck -- ``` -And you can define a lint target: +And you can define a lint target for your [rules_shell] targets: ```starlark -load("@rules_shellcheck//:def.bzl", "shellcheck", "shellcheck_test") +load("@rules_shell//shell:sh_binary.bzl", "sh_binary") +load("@rules_shellcheck//shellcheck:shellcheck_test.bzl", "shellcheck_test") + +sh_binary( + name = "my_binary", + srcs = ["my_binary.sh"], + deps = ["//my/library"], +) shellcheck_test( name = "shellcheck_test", - data = glob(["*.sh"]), + targets = [":my_binary"], tags = ["lint"], format = "gcc", severity = "warning", ) ``` +`targets` accepts anything providing `ShInfo` or `ShBinaryInfo` (`sh_binary`, `sh_library`, …). Sources of those targets +and of their transitive `deps` are linted, with each source directory passed as `--source-path` so `source` directives resolve. + +Generated sources are skipped by default. Set `check_generated` to `1` to lint them, `0` to never lint them, or leave it at +`-1` to defer to the `//shellcheck/settings:check_generated` flag described below. + ## Configuring the shellcheck aspect `shellcheck_aspect` runs `shellcheck` against any target that provides `ShInfo` or `ShBinaryInfo` (e.g. `sh_binary` and `sh_library` from [rules_shell]) as part of a normal `bazel build`. Enable it by adding the following to your `.bazelrc`: @@ -51,6 +64,14 @@ build --@rules_shellcheck//shellcheck/settings:severity=warning Because these are standard `string_flag`s, you can also flip them per-command with `--config` groups or with [`transitions`][transitions] if you want a specific target to lint under different settings. +### Generated files + +The aspect (and any `shellcheck_test` with `check_generated = -1`) only lints source files unless you flip the flag: + +``` +build --@rules_shellcheck//shellcheck/settings:check_generated +``` + ### Skipping targets Add any of the following tags to a target to opt it out of `shellcheck_aspect`: diff --git a/examples/check_glob/BUILD.bazel b/examples/check_glob/BUILD.bazel index 2c578fe..1b8badc 100644 --- a/examples/check_glob/BUILD.bazel +++ b/examples/check_glob/BUILD.bazel @@ -1,9 +1,15 @@ +load("@rules_shell//shell:sh_library.bzl", "sh_library") load("@rules_shellcheck//:def.bzl", "shellcheck_test") +sh_library( + name = "scripts", + srcs = glob(["src/**/*.sh"]), +) + shellcheck_test( name = "shellcheck_all", - data = glob(["src/**/*.sh"]), tags = ["lint"], + targets = [":scripts"], ) test_suite( diff --git a/examples/check_glob/MODULE.bazel b/examples/check_glob/MODULE.bazel index f3e86c0..85c91fd 100644 --- a/examples/check_glob/MODULE.bazel +++ b/examples/check_glob/MODULE.bazel @@ -4,6 +4,7 @@ module( compatibility_level = 1, ) +bazel_dep(name = "rules_shell", version = "0.7.1") bazel_dep(name = "rules_shellcheck", version = "0.0.0") local_path_override( module_name = "rules_shellcheck", diff --git a/examples/optional_attributes/BUILD.bazel b/examples/optional_attributes/BUILD.bazel index 562a7a9..506e03c 100644 --- a/examples/optional_attributes/BUILD.bazel +++ b/examples/optional_attributes/BUILD.bazel @@ -1,9 +1,15 @@ +load("@rules_shell//shell:sh_binary.bzl", "sh_binary") load("@rules_shellcheck//:def.bzl", "shellcheck_test") +sh_binary( + name = "script", + srcs = ["script.sh"], +) + shellcheck_test( name = "shellcheck_all", - data = glob(["*.sh"]), format = "gcc", severity = "warning", tags = ["lint"], + targets = [":script"], ) diff --git a/examples/optional_attributes/MODULE.bazel b/examples/optional_attributes/MODULE.bazel index 87e18f1..5f778d6 100644 --- a/examples/optional_attributes/MODULE.bazel +++ b/examples/optional_attributes/MODULE.bazel @@ -4,6 +4,7 @@ module( compatibility_level = 1, ) +bazel_dep(name = "rules_shell", version = "0.7.1") bazel_dep(name = "rules_shellcheck", version = "0.0.0") local_path_override( module_name = "rules_shellcheck", diff --git a/shellcheck/internal/rules.bzl b/shellcheck/internal/rules.bzl index 0296d92..f6d7ead 100644 --- a/shellcheck/internal/rules.bzl +++ b/shellcheck/internal/rules.bzl @@ -14,11 +14,8 @@ set -eu {shellcheck} {args} """ -_BATCH_CONTENT = """\ -@ECHO OFF - -{shellcheck} {args} -""" +# Batch files must use CRLF line endings. +_BATCH_CONTENT = "@ECHO OFF\r\n\r\n{shellcheck} {args}\r\n" def shellcheck_test_impl(ctx, expect_fail = False): """The implementation of the `shellcheck_test` rule. @@ -46,15 +43,35 @@ def shellcheck_test_impl(ctx, expect_fail = False): if ctx.attr.severity: cmd.append("--severity={}".format(ctx.attr.severity)) + check_generated = ctx.attr.check_generated == 1 or ( + ctx.attr.check_generated == -1 and ctx.attr._check_generated[BuildSettingInfo].value + ) + target_srcs = [ + src + for src in depset(transitive = [ + srcs + for target in ctx.attr.targets + for srcs in (target[ShellcheckSrcsInfo].srcs, target[ShellcheckSrcsInfo].transitive_srcs) + ]).to_list() + if src.is_source or check_generated + ] + files = ctx.files.data + target_srcs + + # Linting runs from the runfiles tree, so `--source-path` is derived from + # `short_path` rather than the exec root paths in `ShellcheckSrcsInfo`. + source_paths = depset([src.short_path.rpartition("/")[0] or "." for src in target_srcs]).to_list() + shellcheck_path = toolchain.shellcheck.short_path shellcheck_rc = toolchain.shellcheckrc.short_path - srcs = [f.short_path for f in ctx.files.data] + srcs = [f.short_path for f in files] if is_windows: shellcheck_path.replace("/", "\\") shellcheck_rc.replace("/", "\\") srcs = [src.replace("/", "\\") for src in srcs] + source_paths = [path.replace("/", "\\") for path in source_paths] cmd.append("--rcfile={}".format(shellcheck_rc)) + cmd.extend(["--source-path={}".format(path) for path in source_paths]) cmd.extend(srcs) if expect_fail: @@ -73,36 +90,12 @@ def shellcheck_test_impl(ctx, expect_fail = False): DefaultInfo( executable = executable, runfiles = ctx.runfiles( - files = [toolchain.shellcheck, toolchain.shellcheckrc] + ctx.files.data, + files = [toolchain.shellcheck, toolchain.shellcheckrc] + files, transitive_files = toolchain.all_files, ), ), ] -ATTRS = { - "data": attr.label_list( - allow_files = True, - ), - "format": attr.string( - values = ["checkstyle", "diff", "gcc", "json", "json1", "quiet", "tty"], - doc = "The format of the outputted lint results.", - ), - "severity": attr.string( - values = ["error", "info", "style", "warning"], - doc = "The severity of the lint results.", - ), - "_windows_constraint": attr.label( - default = Label("@platforms//os:windows"), - ), -} - -shellcheck_test = rule( - implementation = shellcheck_test_impl, - attrs = ATTRS, - test = True, - toolchains = [TOOLCHAIN_TYPE], -) - ShellcheckSrcsInfo = provider( doc = "A provider containing relevant data for linting.", fields = { @@ -144,6 +137,46 @@ _shellcheck_srcs_aspect = aspect( implementation = _shellcheck_srcs_aspect_impl, ) +ATTRS = { + "check_generated": attr.int( + doc = ( + "Whether to lint generated files collected from `targets`: `0` never, " + + "`1` always, `-1` defer to `//shellcheck/settings:check_generated`." + ), + default = -1, + values = [-1, 0, 1], + ), + "data": attr.label_list( + allow_files = True, + ), + "format": attr.string( + values = ["checkstyle", "diff", "gcc", "json", "json1", "quiet", "tty"], + doc = "The format of the outputted lint results.", + ), + "severity": attr.string( + values = ["error", "info", "style", "warning"], + doc = "The severity of the lint results.", + ), + "targets": attr.label_list( + doc = "`rules_shell` targets whose sources, and those of their transitive `deps`, are linted.", + providers = [[ShInfo], [ShBinaryInfo]], + aspects = [_shellcheck_srcs_aspect], + ), + "_check_generated": attr.label( + default = Label("//shellcheck/settings:check_generated"), + ), + "_windows_constraint": attr.label( + default = Label("@platforms//os:windows"), + ), +} + +shellcheck_test = rule( + implementation = shellcheck_test_impl, + attrs = ATTRS, + test = True, + toolchains = [TOOLCHAIN_TYPE], +) + def _shellcheck_aspect_impl(target, ctx): if target.label.workspace_root.startswith("external"): return [] @@ -163,10 +196,11 @@ def _shellcheck_aspect_impl(target, ctx): src_info = target[ShellcheckSrcsInfo] + check_generated = ctx.attr._check_generated[BuildSettingInfo].value srcs = [ src for src in src_info.srcs.to_list() - if src.is_source + if src.is_source or check_generated ] if not srcs: @@ -184,7 +218,7 @@ def _shellcheck_aspect_impl(target, ctx): ]) format = ctx.attr._format[BuildSettingInfo].value - severity = ctx.attr._format[BuildSettingInfo].value + severity = ctx.attr._severity[BuildSettingInfo].value output = ctx.actions.declare_file("{}.shellcheck.ok".format(target.label.name)) @@ -228,6 +262,9 @@ shellcheck_aspect = aspect( doc = "An aspect for performing shellcheck checks on `rules_shell` rules.", implementation = _shellcheck_aspect_impl, attrs = { + "_check_generated": attr.label( + default = Label("//shellcheck/settings:check_generated"), + ), "_format": attr.label( default = Label("//shellcheck/settings:format"), ), diff --git a/shellcheck/settings/BUILD.bazel b/shellcheck/settings/BUILD.bazel index d16a346..5c21a84 100644 --- a/shellcheck/settings/BUILD.bazel +++ b/shellcheck/settings/BUILD.bazel @@ -1,4 +1,4 @@ -load("@bazel_skylib//rules:common_settings.bzl", "string_flag") +load("@bazel_skylib//rules:common_settings.bzl", "bool_flag", "string_flag") filegroup( name = "distribution", @@ -6,6 +6,12 @@ filegroup( visibility = ["//shellcheck:__pkg__"], ) +bool_flag( + name = "check_generated", + build_setting_default = False, + visibility = ["//visibility:public"], +) + string_flag( name = "format", build_setting_default = "", diff --git a/tests/BUILD.bazel b/tests/BUILD.bazel index 743ab2b..15e9491 100644 --- a/tests/BUILD.bazel +++ b/tests/BUILD.bazel @@ -1,4 +1,6 @@ +load("@bazel_skylib//rules:copy_file.bzl", "copy_file") load("@rules_shell//shell:sh_binary.bzl", "sh_binary") +load("@rules_shell//shell:sh_library.bzl", "sh_library") load(":test.bzl", shellcheck_test = "shellcheck_internal_test") shellcheck_test( @@ -26,3 +28,65 @@ sh_binary( name = "good", srcs = ["testdata/good.sh"], ) + +shellcheck_test( + name = "targets_test", + targets = [":good"], +) + +shellcheck_test( + name = "targets_fail_test", + expect_fail = True, + targets = [":bad"], +) + +copy_file( + name = "generated_bad_sh", + src = "testdata/bad.sh", + out = "generated_bad.sh", +) + +sh_library( + name = "generated_bad", + srcs = ["generated_bad.sh"], + # The generated source is intentionally unclean. The aspect skips it by + # default but would lint it under `--//shellcheck/settings:check_generated`. + tags = ["no_shellcheck"], +) + +# `check_generated = 0` never lints the generated source, so the accompanying +# clean file is all shellcheck sees. +shellcheck_test( + name = "generated_excluded_test", + check_generated = 0, + data = ["testdata/good.sh"], + targets = [":generated_bad"], +) + +shellcheck_test( + name = "generated_included_test", + check_generated = 1, + expect_fail = True, + targets = [":generated_bad"], +) + +copy_file( + name = "generated_good_sh", + src = "testdata/good.sh", + out = "generated_good.sh", +) + +# Linted by the aspect only under `--//shellcheck/settings:check_generated`. +sh_library( + name = "generated_good", + srcs = ["generated_good.sh"], +) + +# `check_generated` defaults to -1, deferring to +# `--//shellcheck/settings:check_generated`. The generated source is clean so +# this passes either way; the flag only changes whether it is linted at all. +shellcheck_test( + name = "generated_global_test", + data = ["testdata/good.sh"], + targets = [":generated_good"], +) diff --git a/tests/follow_source/bin/BUILD.bazel b/tests/follow_source/bin/BUILD.bazel index 4c85a25..3de8b07 100644 --- a/tests/follow_source/bin/BUILD.bazel +++ b/tests/follow_source/bin/BUILD.bazel @@ -1,4 +1,5 @@ load("@rules_shell//shell:sh_binary.bzl", "sh_binary") +load("@rules_shellcheck//:def.bzl", "shellcheck_test") sh_binary( name = "bin", @@ -7,10 +8,7 @@ sh_binary( deps = ["//tests/follow_source/lib1"], ) -# TODO: https://github.com/aignas/rules_shellcheck/issues/53 -# shellcheck_test( -# name = "bin_test", -# data = [ -# ":bin", -# ], -# ) +shellcheck_test( + name = "bin_test", + targets = [":bin"], +) diff --git a/tests/follow_source/lib1/BUILD.bazel b/tests/follow_source/lib1/BUILD.bazel index f0ec8ec..23a6eb0 100644 --- a/tests/follow_source/lib1/BUILD.bazel +++ b/tests/follow_source/lib1/BUILD.bazel @@ -1,4 +1,5 @@ load("@rules_shell//shell:sh_library.bzl", "sh_library") +load("@rules_shellcheck//:def.bzl", "shellcheck_test") sh_library( name = "lib1", @@ -7,10 +8,7 @@ sh_library( deps = ["//tests/follow_source/lib2"], ) -# TODO: https://github.com/aignas/rules_shellcheck/issues/53 -# shellcheck_test( -# name = "lib1_test", -# data = [ -# ":lib1", -# ], -# ) +shellcheck_test( + name = "lib1_test", + targets = [":lib1"], +) diff --git a/tests/follow_source/lib2/BUILD.bazel b/tests/follow_source/lib2/BUILD.bazel index 75524cc..9e0e5b5 100644 --- a/tests/follow_source/lib2/BUILD.bazel +++ b/tests/follow_source/lib2/BUILD.bazel @@ -1,4 +1,5 @@ load("@rules_shell//shell:sh_library.bzl", "sh_library") +load("@rules_shellcheck//:def.bzl", "shellcheck_test") sh_library( name = "lib2", @@ -6,10 +7,7 @@ sh_library( visibility = ["//tests/follow_source:__subpackages__"], ) -# TODO: https://github.com/aignas/rules_shellcheck/issues/53 -# shellcheck_test( -# name = "lib2_test", -# data = [ -# ":lib2", -# ], -# ) +shellcheck_test( + name = "lib2_test", + targets = [":lib2"], +)