Skip to content

Commit

Permalink
fix: don't run eslint with zero files to check
Browse files Browse the repository at this point in the history
Fixes #368
  • Loading branch information
alexeagle committed Aug 21, 2024
1 parent a5b32db commit 9b9e781
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 13 deletions.
10 changes: 9 additions & 1 deletion example/src/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
load("@aspect_rules_lint//format:defs.bzl", "format_test")
load("@aspect_rules_swc//swc:defs.bzl", "swc")
load("@aspect_rules_ts//ts:defs.bzl", "ts_config", "ts_project")
load("@io_bazel_rules_go//go:def.bzl", "go_binary")
load("@rules_kotlin//kotlin:jvm.bzl", "kt_jvm_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
load("//tools/lint:linters.bzl", "eslint_test")

package(default_visibility = ["//visibility:public"])

Expand Down Expand Up @@ -35,13 +37,19 @@ ts_project(
name = "ts",
srcs = ["file.ts"],
declaration = True,
transpiler = "tsc",
transpiler = swc,
deps = [
":ts_dep",
"//:node_modules/dayjs",
],
)

# Regression test for https://github.com/aspect-build/rules_lint/issues/368
eslint_test(
name = "eslint_empty_report",
srcs = [":ts"],
)

proto_library(
name = "unused",
srcs = ["unused.proto"],
Expand Down
5 changes: 1 addition & 4 deletions example/src/subdir/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
load("@aspect_rules_js//js:defs.bzl", "js_library")
load("@aspect_rules_swc//swc:defs.bzl", "swc")
load("@aspect_rules_ts//ts:defs.bzl", "ts_project")

exports_files(["ruff.toml"])
Expand All @@ -18,9 +17,7 @@ ts_project(
name = "ts",
srcs = ["test.ts"],
declaration = True,
# verify that even when the macro expands to have js_library targets
# we don't try to lint the output javascript files
transpiler = swc,
transpiler = "tsc",
tsconfig = "//src:tsconfig",
deps = [
"//:node_modules/@types/node",
Expand Down
7 changes: 6 additions & 1 deletion example/test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,12 @@ pmd_test(

eslint_test(
name = "eslint",
srcs = ["//src:ts"],
# NB: we must lint the `ts_typings` target that has the .ts files in srcs,
# not "//src:ts" which is just a js_library re-export.
# For end users it's not obvious that you have to "see inside" the macro to know
# which ts_project output target to lint.
# See https://github.com/aspect-build/rules_lint/issues/369
srcs = ["//src:ts_typings"],
# Expected to fail based on current content of the file.
# Normally you'd fix the file instead of tagging this test.
tags = ["manual"],
Expand Down
6 changes: 5 additions & 1 deletion lint/eslint.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ See the [react example](https://github.com/bazelbuild/examples/blob/b498bb106b20

load("@aspect_bazel_lib//lib:copy_to_bin.bzl", "COPY_FILE_TO_BIN_TOOLCHAINS", "copy_files_to_bin_actions")
load("@aspect_rules_js//js:libs.bzl", "js_lib_helpers")
load("//lint/private:lint_aspect.bzl", "LintOptionsInfo", "filter_srcs", "output_files", "patch_and_output_files", "should_visit")
load("//lint/private:lint_aspect.bzl", "LintOptionsInfo", "filter_srcs", "noop_lint_action", "output_files", "patch_and_output_files", "should_visit")

_MNEMONIC = "AspectRulesLintESLint"

Expand Down Expand Up @@ -214,6 +214,10 @@ def _eslint_aspect_impl(target, ctx):
# 2: Force 256 color support even when a tty isn't detected
color_env = {"FORCE_COLOR": "2"} if ctx.attr._options[LintOptionsInfo].color else {}

if len(files_to_lint) == 0:
noop_lint_action(ctx, outputs)
return [info]

# 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, format = ctx.attr._stylish_formatter, env = color_env)
Expand Down
13 changes: 7 additions & 6 deletions lint/private/lint_aspect.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,9 @@ def filter_srcs(rule):
def noop_lint_action(ctx, outputs):
"""Action that creates expected outputs when no files are provided to a lint action.
This is needed for linters that error when they are given no srcs to inspect.
It is also a performance optimisation in other cases.
This is needed for a linter that doel the wrong thing when given zero srcs to inspect,
for example it might as error or try to lint all files it can find.
It is also a performance optimisation in other cases so we use this everywhere.
Args:
ctx: Bazel Rule or Aspect evaluation context
Expand All @@ -144,12 +145,12 @@ def noop_lint_action(ctx, outputs):

# NB: if we write JSON machine-readable outputs, then an empty file won't be appropriate
if outputs.human.exit_code:
commands.append("echo 0 > {}".format(outputs.human.exit_code.path))
outs += [outputs.human.exit_code]
commands.append("echo 0 > {}".format(outputs.human.exit_code.path))
outs.append(outputs.human.exit_code)

if outputs.machine.exit_code:
commands.append("echo 0 > {}".format(outputs.machine.exit_code.path))
outs += [outputs.machine.exit_code]
commands.append("echo 0 > {}".format(outputs.machine.exit_code.path))
outs.append(outputs.machine.exit_code)

if hasattr(outputs, "patch"):
commands.append("touch {}".format(outputs.patch.path))
Expand Down

0 comments on commit 9b9e781

Please sign in to comment.