From ede851ec12e5f54e1cf606193cbf42e903c2b862 Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Wed, 9 Feb 2022 09:06:27 -0800 Subject: [PATCH 1/2] Addressed some new buildifier defects --- cargo/cargo_bootstrap.bzl | 1 - cargo/private/cargo_utils.bzl | 3 +-- crate_universe/defs.bzl | 2 +- crate_universe/private/util.bzl | 2 +- proto/proto.bzl | 2 -- rust/platform/triple_mappings.bzl | 2 +- rust/private/clippy.bzl | 2 -- .../dummy_cc_toolchain/dummy_cc_toolchain.bzl | 2 +- rust/private/rust.bzl | 4 ++-- rust/private/rust_analyzer.bzl | 2 +- rust/private/rustc.bzl | 24 +++---------------- rust/private/rustdoc.bzl | 2 -- rust/private/transitions.bzl | 6 ++--- rust/private/utils.bzl | 19 +++++++-------- rust/toolchain.bzl | 1 + test/extra_exec_rustc_flags/defs.bzl | 2 +- .../force_all_deps_direct_test.bzl | 1 - .../location_expansion_test.bzl | 1 - test/unit/native_deps/native_deps_test.bzl | 11 --------- .../use_libtest_harness_test.bzl | 1 - 20 files changed, 25 insertions(+), 65 deletions(-) diff --git a/cargo/cargo_bootstrap.bzl b/cargo/cargo_bootstrap.bzl index c75b88b060..6d6cdcff8c 100644 --- a/cargo/cargo_bootstrap.bzl +++ b/cargo/cargo_bootstrap.bzl @@ -188,7 +188,6 @@ def _cargo_bootstrap_repository_impl(repository_ctx): rustc_template = repository_ctx.attr.rust_toolchain_rustc_template tools = get_rust_tools( - repository_ctx = repository_ctx, cargo_template = cargo_template, rustc_template = rustc_template, host_triple = host_triple, diff --git a/cargo/private/cargo_utils.bzl b/cargo/private/cargo_utils.bzl index 5e7cade716..6c1063b183 100644 --- a/cargo/private/cargo_utils.bzl +++ b/cargo/private/cargo_utils.bzl @@ -151,11 +151,10 @@ def _resolve_repository_template( return template -def get_rust_tools(repository_ctx, cargo_template, rustc_template, host_triple, version): +def get_rust_tools(cargo_template, rustc_template, host_triple, version): """Retrieve `cargo` and `rustc` labels based on the host triple. Args: - repository_ctx (repository_ctx): The rule's context object cargo_template (str): A template used to identify the label of the host `cargo` binary. rustc_template (str): A template used to identify the label of the host `rustc` binary. host_triple (struct): The host's triple. See `@rules_rust//rust/platform:triple.bzl`. diff --git a/crate_universe/defs.bzl b/crate_universe/defs.bzl index 43907c430b..32deef15b5 100644 --- a/crate_universe/defs.bzl +++ b/crate_universe/defs.bzl @@ -377,7 +377,7 @@ def _override( if not type(dep_val) == "dict": fail("The {} attribute should be a dictionary".format(dep_key)) - for target, deps in dep_val.items(): + for deps in dep_val.values(): if not type(deps) == "list" or any([type(x) != "string" for x in deps]): fail("The {} values should be lists of strings".format(dep_key)) diff --git a/crate_universe/private/util.bzl b/crate_universe/private/util.bzl index 35de602162..a28d1e83d4 100644 --- a/crate_universe/private/util.bzl +++ b/crate_universe/private/util.bzl @@ -123,7 +123,7 @@ def get_cargo_and_rustc(repository_ctx, host_triple): version_str = repository_ctx.attr.version # Get info about the current host's tool locations - (host_triple, resolver_triple) = get_host_triple(repository_ctx) + host_triple, _ = get_host_triple(repository_ctx) system = triple_to_system(host_triple) extension = system_to_binary_ext(system) arch = triple_to_arch(host_triple) diff --git a/proto/proto.bzl b/proto/proto.bzl index c7b4c8f3ff..5fd1f4f1f1 100644 --- a/proto/proto.bzl +++ b/proto/proto.bzl @@ -212,8 +212,6 @@ def _rust_proto_compile(protos, descriptor_sets, imports, crate_name, ctx, is_gr output_hash, )) - toolchain = find_toolchain(ctx) - # Gather all dependencies for compilation compile_action_deps = depset( transform_deps( diff --git a/rust/platform/triple_mappings.bzl b/rust/platform/triple_mappings.bzl index f7d42cbbae..0614eac34e 100644 --- a/rust/platform/triple_mappings.bzl +++ b/rust/platform/triple_mappings.bzl @@ -172,7 +172,7 @@ def cpu_arch_to_constraints(cpu_arch): return ["@platforms//cpu:{}".format(plat_suffix)] -def vendor_to_constraints(vendor): +def vendor_to_constraints(_vendor): # TODO(acmcarther): Review: # # My current understanding is that vendors can't have a material impact on diff --git a/rust/private/clippy.bzl b/rust/private/clippy.bzl index 53a86f1b96..8525a16aec 100644 --- a/rust/private/clippy.bzl +++ b/rust/private/clippy.bzl @@ -61,10 +61,8 @@ def _clippy_aspect_impl(target, ctx): toolchain = find_toolchain(ctx) cc_toolchain, feature_configuration = find_cc_toolchain(ctx) - crate_type = crate_info.type dep_info, build_info, linkstamps = collect_deps( - label = ctx.label, deps = crate_info.deps, proc_macro_deps = crate_info.proc_macro_deps, aliases = crate_info.aliases, diff --git a/rust/private/dummy_cc_toolchain/dummy_cc_toolchain.bzl b/rust/private/dummy_cc_toolchain/dummy_cc_toolchain.bzl index 67c7d457fe..8ca8c8f4ae 100644 --- a/rust/private/dummy_cc_toolchain/dummy_cc_toolchain.bzl +++ b/rust/private/dummy_cc_toolchain/dummy_cc_toolchain.bzl @@ -1,5 +1,5 @@ # buildifier: disable=module-docstring -def _dummy_cc_toolchain_impl(ctx): +def _dummy_cc_toolchain_impl(_ctx): # The `all_files` attribute is referenced by rustc_compile_action(). return [platform_common.ToolchainInfo(all_files = depset([]))] diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index a17d751dca..def840ec81 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -27,11 +27,11 @@ load( # TODO(marco): Separate each rule into its own file. -def _assert_no_deprecated_attributes(ctx): +def _assert_no_deprecated_attributes(_ctx): """Forces a failure if any deprecated attributes were specified Args: - ctx (ctx): The current rule's context object + _ctx (ctx): The current rule's context object """ pass diff --git a/rust/private/rust_analyzer.bzl b/rust/private/rust_analyzer.bzl index 4a22f8428d..e28ca30c42 100644 --- a/rust/private/rust_analyzer.bzl +++ b/rust/private/rust_analyzer.bzl @@ -221,7 +221,7 @@ rust_analyzer_detect_sysroot = rule( """), ) -def _rust_analyzer_impl(ctx): +def _rust_analyzer_impl(_ctx): pass rust_analyzer = rule( diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 2701424a82..869a7953c4 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -118,7 +118,6 @@ def _are_linkstamps_supported(feature_configuration, has_grep_includes): has_grep_includes) def collect_deps( - label, deps, proc_macro_deps, aliases, @@ -126,7 +125,6 @@ def collect_deps( """Walks through dependencies and collects the transitive dependencies. Args: - label (str): Label of the current target. deps (list): The deps from ctx.attr.deps. proc_macro_deps (list): The proc_macro deps from ctx.attr.proc_macro_deps. aliases (dict): A dict mapping aliased targets to their actual Crate information. @@ -142,7 +140,6 @@ def collect_deps( direct_crates = [] transitive_crates = [] transitive_noncrates = [] - transitive_noncrate_libs = [] transitive_build_infos = [] build_info = None linkstamps = [] @@ -192,7 +189,6 @@ def collect_deps( "targets.") transitive_crates_depset = depset(transitive = transitive_crates) - transitive_libs = depset([]) return ( rust_common.dep_info( @@ -301,20 +297,12 @@ def get_linker_and_args(ctx, attr, cc_toolchain, feature_configuration, rpaths): return ld, link_args, link_env def _process_build_scripts( - ctx, - file, - crate_info, build_info, - dep_info, compile_inputs): """Gathers the outputs from a target's `cargo_build_script` action. Args: - ctx (ctx): The rule's context object - file (File): A struct containing files defined in label type attributes marked as `allow_single_file`. - crate_info (CrateInfo): The Crate information of the crate to process build scripts for. build_info (BuildInfo): The target Build's dependency info. - dep_info (Depinfo): The target Crate's dependency info. compile_inputs (depset): A set of all files that will participate in the build. Returns: @@ -324,7 +312,7 @@ def _process_build_scripts( - (File): An optional path to a generated environment file from a `cargo_build_script` target - (list): All direct and transitive build flags from the current build info. """ - extra_inputs, out_dir, build_env_file, build_flags_files = _create_extra_input_args(ctx, file, build_info, dep_info) + extra_inputs, out_dir, build_env_file, build_flags_files = _create_extra_input_args(build_info) if extra_inputs: compile_inputs = depset(extra_inputs, transitive = [compile_inputs]) return compile_inputs, out_dir, build_env_file, build_flags_files @@ -447,7 +435,7 @@ def collect_inputs( ) build_env_files = getattr(files, "rustc_env_files", []) - compile_inputs, out_dir, build_env_file, build_flags_files = _process_build_scripts(ctx, file, crate_info, build_info, dep_info, compile_inputs) + compile_inputs, out_dir, build_env_file, build_flags_files = _process_build_scripts(build_info, compile_inputs) if build_env_file: build_env_files = [f for f in build_env_files] + [build_env_file] compile_inputs = depset(build_env_files, transitive = [compile_inputs]) @@ -694,7 +682,6 @@ def rustc_compile_action( crate_info, output_hash = None, rust_flags = [], - environ = {}, force_all_deps_direct = False): """Create and run a rustc compile action based on the current rule's attributes @@ -705,7 +692,6 @@ def rustc_compile_action( crate_info (CrateInfo): The CrateInfo provider for the current target. output_hash (str, optional): The hashed path of the crate root. Defaults to None. rust_flags (list, optional): Additional flags to pass to rustc. Defaults to []. - environ (dict, optional): A set of makefile expandable environment variables for the action force_all_deps_direct (bool, optional): Whether to pass the transitive rlibs with --extern to the commandline as opposed to -L. @@ -718,7 +704,6 @@ def rustc_compile_action( cc_toolchain, feature_configuration = find_cc_toolchain(ctx) dep_info, build_info, linkstamps = collect_deps( - label = ctx.label, deps = crate_info.deps, proc_macro_deps = crate_info.proc_macro_deps, aliases = crate_info.aliases, @@ -943,14 +928,11 @@ def add_edition_flags(args, crate): if crate.edition != "2015": args.add("--edition={}".format(crate.edition)) -def _create_extra_input_args(ctx, file, build_info, dep_info): +def _create_extra_input_args(build_info): """Gather additional input arguments from transitive dependencies Args: - ctx (ctx): The rule's context object - file (struct): A struct containing files defined in label type attributes marked as `allow_single_file`. build_info (BuildInfo): The BuildInfo provider from the target Crate's set of inputs. - dep_info (DepInfo): The Depinfo provider form the target Crate's set of inputs. Returns: tuple: A tuple of the following items: diff --git a/rust/private/rustdoc.bzl b/rust/private/rustdoc.bzl index 987b4461c0..59bf404cd4 100644 --- a/rust/private/rustdoc.bzl +++ b/rust/private/rustdoc.bzl @@ -74,7 +74,6 @@ def rustdoc_compile_action( cc_toolchain, feature_configuration = find_cc_toolchain(ctx) dep_info, build_info, linkstamps = collect_deps( - label = ctx.label, deps = crate_info.deps, proc_macro_deps = crate_info.proc_macro_deps, aliases = crate_info.aliases, @@ -161,7 +160,6 @@ def _rust_doc_impl(ctx): crate = ctx.attr.crate crate_info = crate[rust_common.crate_info] - dep_info = crate[rust_common.dep_info] output_dir = ctx.actions.declare_directory("{}.rustdoc".format(ctx.label.name)) diff --git a/rust/private/transitions.bzl b/rust/private/transitions.bzl index 0888118946..5680c7be54 100644 --- a/rust/private/transitions.bzl +++ b/rust/private/transitions.bzl @@ -1,11 +1,11 @@ # buildifier: disable=module-docstring -def _wasm_bindgen_transition(settings, attr): +def _wasm_bindgen_transition(_settings, _attr): """The implementation of the `wasm_bindgen_transition` transition Args: - settings (dict): A dict {String:Object} of all settings declared + _settings (dict): A dict {String:Object} of all settings declared in the inputs parameter to `transition()` - attr (dict): A dict of attributes and values of the rule to which + _attr (dict): A dict of attributes and values of the rule to which the transition is attached Returns: diff --git a/rust/private/utils.bzl b/rust/private/utils.bzl index 5d36169291..be633181ad 100644 --- a/rust/private/utils.bzl +++ b/rust/private/utils.bzl @@ -62,14 +62,13 @@ def relativize(path, start): src_parts = _path_parts(start) dest_parts = _path_parts(path) n = 0 - done = False for src_part, dest_part in zip(src_parts, dest_parts): if src_part != dest_part: break n += 1 relative_path = "" - for i in range(n, len(src_parts)): + for _ in range(n, len(src_parts)): relative_path += "../" relative_path += "/".join(dest_parts[n:]) @@ -492,13 +491,6 @@ def decode_crate_name_as_label_for_testing(crate_name): def _replace_all(string, substitutions): """Replaces occurrences of the given patterns in `string`. - Args: - string (string): the string in which the replacements should be performed. - substitutions: the list of patterns and replacements to apply. - - Returns: - A string with the appropriate substitutions performed. - There are a few reasons this looks complicated: * The substitutions are performed with some priority, i.e. patterns that are listed first in `substitutions` are higher priority than patterns that are @@ -513,11 +505,18 @@ def _replace_all(string, substitutions): pattern matches *earlier* in the string.) (E.g. "_quotedot_" encodes to "_quotequote_dot_". Note that "_quotequote_" and "_dot_" both occur in this string, and overlap.). + + Args: + string (string): the string in which the replacements should be performed. + substitutions: the list of patterns and replacements to apply. + + Returns: + A string with the appropriate substitutions performed. """ # Find the highest-priority pattern matches. plan = {} - for subst_index, (pattern, replacement) in enumerate(substitutions): + for _subst_index, (pattern, replacement) in enumerate(substitutions): for pattern_start in range(len(string)): if not pattern_start in plan and string.startswith(pattern, pattern_start): plan[pattern_start] = (len(pattern), replacement) diff --git a/rust/toolchain.bzl b/rust/toolchain.bzl index 9f4282e55c..b5237b17f5 100644 --- a/rust/toolchain.bzl +++ b/rust/toolchain.bzl @@ -267,6 +267,7 @@ def _rust_toolchain_impl(ctx): # In cases where the toolchain uses the Rust standard library, calculate sysroot path sysroot_path = None + rust_std_files_list = [] if rust_std: # Calculate the rustc sysroot path by using a file from the rust-std bundle rust_std_files_list = rust_std.files.to_list() diff --git a/test/extra_exec_rustc_flags/defs.bzl b/test/extra_exec_rustc_flags/defs.bzl index aca58b69af..790cdc5677 100644 --- a/test/extra_exec_rustc_flags/defs.bzl +++ b/test/extra_exec_rustc_flags/defs.bzl @@ -1,6 +1,6 @@ """Test transitions to test extra_exec_rustc_flags.""" -def _extra_exec_rustc_flags_transition_impl(settings, attr): +def _extra_exec_rustc_flags_transition_impl(_settings, attr): return { "//:extra_exec_rustc_flags": attr.extra_exec_rustc_flags, } diff --git a/test/unit/force_all_deps_direct/force_all_deps_direct_test.bzl b/test/unit/force_all_deps_direct/force_all_deps_direct_test.bzl index 7e0964aa08..6cb5147c1a 100644 --- a/test/unit/force_all_deps_direct/force_all_deps_direct_test.bzl +++ b/test/unit/force_all_deps_direct/force_all_deps_direct_test.bzl @@ -9,7 +9,6 @@ def _force_all_deps_direct_rustc_flags_test(ctx): env = analysistest.begin(ctx) tut = analysistest.target_under_test(env) action = tut.actions[1] - argv = action.argv assert_action_mnemonic(env, action, "Rustc") assert_argv_contains_prefix( env, diff --git a/test/unit/location_expansion/location_expansion_test.bzl b/test/unit/location_expansion/location_expansion_test.bzl index c2aa1940e6..caec57c89b 100644 --- a/test/unit/location_expansion/location_expansion_test.bzl +++ b/test/unit/location_expansion/location_expansion_test.bzl @@ -8,7 +8,6 @@ def _location_expansion_rustc_flags_test(ctx): env = analysistest.begin(ctx) tut = analysistest.target_under_test(env) action = tut.actions[0] - argv = action.argv assert_action_mnemonic(env, action, "Rustc") assert_argv_contains(env, action, "test/unit/location_expansion/mylibrary.rs") expected = "@${pwd}/" + ctx.bin_dir.path + "/test/unit/location_expansion/generated_flag.data" diff --git a/test/unit/native_deps/native_deps_test.bzl b/test/unit/native_deps/native_deps_test.bzl index ec4201bbd8..5755f5d2e6 100644 --- a/test/unit/native_deps/native_deps_test.bzl +++ b/test/unit/native_deps/native_deps_test.bzl @@ -19,17 +19,6 @@ def _native_dep_lib_name(ctx): else: return "libnative_dep.a" -def _lib_has_no_native_libs_test_impl(ctx): - env = analysistest.begin(ctx) - tut = analysistest.target_under_test(env) - action = tut.actions[0] - assert_argv_contains(env, action, "--crate-type=lib") - assert_argv_contains_prefix_suffix(env, action, "-Lnative=", "/native_deps") - assert_argv_contains_not(env, action, "-lstatic=native_dep") - assert_argv_contains_not(env, action, "-ldylib=native_dep") - assert_argv_contains_prefix_not(env, action, "--codegen=linker=") - return analysistest.end(env) - def _rlib_has_no_native_libs_test_impl(ctx): env = analysistest.begin(ctx) tut = analysistest.target_under_test(env) diff --git a/test/unit/use_libtest_harness/use_libtest_harness_test.bzl b/test/unit/use_libtest_harness/use_libtest_harness_test.bzl index 72ec2b7770..fd7907da0e 100644 --- a/test/unit/use_libtest_harness/use_libtest_harness_test.bzl +++ b/test/unit/use_libtest_harness/use_libtest_harness_test.bzl @@ -8,7 +8,6 @@ def _use_libtest_harness_rustc_flags_test_impl(ctx): env = analysistest.begin(ctx) tut = analysistest.target_under_test(env) action = tut.actions[0] - argv = action.argv assert_action_mnemonic(env, action, "Rustc") assert_argv_contains(env, action, "test/unit/use_libtest_harness/mytest.rs") assert_argv_contains(env, action, "--test") From 7bc685e14581e7b4ad37bc95fefdc2137c8d7c1a Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Wed, 9 Feb 2022 11:41:03 -0800 Subject: [PATCH 2/2] Update rust/private/utils.bzl Co-authored-by: Daniel Wagner-Hall --- rust/private/utils.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/private/utils.bzl b/rust/private/utils.bzl index be633181ad..0247a1ffc4 100644 --- a/rust/private/utils.bzl +++ b/rust/private/utils.bzl @@ -516,7 +516,7 @@ def _replace_all(string, substitutions): # Find the highest-priority pattern matches. plan = {} - for _subst_index, (pattern, replacement) in enumerate(substitutions): + for pattern, replacement in substitutions: for pattern_start in range(len(string)): if not pattern_start in plan and string.startswith(pattern, pattern_start): plan[pattern_start] = (len(pattern), replacement)