Add support for --external-source to shellcheck_aspect (#54)

This relates to #53 but
only implements this support for `shellcheck_aspect`. The reason for
comes from `shellcheck_aspect` having baked in assumptions around
operating exclusively on `sh_*` rules (related to
bazel-contrib/rules_shell#16) which enabled the
rules to further align with adding this support for `rules_shell`
targets with the aspect.

This change also eliminates the unnecessary generation of files for each
time the aspect runs.

Author: UebelAndre

shellcheck/.shellcheckrc

@@ -1 +1,2 @@
 # https://github.com/koalaman/shellcheck/blob/v0.11.0/shellcheck.1.md#rc-files
+external-sources=true

shellcheck/internal/BUILD.bazel

@@ -3,3 +3,12 @@ filegroup(
     srcs = glob(["*"]),
     visibility = ["//shellcheck:__pkg__"],
 )
+
+filegroup(
+    name = "aspect_runner",
+    srcs = select({
+        "@platforms//os:windows": ["aspect_runner.bat"],
+        "//conditions:default": ["aspect_runner.sh"],
+    }),
+    visibility = ["//visibility:public"],
+)

shellcheck/internal/aspect_runner.bat

@@ -0,0 +1,6 @@
+#!/bin/sh
+
+set -eu
+
+echo "" > "${SHELLCHECK_ASPECT_OUTPUT}"
+exec $@

shellcheck/internal/aspect_runner.sh

@@ -0,0 +1,6 @@
+#!/bin/sh
+
+set -eu
+
+echo "" > "${SHELLCHECK_ASPECT_OUTPUT}"
+exec $@

shellcheck/internal/rules.bzl

@@ -71,7 +71,7 @@ def shellcheck_test_impl(ctx, expect_fail = False):
         DefaultInfo(
             executable = executable,
             runfiles = ctx.runfiles(
-                files = [toolchain.shellcheck] + ctx.files.data,
+                files = [toolchain.shellcheck, toolchain.shellcheckrc] + ctx.files.data,
                 transitive_files = toolchain.all_files,
             ),
         ),
@@ -101,19 +101,52 @@ shellcheck_test = rule(
     toolchains = [TOOLCHAIN_TYPE],
 )
 
-_ASPECT_SHELL_CONTENT = """\
-#!/bin/sh
-
-echo '' > '{output}'
-exec '{shellcheck}' $@
-"""
+ShellcheckSrcsInfo = provider(
+    doc = "A provider containing relevant data for linting.",
+    fields = {
+        "source_paths": "depset[str]: `--source-path` target paths.",
+        "srcs": "depset[File]: Sources collected from the target.",
+        "transitive_source_paths": "depset[str]: Transitive source paths collected from dependencies.",
+        "transitive_srcs": "depset[File]: Transitive sources collected from dependencies.",
+    },
+)
 
-_ASPECT_BATCH_CONTENT = """\
-@ECHO OFF
+def _shellcheck_srcs_aspect_impl(_target, ctx):
+    # TODO: Replace when a `rules_shell` provider is available
+    # https://github.com/bazelbuild/rules_shell/issues/16
+    rule_name = ctx.rule.kind
+    if rule_name not in ["sh_binary", "sh_test", "sh_library"]:
+        return []
 
-echo "" > {output}
-{shellcheck} %*
-"""
+    srcs = getattr(ctx.rule.files, "srcs", [])
+    source_paths = [src.dirname for src in srcs]
+
+    transitive_srcs = []
+    transitive_source_paths = []
+
+    for dep in getattr(ctx.rule.attr, "deps", []):
+        if ShellcheckSrcsInfo in dep:
+            transitive_srcs.extend([
+                dep[ShellcheckSrcsInfo].srcs,
+                dep[ShellcheckSrcsInfo].transitive_srcs,
+            ])
+            transitive_source_paths.extend([
+                dep[ShellcheckSrcsInfo].source_paths,
+                dep[ShellcheckSrcsInfo].transitive_source_paths,
+            ])
+
+    return [ShellcheckSrcsInfo(
+        srcs = depset(srcs),
+        source_paths = depset(source_paths),
+        transitive_srcs = depset(transitive = transitive_srcs),
+        transitive_source_paths = depset(transitive = transitive_source_paths),
+    )]
+
+_shellcheck_srcs_aspect = aspect(
+    doc = "An aspect for collecting data about how to lint the target.",
+    attr_aspects = ["deps"],
+    implementation = _shellcheck_srcs_aspect_impl,
+)
 
 def _shellcheck_aspect_impl(target, ctx):
     if target.label.workspace_root.startswith("external"):
@@ -129,14 +162,14 @@ def _shellcheck_aspect_impl(target, ctx):
         if tag.replace("-", "_").lower() in ignore_tags:
             return []
 
-    # TODO: https://github.com/aignas/rules_shellcheck/issues/23
-    rule_name = ctx.rule.kind
-    if rule_name not in ["sh_binary", "sh_test", "sh_library"]:
+    if ShellcheckSrcsInfo not in target:
         return []
 
+    src_info = target[ShellcheckSrcsInfo]
+
     srcs = [
         src
-        for src in getattr(ctx.rule.files, "srcs", [])
+        for src in src_info.srcs.to_list()
         if src.is_source
     ]
 
@@ -145,8 +178,8 @@ def _shellcheck_aspect_impl(target, ctx):
 
     toolchain = ctx.toolchains[TOOLCHAIN_TYPE]
 
-    inputs_direct = getattr(ctx.rule.files, "srcs", []) + getattr(ctx.rule.files, "data", [])
-    inputs_transitive = []
+    inputs_direct = [toolchain.shellcheckrc] + getattr(ctx.rule.files, "data", [])
+    inputs_transitive = [src_info.srcs, src_info.transitive_srcs]
 
     if DefaultInfo in target:
         inputs_transitive.extend([
@@ -157,25 +190,14 @@ def _shellcheck_aspect_impl(target, ctx):
     format = ctx.attr._format[BuildSettingInfo].value
     severity = ctx.attr._format[BuildSettingInfo].value
 
-    shellcheck = toolchain.shellcheck
-    is_windows = True if shellcheck.basename.endswith(".exe") else False
-
-    executable = ctx.actions.declare_file("{}.shellcheck.{}".format(target.label.name, "bat" if is_windows else "sh"))
     output = ctx.actions.declare_file("{}.shellcheck.ok".format(target.label.name))
 
-    ctx.actions.write(
-        output = executable,
-        content = (_ASPECT_BATCH_CONTENT if is_windows else _ASPECT_SHELL_CONTENT).format(
-            output = output.path,
-            shellcheck = shellcheck.path,
-        ),
-        is_executable = True,
-    )
-
-    tools = depset([shellcheck], transitive = [toolchain.all_files])
+    tools = depset([toolchain.shellcheck], transitive = [toolchain.all_files])
 
     args = ctx.actions.args()
+    args.add(toolchain.shellcheck)
     args.add(toolchain.shellcheckrc, format = "--rcfile=%s")
+    args.add_all(src_info.source_paths, format_each = "--source-path=%s")
 
     if format:
         args.add(format, format = "--format=%s")
@@ -188,10 +210,12 @@ def _shellcheck_aspect_impl(target, ctx):
     ctx.actions.run(
         mnemonic = "Shellcheck",
         progress_message = "Shellcheck {}".format(target.label),
-        executable = executable,
+        executable = ctx.file._runner,
         inputs = depset(inputs_direct, transitive = inputs_transitive),
         arguments = [args],
-        env = ctx.configuration.default_shell_env,
+        env = ctx.configuration.default_shell_env | {
+            "SHELLCHECK_ASPECT_OUTPUT": output.path,
+        },
         tools = tools,
         outputs = [output],
     )
@@ -209,9 +233,14 @@ shellcheck_aspect = aspect(
         "_format": attr.label(
             default = Label("//shellcheck/settings:format"),
         ),
+        "_runner": attr.label(
+            allow_single_file = True,
+            default = Label("//shellcheck/internal:aspect_runner"),
+        ),
         "_severity": attr.label(
             default = Label("//shellcheck/settings:severity"),
         ),
     },
     toolchains = [TOOLCHAIN_TYPE],
+    requires = [_shellcheck_srcs_aspect],
 )

tests/follow_source/BUILD.bazel

@@ -0,0 +1,16 @@
+load("@rules_shell//shell:sh_binary.bzl", "sh_binary")
+
+sh_binary(
+    name = "bin",
+    srcs = ["bin.sh"],
+    visibility = ["//tests/follow_source:__subpackages__"],
+    deps = ["//tests/follow_source/lib1"],
+)
+
+# TODO: https://github.com/aignas/rules_shellcheck/issues/53
+# shellcheck_test(
+#     name = "bin_test",
+#     data = [
+#         ":bin",
+#     ],
+# )

tests/follow_source/bin/BUILD.bazel

@@ -0,0 +1,8 @@
+#!/bin/bash
+set -euo pipefail
+
+# Source the external library (same directory in runfiles).
+source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../lib1/lib1.sh"
+
+hello_from_lib1
+echo "Done"

tests/follow_source/bin/bin.sh

@@ -0,0 +1,16 @@
+load("@rules_shell//shell:sh_library.bzl", "sh_library")
+
+sh_library(
+    name = "lib1",
+    srcs = ["lib1.sh"],
+    visibility = ["//tests/follow_source:__subpackages__"],
+    deps = ["//tests/follow_source/lib2"],
+)
+
+# TODO: https://github.com/aignas/rules_shellcheck/issues/53
+# shellcheck_test(
+#     name = "lib1_test",
+#     data = [
+#         ":lib1",
+#     ],
+# )

tests/follow_source/lib1/BUILD.bazel

@@ -0,0 +1,9 @@
+#!/bin/bash
+
+# Source the external library (same directory in runfiles).
+source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../lib2/lib2.sh"
+
+hello_from_lib() {
+    echo "Hello from library 1"
+    hello_from_lib2
+}