Add setting clippy_output_diagnostics to match rustc_output_[...] (#3528)

Right now, capturing clippy's diagnostics is in a weird place.
`capture_clippy_output` exists, but it has drastically different
behavior from `rustc_output_diagnostics` in that it captures the
rendered error output and hides it from the terminal, whereas
`rustc_output_diagnostics` captures the raw JSON and still prints out
the rendered diagnostics.

The loss of the JSON is a bit of an issue. With that, the only way to
get JSON output from the command-line is to set `clippy_error_format`,
which will necessarily *clear the analysis cache* and rebuild all the
targets, meaning that you can't get both rendered output *and* JSON
output without either rebuilds in-between or relying on custom
transitions.

This adds the missing equivalent to `rustc_output_diagnostics` in the
form of a new option, `clippy_output_diagnostics`. In an ideal world,
I'd imagine this would be the *only* option to capture clippy output
(since for most intents and purposes, it's strictly a superset of
`capture_clippy_output`), but using a separate option and output group
preserves backwards compatibility while retaining the parallel naming of
`rustc_output_diagnostics` vs `clippy_output_diagnostics`, as well as
the output groups `rustc_output` vs `clippy_output`.

Author: refi64

rust/private/BUILD.bazel

@@ -28,6 +28,7 @@ bzl_library(
         ":rules_cc_bzl_lib",
         "//rust/platform:bzl_lib",
         "@bazel_skylib//lib:paths",
+        "@bazel_skylib//lib:structs",
         "@bazel_skylib//rules:common_settings",
     ],
 )

rust/private/clippy.bzl

@@ -14,8 +14,15 @@
 
 """A module defining clippy rules"""
 
+load("@bazel_skylib//lib:structs.bzl", "structs")
 load("//rust/private:common.bzl", "rust_common")
-load("//rust/private:providers.bzl", "CaptureClippyOutputInfo", "ClippyInfo", "LintsInfo")
+load(
+    "//rust/private:providers.bzl",
+    "CaptureClippyOutputInfo",
+    "ClippyInfo",
+    "ClippyOutputDiagnosticsInfo",
+    "LintsInfo",
+)
 load(
     "//rust/private:rustc.bzl",
     "collect_deps",
@@ -146,6 +153,13 @@ def _clippy_aspect_impl(target, ctx):
         "_clippy_error_format" if use_clippy_error_format else "_error_format",
     )
 
+    clippy_diagnostics = None
+    if ctx.attr._clippy_output_diagnostics[ClippyOutputDiagnosticsInfo].output_diagnostics:
+        clippy_diagnostics = ctx.actions.declare_file(ctx.label.name + ".clippy.diagnostics", sibling = crate_info.output)
+        crate_info_dict = structs.to_dict(crate_info)
+        crate_info_dict["rustc_output"] = clippy_diagnostics
+        crate_info = rust_common.create_crate_info(**crate_info_dict)
+
     args, env = construct_arguments(
         ctx = ctx,
         attr = ctx.rule.attr,
@@ -165,6 +179,7 @@ def _clippy_aspect_impl(target, ctx):
         build_flags_files = build_flags_files,
         emit = ["dep-info", "metadata"],
         skip_expanding_rustc_env = True,
+        use_json_output = bool(clippy_diagnostics),
         error_format = error_format,
     )
 
@@ -217,7 +232,7 @@ def _clippy_aspect_impl(target, ctx):
     ctx.actions.run(
         executable = ctx.executable._process_wrapper,
         inputs = compile_inputs,
-        outputs = [clippy_out],
+        outputs = [clippy_out] + [x for x in [clippy_diagnostics] if x],
         env = env,
         tools = [toolchain.clippy_driver],
         arguments = args.all,
@@ -226,8 +241,12 @@ def _clippy_aspect_impl(target, ctx):
         toolchain = "@rules_rust//rust:toolchain_type",
     )
 
+    output_group_info = {"clippy_checks": depset([clippy_out])}
+    if clippy_diagnostics:
+        output_group_info["clippy_output"] = depset([clippy_diagnostics])
+
     return [
-        OutputGroupInfo(clippy_checks = depset([clippy_out])),
+        OutputGroupInfo(**output_group_info),
         ClippyInfo(output = depset([clippy_out])),
     ]
 
@@ -255,6 +274,10 @@ rust_clippy_aspect = aspect(
             doc = "Arguments to pass to clippy",
             default = Label("//rust/settings:clippy_flags"),
         ),
