Skip to content

Commit

Permalink
fix(eslint): print relative paths in stylish output (#337)
Browse files Browse the repository at this point in the history
* fix(eslint): print relative paths in stylish output

* refactor: safer logic pointed out in code review
  • Loading branch information
alexeagle authored Jul 18, 2024
1 parent d0bd08a commit 0e9dc31
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 9 deletions.
10 changes: 8 additions & 2 deletions lint/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
)

Expand Down
29 changes: 22 additions & 7 deletions lint/eslint.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -162,19 +162,29 @@ 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,
}),
)

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],
Expand Down Expand Up @@ -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]

Expand Down Expand Up @@ -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",
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
Expand Down
144 changes: 144 additions & 0 deletions lint/eslint.stylish-formatter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
// 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 idx = eslintEntry.lastIndexOf("node_modules");
if (idx < 0) {
throw new Error(
"node_modules not found in eslint entry point " + eslintEntry
);
}
const searchPath = eslintEntry.substring(0, idx);
// 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) : "";
};

0 comments on commit 0e9dc31

Please sign in to comment.