From a398bbbb3aedf008a94525e26dc3af0f04f5ebc2 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Wed, 17 Jul 2024 16:52:32 -0700 Subject: [PATCH] fix(eslint): print relative paths in stylish output --- lint/BUILD.bazel | 10 +- lint/eslint.bzl | 29 +++- ...rmatter.js => eslint.compact-formatter.js} | 1 + lint/eslint.stylish-formatter.js | 141 ++++++++++++++++++ 4 files changed, 172 insertions(+), 9 deletions(-) rename lint/{eslint.bazel-formatter.js => eslint.compact-formatter.js} (94%) create mode 100644 lint/eslint.stylish-formatter.js diff --git a/lint/BUILD.bazel b/lint/BUILD.bazel index 6b616f44..74b08b74 100644 --- a/lint/BUILD.bazel +++ b/lint/BUILD.bazel @@ -108,8 +108,14 @@ 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"], +) + +js_library( + name = "eslint.stylish-formatter", + srcs = ["eslint.stylish-formatter.js"], visibility = ["//visibility:public"], ) diff --git a/lint/eslint.bzl b/lint/eslint.bzl index d1b78caf..5b960f6c 100644 --- a/lint/eslint.bzl +++ b/lint/eslint.bzl @@ -162,11 +162,21 @@ def eslint_fix(ctx, executable, srcs, patch, stdout, exit_code, format = "stylis """ patch_cfg = ctx.actions.declare_file("_{}.patch_cfg".format(ctx.label.name)) + file_inputs = [ctx.attr._workaround_17660] + args = ["--fix"] + + if type(format) == "string": + args.extend(["--format", format]) + else: + args.extend(["--format", "../../../" + format.files.to_list()[0].path]) + file_inputs.append(format) + args.extend([s.short_path for s in srcs]) + ctx.actions.write( output = patch_cfg, content = json.encode({ "linter": executable._eslint.path, - "args": ["--fix", "--format", format] + [s.short_path for s in srcs], + "args": args, "env": dict(env, **{"BAZEL_BINDIR": ctx.bin_dir.path}), "files_to_diff": [s.path for s in srcs], "output": patch.path, @@ -174,7 +184,7 @@ def eslint_fix(ctx, executable, srcs, patch, stdout, exit_code, format = "stylis ) ctx.actions.run( - inputs = _gather_inputs(ctx, srcs, [ctx.attr._workaround_17660]) + [patch_cfg], + inputs = _gather_inputs(ctx, srcs, file_inputs) + [patch_cfg], outputs = [patch, stdout, exit_code], executable = executable._patcher, arguments = [patch_cfg.path], @@ -206,12 +216,12 @@ def _eslint_aspect_impl(target, ctx): # eslint can produce a patch file at the same time it reports the unpatched violations if hasattr(outputs, "patch"): - eslint_fix(ctx, ctx.executable, files_to_lint, outputs.patch, outputs.human.out, outputs.human.exit_code, env = color_env) + eslint_fix(ctx, ctx.executable, files_to_lint, outputs.patch, outputs.human.out, outputs.human.exit_code, format = ctx.attr._stylish_formatter, env = color_env) else: - eslint_action(ctx, ctx.executable, files_to_lint, outputs.human.out, outputs.human.exit_code, env = color_env) + eslint_action(ctx, ctx.executable, files_to_lint, outputs.human.out, outputs.human.exit_code, format = ctx.attr._stylish_formatter, env = color_env) # TODO(alex): if we run with --fix, this will report the issues that were fixed. Does a machine reader want to know about them? - eslint_action(ctx, ctx.executable, files_to_lint, outputs.machine.out, outputs.machine.exit_code, format = ctx.attr._formatter) + eslint_action(ctx, ctx.executable, files_to_lint, outputs.machine.out, outputs.machine.exit_code, format = ctx.attr._compact_formatter) return [info] @@ -258,8 +268,13 @@ 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", + ), + "_stylish_formatter": attr.label( + default = "@aspect_rules_lint//lint:eslint.stylish-formatter", allow_single_file = True, cfg = "exec", ), diff --git a/lint/eslint.bazel-formatter.js b/lint/eslint.compact-formatter.js similarity index 94% rename from lint/eslint.bazel-formatter.js rename to lint/eslint.compact-formatter.js index d4ec9da3..f3e6f30f 100644 --- a/lint/eslint.bazel-formatter.js +++ b/lint/eslint.compact-formatter.js @@ -27,6 +27,7 @@ module.exports = function (results, context) { total += messages.length; messages.forEach((message) => { + // LOCAL MODIFICATION: print path relative to the working directory output += `${path.relative(context.cwd, result.filePath)}: `; output += `line ${message.line || 0}`; output += `, col ${message.column || 0}`; diff --git a/lint/eslint.stylish-formatter.js b/lint/eslint.stylish-formatter.js new file mode 100644 index 00000000..cfdd4d37 --- /dev/null +++ b/lint/eslint.stylish-formatter.js @@ -0,0 +1,141 @@ +// Fork of 'stylish' 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/stylish.js +/** + * @fileoverview Stylish reporter + * @author Sindre Sorhus + */ +"use strict"; + +/** + * LOCAL MODIFICATION: + * The three eslint dependencies should be loaded from the user's node_modules tree, not from rules_lint. + */ + +// This script is used as a command-line flag to eslint, so the command line is "node eslint.js --format this_script.js" +// That means we can grab the path of the eslint entry point, which is beneath its node modules tree. +const eslintEntry = process.argv[1]; +// Walk up the tree to the location where eslint normally roots the searchPath of its require() calls +const searchPath = eslintEntry.substring( + 0, + eslintEntry.length - "/node_modules/eslint/bin/eslint.js".length +); +// Modify the upstream code to pass through an explicit `require.resolve` that starts from eslint +const chalk = require(require.resolve("chalk", { paths: [searchPath] })), + stripAnsi = require(require.resolve("strip-ansi", { paths: [searchPath] })), + table = require(require.resolve("text-table", { paths: [searchPath] })); + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +/** + * Given a word and a count, append an s if count is not one. + * @param {string} word A word in its singular form. + * @param {int} count A number controlling whether word should be pluralized. + * @returns {string} The original word with an s on the end if count is not one. + */ +function pluralize(word, count) { + return count === 1 ? word : `${word}s`; +} + +//------------------------------------------------------------------------------ +// Public Interface +//------------------------------------------------------------------------------ + +module.exports = function (results, context) { + let output = "\n", + errorCount = 0, + warningCount = 0, + fixableErrorCount = 0, + fixableWarningCount = 0, + summaryColor = "yellow"; + + results.forEach((result) => { + const messages = result.messages; + + if (messages.length === 0) { + return; + } + + errorCount += result.errorCount; + warningCount += result.warningCount; + fixableErrorCount += result.fixableErrorCount; + fixableWarningCount += result.fixableWarningCount; + + // LOCAL MODIFICATION: print path relative to the working directory + output += `${chalk.underline( + require("node:path").relative(context.cwd, result.filePath) + )}\n`; + + output += `${table( + messages.map((message) => { + let messageType; + + if (message.fatal || message.severity === 2) { + messageType = chalk.red("error"); + summaryColor = "red"; + } else { + messageType = chalk.yellow("warning"); + } + + return [ + "", + message.line || 0, + message.column || 0, + messageType, + message.message.replace(/([^ ])\.$/u, "$1"), + chalk.dim(message.ruleId || ""), + ]; + }), + { + align: ["", "r", "l"], + stringLength(str) { + return stripAnsi(str).length; + }, + } + ) + .split("\n") + .map((el) => + el.replace(/(\d+)\s+(\d+)/u, (m, p1, p2) => chalk.dim(`${p1}:${p2}`)) + ) + .join("\n")}\n\n`; + }); + + const total = errorCount + warningCount; + + if (total > 0) { + output += chalk[summaryColor].bold( + [ + "\u2716 ", + total, + pluralize(" problem", total), + " (", + errorCount, + pluralize(" error", errorCount), + ", ", + warningCount, + pluralize(" warning", warningCount), + ")\n", + ].join("") + ); + + if (fixableErrorCount > 0 || fixableWarningCount > 0) { + output += chalk[summaryColor].bold( + [ + " ", + fixableErrorCount, + pluralize(" error", fixableErrorCount), + " and ", + fixableWarningCount, + pluralize(" warning", fixableWarningCount), + " potentially fixable with the `--fix` option.\n", + ].join("") + ); + } + } + + // Resets output color, for prevent change on top level + return total > 0 ? chalk.reset(output) : ""; +};