Skip to content

Commit

Permalink
Merge pull request #1010 from godot-rust/qol/nodepath-subpath-clippy
Browse files Browse the repository at this point in the history
Follow clippy 1.84; limit `NodePath::subpath()` polyfill again
  • Loading branch information
Bromeon authored Jan 11, 2025
2 parents 6a5d19d + a23bf9f commit 3dafef1
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 46 deletions.
14 changes: 8 additions & 6 deletions godot-codegen/src/special_cases/codegen_special_cases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,16 @@ pub(crate) fn is_class_method_excluded(method: &JsonClassMethod, ctx: &mut Conte
};

// Exclude if return type contains an excluded type.
if method.return_value.as_ref().map_or(false, |ret| {
is_arg_or_return_excluded(ret.type_.as_str(), ctx)
}) {
if method
.return_value
.as_ref()
.is_some_and(|ret| is_arg_or_return_excluded(ret.type_.as_str(), ctx))
{
return true;
}

// Exclude if any argument contains an excluded type.
if method.arguments.as_ref().map_or(false, |args| {
if method.arguments.as_ref().is_some_and(|args| {
args.iter()
.any(|arg| is_arg_or_return_excluded(arg.type_.as_str(), ctx))
}) {
Expand All @@ -107,8 +109,8 @@ pub(crate) fn is_utility_function_excluded(
function
.return_type
.as_ref()
.map_or(false, |ret| is_type_excluded(ret.as_str(), ctx))
|| function.arguments.as_ref().map_or(false, |args| {
.is_some_and(|ret| is_type_excluded(ret.as_str(), ctx))
|| function.arguments.as_ref().is_some_and(|args| {
args.iter()
.any(|arg| is_type_excluded(arg.type_.as_str(), ctx))
})
Expand Down
32 changes: 18 additions & 14 deletions godot-core/src/builtin/string/node_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use std::fmt;

use godot_ffi as sys;
use godot_ffi::{ffi_methods, GodotFfi};
use godot_ffi::{ffi_methods, GdextBuild, GodotFfi};

use crate::builtin::inner;

Expand Down Expand Up @@ -142,19 +142,23 @@ impl NodePath {
#[cfg(since_api = "4.3")]
#[doc(alias = "slice")]
pub fn subpath(&self, begin: i32, exclusive_end: i32) -> NodePath {
// Polyfill for bug https://github.com/godotengine/godot/pull/100954.
// TODO(v0.3) make polyfill (everything but last line) conditional if PR is merged in 4.4.
let name_count = self.get_name_count() as i32;
let subname_count = self.get_subname_count() as i32;
let total_count = name_count + subname_count;

let mut begin = begin.clamp(-total_count, total_count);
if begin < 0 {
begin += total_count;
}
if begin > name_count {
begin += 1;
}
// Polyfill for bug https://github.com/godotengine/godot/pull/100954, fixed in 4.4.
let begin = if GdextBuild::since_api("4.4") {
begin
} else {
let name_count = self.get_name_count() as i32;
let subname_count = self.get_subname_count() as i32;
let total_count = name_count + subname_count;

let mut begin = begin.clamp(-total_count, total_count);
if begin < 0 {
begin += total_count;
}
if begin > name_count {
begin += 1;
}
begin
};

self.as_inner().slice(begin as i64, exclusive_end as i64)
}
Expand Down
6 changes: 3 additions & 3 deletions godot-core/src/builtin/variant/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,9 +453,9 @@ impl Drop for Variant {
// Variant is not Eq because it can contain floats and other types composed of floats.
impl PartialEq for Variant {
fn eq(&self, other: &Self) -> bool {
Self::evaluate(self, other, VariantOperator::EQUAL)
.map(|v| v.to::<bool>())
.unwrap_or(false) // if there is no defined conversion, then they are non-equal
Self::evaluate(self, other, VariantOperator::EQUAL) //.
.is_some_and(|v| v.to::<bool>())
// If there is no defined conversion (-> None), then they are non-equal.
}
}

Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/meta/args/ref_arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,6 @@ where
}

fn is_null(&self) -> bool {
self.shared_ref.map(|r| r.is_null()).unwrap_or(true)
self.shared_ref.map_or(true, T::is_null)
}
}
6 changes: 2 additions & 4 deletions godot-core/src/obj/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,7 @@ impl MemDynamic {
/// Check whether dynamic type is ref-counted.
fn inherits_refcounted<T: GodotClass>(obj: &RawGd<T>) -> bool {
obj.instance_id_unchecked()
.map(|id| id.is_ref_counted())
.unwrap_or(false)
.is_some_and(|id| id.is_ref_counted())
}
}
impl Sealed for MemDynamic {}
Expand Down Expand Up @@ -301,8 +300,7 @@ impl DynMemory for MemDynamic {
out!(" MemDyn::dec <{}>", std::any::type_name::<T>());
if obj
.instance_id_unchecked()
.map(|id| id.is_ref_counted())
.unwrap_or(false)
.is_some_and(|id| id.is_ref_counted())
{
// Will call `RefCounted::unreference()` which checks for liveness.
MemRefCounted::maybe_dec_ref(obj)
Expand Down
3 changes: 1 addition & 2 deletions godot-core/src/obj/raw_gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,7 @@ impl<T: GodotClass> RawGd<T> {
pub(crate) fn is_instance_valid(&self) -> bool {
self.cached_rtti
.as_ref()
.map(|rtti| rtti.instance_id().lookup_validity())
.unwrap_or(false)
.is_some_and(|rtti| rtti.instance_id().lookup_validity())
}

// See use-site for explanation.
Expand Down
2 changes: 1 addition & 1 deletion godot-ffi/src/toolbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ fn strip_module_paths(full_name: &str) -> String {
result.push(c);

// Handle spaces after commas for readability.
if c == ',' && chars.peek().map_or(false, |&next_c| next_c != ' ') {
if c == ',' && chars.peek().is_some_and(|&next_c| next_c != ' ') {
result.push(' ');
}
}
Expand Down
20 changes: 8 additions & 12 deletions godot-macros/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ fn delimiter_opening_char(delimiter: Delimiter) -> char {
/// declaration of the form `impl MyTrait for SomeType`. The type `SomeType` is irrelevant in this example.
pub(crate) fn is_impl_named(original_impl: &venial::Impl, name: &str) -> bool {
let trait_name = original_impl.trait_ty.as_ref().unwrap(); // unwrap: already checked outside
extract_typename(trait_name).map_or(false, |seg| seg.ident == name)
extract_typename(trait_name).is_some_and(|seg| seg.ident == name)
}

/// Validates either:
Expand Down Expand Up @@ -228,27 +228,23 @@ pub(crate) fn path_is_single(path: &[TokenTree], expected: &str) -> bool {

pub(crate) fn path_ends_with(path: &[TokenTree], expected: &str) -> bool {
// Could also use TypeExpr::as_path(), or fn below this one.
path.last()
.map(|last| last.to_string() == expected)
.unwrap_or(false)
path.last().is_some_and(|last| last.to_string() == expected)
}

pub(crate) fn path_ends_with_complex(path: &venial::TypeExpr, expected: &str) -> bool {
path.as_path()
.map(|path| {
path.segments
.last()
.map_or(false, |seg| seg.ident == expected)
})
.unwrap_or(false)
path.as_path().is_some_and(|path| {
path.segments
.last()
.is_some_and(|seg| seg.ident == expected)
})
}

pub(crate) fn extract_cfg_attrs(
attrs: &[venial::Attribute],
) -> impl IntoIterator<Item = &venial::Attribute> {
attrs.iter().filter(|attr| {
attr.get_single_path_segment()
.map_or(false, |name| name == "cfg")
.is_some_and(|name| name == "cfg")
})
}

Expand Down
4 changes: 1 addition & 3 deletions itest/rust/src/framework/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,7 @@ fn print_test_pre(test_case: &str, test_file: String, last_file: &mut Option<Str

fn print_file_header(file: String, last_file: &mut Option<String>) {
// Check if we need to open a new category for a file.
let print_file = last_file
.as_ref()
.map_or(true, |last_file| last_file != &file);
let print_file = last_file.as_ref() != Some(&file);

if print_file {
println!("\n {}:", extract_file_subtitle(&file));
Expand Down

0 comments on commit 3dafef1

Please sign in to comment.