Eliminate Rustfmt action in Bindgen rules. Bindgen can run rustfmt (#2025)

* Eliminate Rustfmt action in Bindgen rules. Bindgen can run rustfmt

* Remove conflicting attributes.

* Regenerate documentation

Author: UebelAndre

bindgen/bindgen.bzl

@@ -41,7 +41,6 @@ def rust_bindgen_library(
         cc_lib,
         bindgen_flags = None,
         clang_flags = None,
-        rustfmt = True,
         **kwargs):
     """Generates a rust source file for `header`, and builds a rust_library.
 
@@ -53,7 +52,6 @@ def rust_bindgen_library(
         cc_lib (str): The label of the cc_library that contains the .h file. This is used to find the transitive includes.
         bindgen_flags (list, optional): Flags to pass directly to the bindgen executable. See https://rust-lang.github.io/rust-bindgen/ for details.
         clang_flags (list, optional): Flags to pass directly to the clang executable.
-        rustfmt (bool, optional): Enable or disable running rustfmt on the generated file.
         **kwargs: Arguments to forward to the underlying `rust_library` rule.
     """
 
@@ -71,7 +69,6 @@ def rust_bindgen_library(
         cc_lib = cc_lib,
         bindgen_flags = bindgen_flags or [],
         clang_flags = clang_flags or [],
-        rustfmt = rustfmt,
         tags = tags,
     )
 
@@ -85,7 +82,6 @@ def rust_bindgen_library(
 
 def _rust_bindgen_impl(ctx):
     toolchain = ctx.toolchains[Label("//bindgen:toolchain_type")]
-    rustfmt_toolchain = ctx.toolchains[Label("//rust/rustfmt:toolchain_type")]
 
     # nb. We can't grab the cc_library`s direct headers, so a header must be provided.
     cc_lib = ctx.attr.cc_lib
@@ -96,57 +92,58 @@ def _rust_bindgen_impl(ctx):
 
     toolchain = ctx.toolchains[Label("//bindgen:toolchain_type")]
     bindgen_bin = toolchain.bindgen
-    rustfmt_bin = rustfmt_toolchain.rustfmt
     clang_bin = toolchain.clang
     libclang = toolchain.libclang
     libstdcxx = toolchain.libstdcxx
 
-    # rustfmt is not where bindgen expects to find it, so we format manually
-    bindgen_args = ["--no-rustfmt-bindings"] + ctx.attr.bindgen_flags
-    clang_args = ctx.attr.clang_flags
-
     output = ctx.outputs.out
+    tools = depset([clang_bin])
 
     # libclang should only have 1 output file
     libclang_dir = _get_libs_for_static_executable(libclang).to_list()[0].dirname
     include_directories = cc_lib[CcInfo].compilation_context.includes.to_list()
     quote_include_directories = cc_lib[CcInfo].compilation_context.quote_includes.to_list()
     system_include_directories = cc_lib[CcInfo].compilation_context.system_includes.to_list()
 
-    # Vanilla usage of bindgen produces formatted output, here we do the same if we have `rustfmt` in our toolchain.
-    run_rustfmt = toolchain.default_rustfmt or ctx.attr.rustfmt
-    if run_rustfmt:
-        unformatted_output = ctx.actions.declare_file(output.basename + ".unformatted")
-    else:
-        unformatted_output = output
+    env = {
+        "CLANG_PATH": clang_bin.path,
+        "LIBCLANG_PATH": libclang_dir,
+        "RUST_BACKTRACE": "1",
+    }
 
     args = ctx.actions.args()
-    args.add_all(bindgen_args)
+
+    # Configure Bindgen Arguments
+    args.add_all(ctx.attr.bindgen_flags)
     args.add(header.path)
-    args.add("--output", unformatted_output)
+    args.add("--output", output)
+
+    # Vanilla usage of bindgen produces formatted output, here we do the same if we have `rustfmt` in our toolchain.
+    rustfmt_toolchain = ctx.toolchains[Label("//rust/rustfmt:toolchain_type")]
+    if toolchain.default_rustfmt:
+        # Bindgen is able to find rustfmt using the RUSTFMT environment variable
+        env.update({"RUSTFMT": rustfmt_toolchain.rustfmt.path})
+        tools = depset(transitive = [tools, rustfmt_toolchain.all_files])
+    else:
+        args.add("--no-rustfmt-bindings")
+
+    # Configure Clang Arguments
     args.add("--")
     args.add_all(include_directories, before_each = "-I")
     args.add_all(quote_include_directories, before_each = "-iquote")
     args.add_all(system_include_directories, before_each = "-isystem")
-    args.add_all(clang_args)
+    args.add_all(ctx.attr.clang_flags)
 
-    env = {
-        "CLANG_PATH": clang_bin.path,
-        "LIBCLANG_PATH": libclang_dir,
-        "RUST_BACKTRACE": "1",
-    }
     cc_toolchain, feature_configuration = find_cc_toolchain(ctx)
     _, _, linker_env = get_linker_and_args(ctx, ctx.attr, "bin", cc_toolchain, feature_configuration, None)
     env.update(**linker_env)
 
-    tools = depset([clang_bin])
-
     # Allow sysroots configured by the toolchain to be added to Clang arguments.
     if "no-rust-bindgen-cc-sysroot" not in ctx.features:
         if cc_toolchain.sysroot:
             tools = depset(transitive = [tools, cc_toolchain.all_files])
             sysroot_args = ["--sysroot", cc_toolchain.sysroot]
-            for arg in clang_args:
+            for arg in ctx.attr.clang_flags:
                 if arg.startswith("--sysroot"):
                     sysroot_args = []
                     break
@@ -169,32 +166,14 @@ def _rust_bindgen_impl(ctx):
                 _get_libs_for_static_executable(libstdcxx),
             ] if libstdcxx else []),
         ),
