Skip to content

Commit

Permalink
fix: workaround eslint issue without resorting to run_shell
Browse files Browse the repository at this point in the history
  • Loading branch information
alexeagle committed Oct 19, 2023
1 parent 97b6dc9 commit 6598c9c
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 4 deletions.
22 changes: 21 additions & 1 deletion example/test/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
"Demonstrates how to enforce zero-lint-tolerance policy with tests"

load("//tools:lint.bzl", "flake8_test", "pmd_test")
load("@aspect_rules_ts//ts:defs.bzl", "ts_project")
load("//tools:lint.bzl", "eslint_test", "flake8_test", "pmd_test")

ts_project(
name = "no_violations",
srcs = ["no_violations.ts"],
tsconfig = {},
)

eslint_test(
name = "eslint_empty_report",
srcs = [":no_violations"],
)

flake8_test(
name = "flake8",
Expand All @@ -17,3 +29,11 @@ pmd_test(
# Normally you'd fix the file instead of tagging this test.
tags = ["manual"],
)

eslint_test(
name = "eslint",
srcs = ["//src:ts"],
# Expected to fail based on current content of the file.
# Normally you'd fix the file instead of tagging this test.
tags = ["manual"],
)
1 change: 1 addition & 0 deletions example/test/no_violations.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const a = "this is fine";
2 changes: 2 additions & 0 deletions example/tools/lint.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ eslint = eslint_aspect(
config = "@@//:eslintrc",
)

eslint_test = make_lint_test(aspect = eslint)

flake8 = flake8_aspect(
binary = "@@//:flake8",
config = "@@//:.flake8",
Expand Down
7 changes: 7 additions & 0 deletions lint/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
load("@aspect_rules_js//js:defs.bzl", "js_library")
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")

exports_files(glob(["*.bzl"]) + ["lint_test.sh"])

js_library(
name = "eslint.workaround_17660",
srcs = ["eslint.workaround_17660.js"],
visibility = ["//visibility:public"],
)

bzl_library(
name = "buf",
srcs = ["buf.bzl"],
Expand Down
15 changes: 12 additions & 3 deletions lint/eslint.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ def eslint_action(ctx, executable, srcs, report, use_exit_code = False):

args = ctx.actions.args()

# Workaround: create an empty report 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 = "=")

# require explicit path to the eslintrc file, don't search for one
args.add("--no-eslintrc")

Expand All @@ -47,7 +51,7 @@ def eslint_action(ctx, executable, srcs, report, use_exit_code = False):

# Add the config file along with any deps it has on npm packages
inputs.extend(js_lib_helpers.gather_files_from_js_providers(
[ctx.attr._config_file],
[ctx.attr._config_file, ctx.attr._workaround_17660],
include_transitive_sources = True,
include_declarations = False,
include_npm_linked_packages = True,
Expand All @@ -56,7 +60,7 @@ def eslint_action(ctx, executable, srcs, report, use_exit_code = False):
outputs = [report]

if not use_exit_code:
exit_code_out = ctx.actions.declare_file("exit_code_out")
exit_code_out = ctx.actions.declare_file("_{}.exit_code_out".format(ctx.label.name))
outputs.append(exit_code_out)
env["JS_BINARY__EXIT_CODE_OUTPUT_FILE"] = exit_code_out.path

Expand All @@ -71,7 +75,7 @@ def eslint_action(ctx, executable, srcs, report, use_exit_code = False):

# buildifier: disable=function-docstring
def _eslint_aspect_impl(target, ctx):
if ctx.rule.kind in ["ts_project_rule"]:
if ctx.rule.kind in ["ts_project", "ts_project_rule"]:
report = ctx.actions.declare_file(target.label.name + ".eslint-report.txt")
eslint_action(ctx, ctx.executable, ctx.rule.files.srcs, report, ctx.attr.fail_on_violation)
results = depset([report])
Expand Down Expand Up @@ -107,5 +111,10 @@ def eslint_aspect(binary, config):
default = config,
allow_single_file = True,
),
"_workaround_17660": attr.label(
default = "@aspect_rules_lint//lint:eslint.workaround_17660",
allow_single_file = True,
cfg = "exec",
),
},
)
9 changes: 9 additions & 0 deletions lint/eslint.workaround_17660.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Workaround for https://github.com/eslint/eslint/issues/17660
// Use as a --require script so that this script is evaluated before the eslint entry point.
// Creates an empty report file just in case eslint doesn't try to write one.
const fs = require("fs");
for (let i = 0; i < process.argv.length; i++) {
if (process.argv[i] == "--output-file") {
fs.closeSync(fs.openSync(process.argv[i + 1], "w"));
}
}

0 comments on commit 6598c9c

Please sign in to comment.