diff --git a/example/lint.sh b/example/lint.sh index f4e6c199..7bb1fed4 100755 --- a/example/lint.sh +++ b/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 diff --git a/lint/BUILD.bazel b/lint/BUILD.bazel index 01f2d5fb..11ba225a 100644 --- a/lint/BUILD.bazel +++ b/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"], ) diff --git a/lint/buf.bzl b/lint/buf.bzl index edc83a25..eb1ac333 100644 --- a/lint/buf.bzl +++ b/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"]): diff --git a/lint/clang_tidy.bzl b/lint/clang_tidy.bzl index 4b2eeb6f..e4ceb15c 100644 --- a/lint/clang_tidy.bzl +++ b/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): diff --git a/lint/eslint.bazel-formatter.js b/lint/eslint.bazel-formatter.js deleted file mode 100644 index d4ec9da3..00000000 --- a/lint/eslint.bazel-formatter.js +++ /dev/null @@ -1,45 +0,0 @@ -// Fork of 'compact' plugin that prints relative paths. -// This allows an editor to navigate to the location of the lint warning even though we present -// eslint with paths underneath a bazel sandbox folder. -// from https://github.com/eslint/eslint/blob/331cf62024b6c7ad4067c14c593f116576c3c861/lib/cli-engine/formatters/compact.js -const path = require("node:path"); - -/** - * Returns the severity of warning or error - * @param {Object} message message object to examine - * @returns {string} severity level - * @private - */ -function getMessageType(message) { - if (message.fatal || message.severity === 2) { - return "Error"; - } - return "Warning"; -} - -module.exports = function (results, context) { - let output = "", - total = 0; - - results.forEach((result) => { - const messages = result.messages; - - total += messages.length; - - messages.forEach((message) => { - output += `${path.relative(context.cwd, result.filePath)}: `; - output += `line ${message.line || 0}`; - output += `, col ${message.column || 0}`; - output += `, ${getMessageType(message)}`; - output += ` - ${message.message}`; - output += message.ruleId ? ` (${message.ruleId})` : ""; - output += "\n"; - }); - }); - - if (total > 0) { - output += `\n${total} problem${total !== 1 ? "s" : ""}`; - } - - return output; -}; diff --git a/lint/eslint.bzl b/lint/eslint.bzl index 3aa37f16..07e6c888 100644 --- a/lint/eslint.bzl +++ b/lint/eslint.bzl @@ -59,14 +59,14 @@ 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, @@ -74,7 +74,7 @@ def _gather_inputs(ctx, srcs): 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,7 +167,7 @@ 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, @@ -163,7 +175,7 @@ def eslint_fix(ctx, executable, srcs, patch, stdout, exit_code): ) 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", ), diff --git a/lint/eslint.compact-formatter.js b/lint/eslint.compact-formatter.js new file mode 100644 index 00000000..b4266a5d --- /dev/null +++ b/lint/eslint.compact-formatter.js @@ -0,0 +1,67 @@ +// Fork of 'compact' plugin that prints relative paths. +// This allows an editor to navigate to the location of the lint warning even though we present +// eslint with paths underneath a bazel sandbox folder. + +/** + * Vendored from https://github.com/eslint/eslint/blob/331cf62024b6c7ad4067c14c593f116576c3c861/lib/cli-engine/formatters/compact.js +Copyright OpenJS Foundation and other contributors, + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. + */ +const path = require("node:path"); + +/** + * Returns the severity of warning or error + * @param {Object} message message object to examine + * @returns {string} severity level + * @private + */ +function getMessageType(message) { + if (message.fatal || message.severity === 2) { + return "Error"; + } + return "Warning"; +} + +module.exports = function (results, context) { + let output = "", + total = 0; + + results.forEach((result) => { + const messages = result.messages; + + total += messages.length; + + messages.forEach((message) => { + output += `${path.relative(context.cwd, result.filePath)}: `; + output += `line ${message.line || 0}`; + output += `, col ${message.column || 0}`; + output += `, ${getMessageType(message)}`; + output += ` - ${message.message}`; + output += message.ruleId ? ` (${message.ruleId})` : ""; + output += "\n"; + }); + }); + + if (total > 0) { + output += `\n${total} problem${total !== 1 ? "s" : ""}`; + } + + return output; +}; diff --git a/lint/flake8.bzl b/lint/flake8.bzl index e80a849a..c7220eb5 100644 --- a/lint/flake8.bzl +++ b/lint/flake8.bzl @@ -53,7 +53,9 @@ def flake8_action(ctx, executable, srcs, config, stdout, exit_code = None): args.add_all(srcs) args.add(config, format = "--config=%s") - if exit_code: + if exit_code == "discard": + command = "{flake8} $@ >{stdout} || true" + elif exit_code: command = "{flake8} $@ >{stdout}; echo $? > " + exit_code.path outputs.append(exit_code) else: @@ -76,14 +78,16 @@ def _flake8_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) files_to_lint = filter_srcs(ctx.rule) if len(files_to_lint) == 0: - dummy_successful_lint_action(ctx, report, exit_code) + dummy_successful_lint_action(ctx, output, exit_code) else: - flake8_action(ctx, ctx.executable._flake8, files_to_lint, ctx.file._config_file, report, exit_code) + flake8_action(ctx, ctx.executable._flake8, files_to_lint, ctx.file._config_file, output, exit_code) + if report: + flake8_action(ctx, ctx.executable._flake8, files_to_lint, ctx.file._config_file, report, exit_code = "discard") return [info] def lint_flake8_aspect(binary, config, rule_kinds = ["py_binary", "py_library"]): diff --git a/lint/ktlint.bzl b/lint/ktlint.bzl index fd119737..ccbbe5dd 100644 --- a/lint/ktlint.bzl +++ b/lint/ktlint.bzl @@ -56,7 +56,7 @@ load("//lint/private:lint_aspect.bzl", "LintOptionsInfo", "dummy_successful_lint _MNEMONIC = "AspectRulesLintKTLint" -def ktlint_action(ctx, executable, srcs, editorconfig, stdout, baseline_file, java_runtime, ruleset_jar = None, exit_code = None): +def ktlint_action(ctx, executable, srcs, editorconfig, stdout, baseline_file, java_runtime, ruleset_jar = None, exit_code = None, user_args = []): """ Runs ktlint as build action in Bazel. Adapter for wrapping Bazel around @@ -73,9 +73,11 @@ def ktlint_action(ctx, executable, srcs, editorconfig, stdout, baseline_file, ja ruleset_jar: An optional, custom ktlint ruleset jar. exit_code: output file to write the exit code. If None, then fail the build when ktlint exits non-zero. + user_args: additional CLI arguments to the ktlint command """ args = ctx.actions.args() + args.add_all(user_args) inputs = srcs outputs = [stdout] @@ -108,7 +110,9 @@ def ktlint_action(ctx, executable, srcs, editorconfig, stdout, baseline_file, ja # This makes hermetic java available to ktlint executable command = "export PATH=$PATH:$JAVA_HOME/bin\n" - if exit_code: + if exit_code == "discard": + command += "{ktlint} $@ >{stdout} || true" + elif exit_code: # Don't fail ktlint and just report the violations command += "{ktlint} $@ >{stdout}; echo $? >" + exit_code.path outputs.append(exit_code) @@ -130,7 +134,7 @@ def _ktlint_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) ruleset_jar = None if hasattr(ctx.attr, "_ruleset_jar"): ruleset_jar = ctx.file._ruleset_jar @@ -138,9 +142,11 @@ def _ktlint_aspect_impl(target, ctx): files_to_lint = filter_srcs(ctx.rule) if len(files_to_lint) == 0: - dummy_successful_lint_action(ctx, report, exit_code) + dummy_successful_lint_action(ctx, output, exit_code) else: - ktlint_action(ctx, ctx.executable._ktlint, files_to_lint, ctx.file._editorconfig, report, ctx.file._baseline_file, ctx.attr._java_runtime, ruleset_jar, exit_code) + ktlint_action(ctx, ctx.executable._ktlint, files_to_lint, ctx.file._editorconfig, output, ctx.file._baseline_file, ctx.attr._java_runtime, ruleset_jar, exit_code, ["--color"]) + if report: + ktlint_action(ctx, ctx.executable._ktlint, files_to_lint, ctx.file._editorconfig, report, ctx.file._baseline_file, ctx.attr._java_runtime, ruleset_jar, exit_code = "discard") return [info] def lint_ktlint_aspect(binary, editorconfig, baseline_file, ruleset_jar = None, rule_kinds = ["kt_jvm_library", "kt_jvm_binary", "kt_js_library"]): diff --git a/lint/pmd.bzl b/lint/pmd.bzl index e4116fa9..fff54482 100644 --- a/lint/pmd.bzl +++ b/lint/pmd.bzl @@ -59,7 +59,9 @@ def pmd_action(ctx, executable, srcs, rulesets, stdout, exit_code = None): src_args.use_param_file("%s", use_always = True) src_args.add_all(srcs) - if exit_code: + if exit_code == "discard": + command = "{PMD} $@ >{stdout} || true" + elif exit_code: command = "{PMD} $@ >{stdout}; echo $? > " + exit_code.path outputs.append(exit_code) else: @@ -83,11 +85,13 @@ def _pmd_aspect_impl(target, ctx): files_to_lint = filter_srcs(ctx.rule) - 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: - pmd_action(ctx, ctx.executable._pmd, files_to_lint, ctx.files._rulesets, report, exit_code) + pmd_action(ctx, ctx.executable._pmd, files_to_lint, ctx.files._rulesets, output, exit_code) + if report: + pmd_action(ctx, ctx.executable._pmd, files_to_lint, ctx.files._rulesets, report, exit_code = "discard") return [info] def lint_pmd_aspect(binary, rulesets, rule_kinds = ["java_binary", "java_library"]): diff --git a/lint/private/lint_aspect.bzl b/lint/private/lint_aspect.bzl index 7e76b158..6cc10f78 100644 --- a/lint/private/lint_aspect.bzl +++ b/lint/private/lint_aspect.bzl @@ -46,10 +46,20 @@ def should_visit(rule, allow_kinds, allow_filegroup_tags = []): _OUTFILE_FORMAT = "{label}.{mnemonic}.{suffix}" -# buildifier: disable=function-docstring def report_files(mnemonic, target, ctx): + """Declare linter output files. + + Args: + mnemonic: used as part of the filename + target: the target being visited by a linter aspect + ctx: the aspect context + + Returns: + 4-tuple of output (human-readable stdout), report (machine-parsable), exit code of the tool, and the OutputGroupInfo provider + """ + output = ctx.actions.declare_file(_OUTFILE_FORMAT.format(label = target.label.name, mnemonic = mnemonic, suffix = "txt")) report = ctx.actions.declare_file(_OUTFILE_FORMAT.format(label = target.label.name, mnemonic = mnemonic, suffix = "report")) - outs = [report] + outs = [output, report] if ctx.attr._options[LintOptionsInfo].fail_on_violation: # Fail on violation means the exit code is reported to Bazel as the action result exit_code = None @@ -59,7 +69,11 @@ def report_files(mnemonic, target, ctx): # and interpreting them. exit_code = ctx.actions.declare_file(_OUTFILE_FORMAT.format(label = target.label.name, mnemonic = mnemonic, suffix = "exit_code")) outs.append(exit_code) - return report, exit_code, OutputGroupInfo(rules_lint_report = depset(outs)) + + return output, report, exit_code, OutputGroupInfo( + rules_lint_output = depset([output]), + rules_lint_report = depset([f for f in [report, exit_code] if f]), + ) def patch_file(mnemonic, target, ctx): patch = ctx.actions.declare_file(_OUTFILE_FORMAT.format(label = target.label.name, mnemonic = mnemonic, suffix = "patch")) @@ -69,8 +83,9 @@ def patch_file(mnemonic, target, ctx): # So we need a separate function to return both. def patch_and_report_files(*args): patch, _ = patch_file(*args) - report, exit_code, _ = report_files(*args) - return patch, report, exit_code, OutputGroupInfo( + output, report, exit_code, _ = report_files(*args) + return patch, output, report, exit_code, OutputGroupInfo( + rules_lint_output = depset([output]), rules_lint_report = depset([f for f in [report, exit_code] if f]), rules_lint_patch = depset([patch]), ) diff --git a/lint/ruff.bzl b/lint/ruff.bzl index 4f613427..d0061b41 100644 --- a/lint/ruff.bzl +++ b/lint/ruff.bzl @@ -55,7 +55,7 @@ load(":ruff_versions.bzl", "RUFF_VERSIONS") _MNEMONIC = "AspectRulesLintRuff" -def ruff_action(ctx, executable, srcs, config, stdout, exit_code = None): +def ruff_action(ctx, executable, srcs, config, stdout, exit_code = None, env = {}): """Run ruff as an action under Bazel. Ruff will select the configuration file to use for each source file, as documented here: @@ -80,6 +80,7 @@ def ruff_action(ctx, executable, srcs, config, stdout, exit_code = None): exit_code: output file to write the exit code. If None, then fail the build when ruff exits non-zero. See https://github.com/astral-sh/ruff/blob/dfe4291c0b7249ae892f5f1d513e6f1404436c13/docs/linter.md#exit-codes + env: environment variables - note that ruff accepts many command-line flags via environment as well """ inputs = srcs + config outputs = [stdout] @@ -90,7 +91,9 @@ def ruff_action(ctx, executable, srcs, config, stdout, exit_code = None): args.add("check") args.add_all(srcs) - if exit_code: + if exit_code == "discard": + command = "{ruff} $@ >{stdout} || true" + elif exit_code: command = "{ruff} $@ >{stdout}; echo $? >" + exit_code.path outputs.append(exit_code) else: @@ -102,6 +105,7 @@ def ruff_action(ctx, executable, srcs, config, stdout, exit_code = None): outputs = outputs, command = command.format(ruff = executable.path, stdout = stdout.path), arguments = [args], + env = env, mnemonic = _MNEMONIC, progress_message = "Linting %{label} with Ruff", tools = [executable], @@ -153,19 +157,23 @@ def _ruff_aspect_impl(target, ctx): return [] files_to_lint = filter_srcs(ctx.rule) - + report = 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) + dummy_successful_lint_action(ctx, output, exit_code, patch) else: - ruff_fix(ctx, ctx.executable, files_to_lint, ctx.files._config_files, patch, report, exit_code) + ruff_fix(ctx, ctx.executable, files_to_lint, ctx.files._config_files, patch, output, 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: - ruff_action(ctx, ctx.executable._ruff, files_to_lint, ctx.files._config_files, report, exit_code) + ruff_action(ctx, ctx.executable._ruff, files_to_lint, ctx.files._config_files, output, exit_code, env = {"FORCE_COLOR": "1"}) + + if report: + ruff_action(ctx, ctx.executable._ruff, files_to_lint, ctx.files._config_files, report, exit_code = "discard") + return [info] def lint_ruff_aspect(binary, configs, rule_kinds = ["py_binary", "py_library", "py_test"]): @@ -213,7 +221,6 @@ def _ruff_workaround_20269_impl(rctx): # download_and_extract has a bug due to the use of Apache Commons library within Bazel, # See https://github.com/bazelbuild/bazel/issues/20269 # To workaround, we fetch the file and then use the BSD tar on the system to extract it. - # TODO: remove for users on Bazel 8 (or maybe sooner if that fix is cherry-picked) rctx.download(sha256 = rctx.attr.sha256, url = rctx.attr.url, output = "ruff.tar.gz") tar_cmd = [rctx.which("tar"), "xzf", "ruff.tar.gz"] if rctx.attr.strip_prefix: diff --git a/lint/shellcheck.bzl b/lint/shellcheck.bzl index 786b07bb..fc55aadf 100644 --- a/lint/shellcheck.bzl +++ b/lint/shellcheck.bzl @@ -44,7 +44,9 @@ def shellcheck_action(ctx, executable, srcs, config, stdout, exit_code = None, o args.add_all(srcs) outputs = [stdout] - if exit_code: + if exit_code == "discard": + command = "{shellcheck} $@ >{stdout} || true" + elif exit_code: command = "{shellcheck} $@ >{stdout}; echo $? >" + exit_code.path outputs.append(exit_code) else: @@ -72,21 +74,25 @@ def _shellcheck_aspect_impl(target, ctx): files_to_lint = filter_srcs(ctx.rule) 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) discard_exit_code = ctx.actions.declare_file(_OUTFILE_FORMAT.format(label = target.label.name, mnemonic = _MNEMONIC, suffix = "patch_exit_code")) if len(files_to_lint) == 0: dummy_successful_lint_action(ctx, patch, discard_exit_code) else: shellcheck_action(ctx, ctx.executable._shellcheck, files_to_lint, ctx.file._config_file, patch, discard_exit_code, ["--format", "diff"]) 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: # shellcheck does not have a --fix mode that applies fixes for some violations while reporting others. # So we must run a second action to populate the human-readable report. - shellcheck_action(ctx, ctx.executable._shellcheck, files_to_lint, ctx.file._config_file, report, exit_code) + shellcheck_action(ctx, ctx.executable._shellcheck, files_to_lint, ctx.file._config_file, output, exit_code, options = ["--color"]) + + if report: + shellcheck_action(ctx, ctx.executable._shellcheck, files_to_lint, ctx.file._config_file, report, exit_code = "discard") + return [info] def lint_shellcheck_aspect(binary, config, rule_kinds = ["sh_binary", "sh_library"]): diff --git a/lint/vale.bzl b/lint/vale.bzl index 358ed156..b275ac68 100644 --- a/lint/vale.bzl +++ b/lint/vale.bzl @@ -70,7 +70,7 @@ load(":vale_versions.bzl", "VALE_VERSIONS") _MNEMONIC = "AspectRulesLintVale" -def vale_action(ctx, executable, srcs, styles, config, stdout, exit_code = None): +def vale_action(ctx, executable, srcs, styles, config, stdout, exit_code = None, format = "CLI"): """Run Vale as an action under Bazel. Args: @@ -82,6 +82,7 @@ def vale_action(ctx, executable, srcs, styles, config, stdout, exit_code = None) stdout: output file containing stdout of Vale exit_code: output file containing Vale exit code. If None, then fail the build when Vale exits non-zero. + format: the value for the --output CLI flag """ inputs = srcs + [config] env = {} @@ -96,10 +97,12 @@ def vale_action(ctx, executable, srcs, styles, config, stdout, exit_code = None) args = ctx.actions.args() args.add_all(srcs) args.add_all(["--config", config]) - args.add_all(["--output", "line"]) + args.add_all(["--output", format]) outputs = [stdout] - if exit_code: + if exit_code == "discard": + command = "{vale} $@ >{stdout} || true" + elif exit_code: command = "{vale} $@ >{stdout}; echo $? > " + exit_code.path outputs.append(exit_code) else: @@ -122,19 +125,22 @@ def vale_action(ctx, executable, srcs, styles, config, stdout, exit_code = None) # buildifier: disable=function-docstring def _vale_aspect_impl(target, ctx): - if should_visit(ctx.rule, ctx.attr._rule_kinds, ctx.attr._filegroup_tags): - report, exit_code, info = report_files(_MNEMONIC, target, ctx) - styles = None - if ctx.files._styles: - if len(ctx.files._styles) != 1: - fail("Only a single directory should be in styles") - styles = ctx.files._styles[0] - if not styles.is_directory: - fail("Styles should be a directory containing installed styles") - vale_action(ctx, ctx.executable._vale, ctx.rule.files.srcs, styles, ctx.file._config, report, exit_code) - return [info] - - return [] + if not should_visit(ctx.rule, ctx.attr._rule_kinds, ctx.attr._filegroup_tags): + return [] + + output, report, exit_code, info = report_files(_MNEMONIC, target, ctx) + styles = None + if ctx.files._styles: + if len(ctx.files._styles) != 1: + fail("Only a single directory should be in styles") + styles = ctx.files._styles[0] + if not styles.is_directory: + fail("Styles should be a directory containing installed styles") + vale_action(ctx, ctx.executable._vale, ctx.rule.files.srcs, styles, ctx.file._config, output, exit_code) + + if report: + vale_action(ctx, ctx.executable._vale, ctx.rule.files.srcs, styles, ctx.file._config, report, exit_code = "discard", format = "line") + return [info] # There's no "official" markdown_library rule. # Users might want to try https://github.com/dwtj/dwtj_rules_markdown but we expect many won't