Skip to content

chore: fix last clippy lints and enforce in CI #3106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 12 additions & 45 deletions .github/workflows/bindgen.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,27 @@ on:
- main

jobs:
rustfmt-clippy:
rustfmt:
runs-on: ubuntu-latest

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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This TODO is still accurate afaict, let's keep it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I had this written before and somehow didn't submit. Nevermind tho

toolchain: nightly
components: rustfmt, clippy
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
run: cargo clippy --all-targets --workspace --exclude bindgen-integration --exclude tests_expectations -- -D warnings

msrv:
runs-on: ubuntu-latest
Expand Down Expand Up @@ -64,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:
Expand All @@ -79,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`
Expand All @@ -95,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
Expand All @@ -112,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

Expand Down Expand Up @@ -174,21 +156,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:
Expand Down Expand Up @@ -221,7 +188,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.
Expand Down
2 changes: 1 addition & 1 deletion bindgen-tests/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl ParseCallbacks for ItemDiscovery {

fn test_item_discovery_callback(
header: &str,
expected: HashMap<DiscoveredItemId, DiscoveredItem>,
expected: &HashMap<DiscoveredItemId, DiscoveredItem>,
) {
let discovery = ItemDiscovery::default();
let info = Rc::clone(&discovery.0);
Expand All @@ -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]
Expand Down Expand Up @@ -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]
Expand All @@ -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) {
Expand Down
30 changes: 14 additions & 16 deletions bindgen-tests/tests/quickchecking/src/fuzzers.rs
Original file line number Diff line number Diff line change
@@ -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*`.
Expand Down Expand Up @@ -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(())
}
}

Expand Down Expand Up @@ -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 }
}
Expand All @@ -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}");
}
}

Expand Down Expand Up @@ -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}");
}
}

Expand Down Expand Up @@ -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}");
}
}

Expand Down Expand Up @@ -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}");
}
}

Expand Down Expand Up @@ -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}");
}
}

Expand Down Expand Up @@ -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(())
}
}

Expand All @@ -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(())
}
}

Expand Down
6 changes: 3 additions & 3 deletions bindgen-tests/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}
Expand Down Expand Up @@ -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?")
};
Expand Down Expand Up @@ -187,7 +187,7 @@ fn compare_generated_header(
header.display(),
looked_at,
),
};
}

let (builder, roundtrip_builder) = builder.into_builder(check_roundtrip)?;

Expand Down
14 changes: 7 additions & 7 deletions bindgen/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3075,7 +3075,7 @@ impl Method {
parent: parent_id,
final_name: name.clone(),
},
)
);
});

let mut function_name = function_item.canonical_name(ctx);
Expand Down Expand Up @@ -3138,7 +3138,7 @@ impl Method {
exprs[0] = quote! {
self
};
};
}

let call = quote! {
#function_name (#( #exprs ),* )
Expand Down Expand Up @@ -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<proc_macro2::TokenStream>,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions bindgen/codegen/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -384,7 +384,7 @@ impl<'a> CSerialize<'a> for Type {
loc: get_loc(item),
})
}
};
}

if !stack.is_empty() {
write!(writer, " ")?;
Expand Down
6 changes: 3 additions & 3 deletions bindgen/codegen/struct_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Loading
Loading