Skip to content

Commit

Permalink
Multiple minor lint fixes
Browse files Browse the repository at this point in the history
Added all detected pedantic lints, and allowed them all to keep track of them because some should be fixed.  Also fixed a few minor simple ones.
  • Loading branch information
nyurik authored and pvdrz committed Jan 2, 2025
1 parent 0917399 commit 78adb33
Show file tree
Hide file tree
Showing 13 changed files with 87 additions and 64 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/bindgen.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
run: cargo fmt -- --check

- name: Run clippy
run: cargo clippy --tests
run: cargo clippy --all-targets --workspace --exclude bindgen-integration --exclude tests_expectations

msrv:
runs-on: ubuntu-latest
Expand Down
53 changes: 43 additions & 10 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,51 @@ syn = "2.0"
tempfile = "3"

[workspace.lints.rust]
# unused_qualifications = "warn"
unused_qualifications = "warn"

[workspace.lints.clippy]
# disallowed-names = "allow"
# manual-c-str-literals = "allow"
# missing-safety-doc = "allow"
# op-ref = "allow"
# ptr-offset-with-cast = "allow"
# too-many-arguments = "allow"
# transmute-int-to-bool = "allow"
# unnecessary-cast = "allow"
# useless-transmute = "allow"
pedantic = { level = "warn", priority = -1 }

cast_possible_truncation = "allow"
cast_possible_wrap = "allow"
cast_precision_loss = "allow"
cast_sign_loss = "allow"
checked_conversions = "allow"
default_trait_access = "allow"
explicit_into_iter_loop = "allow"
flat_map_option = "allow"
ignored_unit_patterns = "allow"
implicit_hasher = "allow"
inconsistent_struct_constructor = "allow"
items_after_statements = "allow"
maybe_infinite_iter = "allow"
missing_errors_doc = "allow"
missing_panics_doc = "allow"
module_name_repetitions = "allow"
must_use_candidate = "allow"
ptr_as_ptr = "allow"
redundant_closure_for_method_calls = "allow"
return_self_not_must_use = "allow"
#should_panic_without_expect = "allow"
similar_names = "allow"
struct_excessive_bools = "allow"
struct_field_names = "allow"
unnecessary_wraps = "allow"
unnested_or_patterns = "allow"
unreadable_literal = "allow"
used_underscore_binding = "allow"
wildcard_imports = "allow"

# TODO
borrow_as_ptr = "allow"
match_same_arms = "allow"
trivially_copy_pass_by_ref = "allow"
needless_pass_by_value = "allow"
unused_self = "allow"

# Theese seem to be ok to ignore for now
enum_glob_use = "allow"
too_many_lines = "allow"

# Config for 'cargo release'
[workspace.metadata.release]
Expand Down
19 changes: 7 additions & 12 deletions bindgen-tests/tests/parse_callbacks/item_discovery_callback/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,10 @@ pub fn compare_item_caches(generated: ItemCache, expected: ItemCache) {
)
});

if found.is_none() {
panic!(
"Missing Expected Item: {:#?}\n in {:#?}",
expected_item, generated
);
}
assert!(
found.is_some(),
"Missing Expected Item: {expected_item:#?}\n in {generated:#?}"
);
}
}

