From dfaefb7161ff0de9af47b47e45b96756affff1f0 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sat, 1 Feb 2025 17:54:39 -0500 Subject: [PATCH 1/6] chore: fix new clippy lints, enforce in CI --- .github/workflows/bindgen.yml | 10 +--------- bindgen-tests/build.rs | 2 +- bindgen/ir/dot.rs | 2 +- bindgen/lib.rs | 2 +- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/.github/workflows/bindgen.yml b/.github/workflows/bindgen.yml index 3a90263bd0..4e8f94c5b7 100644 --- a/.github/workflows/bindgen.yml +++ b/.github/workflows/bindgen.yml @@ -18,19 +18,11 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Install nightly - uses: dtolnay/rust-toolchain@master - with: - # TODO: Should ideally be stable, but we use some nightly-only - # features. - toolchain: nightly - components: rustfmt, clippy - - name: Run rustfmt run: cargo fmt -- --check - name: Run clippy - run: cargo clippy --all-targets --workspace --exclude bindgen-integration --exclude tests_expectations + run: cargo clippy --all-targets --workspace --exclude bindgen-integration --exclude tests_expectations -- -D warnings msrv: runs-on: ubuntu-latest diff --git a/bindgen-tests/build.rs b/bindgen-tests/build.rs index b261ed0a35..8fb33716a1 100644 --- a/bindgen-tests/build.rs +++ b/bindgen-tests/build.rs @@ -23,7 +23,7 @@ pub fn main() { for entry in entries { // TODO: file_is_cpp() in bindgen/lib.rs checks for hpp,hxx,hh, and h++ - should this be consistent? - if entry.path().extension().map_or(false, |ext| { + if entry.path().extension().is_some_and(|ext| { ext.eq_ignore_ascii_case("h") || ext.eq_ignore_ascii_case("hpp") }) { let func = entry diff --git a/bindgen/ir/dot.rs b/bindgen/ir/dot.rs index 0ccee42c0d..9bfc559f41 100644 --- a/bindgen/ir/dot.rs +++ b/bindgen/ir/dot.rs @@ -41,7 +41,7 @@ where if is_allowlisted { "black" } else { "gray" } )?; item.dot_attributes(ctx, &mut dot_file)?; - writeln!(&mut dot_file, r#" >];"#)?; + writeln!(&mut dot_file, " >];")?; item.trace( ctx, diff --git a/bindgen/lib.rs b/bindgen/lib.rs index 5a946e6d13..5423f00495 100644 --- a/bindgen/lib.rs +++ b/bindgen/lib.rs @@ -86,7 +86,7 @@ pub const DEFAULT_ANON_FIELDS_PREFIX: &str = "__bindgen_anon_"; const DEFAULT_NON_EXTERN_FNS_SUFFIX: &str = "__extern"; fn file_is_cpp(name_file: &str) -> bool { - Path::new(name_file).extension().map_or(false, |ext| { + Path::new(name_file).extension().is_some_and(|ext| { ext.eq_ignore_ascii_case("hpp") || ext.eq_ignore_ascii_case("hxx") || ext.eq_ignore_ascii_case("hh") || From 14681968b788c6e986961f96849dc15fbf217bfa Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sat, 1 Feb 2025 18:28:01 -0500 Subject: [PATCH 2/6] remove check-cfg, separate clippy from nightly --- .github/workflows/bindgen.yml | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/.github/workflows/bindgen.yml b/.github/workflows/bindgen.yml index 4e8f94c5b7..94022c21e1 100644 --- a/.github/workflows/bindgen.yml +++ b/.github/workflows/bindgen.yml @@ -12,14 +12,24 @@ on: - main jobs: - rustfmt-clippy: + rustfmt: runs-on: ubuntu-latest - steps: - uses: actions/checkout@v4 + - name: Install nightly + uses: dtolnay/rust-toolchain@master + with: + toolchain: nightly + components: rustfmt + - name: Run rustfmt - run: cargo fmt -- --check + run: cargo +nightly fmt -- --check + + clippy: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 - name: Run clippy run: cargo clippy --all-targets --workspace --exclude bindgen-integration --exclude tests_expectations -- -D warnings @@ -166,21 +176,6 @@ jobs: BINDGEN_RUST_FOR_LINUX_TEST: ${{matrix.os == 'ubuntu-latest' && matrix.llvm_version == '16.0' && matrix.feature_extra_asserts == 0 && 1 || 0}} run: ./ci/test.sh - check-cfg: - runs-on: ubuntu-latest - env: - RUSTFLAGS: -D warnings - steps: - - uses: actions/checkout@v4 - - - name: Install nightly - uses: dtolnay/rust-toolchain@master - with: - toolchain: nightly - - - name: Check cfg - run: cargo check -Z unstable-options -Z check-cfg - test-book: runs-on: ubuntu-latest steps: @@ -213,7 +208,7 @@ jobs: # separately. success: runs-on: ubuntu-latest - needs: [rustfmt-clippy, msrv, minimal, docs, quickchecking, test-expectations, test, check-cfg, test-book, test-no-headers] + needs: [rustfmt, clippy, msrv, minimal, docs, quickchecking, test-expectations, test, test-book, test-no-headers] # GitHub branch protection is exceedingly silly and treats "jobs skipped # because a dependency failed" as success. So we have to do some # contortions to ensure the job fails if any of its dependencies fails. From 62bae2fd0e99102f6ff0725ace8c897d520496fd Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sat, 1 Feb 2025 18:31:05 -0500 Subject: [PATCH 3/6] remove stable rust installation --- .github/workflows/bindgen.yml | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/.github/workflows/bindgen.yml b/.github/workflows/bindgen.yml index 94022c21e1..1778de1d2b 100644 --- a/.github/workflows/bindgen.yml +++ b/.github/workflows/bindgen.yml @@ -66,12 +66,7 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Install stable - uses: dtolnay/rust-toolchain@master - with: - toolchain: stable - - - name: Check without default features + - name: Check without default features run: cargo check -p bindgen --no-default-features --features=runtime docs: @@ -81,12 +76,7 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Install stable - uses: dtolnay/rust-toolchain@master - with: - toolchain: stable - - - name: Generate documentation for `bindgen` + - name: Generate documentation for `bindgen` run: cargo doc --document-private-items --no-deps -p bindgen - name: Generate documentation for `bindgen-cli` @@ -97,11 +87,6 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Install stable - uses: dtolnay/rust-toolchain@master - with: - toolchain: stable - # TODO: Actually run quickchecks once `bindgen` is reliable enough. - name: Build quickcheck tests run: cd bindgen-tests/tests/quickchecking && cargo test @@ -114,11 +99,6 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Install stable - uses: dtolnay/rust-toolchain@master - with: - toolchain: stable - - name: Test expectations run: cd bindgen-tests/tests/expectations && cargo test From ae9378dbc63edd3e0ad9dbf437f28941b2267e3e Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sun, 2 Feb 2025 20:32:47 -0500 Subject: [PATCH 4/6] another minor clippy lint --- bindgen/codegen/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index 13c0b60526..1d5796b4b4 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -4729,7 +4729,7 @@ impl CodeGenerator for Function { mangled_name, Some(abi), )) - .then(|| mangled_name) + .then_some(mangled_name) }); if let Some(link_name) = link_name_attr { From 30321c45eb5d6a3e68c7fb599754b2eb6732ffef Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Tue, 11 Feb 2025 13:19:49 -0500 Subject: [PATCH 5/6] fix clippy issues --- bindgen/ir/context.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bindgen/ir/context.rs b/bindgen/ir/context.rs index 99c75d63d8..37f8d5968d 100644 --- a/bindgen/ir/context.rs +++ b/bindgen/ir/context.rs @@ -2108,13 +2108,13 @@ If you encounter an error missing from this list, please file an issue or a PR!" pch.clone().into_boxed_str(), ]; let mut skip_next = false; - for arg in self.options.fallback_clang_args.iter() { + for arg in &self.options.fallback_clang_args { if arg.as_ref() == "-include" { skip_next = true; } else if skip_next { skip_next = false; } else { - c_args.push(arg.clone()) + c_args.push(arg.clone()); } } self.fallback_tu = From 4f603a2f02a1473c15ac8fd19413cf390c719238 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Wed, 9 Apr 2025 16:15:14 -0400 Subject: [PATCH 6/6] fix all clippy lints --- .../item_discovery_callback/mod.rs | 8 ++--- .../tests/quickchecking/src/fuzzers.rs | 30 +++++++++---------- bindgen-tests/tests/tests.rs | 6 ++-- bindgen/codegen/mod.rs | 12 ++++---- bindgen/codegen/serialize.rs | 4 +-- bindgen/codegen/struct_layout.rs | 6 ++-- bindgen/ir/analysis/derive.rs | 4 +-- bindgen/ir/comp.rs | 2 +- bindgen/ir/context.rs | 4 +-- bindgen/ir/item.rs | 4 +-- bindgen/lib.rs | 6 ++-- 11 files changed, 42 insertions(+), 44 deletions(-) diff --git a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs index 645235b254..6a6d0149f5 100644 --- a/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs +++ b/bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs @@ -20,7 +20,7 @@ impl ParseCallbacks for ItemDiscovery { fn test_item_discovery_callback( header: &str, - expected: HashMap, + expected: &HashMap, ) { let discovery = ItemDiscovery::default(); let info = Rc::clone(&discovery.0); @@ -34,7 +34,7 @@ fn test_item_discovery_callback( .generate() .expect("TODO: panic message"); - compare_item_caches(&info.borrow(), &expected); + compare_item_caches(&info.borrow(), expected); } #[test] @@ -103,7 +103,7 @@ fn test_item_discovery_callback_c() { ), ]); test_item_discovery_callback( - "/tests/parse_callbacks/item_discovery_callback/header_item_discovery.h", expected); + "/tests/parse_callbacks/item_discovery_callback/header_item_discovery.h", &expected); } #[test] @@ -125,7 +125,7 @@ fn test_item_discovery_callback_cpp() { ), ]); test_item_discovery_callback( - "/tests/parse_callbacks/item_discovery_callback/header_item_discovery.hpp", expected); + "/tests/parse_callbacks/item_discovery_callback/header_item_discovery.hpp", &expected); } pub fn compare_item_caches(generated: &ItemCache, expected: &ItemCache) { diff --git a/bindgen-tests/tests/quickchecking/src/fuzzers.rs b/bindgen-tests/tests/quickchecking/src/fuzzers.rs index eea0393239..176636f66f 100644 --- a/bindgen-tests/tests/quickchecking/src/fuzzers.rs +++ b/bindgen-tests/tests/quickchecking/src/fuzzers.rs @@ -1,5 +1,6 @@ use quickcheck::{Arbitrary, Gen}; use std::fmt; +use std::fmt::Write as _; /// `BaseTypeC` is used in generation of C headers to represent the C language's /// primitive types as well as `void*`. @@ -223,11 +224,10 @@ impl Arbitrary for DeclarationListC { /// Enables to string and format for `DeclarationListC` types. impl fmt::Display for DeclarationListC { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut display = String::new(); for decl in &self.decls { - display += &format!("{decl}"); + write!(f, "{decl}")?; } - write!(f, "{display}") + Ok(()) } } @@ -330,7 +330,7 @@ impl Arbitrary for ArrayDimensionC { for _ in 1..dimensions { // 16 is an arbitrary "not too big" number for capping array size. - def += &format!("[{}]", gen_range(g, lower_bound, 16)); + let _ = write!(def, "[{}]", gen_range(g, lower_bound, 16)); } ArrayDimensionC { def } } @@ -347,7 +347,7 @@ impl fmt::Display for ArrayDimensionC { /// identifiers unique. impl MakeUnique for BasicTypeDeclarationC { fn make_unique(&mut self, stamp: usize) { - self.ident_id += &format!("_{stamp}"); + let _ = write!(self.ident_id, "_{stamp}"); } } @@ -384,7 +384,7 @@ impl fmt::Display for BasicTypeDeclarationC { /// identifiers unique. impl MakeUnique for StructDeclarationC { fn make_unique(&mut self, stamp: usize) { - self.ident_id += &format!("_{stamp}"); + let _ = write!(self.ident_id, "_{stamp}"); } } @@ -432,7 +432,7 @@ impl fmt::Display for StructDeclarationC { /// identifiers unique. impl MakeUnique for UnionDeclarationC { fn make_unique(&mut self, stamp: usize) { - self.ident_id += &format!("_{stamp}"); + let _ = write!(self.ident_id, "_{stamp}"); } } @@ -480,7 +480,7 @@ impl fmt::Display for UnionDeclarationC { /// `FunctionPointerDeclarationC` identifiers unique. impl MakeUnique for FunctionPointerDeclarationC { fn make_unique(&mut self, stamp: usize) { - self.ident_id += &format!("_{stamp}"); + let _ = write!(self.ident_id, "_{stamp}"); } } @@ -517,7 +517,7 @@ impl fmt::Display for FunctionPointerDeclarationC { /// identifiers unique. impl MakeUnique for FunctionPrototypeC { fn make_unique(&mut self, stamp: usize) { - self.ident_id += &format!("_{stamp}"); + let _ = write!(self.ident_id, "_{stamp}"); } } @@ -586,14 +586,13 @@ impl Arbitrary for ParameterListC { /// Enables to string and format for `ParameterListC` types. impl fmt::Display for ParameterListC { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut display = String::new(); for (i, p) in self.params.iter().enumerate() { match i { - 0 => display += &format!("{p}"), - _ => display += &format!(",{p}"), + 0 => write!(f, "{p}")?, + _ => write!(f, ",{p}")?, } } - write!(f, "{display}") + Ok(()) } } @@ -612,11 +611,10 @@ impl Arbitrary for HeaderC { /// Enables to string and format for `HeaderC` types. impl fmt::Display for HeaderC { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut display = String::new(); for decl in &self.def.decls { - display += &format!("{decl}"); + write!(f, "{decl}")?; } - write!(f, "{display}") + Ok(()) } } diff --git a/bindgen-tests/tests/tests.rs b/bindgen-tests/tests/tests.rs index b808dfcd43..3cc7cbe415 100644 --- a/bindgen-tests/tests/tests.rs +++ b/bindgen-tests/tests/tests.rs @@ -27,7 +27,7 @@ fn should_overwrite_expected() -> bool { return true; } assert!( - var == "0" || var == "", + var == "0" || var.is_empty(), "Invalid value of BINDGEN_OVERWRITE_EXPECTED" ); } @@ -57,7 +57,7 @@ fn error_diff_mismatch( if let Some(var) = env::var_os("BINDGEN_TESTS_DIFFTOOL") { //usecase: var = "meld" -> You can hand check differences let Some(std::path::Component::Normal(name)) = - filename.components().last() + filename.components().next_back() else { panic!("Why is the header variable so weird?") }; @@ -187,7 +187,7 @@ fn compare_generated_header( header.display(), looked_at, ), - }; + } let (builder, roundtrip_builder) = builder.into_builder(check_roundtrip)?; diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index 1d5796b4b4..8da10ff051 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -3075,7 +3075,7 @@ impl Method { parent: parent_id, final_name: name.clone(), }, - ) + ); }); let mut function_name = function_item.canonical_name(ctx); @@ -3138,7 +3138,7 @@ impl Method { exprs[0] = quote! { self }; - }; + } let call = quote! { #function_name (#( #exprs ),* ) @@ -3282,7 +3282,7 @@ impl FromStr for EnumVariation { struct EnumBuilder { /// Type identifier of the enum. /// - /// This is the base name, i.e. for ModuleConst enums, this does not include the module name. + /// This is the base name, i.e. for `ModuleConst` enums, this does not include the module name. enum_type: Ident, /// Attributes applying to the enum type attrs: Vec, @@ -3453,7 +3453,7 @@ impl EnumBuilder { } fn newtype_bitfield_impl( - prefix: Ident, + prefix: &Ident, rust_ty: &syn::Type, ) -> proc_macro2::TokenStream { let rust_ty_name = &rust_ty; @@ -3593,7 +3593,7 @@ impl EnumBuilder { let prefix = ctx.trait_prefix(); let bitfield_impl_opt = is_bitfield - .then(|| Self::newtype_bitfield_impl(prefix, rust_ty)); + .then(|| Self::newtype_bitfield_impl(&prefix, rust_ty)); quote! { // Previously variant impls where before the enum definition. @@ -4625,7 +4625,7 @@ impl CodeGenerator for Function { FunctionKind::Method(ref method_kind) => { method_kind.is_pure_virtual() } - _ => false, + FunctionKind::Function => false, }; if is_pure_virtual && !ctx.options().generate_pure_virtual_functions { // Pure virtual methods have no actual symbol, so we can't generate diff --git a/bindgen/codegen/serialize.rs b/bindgen/codegen/serialize.rs index c7bb6cecef..9af48aa8ff 100644 --- a/bindgen/codegen/serialize.rs +++ b/bindgen/codegen/serialize.rs @@ -368,7 +368,7 @@ impl<'a> CSerialize<'a> for Type { match comp_info.kind() { CompKind::Struct => write!(writer, "struct {name}")?, CompKind::Union => write!(writer, "union {name}")?, - }; + } } TypeKind::Enum(_enum_ty) => { if self.is_const() { @@ -384,7 +384,7 @@ impl<'a> CSerialize<'a> for Type { loc: get_loc(item), }) } - }; + } if !stack.is_empty() { write!(writer, " ")?; diff --git a/bindgen/codegen/struct_layout.rs b/bindgen/codegen/struct_layout.rs index 0d97b8cc20..0d2e6a05c5 100644 --- a/bindgen/codegen/struct_layout.rs +++ b/bindgen/codegen/struct_layout.rs @@ -266,11 +266,11 @@ impl<'a> StructLayoutTracker<'a> { } }; - if !is_union { - self.latest_offset += field_layout.size; - } else { + if is_union { self.latest_offset = cmp::max(self.latest_offset, field_layout.size); + } else { + self.latest_offset += field_layout.size; } self.latest_field_layout = Some(field_layout); self.max_field_align = diff --git a/bindgen/ir/analysis/derive.rs b/bindgen/ir/analysis/derive.rs index 6c66998bee..eaa20fff46 100644 --- a/bindgen/ir/analysis/derive.rs +++ b/bindgen/ir/analysis/derive.rs @@ -197,7 +197,7 @@ impl CannotDerive<'_> { self.derive_trait ); } - }; + } return layout_can_derive; } @@ -355,7 +355,7 @@ impl CannotDerive<'_> { self.derive_trait ); } - }; + } return layout_can_derive; } } diff --git a/bindgen/ir/comp.rs b/bindgen/ir/comp.rs index 15f9cb4655..655e0f1fa5 100644 --- a/bindgen/ir/comp.rs +++ b/bindgen/ir/comp.rs @@ -1855,7 +1855,7 @@ impl IsOpaque for CompInfo { // See https://github.com/rust-lang/rust-bindgen/issues/537 and // https://github.com/rust-lang/rust/issues/33158 if self.is_packed(ctx, layout.as_ref()) && - layout.map_or(false, |l| l.align > 1) + layout.is_some_and(|l| l.align > 1) { warn!("Found a type that is both packed and aligned to greater than \ 1; Rust before version 1.33 doesn't have `#[repr(packed(N))]`, so we \ diff --git a/bindgen/ir/context.rs b/bindgen/ir/context.rs index 37f8d5968d..3f9e16ac9b 100644 --- a/bindgen/ir/context.rs +++ b/bindgen/ir/context.rs @@ -931,7 +931,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" *ty.kind() { typerefs.push((id, *ty, loc, parent_id)); - }; + } } typerefs } @@ -3076,7 +3076,7 @@ impl TemplateParameters for PartialType { num_params += 1; } _ => {} - }; + } clang_sys::CXChildVisit_Continue }); num_params diff --git a/bindgen/ir/item.rs b/bindgen/ir/item.rs index 8b0d24cd04..25c3c258aa 100644 --- a/bindgen/ir/item.rs +++ b/bindgen/ir/item.rs @@ -727,7 +727,7 @@ impl Item { to.push_str(&self.canonical_name(ctx)); if let ItemKind::Type(ref ty) = *self.kind() { if let TypeKind::TemplateInstantiation(ref inst) = *ty.kind() { - to.push_str(&format!("_open{level}_")); + let _ = write!(to, "_open{level}_"); for arg in inst.template_arguments() { arg.into_resolver() .through_type_refs() @@ -735,7 +735,7 @@ impl Item { .push_disambiguated_name(ctx, to, level + 1); to.push('_'); } - to.push_str(&format!("close{level}")); + let _ = write!(to, "close{level}"); } } } diff --git a/bindgen/lib.rs b/bindgen/lib.rs index 5423f00495..12ac8a2998 100644 --- a/bindgen/lib.rs +++ b/bindgen/lib.rs @@ -575,7 +575,7 @@ impl BindgenOptions { self.parse_callbacks .iter() .filter_map(|cb| f(cb.as_ref())) - .last() + .next_back() } fn all_callbacks( @@ -796,7 +796,7 @@ impl Bindings { 0, format!("--target={effective_target}").into_boxed_str(), ); - }; + } fn detect_include_paths(options: &mut BindgenOptions) { if !options.detect_include_paths { @@ -1209,7 +1209,7 @@ pub fn clang_version() -> ClangVersion { }; } } - }; + } ClangVersion { parsed: None, full: raw_v.clone(),