From cab298c579e33c72829d5e321612306f34746c77 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Wed, 21 Aug 2024 08:42:25 -0400 Subject: [PATCH] fix: don't run eslint with zero files to check (#370) * chore: introduce a ts_project transpiler macro maybe reproduces a bug * fix: don't run eslint with zero files to check Fixes #368 * chore: also add swc to WORKSPACE --- example/MODULE.bazel | 5 +++-- example/WORKSPACE.bazel | 26 ++++++++++++++++++++------ example/src/BUILD.bazel | 10 +++++++++- example/src/tsconfig.json | 3 ++- example/test/BUILD.bazel | 7 ++++++- lint/eslint.bzl | 6 +++++- lint/private/lint_aspect.bzl | 13 +++++++------ 7 files changed, 52 insertions(+), 18 deletions(-) diff --git a/example/MODULE.bazel b/example/MODULE.bazel index 794f03e5..2fbeca6c 100644 --- a/example/MODULE.bazel +++ b/example/MODULE.bazel @@ -2,8 +2,9 @@ bazel_dep(name = "aspect_rules_lint", version = "0.0.0") bazel_dep(name = "aspect_bazel_lib", version = "2.7.7") -bazel_dep(name = "aspect_rules_js", version = "2.0.0-rc0") -bazel_dep(name = "aspect_rules_ts", version = "3.0.0-rc0") +bazel_dep(name = "aspect_rules_js", version = "2.0.0") +bazel_dep(name = "aspect_rules_swc", version = "2.0.0") +bazel_dep(name = "aspect_rules_ts", version = "3.0.0") bazel_dep(name = "rules_buf", version = "0.3.0") bazel_dep(name = "bazel_skylib", version = "1.4.2") bazel_dep(name = "toolchains_llvm", version = "0.10.3") diff --git a/example/WORKSPACE.bazel b/example/WORKSPACE.bazel index 4bcf4a86..9ab357f7 100644 --- a/example/WORKSPACE.bazel +++ b/example/WORKSPACE.bazel @@ -45,9 +45,9 @@ http_archive( http_archive( name = "aspect_rules_js", - sha256 = "389021e29b3aeed2f6fb3a7a1478f8fc52947a6500b198a7ec0f3358c2842415", - strip_prefix = "rules_js-2.0.0-rc0", - url = "https://github.com/aspect-build/rules_js/releases/download/v2.0.0-rc0/rules_js-v2.0.0-rc0.tar.gz", + sha256 = "6b7e73c35b97615a09281090da3645d9f03b2a09e8caa791377ad9022c88e2e6", + strip_prefix = "rules_js-2.0.0", + url = "https://github.com/aspect-build/rules_js/releases/download/v2.0.0/rules_js-v2.0.0.tar.gz", ) http_archive( @@ -98,9 +98,9 @@ npm_repositories() http_archive( name = "aspect_rules_ts", - sha256 = "3ea5cdb825d5dbffe286b3d9c5197a2648cf04b5e6bd8b913a45823cdf0ae960", - strip_prefix = "rules_ts-3.0.0-rc0", - url = "https://github.com/aspect-build/rules_ts/releases/download/v3.0.0-rc0/rules_ts-v3.0.0-rc0.tar.gz", + sha256 = "ee7dcc35faef98f3050df9cf26f2a72ef356cab8ad927efb1c4dc119ac082a19", + strip_prefix = "rules_ts-3.0.0", + url = "https://github.com/aspect-build/rules_ts/releases/download/v3.0.0/rules_ts-v3.0.0.tar.gz", ) load("@aspect_rules_ts//ts:repositories.bzl", "rules_ts_dependencies") @@ -109,6 +109,20 @@ rules_ts_dependencies( ts_version_from = "//:package.json", ) +http_archive( + name = "aspect_rules_swc", + sha256 = "d63d7b283249fa942f78d2716ecff3edbdc10104ee1b9a6b9464ece471ef95ea", + strip_prefix = "rules_swc-2.0.0", + url = "https://github.com/aspect-build/rules_swc/releases/download/v2.0.0/rules_swc-v2.0.0.tar.gz", +) + +load("@aspect_rules_swc//swc:repositories.bzl", "LATEST_SWC_VERSION", "swc_register_toolchains") + +swc_register_toolchains( + name = "swc", + swc_version = LATEST_SWC_VERSION, +) + http_archive( name = "buildifier_prebuilt", sha256 = "72b5bb0853aac597cce6482ee6c62513318e7f2c0050bc7c319d75d03d8a3875", diff --git a/example/src/BUILD.bazel b/example/src/BUILD.bazel index c95c5a61..4cbcdae9 100644 --- a/example/src/BUILD.bazel +++ b/example/src/BUILD.bazel @@ -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"]) @@ -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"], diff --git a/example/src/tsconfig.json b/example/src/tsconfig.json index f7393fc9..fa30564f 100644 --- a/example/src/tsconfig.json +++ b/example/src/tsconfig.json @@ -3,5 +3,6 @@ "declaration": true, "esModuleInterop": true, "strictNullChecks": true - } + }, + "exclude": [] } diff --git a/example/test/BUILD.bazel b/example/test/BUILD.bazel index aea37d98..60d2c3e6 100644 --- a/example/test/BUILD.bazel +++ b/example/test/BUILD.bazel @@ -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"], diff --git a/lint/eslint.bzl b/lint/eslint.bzl index 62868989..8819dff9 100644 --- a/lint/eslint.bzl +++ b/lint/eslint.bzl @@ -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" @@ -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) diff --git a/lint/private/lint_aspect.bzl b/lint/private/lint_aspect.bzl index 499dbf7b..cf66fd75 100644 --- a/lint/private/lint_aspect.bzl +++ b/lint/private/lint_aspect.bzl @@ -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 does the wrong thing when given zero srcs to inspect, + for example it might error, or try to lint all files it can find. + It is also a performance optimisation in other cases; use this in every linter implementation. Args: ctx: Bazel Rule or Aspect evaluation context @@ -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))