Add support for linting generated sources

.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

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`.

README.md

@@ -12,20 +12,33 @@ Then `shellcheck` can be accessed by running:
 bazel run @rules_shellcheck//:shellcheck -- <parameters>
 ```
 
-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`:

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(

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",

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"],
 )

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",

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"),
         ),

shellcheck/settings/BUILD.bazel

@@ -1,11 +1,17 @@
-load("@bazel_skylib//rules:common_settings.bzl", "string_flag")
+load("@bazel_skylib//rules:common_settings.bzl", "bool_flag", "string_flag")
 
 filegroup(
     name = "distribution",
     srcs = glob(["*"]),
     visibility = ["//shellcheck:__pkg__"],
 )
 
+bool_flag(
+    name = "check_generated",
+    build_setting_default = False,
+    visibility = ["//visibility:public"],
+)
+
 string_flag(
     name = "format",
     build_setting_default = "",

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"],
+)