-        outputs = [unformatted_output],
+        outputs = [output],
         mnemonic = "RustBindgen",
         progress_message = "Generating bindings for {}..".format(header.path),
         env = env,
         arguments = [args],
         tools = tools,
     )
 
-    if run_rustfmt:
-        rustfmt_args = ctx.actions.args()
-        rustfmt_args.add("--stdout-file", output)
-        rustfmt_args.add("--")
-        rustfmt_args.add(rustfmt_bin)
-        rustfmt_args.add("--emit", "stdout")
-        rustfmt_args.add("--quiet")
-        rustfmt_args.add(unformatted_output)
-
-        ctx.actions.run(
-            executable = ctx.executable._process_wrapper,
-            inputs = [unformatted_output],
-            outputs = [output],
-            arguments = [rustfmt_args],
-            tools = [rustfmt_toolchain.all_files],
-            mnemonic = "RustfmtBindgen",
-        )
-
 rust_bindgen = rule(
     doc = "Generates a rust source file from a cc_library and a header.",
     implementation = _rust_bindgen_impl,
@@ -213,10 +192,6 @@ rust_bindgen = rule(
             doc = "The `.h` file to generate bindings for.",
             allow_single_file = True,
         ),
-        "rustfmt": attr.bool(
-            doc = "Enable or disable running rustfmt on the generated file.",
-            default = True,
-        ),
         "_cc_toolchain": attr.label(
             default = Label("@bazel_tools//tools/cpp:current_cc_toolchain"),
         ),
@@ -239,12 +214,6 @@ rust_bindgen = rule(
 )
 
 def _rust_bindgen_toolchain_impl(ctx):
-    if ctx.attr.rustfmt:
-        # buildifier: disable=print
-        print("The `rustfmt` attribute is deprecated. Please remove it on {} and register a `rustfmt_toolchain` instead.".format(
-            ctx.label,
-        ))
-
     return platform_common.ToolchainInfo(
         bindgen = ctx.executable.bindgen,
         clang = ctx.executable.clang,
@@ -309,11 +278,5 @@ For additional information, see the [Bazel toolchains documentation](https://doc
             providers = [CcInfo],
             mandatory = False,
         ),
-        "rustfmt": attr.label(
-            doc = "**Deprecated**: Instead, register a `rustfmt_toolchain` and refer to the `rust_bindgen_toolchain.default_rustfmt` and `rust_bindgen.rustfmt` attributes.",
-            executable = True,
-            cfg = "exec",
-            mandatory = False,
-        ),
     },
 )

docs/flatten.md

@@ -351,7 +351,7 @@ is available under the key `dsym_folder` in `OutputGroupInfo`.
 ## rust_bindgen
 
 <pre>
-rust_bindgen(<a href="#rust_bindgen-name">name</a>, <a href="#rust_bindgen-bindgen_flags">bindgen_flags</a>, <a href="#rust_bindgen-cc_lib">cc_lib</a>, <a href="#rust_bindgen-clang_flags">clang_flags</a>, <a href="#rust_bindgen-header">header</a>, <a href="#rust_bindgen-rustfmt">rustfmt</a>)
+rust_bindgen(<a href="#rust_bindgen-name">name</a>, <a href="#rust_bindgen-bindgen_flags">bindgen_flags</a>, <a href="#rust_bindgen-cc_lib">cc_lib</a>, <a href="#rust_bindgen-clang_flags">clang_flags</a>, <a href="#rust_bindgen-header">header</a>)
 </pre>
 
 Generates a rust source file from a cc_library and a header.