+        "_clippy_output_diagnostics": attr.label(
+            doc = "Value of the `clippy_output_diagnostics` build setting",
+            default = "//rust/settings:clippy_output_diagnostics",
+        ),
         "_config": attr.label(
             doc = "The `clippy.toml` file used for configuration",
             allow_single_file = True,
@@ -395,3 +418,25 @@ capture_clippy_output = rule(
     implementation = _capture_clippy_output_impl,
     build_setting = config.bool(flag = True),
 )
+
+def _clippy_output_diagnostics_impl(ctx):
+    """Implementation of the `clippy_output_diagnostics` rule
+
+    Args:
+        ctx (ctx): The rule's context object
+
+    Returns:
+        list: A list containing the CaptureClippyOutputInfo provider
+    """
+    return [ClippyOutputDiagnosticsInfo(output_diagnostics = ctx.build_setting_value)]
+
+clippy_output_diagnostics = rule(
+    doc = (
+        "Setting this flag from the command line with `--@rules_rust//rust/settings:clippy_output_diagnostics` " +
+        "makes rules_rust save lippy json output (suitable for consumption by rust-analyzer) in a file, " +
+        "available from the `clippy_output` output group. This is the clippy equivalent of " +
+        "`@rules_rust//settings:rustc_output_diagnostics`."
+    ),
+    implementation = _clippy_output_diagnostics_impl,
+    build_setting = config.bool(flag = True),
+)

rust/private/providers.bzl

@@ -148,6 +148,11 @@ CaptureClippyOutputInfo = provider(
     fields = {"capture_output": "Value of the `capture_clippy_output` build setting"},
 )
 
+ClippyOutputDiagnosticsInfo = provider(
+    doc = "Value of the `clippy_output_diagnostics` build setting",
+    fields = {"output_diagnostics": "Value of the `clippy_output_diagnostics` build setting"},
+)
+
 ClippyInfo = provider(
     doc = "Provides information on a clippy run.",
     fields = {

rust/settings/BUILD.bazel

@@ -6,6 +6,7 @@ load(
     "clippy_error_format",
     "clippy_flag",
     "clippy_flags",
+    "clippy_output_diagnostics",
     "clippy_toml",
     "codegen_units",
     "error_format",
@@ -63,6 +64,8 @@ clippy_flag()
 
 clippy_flags()
 
+clippy_output_diagnostics()
+
 clippy_toml()
 
 codegen_units()

rust/settings/settings.bzl

@@ -14,6 +14,7 @@ load(
     _capture_clippy_output = "capture_clippy_output",
     _clippy_flag = "clippy_flag",
     _clippy_flags = "clippy_flags",
+    _clippy_output_diagnostics = "clippy_output_diagnostics",
 )
 load("//rust/private:lto.bzl", "rust_lto_flag")
 load(
@@ -346,6 +347,20 @@ def rustc_output_diagnostics():
         visibility = ["//visibility:public"],
     )
 
+# buildifier: disable=unnamed-macro
+def clippy_output_diagnostics():
+    """A flag to enable the `clippy_output_diagnostics` setting.
+
+    If this flag is true, rules_rust will save clippy json output (suitable for consumption
+    by rust-analyzer) in a file, available from the `clippy_output` output group. This is the
+    clippy equivalent of `rustc_output_diagnostics`.
+    """
+    _clippy_output_diagnostics(
+        name = "clippy_output_diagnostics",
+        build_setting_default = False,
+        visibility = ["//visibility:public"],
+    )
+
 # buildifier: disable=unnamed-macro
 def clippy_flags():
     """This setting may be used to pass extra options to clippy from the command line.

test/unit/clippy/clippy_test.bzl

@@ -2,15 +2,15 @@
 
 load("@bazel_skylib//lib:unittest.bzl", "analysistest")
 load("//rust:defs.bzl", "rust_clippy_aspect")
-load("//test/unit:common.bzl", "assert_argv_contains", "assert_list_contains_adjacent_elements")
+load("//test/unit:common.bzl", "assert_argv_contains", "assert_argv_contains_prefix_suffix", "assert_list_contains_adjacent_elements")
 
 def _find_clippy_action(actions):
     for action in actions:
         if action.mnemonic == "Clippy":
             return action
     fail("Failed to find Clippy action")
 
-def _clippy_aspect_action_has_flag_impl(ctx, flags):
+def _clippy_aspect_action_has_flag_impl(ctx, flags, *, prefix_suffix_flags = []):
     env = analysistest.begin(ctx)
     target = analysistest.target_under_test(env)
 
@@ -23,6 +23,8 @@ def _clippy_aspect_action_has_flag_impl(ctx, flags):
             clippy_action,
             flag,
         )
+    for (prefix, suffix) in prefix_suffix_flags:
+        assert_argv_contains_prefix_suffix(env, clippy_action, prefix, suffix)
 
     clippy_checks = target[OutputGroupInfo].clippy_checks.to_list()
     if len(clippy_checks) != 1:
@@ -123,6 +125,17 @@ clippy_aspect_with_clippy_error_format_test = make_clippy_aspect_unittest(
     },
 )
 
+clippy_aspect_with_output_diagnostics_test = make_clippy_aspect_unittest(
+    lambda ctx: _clippy_aspect_action_has_flag_impl(
+        ctx,
+        ["--error-format=json", "--output-file"],
+        prefix_suffix_flags = [("", "/ok_library.clippy.diagnostics")],
+    ),
+    config_settings = {
+        str(Label("//rust/settings:clippy_output_diagnostics")): True,
+    },
+)
+
 def clippy_test_suite(name):
     """Entry-point macro called from the BUILD file.
 
@@ -165,6 +178,11 @@ def clippy_test_suite(name):
         target_under_test = Label("//test/clippy:ok_library"),
     )
 
+    clippy_aspect_with_output_diagnostics_test(
+        name = "clippy_aspect_with_output_diagnostics_test",
+        target_under_test = Label("//test/clippy:ok_library"),
+    )
+
     native.test_suite(
         name = name,
         tests = [
@@ -176,5 +194,6 @@ def clippy_test_suite(name):
             ":test_clippy_aspect_with_explicit_flags_test",
             ":clippy_aspect_without_clippy_error_format_test",
             ":clippy_aspect_with_clippy_error_format_test",
+            ":clippy_aspect_with_output_diagnostics_test",
         ],
     )