Add clippy_error_format setting. (#3496)

This allows the error format for Clippy to be set independently of that
for rustc.

As this is a backwards-incompatible change, it is guarded by a feature
flag `incompatible_change_clippy_error_format`.

For the motivation for this change, see #3494.

Author: martinboehme

BUILD.bazel

@@ -13,6 +13,12 @@ alias(
     visibility = ["//visibility:public"],
 )
 
+alias(
+    name = "clippy_error_format",
+    actual = "//rust/settings:clippy_error_format",
+    visibility = ["//visibility:public"],
+)
+
 # This setting may be changed from the command line to generate rustc diagnostics.
 alias(
     name = "rustc_output_diagnostics",

rust/private/clippy.bzl

@@ -21,13 +21,15 @@ load(
     "collect_deps",
     "collect_inputs",
     "construct_arguments",
+    "get_error_format",
 )
 load(
     "//rust/private:utils.bzl",
     "determine_output_hash",
     "find_cc_toolchain",
     "find_toolchain",
 )
+load("//rust/settings:incompatible.bzl", "IncompatibleFlagInfo")
 
 ClippyFlagsInfo = provider(
     doc = "Pass each value as an additional flag to clippy invocations",
@@ -138,6 +140,12 @@ def _clippy_aspect_impl(target, ctx):
         lint_files,
     )
 
+    use_clippy_error_format = ctx.attr._incompatible_change_clippy_error_format[IncompatibleFlagInfo].enabled
+    error_format = get_error_format(
+        ctx.attr,
+        "_clippy_error_format" if use_clippy_error_format else "_error_format",
+    )
+
     args, env = construct_arguments(
         ctx = ctx,
         attr = ctx.rule.attr,
@@ -157,6 +165,7 @@ def _clippy_aspect_impl(target, ctx):
         build_flags_files = build_flags_files,
         emit = ["dep-info", "metadata"],
         skip_expanding_rustc_env = True,
+        error_format = error_format,
     )
 
     if crate_info.is_test:
@@ -233,6 +242,10 @@ rust_clippy_aspect = aspect(
             doc = "Value of the `capture_clippy_output` build setting",
             default = Label("//rust/settings:capture_clippy_output"),
         ),
+        "_clippy_error_format": attr.label(
+            doc = "The desired `--error-format` flags for clippy",
+            default = "//rust/settings:clippy_error_format",
+        ),
         "_clippy_flag": attr.label(
             doc = "Arguments to pass to clippy." +
                   "Multiple uses are accumulated and appended after the extra_rustc_flags.",
@@ -248,12 +261,16 @@ rust_clippy_aspect = aspect(
             default = Label("//rust/settings:clippy.toml"),
         ),
         "_error_format": attr.label(
-            doc = "The desired `--error-format` flags for clippy",
+            doc = "The desired `--error-format` flags for rustc",
             default = "//rust/settings:error_format",
         ),
         "_extra_rustc_flag": attr.label(
             default = Label("//rust/settings:extra_rustc_flag"),
         ),
+        "_incompatible_change_clippy_error_format": attr.label(
+            doc = "Whether to use the _clippy_error_format attribute",
+            default = "//rust/settings:incompatible_change_clippy_error_format",
+        ),
         "_per_crate_rustc_flag": attr.label(
             default = Label("//rust/settings:experimental_per_crate_rustc_flag"),
         ),

rust/private/rustc.bzl

@@ -833,7 +833,8 @@ def construct_arguments(
         use_json_output = False,
         build_metadata = False,
         force_depend_on_objects = False,
-        skip_expanding_rustc_env = False):
+        skip_expanding_rustc_env = False,
+        error_format = None):
     """Builds an Args object containing common rustc flags
 
     Args:
@@ -865,6 +866,7 @@ def construct_arguments(
         build_metadata (bool): Generate CLI arguments for building *only* .rmeta files. This requires use_json_output.
         force_depend_on_objects (bool): Force using `.rlib` object files instead of metadata (`.rmeta`) files even if they are available.
         skip_expanding_rustc_env (bool): Whether to skip expanding CrateInfo.rustc_env_attr
+        error_format (str, optional): Error format to pass to the `--error-format` command line argument. If set to None, uses the "_error_format" entry in `attr`.
 
     Returns:
         tuple: A tuple of the following items
@@ -938,9 +940,8 @@ def construct_arguments(
     rustc_flags.add(crate_info.name, format = "--crate-name=%s")
     rustc_flags.add(crate_info.type, format = "--crate-type=%s")
 
-    error_format = "human"
-    if hasattr(attr, "_error_format"):
-        error_format = attr._error_format[ErrorFormatInfo].error_format
+    if error_format == None:
+        error_format = get_error_format(attr, "_error_format")
 
     if use_json_output:
         # If --error-format was set to json, we just pass the output through
@@ -2254,6 +2255,11 @@ error_format = rule(
     build_setting = config.string(flag = True),
 )
 
+def get_error_format(attr, attr_name):
+    if hasattr(attr, attr_name):
+        return getattr(attr, attr_name)[ErrorFormatInfo].error_format
+    return "human"
+
 def _rustc_output_diagnostics_impl(ctx):
     """Implementation of the `rustc_output_diagnostics` rule
 

rust/settings/BUILD.bazel

@@ -2,6 +2,7 @@ load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
 load(
     ":settings.bzl",
     "capture_clippy_output",
+    "clippy_error_format",
     "clippy_flag",
     "clippy_flags",
     "clippy_toml",
@@ -17,6 +18,7 @@ load(
     "extra_exec_rustc_flags",
     "extra_rustc_flag",
     "extra_rustc_flags",
+    "incompatible_change_clippy_error_format",
     "incompatible_change_rust_test_compilation_output_directory",
     "incompatible_do_not_include_data_in_compile_data",
     "lto",
@@ -61,6 +63,8 @@ codegen_units()
 
 error_format()
 
+clippy_error_format()
+
 experimental_link_std_dylib()
 
 experimental_per_crate_rustc_flag()
@@ -81,6 +85,8 @@ extra_rustc_flag()
 
 extra_rustc_flags()
 
+incompatible_change_clippy_error_format()
+
 incompatible_change_rust_test_compilation_output_directory()
 
 incompatible_do_not_include_data_in_compile_data()

rust/settings/settings.bzl

@@ -237,6 +237,28 @@ def error_format():
         build_setting_default = "human",
     )
 
+# buildifier: disable=unnamed-macro
+def clippy_error_format():
+    """This setting may be changed from the command line to generate machine readable errors.
+    """
+    _error_format(
+        name = "clippy_error_format",
+        build_setting_default = "human",
+    )
+
+# buildifier: disable=unnamed-macro
+def incompatible_change_clippy_error_format():
+    """A flag to enable the `clippy_error_format` setting.
+
+    If this flag is true, Clippy uses the format set in `clippy_error_format` to
+    format its diagnostics; otherwise, it uses the format set in `error_format`.
+    """
+    incompatible_flag(
+        name = "incompatible_change_clippy_error_format",
+        build_setting_default = False,
+        issue = "https://github.com/bazelbuild/rules_rust/issues/3494",
+    )
+
 # buildifier: disable=unnamed-macro
 def rustc_output_diagnostics():
     """This setting may be changed from the command line to generate rustc diagnostics.

test/unit/clippy/clippy_test.bzl

@@ -99,6 +99,30 @@ clippy_aspect_with_explicit_flags_test = make_clippy_aspect_unittest(
     },
 )
 
+clippy_aspect_without_clippy_error_format_test = make_clippy_aspect_unittest(
+    lambda ctx: _clippy_aspect_action_has_flag_impl(
+        ctx,
+        ["--error-format=short"],
+    ),
+    config_settings = {
+        str(Label("//rust/settings:error_format")): "short",
+        str(Label("//rust/settings:clippy_error_format")): "json",
+        str(Label("//rust/settings:incompatible_change_clippy_error_format")): False,
+    },
+)
+
+clippy_aspect_with_clippy_error_format_test = make_clippy_aspect_unittest(
+    lambda ctx: _clippy_aspect_action_has_flag_impl(
+        ctx,
+        ["--error-format=json"],
+    ),
+    config_settings = {
+        str(Label("//rust/settings:error_format")): "short",
+        str(Label("//rust/settings:clippy_error_format")): "json",
+        str(Label("//rust/settings:incompatible_change_clippy_error_format")): True,
+    },
+)
+
 def clippy_test_suite(name):
     """Entry-point macro called from the BUILD file.
 
@@ -118,6 +142,7 @@ def clippy_test_suite(name):
         name = "test_clippy_aspect_action_has_warnings_flag_test",
         target_under_test = Label("//test/clippy:ok_test"),
     )
+
     clippy_aspect_with_explicit_flags_test(
         name = "binary_clippy_aspect_with_explicit_flags_test",
         target_under_test = Label("//test/clippy:ok_binary"),
@@ -131,6 +156,15 @@ def clippy_test_suite(name):
         target_under_test = Label("//test/clippy:ok_test"),
     )
 
+    clippy_aspect_without_clippy_error_format_test(
+        name = "clippy_aspect_without_clippy_error_format_test",
+        target_under_test = Label("//test/clippy:ok_library"),
+    )
+    clippy_aspect_with_clippy_error_format_test(
+        name = "clippy_aspect_with_clippy_error_format_test",
+        target_under_test = Label("//test/clippy:ok_library"),
+    )
+
     native.test_suite(
         name = name,
         tests = [
@@ -140,5 +174,7 @@ def clippy_test_suite(name):
             ":binary_clippy_aspect_with_explicit_flags_test",
             ":library_clippy_aspect_with_explicit_flags_test",
             ":test_clippy_aspect_with_explicit_flags_test",
+            ":clippy_aspect_without_clippy_error_format_test",
+            ":clippy_aspect_with_clippy_error_format_test",
         ],
     )