@@ -366,15 +366,14 @@ Generates a rust source file from a cc_library and a header.
 | <a id="rust_bindgen-cc_lib"></a>cc_lib |  The cc_library that contains the <code>.h</code> file. This is used to find the transitive includes.   | <a href="https://bazel.build/concepts/labels">Label</a> | optional | <code>None</code> |
 | <a id="rust_bindgen-clang_flags"></a>clang_flags |  Flags to pass directly to the clang executable.   | List of strings | optional | <code>[]</code> |
 | <a id="rust_bindgen-header"></a>header |  The <code>.h</code> file to generate bindings for.   | <a href="https://bazel.build/concepts/labels">Label</a> | optional | <code>None</code> |
-| <a id="rust_bindgen-rustfmt"></a>rustfmt |  Enable or disable running rustfmt on the generated file.   | Boolean | optional | <code>True</code> |
 
 
 <a id="rust_bindgen_toolchain"></a>
 
 ## rust_bindgen_toolchain
 
 <pre>
-rust_bindgen_toolchain(<a href="#rust_bindgen_toolchain-name">name</a>, <a href="#rust_bindgen_toolchain-bindgen">bindgen</a>, <a href="#rust_bindgen_toolchain-clang">clang</a>, <a href="#rust_bindgen_toolchain-default_rustfmt">default_rustfmt</a>, <a href="#rust_bindgen_toolchain-libclang">libclang</a>, <a href="#rust_bindgen_toolchain-libstdcxx">libstdcxx</a>, <a href="#rust_bindgen_toolchain-rustfmt">rustfmt</a>)
+rust_bindgen_toolchain(<a href="#rust_bindgen_toolchain-name">name</a>, <a href="#rust_bindgen_toolchain-bindgen">bindgen</a>, <a href="#rust_bindgen_toolchain-clang">clang</a>, <a href="#rust_bindgen_toolchain-default_rustfmt">default_rustfmt</a>, <a href="#rust_bindgen_toolchain-libclang">libclang</a>, <a href="#rust_bindgen_toolchain-libstdcxx">libstdcxx</a>)
 </pre>
 
 The tools required for the `rust_bindgen` rule.