Expand Down Expand Up @@ -235,12 +233,9 @@ pub fn compare_alias_info(
let expected_aliased = expected.get(expected_alias_for).unwrap();

// We must have the aliased type in the cache
let generated_aliased =
if let Some(generated_aliased) = generated.get(generated_alias_for) {
generated_aliased
} else {
return false;
};
let Some(generated_aliased) = generated.get(generated_alias_for) else {
return false;
};

compare_item_info(expected_aliased, generated_aliased, expected, generated)
}
7 changes: 4 additions & 3 deletions bindgen-tests/tests/quickchecking/src/bin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@ fn parse_tests_count(v: &str) -> Result<u64, String> {
// Parse CLI argument input for fuzzed headers output path.
fn parse_path(v: &str) -> Result<PathBuf, String> {
let path = PathBuf::from(v);
match path.is_dir() {
true => Ok(path),
false => Err(String::from("Provided directory path does not exist.")),
if path.is_dir() {
Ok(path)
} else {
Err(String::from("Provided directory path does not exist."))
}
}

Expand Down
7 changes: 4 additions & 3 deletions bindgen-tests/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ fn should_overwrite_expected() -> bool {
if var == "1" {
return true;
}
if var != "0" && var != "" {
panic!("Invalid value of BINDGEN_OVERWRITE_EXPECTED");
}
assert!(
var == "0" || var == "",
"Invalid value of BINDGEN_OVERWRITE_EXPECTED"
);
}
false
}
Expand Down
2 changes: 1 addition & 1 deletion bindgen/clang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1761,7 +1761,7 @@ impl File {

fn cxstring_to_string_leaky(s: CXString) -> String {
if s.data.is_null() {
return "".to_owned();
return String::new();
}
let c_str = unsafe { CStr::from_ptr(clang_getCString(s) as *const _) };
c_str.to_string_lossy().into_owned()
Expand Down
4 changes: 2 additions & 2 deletions bindgen/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4565,7 +4565,7 @@ impl CodeGenerator for Function {
FunctionKind::Function => {
ctx.options().dynamic_library_name.is_some()
}
_ => false,
FunctionKind::Method(_) => false,
};

// Similar to static member variables in a class template, we can't
Expand Down Expand Up @@ -5878,7 +5878,7 @@ pub(crate) mod utils {

// Check that the mangled name contains the canonical name after the
// prefix
if &mangled_name[1..canonical_name.len() + 1] != canonical_name {
if &mangled_name[1..=canonical_name.len()] != canonical_name {
return false;
}

Expand Down
26 changes: 11 additions & 15 deletions bindgen/ir/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2271,21 +2271,17 @@ If you encounter an error missing from this list, please file an issue or a PR!"
);
}
break;
} else {
// This is _likely_, but not certainly, a macro that's
// been placed just before the namespace keyword.
// Unfortunately, clang tokens don't let us easily see
// through the ifdef tokens, so we don't know what this
// token should really be. Instead of panicking though,
// we warn the user that we assumed the token was blank,
// and then move on.
//
// See also https://github.com/rust-lang/rust-bindgen/issues/1676.
warn!(
"Ignored unknown namespace prefix '{}' at {token:?} in {cursor:?}",
String::from_utf8_lossy(name),
);
}
// This is _likely_, but not certainly, a macro that's
// been placed just before the namespace keyword.
// Unfortunately, clang tokens don't let us easily see
// through the ifdef tokens, so we don't know what this
// token should really be. Instead of panicking though,
// we warn the user that we assumed the token was blank,
// and then move on.
//
// See also https://github.com/rust-lang/rust-bindgen/issues/1676.
warn!("Ignored unknown namespace prefix '{}' at {token:?} in {cursor:?}", String::from_utf8_lossy(name));
}
}
}
Expand Down Expand Up @@ -2613,7 +2609,7 @@ If you encounter an error missing from this list, please file an issue or a PR!"

/// Call if an opaque array is generated
pub(crate) fn generated_opaque_array(&self) {
self.generated_opaque_array.set(true)
self.generated_opaque_array.set(true);
}

/// Whether we need to generate the opaque array type
Expand Down
2 changes: 1 addition & 1 deletion bindgen/ir/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::str::FromStr;

const RUST_DERIVE_FUNPTR_LIMIT: usize = 12;

/// What kind of a function are we looking at?
/// What kind of function are we looking at?
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub(crate) enum FunctionKind {
/// A plain, free function.
Expand Down
13 changes: 4 additions & 9 deletions bindgen/ir/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1856,16 +1856,11 @@ impl Item {
let parent = ctx.root_module().into();

if let Some(id) = ctx.get_type_param(&definition) {
if let Some(with_id) = with_id {
return Some(ctx.build_ty_wrapper(
with_id,
id,
Some(parent),
&ty,
));
return Some(if let Some(with_id) = with_id {
ctx.build_ty_wrapper(with_id, id, Some(parent), &ty)
} else {
return Some(id);
}
id
});
}

// See tests/headers/const_tparam.hpp and
Expand Down
4 changes: 2 additions & 2 deletions bindgen/ir/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,15 +504,15 @@ fn is_invalid_type_param_invalid_remaining() {
}

#[test]
#[should_panic]
#[should_panic(expected = "Unnamed named type")]
fn is_invalid_type_param_unnamed() {
let ty = Type::new(None, None, TypeKind::TypeParam, false);
assert!(ty.is_invalid_type_param());
}

#[test]
fn is_invalid_type_param_empty_name() {
let ty = Type::new(Some("".into()), None, TypeKind::TypeParam, false);
let ty = Type::new(Some(String::new()), None, TypeKind::TypeParam, false);
assert!(ty.is_invalid_type_param());
}

Expand Down
10 changes: 6 additions & 4 deletions bindgen/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,12 @@ 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 {
name_file.ends_with(".hpp") ||
name_file.ends_with(".hxx") ||
name_file.ends_with(".hh") ||
name_file.ends_with(".h++")
Path::new(name_file).extension().map_or(false, |ext| {
ext.eq_ignore_ascii_case("hpp") ||
ext.eq_ignore_ascii_case("hxx") ||
ext.eq_ignore_ascii_case("hh") ||
ext.eq_ignore_ascii_case("h++")
})
}

fn args_are_cpp(clang_args: &[Box<str>]) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion bindgen/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl<'a> Timer<'a> {

/// Returns the time elapsed since the timer's creation
pub fn elapsed(&self) -> Duration {
Instant::now() - self.start
self.start.elapsed()
}

fn print_elapsed(&mut self) {
Expand Down

0 comments on commit 78adb33

Please sign in to comment.