feat: produce new rules_lint_output group for human-readable

Configure linters to be colored and produce prettier output for this new output.
Change lint.sh to present this output rather than the 'report' output which is machine-readable

Fixes #324

example/lint.sh

@@ -46,7 +46,7 @@ args+=(
 	# See https://github.com/aspect-build/rules_ts/pull/574#issuecomment-2073632879
 	"--norun_validations"
 	"--build_event_json_file=$buildevents"
-	"--output_groups=rules_lint_report"
+	"--output_groups=rules_lint_output,rules_lint_report"
 	"--remote_download_regex='.*AspectRulesLint.*'"
 )
 
@@ -58,6 +58,11 @@ if [ $1 == "--fail-on-violation" ]; then
 	)
 	shift
 fi
+
+# Allow a `--fix` option on the command-line.
+# This happens to make output of the linter such as ruff's
+# [*] 1 fixable with the `--fix` option.
+# so that the naive thing of pasting that flag to lint.sh will do what the user expects.
 if [ $1 == "--fix" ]; then
 	fix="patch"
 	args+=(
@@ -78,32 +83,29 @@ bazel build ${args[@]} $@
 # TODO: Maybe this could be hermetic with bazel run @aspect_bazel_lib//tools:jq or sth
 if [ $machine == "Windows" ]; then
     # jq on windows outputs CRLF which breaks this script. https://github.com/jqlang/jq/issues/92
-    valid_reports=$(jq --arg ext report --raw-output "$filter" "$buildevents" | tr -d '\r')
+    valid_outputs=$(jq --arg ext .txt --raw-output "$filter" "$buildevents" | tr -d '\r')
 else
-    valid_reports=$(jq --arg ext report --raw-output "$filter" "$buildevents")
+    valid_outputs=$(jq --arg ext .txt --raw-output "$filter" "$buildevents")
 fi
 
 # Show the results.
-while IFS= read -r report; do
-	# Exclude coverage reports, and check if the report is empty.
-	if [[ "$report" == *coverage.dat ]] || [[ ! -s "$report" ]]; then
+while IFS= read -r output; do
+	# Exclude coverage reports, and check if the output is empty.
+	if [[ "$output" == *coverage.dat ]] || [[ ! -s "$output" ]]; then
 		# Report is empty. No linting errors.
 		continue
 	fi
-	echo "From ${report}:"
-	cat "${report}"
+	echo "From ${output}:"
+	cat "${output}"
 	echo
-done <<<"$valid_reports"
+done <<<"$valid_outputs"
 
-# This happens to make output of the linter such as ruff's
-# [*] 1 fixable with the `--fix` option.
-# so that the naive thing of pasting that flag to lint.sh will do what the user expects.
 if [ -n "$fix" ]; then
-	valid_patches=$valid_reports
+	valid_patches=$valid_outputs
 	while IFS= read -r patch; do
-		# Exclude coverage reports, and check if the report is empty.
+		# Exclude coverage, and check if the patch is empty.
 		if [[ "$patch" == *coverage.dat ]] || [[ ! -s "$patch" ]]; then
-			# Report is empty. No linting errors.
+			# Patch is empty. No linting errors.
 			continue
 		fi
 

lint/BUILD.bazel

@@ -93,8 +93,8 @@ js_library(
 )
 
 js_library(
-    name = "eslint.bazel-formatter",
-    srcs = ["eslint.bazel-formatter.js"],
+    name = "eslint.compact-formatter",
+    srcs = ["eslint.compact-formatter.js"],
     visibility = ["//visibility:public"],
 )
 

lint/buf.bzl

@@ -60,7 +60,9 @@ def buf_lint_action(ctx, buf, protoc, target, stderr, exit_code = None):
     args.add_all(sources)
     outputs = [stderr]
 
-    if exit_code:
+    if exit_code == "discard":
+        command = "{protoc} $@ 2>{stderr} || true"
+    elif exit_code:
         command = "{protoc} $@ 2>{stderr}; echo $? > " + exit_code.path
         outputs.append(exit_code)
     else:
@@ -87,15 +89,24 @@ def _buf_lint_aspect_impl(target, ctx):
     if not should_visit(ctx.rule, ctx.attr._rule_kinds):
         return []
 
-    report, exit_code, info = report_files(_MNEMONIC, target, ctx)
+    output, report, exit_code, info = report_files(_MNEMONIC, target, ctx)
     buf_lint_action(
         ctx,
         ctx.toolchains[ctx.attr._buf_toolchain].cli,
         ctx.toolchains["@rules_proto//proto:toolchain_type"].proto.proto_compiler.executable,
         target,
-        report,
+        output,
         exit_code,
     )
+    if report:
+        buf_lint_action(
+            ctx,
+            ctx.toolchains[ctx.attr._buf_toolchain].cli,
+            ctx.toolchains["@rules_proto//proto:toolchain_type"].proto.proto_compiler.executable,
+            target,
+            report,
+            exit_code = "discard",
+        )
     return [info]
 
 def lint_buf_aspect(config, toolchain = "@rules_buf//tools/protoc-gen-buf-lint:toolchain_type", rule_kinds = ["proto_library"]):

lint/clang_tidy.bzl

@@ -238,6 +238,8 @@ def clang_tidy_action(ctx, compilation_context, executable, srcs, stdout, exit_c
     outputs = [stdout]
     env = {}
     env["CLANG_TIDY__STDOUT_STDERR_OUTPUT_FILE"] = stdout.path
+    if exit_code == "discard":
+        exit_code = ctx.actions.declare_file(name = "_exit_code_discard", sibling = stdout)
     if exit_code:
         env["CLANG_TIDY__EXIT_CODE_OUTPUT_FILE"] = exit_code.path
         outputs.append(exit_code)
@@ -310,17 +312,19 @@ def _clang_tidy_aspect_impl(target, ctx):
     compilation_context = target[CcInfo].compilation_context
 
     if ctx.attr._options[LintOptionsInfo].fix:
-        patch, report, exit_code, info = patch_and_report_files(_MNEMONIC, target, ctx)
+        patch, output, report, exit_code, info = patch_and_report_files(_MNEMONIC, target, ctx)
         if len(files_to_lint) == 0:
             dummy_successful_lint_action(ctx, report, exit_code, patch)
         else:
             clang_tidy_fix(ctx, compilation_context, ctx.executable, files_to_lint, patch, report, exit_code)
     else:
-        report, exit_code, info = report_files(_MNEMONIC, target, ctx)
+        output, report, exit_code, info = report_files(_MNEMONIC, target, ctx)
         if len(files_to_lint) == 0:
-            dummy_successful_lint_action(ctx, report, exit_code)
+            dummy_successful_lint_action(ctx, output, exit_code)
         else:
-            clang_tidy_action(ctx, compilation_context, ctx.executable, files_to_lint, report, exit_code)
+            clang_tidy_action(ctx, compilation_context, ctx.executable, files_to_lint, output, exit_code)
+        if report:
+            clang_tidy_action(ctx, compilation_context, ctx.executable, files_to_lint, report, exit_code = "discard")
     return [info]
 
 def lint_clang_tidy_aspect(binary, configs = [], global_config = [], header_filter = "", lint_target_headers = False, angle_includes_are_system = True, verbose = False):

lint/eslint.bzl

@@ -59,22 +59,22 @@ load("//lint/private:lint_aspect.bzl", "LintOptionsInfo", "dummy_successful_lint
 
 _MNEMONIC = "AspectRulesLintESLint"
 
-def _gather_inputs(ctx, srcs):
+def _gather_inputs(ctx, srcs, files):
     inputs = copy_files_to_bin_actions(ctx, srcs)
 
     # Add the config file along with any deps it has on npm packages
     if "gather_files_from_js_providers" in dir(js_lib_helpers):
         # rules_js 1.x
         js_inputs = js_lib_helpers.gather_files_from_js_providers(
-            ctx.attr._config_files + [ctx.attr._workaround_17660, ctx.attr._formatter],
+            ctx.attr._config_files + files,
             include_transitive_sources = True,
             include_declarations = True,
             include_npm_linked_packages = True,
         )
     else:
         # rules_js 2.x
         js_inputs = js_lib_helpers.gather_files_from_js_infos(
-            ctx.attr._config_files + [ctx.attr._workaround_17660, ctx.attr._formatter],
+            ctx.attr._config_files + files,
             include_sources = True,
             include_transitive_sources = True,
             include_types = True,
@@ -84,7 +84,7 @@ def _gather_inputs(ctx, srcs):
     inputs.extend(js_inputs.to_list())
     return inputs
 
-def eslint_action(ctx, executable, srcs, report, exit_code = None):
+def eslint_action(ctx, executable, srcs, stdout, exit_code = None, formatter = None, env = {}):
     """Create a Bazel Action that spawns an eslint process.
 
     Adapter for wrapping Bazel around
@@ -94,46 +94,58 @@ def eslint_action(ctx, executable, srcs, report, exit_code = None):
         ctx: an action context OR aspect context
         executable: struct with an eslint field
         srcs: list of file objects to lint
-        report: output file containing the stdout or --output-file of eslint
+        stdout: output file containing the stdout or --output-file of eslint
         exit_code: output file containing the exit code of eslint.
             If None, then fail the build when eslint exits non-zero.
+        formatter: script suitable for eslint --format flag
+        env: additional environment variables
     """
 
+    formatter = formatter or ctx.attr._compact_formatter
+
     args = ctx.actions.args()
+    file_inputs = [ctx.attr._workaround_17660]
 
     # TODO: enable if debug config, similar to rules_ts
     # args.add("--debug")
-
-    args.add_all(["--format", "../../../" + ctx.file._formatter.path])
+    if type(formatter) == "string":
+        args.add_all(["--format", formatter])
+    else:
+        args.add_all(["--format", "../../../" + formatter.files.to_list()[0].path])
+        file_inputs.append(formatter)
     args.add_all([s.short_path for s in srcs])
 
-    env = {"BAZEL_BINDIR": ctx.bin_dir.path}
+    if exit_code == "discard":
+        exit_code = ctx.actions.declare_file("_discard_eslint_exit_code", sibling = stdout)
 
     if not exit_code:
         ctx.actions.run_shell(
-            inputs = _gather_inputs(ctx, srcs),
-            outputs = [report],
+            inputs = _gather_inputs(ctx, srcs, file_inputs),
+            outputs = [stdout],
             tools = [executable._eslint],
             arguments = [args],
-            command = executable._eslint.path + " $@ && touch " + report.path,
-            env = env,
+            command = executable._eslint.path + " $@ && touch " + stdout.path,
+            env = dict(env, **{
+                "BAZEL_BINDIR": ctx.bin_dir.path,
+            }),
             mnemonic = _MNEMONIC,
             progress_message = "Linting %{label} with ESLint",
         )
     else:
-        # Workaround: create an empty report file in case eslint doesn't write one
+        # Workaround: create an empty file in case eslint doesn't write one
         # Use `../../..` to return to the execroot?
         args.add_joined(["--node_options", "--require", "../../../" + ctx.file._workaround_17660.path], join_with = "=")
 
-        args.add_all(["--output-file", report.short_path])
-        env["JS_BINARY__EXIT_CODE_OUTPUT_FILE"] = exit_code.path
+        args.add_all(["--output-file", stdout.short_path])
 
         ctx.actions.run(
-            inputs = _gather_inputs(ctx, srcs),
-            outputs = [report, exit_code],
+            inputs = _gather_inputs(ctx, srcs, file_inputs),
+            outputs = [stdout, exit_code],
             executable = executable._eslint,
             arguments = [args],
-            env = env,
+            env = dict(env, **{
+                "JS_BINARY__EXIT_CODE_OUTPUT_FILE": exit_code.path,
+            }),
             mnemonic = _MNEMONIC,
             progress_message = "Linting %{label} with ESLint",
         )
@@ -155,15 +167,15 @@ def eslint_fix(ctx, executable, srcs, patch, stdout, exit_code):
         output = patch_cfg,
         content = json.encode({
             "linter": executable._eslint.path,
-            "args": ["--fix", "--format", "../../../" + ctx.file._formatter.path] + [s.short_path for s in srcs],
+            "args": ["--fix", "--format", "../../../" + ctx.file._compact_formatter.path] + [s.short_path for s in srcs],
             "env": {"BAZEL_BINDIR": ctx.bin_dir.path},
             "files_to_diff": [s.path for s in srcs],
             "output": patch.path,
         }),
     )
 
     ctx.actions.run(
-        inputs = _gather_inputs(ctx, srcs) + [patch_cfg],
+        inputs = _gather_inputs(ctx, srcs, [ctx.attr._workaround_17660, ctx.attr._compact_formatter]) + [patch_cfg],
         outputs = [patch, stdout, exit_code],
         executable = executable._patcher,
         arguments = [patch_cfg.path],
@@ -184,19 +196,27 @@ def _eslint_aspect_impl(target, ctx):
         return []
 
     files_to_lint = filter_srcs(ctx.rule)
+    output = None
 
     if ctx.attr._options[LintOptionsInfo].fix:
-        patch, report, exit_code, info = patch_and_report_files(_MNEMONIC, target, ctx)
+        patch, output, report, exit_code, info = patch_and_report_files(_MNEMONIC, target, ctx)
         if len(files_to_lint) == 0:
             dummy_successful_lint_action(ctx, report, exit_code, patch)
         else:
             eslint_fix(ctx, ctx.executable, files_to_lint, patch, report, exit_code)
     else:
-        report, exit_code, info = report_files(_MNEMONIC, target, ctx)
+        output, report, exit_code, info = report_files(_MNEMONIC, target, ctx)
         if len(files_to_lint) == 0:
-            dummy_successful_lint_action(ctx, report, exit_code)
+            dummy_successful_lint_action(ctx, output, exit_code)
         else:
-            eslint_action(ctx, ctx.executable, files_to_lint, report, exit_code)
+            eslint_action(ctx, ctx.executable, files_to_lint, output, exit_code, formatter = "stylish", env = {
+                # https://www.npmjs.com/package/chalk#chalklevel
+                # Force 256 color support even when a tty isn't detected
+                "FORCE_COLOR": "2",
+            })
+
+    if report:
+        eslint_action(ctx, ctx.executable, files_to_lint, output, exit_code = "discard")
 
     return [info]
 
@@ -243,8 +263,8 @@ def lint_eslint_aspect(binary, configs, rule_kinds = ["js_library", "ts_project"
                 allow_single_file = True,
                 cfg = "exec",
             ),
-            "_formatter": attr.label(
-                default = "@aspect_rules_lint//lint:eslint.bazel-formatter",
+            "_compact_formatter": attr.label(
+                default = "@aspect_rules_lint//lint:eslint.compact-formatter",
                 allow_single_file = True,
                 cfg = "exec",
             ),