@@ -416,7 +415,6 @@ For additional information, see the [Bazel toolchains documentation](https://doc
 | <a id="rust_bindgen_toolchain-default_rustfmt"></a>default_rustfmt |  If set, <code>rust_bindgen</code> targets will always format generated sources with <code>rustfmt</code>.   | Boolean | optional | <code>False</code> |
 | <a id="rust_bindgen_toolchain-libclang"></a>libclang |  A cc_library that provides bindgen's runtime dependency on libclang.   | <a href="https://bazel.build/concepts/labels">Label</a> | optional | <code>None</code> |
 | <a id="rust_bindgen_toolchain-libstdcxx"></a>libstdcxx |  A cc_library that satisfies libclang's libstdc++ dependency. This is used to make the execution of clang hermetic. If None, system libraries will be used instead.   | <a href="https://bazel.build/concepts/labels">Label</a> | optional | <code>None</code> |
-| <a id="rust_bindgen_toolchain-rustfmt"></a>rustfmt |  **Deprecated**: Instead, register a <code>rustfmt_toolchain</code> and refer to the <code>rust_bindgen_toolchain.default_rustfmt</code> and <code>rust_bindgen.rustfmt</code> attributes.   | <a href="https://bazel.build/concepts/labels">Label</a> | optional | <code>None</code> |
 
 
 <a id="rust_clippy"></a>
@@ -1752,7 +1750,7 @@ Declare dependencies needed for bindgen.
 ## rust_bindgen_library
 
 <pre>
-rust_bindgen_library(<a href="#rust_bindgen_library-name">name</a>, <a href="#rust_bindgen_library-header">header</a>, <a href="#rust_bindgen_library-cc_lib">cc_lib</a>, <a href="#rust_bindgen_library-bindgen_flags">bindgen_flags</a>, <a href="#rust_bindgen_library-clang_flags">clang_flags</a>, <a href="#rust_bindgen_library-rustfmt">rustfmt</a>, <a href="#rust_bindgen_library-kwargs">kwargs</a>)
+rust_bindgen_library(<a href="#rust_bindgen_library-name">name</a>, <a href="#rust_bindgen_library-header">header</a>, <a href="#rust_bindgen_library-cc_lib">cc_lib</a>, <a href="#rust_bindgen_library-bindgen_flags">bindgen_flags</a>, <a href="#rust_bindgen_library-clang_flags">clang_flags</a>, <a href="#rust_bindgen_library-kwargs">kwargs</a>)
 </pre>
 
 Generates a rust source file for `header`, and builds a rust_library.
@@ -1770,7 +1768,6 @@ Arguments are the same as `rust_bindgen`, and `kwargs` are passed directly to ru
 | <a id="rust_bindgen_library-cc_lib"></a>cc_lib |  The label of the cc_library that contains the .h file. This is used to find the transitive includes.   |  none |
 | <a id="rust_bindgen_library-bindgen_flags"></a>bindgen_flags |  Flags to pass directly to the bindgen executable. See https://rust-lang.github.io/rust-bindgen/ for details.   |  `None` |
 | <a id="rust_bindgen_library-clang_flags"></a>clang_flags |  Flags to pass directly to the clang executable.   |  `None` |
-| <a id="rust_bindgen_library-rustfmt"></a>rustfmt |  Enable or disable running rustfmt on the generated file.   |  `True` |
 | <a id="rust_bindgen_library-kwargs"></a>kwargs |  Arguments to forward to the underlying <code>rust_library</code> rule.   |  none |
 
 

docs/rust_bindgen.md

@@ -53,7 +53,7 @@ toolchains following the instructions for [rust_bindgen_toolchain](#rust_bindgen
 ## rust_bindgen
 
 <pre>
-rust_bindgen(<a href="#rust_bindgen-name">name</a>, <a href="#rust_bindgen-bindgen_flags">bindgen_flags</a>, <a href="#rust_bindgen-cc_lib">cc_lib</a>, <a href="#rust_bindgen-clang_flags">clang_flags</a>, <a href="#rust_bindgen-header">header</a>, <a href="#rust_bindgen-rustfmt">rustfmt</a>)
+rust_bindgen(<a href="#rust_bindgen-name">name</a>, <a href="#rust_bindgen-bindgen_flags">bindgen_flags</a>, <a href="#rust_bindgen-cc_lib">cc_lib</a>, <a href="#rust_bindgen-clang_flags">clang_flags</a>, <a href="#rust_bindgen-header">header</a>)
 </pre>
 
 Generates a rust source file from a cc_library and a header.
@@ -68,15 +68,14 @@ Generates a rust source file from a cc_library and a header.
 | <a id="rust_bindgen-cc_lib"></a>cc_lib |  The cc_library that contains the <code>.h</code> file. This is used to find the transitive includes.   | <a href="https://bazel.build/concepts/labels">Label</a> | optional | <code>None</code> |
 | <a id="rust_bindgen-clang_flags"></a>clang_flags |  Flags to pass directly to the clang executable.   | List of strings | optional | <code>[]</code> |
 | <a id="rust_bindgen-header"></a>header |  The <code>.h</code> file to generate bindings for.   | <a href="https://bazel.build/concepts/labels">Label</a> | optional | <code>None</code> |
-| <a id="rust_bindgen-rustfmt"></a>rustfmt |  Enable or disable running rustfmt on the generated file.   | Boolean | optional | <code>True</code> |
 
 
 <a id="rust_bindgen_toolchain"></a>
 
 ## rust_bindgen_toolchain
 
 <pre>
-rust_bindgen_toolchain(<a href="#rust_bindgen_toolchain-name">name</a>, <a href="#rust_bindgen_toolchain-bindgen">bindgen</a>, <a href="#rust_bindgen_toolchain-clang">clang</a>, <a href="#rust_bindgen_toolchain-default_rustfmt">default_rustfmt</a>, <a href="#rust_bindgen_toolchain-libclang">libclang</a>, <a href="#rust_bindgen_toolchain-libstdcxx">libstdcxx</a>, <a href="#rust_bindgen_toolchain-rustfmt">rustfmt</a>)
+rust_bindgen_toolchain(<a href="#rust_bindgen_toolchain-name">name</a>, <a href="#rust_bindgen_toolchain-bindgen">bindgen</a>, <a href="#rust_bindgen_toolchain-clang">clang</a>, <a href="#rust_bindgen_toolchain-default_rustfmt">default_rustfmt</a>, <a href="#rust_bindgen_toolchain-libclang">libclang</a>, <a href="#rust_bindgen_toolchain-libstdcxx">libstdcxx</a>)
 </pre>
 
 The tools required for the `rust_bindgen` rule.
@@ -118,7 +117,6 @@ For additional information, see the [Bazel toolchains documentation](https://doc
 | <a id="rust_bindgen_toolchain-default_rustfmt"></a>default_rustfmt |  If set, <code>rust_bindgen</code> targets will always format generated sources with <code>rustfmt</code>.   | Boolean | optional | <code>False</code> |
 | <a id="rust_bindgen_toolchain-libclang"></a>libclang |  A cc_library that provides bindgen's runtime dependency on libclang.   | <a href="https://bazel.build/concepts/labels">Label</a> | optional | <code>None</code> |
 | <a id="rust_bindgen_toolchain-libstdcxx"></a>libstdcxx |  A cc_library that satisfies libclang's libstdc++ dependency. This is used to make the execution of clang hermetic. If None, system libraries will be used instead.   | <a href="https://bazel.build/concepts/labels">Label</a> | optional | <code>None</code> |
-| <a id="rust_bindgen_toolchain-rustfmt"></a>rustfmt |  **Deprecated**: Instead, register a <code>rustfmt_toolchain</code> and refer to the <code>rust_bindgen_toolchain.default_rustfmt</code> and <code>rust_bindgen.rustfmt</code> attributes.   | <a href="https://bazel.build/concepts/labels">Label</a> | optional | <code>None</code> |
 
 
 <a id="rust_bindgen_dependencies"></a>
@@ -138,7 +136,7 @@ Declare dependencies needed for bindgen.
 ## rust_bindgen_library
 
 <pre>
-rust_bindgen_library(<a href="#rust_bindgen_library-name">name</a>, <a href="#rust_bindgen_library-header">header</a>, <a href="#rust_bindgen_library-cc_lib">cc_lib</a>, <a href="#rust_bindgen_library-bindgen_flags">bindgen_flags</a>, <a href="#rust_bindgen_library-clang_flags">clang_flags</a>, <a href="#rust_bindgen_library-rustfmt">rustfmt</a>, <a href="#rust_bindgen_library-kwargs">kwargs</a>)
+rust_bindgen_library(<a href="#rust_bindgen_library-name">name</a>, <a href="#rust_bindgen_library-header">header</a>, <a href="#rust_bindgen_library-cc_lib">cc_lib</a>, <a href="#rust_bindgen_library-bindgen_flags">bindgen_flags</a>, <a href="#rust_bindgen_library-clang_flags">clang_flags</a>, <a href="#rust_bindgen_library-kwargs">kwargs</a>)
 </pre>
 
 Generates a rust source file for `header`, and builds a rust_library.
@@ -156,7 +154,6 @@ Arguments are the same as `rust_bindgen`, and `kwargs` are passed directly to ru
 | <a id="rust_bindgen_library-cc_lib"></a>cc_lib |  The label of the cc_library that contains the .h file. This is used to find the transitive includes.   |  none |
 | <a id="rust_bindgen_library-bindgen_flags"></a>bindgen_flags |  Flags to pass directly to the bindgen executable. See https://rust-lang.github.io/rust-bindgen/ for details.   |  `None` |
 | <a id="rust_bindgen_library-clang_flags"></a>clang_flags |  Flags to pass directly to the clang executable.   |  `None` |
-| <a id="rust_bindgen_library-rustfmt"></a>rustfmt |  Enable or disable running rustfmt on the generated file.   |  `True` |
 | <a id="rust_bindgen_library-kwargs"></a>kwargs |  Arguments to forward to the underlying <code>rust_library</code> rule.   |  none |
 
 

examples/bindgen/BUILD.bazel

@@ -26,27 +26,3 @@ rust_test(
     name = "simple_test",
     crate = ":simple_example",
 )
-
-# Same as above, except disabling formatting on bindgen.
-rust_bindgen_library(
-    name = "simple_bindgen_unformatted",
-    bindgen_flags = [
-        "--allowlist-var=SIMPLE_.*",
-    ],
-    cc_lib = ":simple",
-    crate_name = "simple_bindgen",
-    header = "simple.h",
-    rustfmt = False,
-)
-
-rust_binary(
-    name = "simple_example_unformatted",
-    srcs = ["main.rs"],
-    crate_name = "simple_example",
-    deps = [":simple_bindgen_unformatted"],
-)
-
-rust_test(
-    name = "simple_test_unformatted",
-    crate = ":simple_example_unformatted",
-)