From 3ac00ed52c9d3a2db7a08fc3f45ecd26ae8722e0 Mon Sep 17 00:00:00 2001 From: Johan Andersson Date: Wed, 10 Mar 2021 13:42:55 +0100 Subject: [PATCH] Use Embark lints v0.2 (#128) Switches to use our new standard template for defining shared Rust and Clippy lints for our Embark projects, https://github.com/EmbarkStudios/rust-ecosystem/issues/59. Did a couple of lint fixes from this but didn't want to make the change too intrusive so added some allow exceptions for now that can be revisited --- physx-sys/src/lib.rs | 69 ++++++++++++++++++++++---- physx/src/aggregate.rs | 6 +-- physx/src/articulation_base.rs | 4 +- physx/src/base.rs | 47 +++++++++--------- physx/src/lib.rs | 60 +++++++++++++++++++--- physx/src/owner.rs | 1 + physx/src/physics.rs | 4 +- physx/src/scene.rs | 14 +++--- physx/src/simulation_event_callback.rs | 35 ++++++------- physx/src/visual_debugger.rs | 10 ++-- 10 files changed, 169 insertions(+), 81 deletions(-) diff --git a/physx-sys/src/lib.rs b/physx-sys/src/lib.rs index d5f111de..69145091 100644 --- a/physx-sys/src/lib.rs +++ b/physx-sys/src/lib.rs @@ -1,9 +1,3 @@ -#![allow(non_upper_case_globals)] -#![allow(non_camel_case_types)] -#![allow(non_snake_case)] -#![allow(clippy::unreadable_literal)] -#![allow(clippy::unused_unit)] - //! # 🎳 physx-sys //! //! ![Build Status](https://github.com/EmbarkStudios/physx-rs/workflows/CI/badge.svg) @@ -128,6 +122,63 @@ //! license, shall be dual licensed as above, without any additional terms or //! conditions. +// BEGIN - Embark standard lints v0.2. +// do not change or add/remove here, but one can add exceptions after this section +// for more info see: +#![deny(unsafe_code)] +#![warn( + clippy::all, + clippy::await_holding_lock, + clippy::dbg_macro, + clippy::debug_assert_with_mut_call, + clippy::doc_markdown, + clippy::empty_enum, + clippy::enum_glob_use, + clippy::exit, + clippy::explicit_into_iter_loop, + clippy::filter_map_next, + clippy::fn_params_excessive_bools, + clippy::if_let_mutex, + clippy::imprecise_flops, + clippy::inefficient_to_string, + clippy::let_unit_value, + clippy::linkedlist, + clippy::lossy_float_literal, + clippy::macro_use_imports, + clippy::map_flatten, + clippy::map_unwrap_or, + clippy::match_on_vec_items, + clippy::match_wildcard_for_single_variants, + clippy::mem_forget, + clippy::mismatched_target_os, + clippy::needless_borrow, + clippy::needless_continue, + clippy::option_option, + clippy::pub_enum_variant_names, + clippy::ref_option_ref, + clippy::rest_pat_in_fully_bound_structs, + clippy::string_to_string, + clippy::suboptimal_flops, + clippy::todo, + clippy::unnested_or_patterns, + clippy::unused_self, + clippy::verbose_file_reads, + future_incompatible, + nonstandard_style, + rust_2018_idioms +)] +// END - Embark standard lints v0.2 +// crate-specific exceptions: +#![allow( + unsafe_code, + non_upper_case_globals, + non_camel_case_types, + non_snake_case, + clippy::doc_markdown, // TODO: fixup comments and docs (though annoyingly complains about "PhysX") + clippy::unreadable_literal, + clippy::unused_unit +)] + #[cfg(feature = "structgen")] include!(concat!(env!("OUT_DIR"), "/structgen_out.rs")); @@ -226,9 +277,9 @@ impl Default for SimulationEventCallbackInfo { } } -/// return 0 = PxQueryHitType::eNONE -/// return 1 = PxQueryHitType::eTOUCH -/// return 2 = PxQueryHitType::eBLOCK +/// return 0 = `PxQueryHitType::eNONE` +/// return 1 = `PxQueryHitType::eTOUCH` +/// return 2 = `PxQueryHitType::eBLOCK` pub type RaycastHitCallback = unsafe extern "C" fn( *const PxRigidActor, *const PxFilterData, diff --git a/physx/src/aggregate.rs b/physx/src/aggregate.rs index 54f35289..656c8570 100644 --- a/physx/src/aggregate.rs +++ b/physx/src/aggregate.rs @@ -142,7 +142,7 @@ pub trait Aggregate: Class + Base { PxAggregate_addActor_mut( self.as_mut_ptr(), actor.as_mut_ptr(), - bvh.map(Class::as_ptr).unwrap_or(null()), + bvh.map_or(null(), Class::as_ptr), ) } } @@ -157,7 +157,7 @@ pub trait Aggregate: Class + Base { PxAggregate_addActor_mut( self.as_mut_ptr(), actor.as_mut_ptr(), - bvh.map(Class::as_ptr).unwrap_or(null()), + bvh.map_or(null(), Class::as_ptr), ) } } @@ -172,7 +172,7 @@ pub trait Aggregate: Class + Base { PxAggregate_addActor_mut( self.as_mut_ptr(), actor.as_mut_ptr(), - bvh.map(Class::as_ptr).unwrap_or(null()), + bvh.map_or(null(), Class::as_ptr), ) } } diff --git a/physx/src/articulation_base.rs b/physx/src/articulation_base.rs index 3392c8cc..495786c2 100644 --- a/physx/src/articulation_base.rs +++ b/physx/src/articulation_base.rs @@ -226,9 +226,7 @@ pub trait ArticulationBase: Class + Base { unsafe { (PxArticulationBase_createLink_mut( self.as_mut_ptr(), - parent - .map(|parent| parent.as_mut_ptr()) - .unwrap_or(null_mut()), + parent.map_or(null_mut(), |parent| parent.as_mut_ptr()), pose.as_ptr(), ) as *mut Self::ArticulationLink) .as_mut() diff --git a/physx/src/base.rs b/physx/src/base.rs index 373cbc51..21317314 100644 --- a/physx/src/base.rs +++ b/physx/src/base.rs @@ -65,31 +65,30 @@ pub enum ConcreteType { impl From for ConcreteType { fn from(val: u16) -> Self { - use ConcreteType::*; match val { - 0 => Undefined, - 1 => Heightfield, - 2 => ConvexMesh, - 3 => TriangleMeshBvh33, - 4 => TriangleMeshBvh34, - 5 => RigidDynamic, - 6 => RigidStatic, - 7 => Shape, - 8 => Material, - 9 => Constraint, - 10 => Aggregate, - 11 => Articulation, - 12 => ArticulationReducedCoordinate, - 13 => ArticulationLink, - 14 => ArticulationJoint, - 15 => ArticulationJointReducedCoordinate, - 16 => PruningStructure, - 17 => BvhStructure, - 18 => PhysxCoreCount, - 256 => FirstPhysxExtension, - 512 => FirstVehicleExtension, - 1024 => FirstUserExtension, - _ => Undefined, + 0 => ConcreteType::Undefined, + 1 => ConcreteType::Heightfield, + 2 => ConcreteType::ConvexMesh, + 3 => ConcreteType::TriangleMeshBvh33, + 4 => ConcreteType::TriangleMeshBvh34, + 5 => ConcreteType::RigidDynamic, + 6 => ConcreteType::RigidStatic, + 7 => ConcreteType::Shape, + 8 => ConcreteType::Material, + 9 => ConcreteType::Constraint, + 10 => ConcreteType::Aggregate, + 11 => ConcreteType::Articulation, + 12 => ConcreteType::ArticulationReducedCoordinate, + 13 => ConcreteType::ArticulationLink, + 14 => ConcreteType::ArticulationJoint, + 15 => ConcreteType::ArticulationJointReducedCoordinate, + 16 => ConcreteType::PruningStructure, + 17 => ConcreteType::BvhStructure, + 18 => ConcreteType::PhysxCoreCount, + 256 => ConcreteType::FirstPhysxExtension, + 512 => ConcreteType::FirstVehicleExtension, + 1024 => ConcreteType::FirstUserExtension, + _ => ConcreteType::Undefined, } } } diff --git a/physx/src/lib.rs b/physx/src/lib.rs index c51a6fbf..b2d97e94 100644 --- a/physx/src/lib.rs +++ b/physx/src/lib.rs @@ -1,10 +1,3 @@ -// Author: Tom Olsson -// Copyright © 2019, Embark Studios, all rights reserved. -// Created: 2 April 2019 - -#![warn(clippy::all)] -#![deny(rust_2018_idioms)] - //! # 🎳 physx //! //! ![Build Status](https://github.com/EmbarkStudios/physx-rs/workflows/CI/badge.svg) @@ -100,6 +93,59 @@ //! license, shall be dual licensed as above, without any additional terms or //! conditions. +// BEGIN - Embark standard lints v0.2. +// do not change or add/remove here, but one can add exceptions after this section +// for more info see: +#![deny(unsafe_code)] +#![warn( + clippy::all, + clippy::await_holding_lock, + clippy::dbg_macro, + clippy::debug_assert_with_mut_call, + clippy::doc_markdown, + clippy::empty_enum, + clippy::enum_glob_use, + clippy::exit, + clippy::explicit_into_iter_loop, + clippy::filter_map_next, + clippy::fn_params_excessive_bools, + clippy::if_let_mutex, + clippy::imprecise_flops, + clippy::inefficient_to_string, + clippy::let_unit_value, + clippy::linkedlist, + clippy::lossy_float_literal, + clippy::macro_use_imports, + clippy::map_flatten, + clippy::map_unwrap_or, + clippy::match_on_vec_items, + clippy::match_wildcard_for_single_variants, + clippy::mem_forget, + clippy::mismatched_target_os, + clippy::needless_borrow, + clippy::needless_continue, + clippy::option_option, + clippy::pub_enum_variant_names, + clippy::ref_option_ref, + clippy::rest_pat_in_fully_bound_structs, + clippy::string_to_string, + clippy::suboptimal_flops, + clippy::todo, + clippy::unnested_or_patterns, + clippy::unused_self, + clippy::verbose_file_reads, + future_incompatible, + nonstandard_style, + rust_2018_idioms +)] +// END - Embark standard lints v0.2 +// crate-specific exceptions: +#![allow( + unsafe_code, // this is a safe wrapper of unsafe code, so plenty of unsafe code in here + clippy::doc_markdown, // TODO: fixup comments and docs (though annoyingly complains about "PhysX") + clippy::pub_enum_variant_names, // TODO: revisit, will change API, remove from standard set? +)] + // Utility traits pub mod traits; diff --git a/physx/src/owner.rs b/physx/src/owner.rs index 19b37bb7..0335d264 100644 --- a/physx/src/owner.rs +++ b/physx/src/owner.rs @@ -25,6 +25,7 @@ impl Owner { } /// Consumes the Owner without calling Drop and returns the raw pointer it was wrapping. + #[allow(clippy::mem_forget)] pub fn into_ptr(mut self) -> *mut S where T: Class, diff --git a/physx/src/physics.rs b/physx/src/physics.rs index d1e62162..6238bcb4 100644 --- a/physx/src/physics.rs +++ b/physx/src/physics.rs @@ -717,9 +717,7 @@ impl PhysicsFoundationBuilder { unsafe { phys_PxInitExtensions( physics.as_mut_ptr(), - pvd.as_mut() - .map(|pv| pv.as_mut_ptr()) - .unwrap_or_else(null_mut), + pvd.as_mut().map_or_else(null_mut, |pv| pv.as_mut_ptr()), ) } } else { diff --git a/physx/src/scene.rs b/physx/src/scene.rs index 9ff3c895..a9fc6a45 100644 --- a/physx/src/scene.rs +++ b/physx/src/scene.rs @@ -569,7 +569,7 @@ pub trait Scene: Class + UserData { completion_task: Option<&mut PxBaseTask>, scratch: Option<&mut ScratchBuffer>, ) { - let completion_task = completion_task.map(|t| t as *mut _).unwrap_or(null_mut()); + let completion_task = completion_task.map_or(null_mut(), |t| t as *mut _); let (scratch_ptr, scratch_size) = if let Some(scratch) = scratch { scratch.as_ptr_and_size() @@ -761,19 +761,19 @@ pub trait Scene: Class + UserData { // Callbacks /// # Safety - /// PxContactModifyCallback does not have a safe wrapper, using it requires use of physx_sys. + /// PxContactModifyCallback does not have a safe wrapper, using it requires use of [`physx_sys`]. unsafe fn set_contact_modify_callback(&mut self, callback: &mut PxContactModifyCallback) { PxScene_setContactModifyCallback_mut(self.as_mut_ptr(), callback); } /// # Safety - /// PxContactModifyCallback does not have a safe wrapper, using it requires use of physx_sys. + /// PxContactModifyCallback does not have a safe wrapper, using it requires use of [`physx_sys`]. unsafe fn get_contact_modify_callback(&self) -> &PxContactModifyCallback { &*PxScene_getContactModifyCallback(self.as_ptr()) } /// # Safety - /// PxCCDContactModifyCallback does not have a safe wrapper, using it requires use of physx_sys. + /// PxCCDContactModifyCallback does not have a safe wrapper, using it requires use of [`physx_sys`]. unsafe fn set_ccd_contact_modify_callback( &mut self, callback: &mut PxCCDContactModifyCallback, @@ -782,19 +782,19 @@ pub trait Scene: Class + UserData { } /// # Safety - /// PxCCDContactModifyCallback does not have a safe wrapper, using it requires use of physx_sys. + /// PxCCDContactModifyCallback does not have a safe wrapper, using it requires use of [`physx_sys`]. unsafe fn get_ccd_contact_callback(&self) -> &PxCCDContactModifyCallback { &*PxScene_getCCDContactModifyCallback(self.as_ptr()) } /// # Safety - /// PxBroadPhaseCallback does not have a safe wrapper, using it requires use of physx_sys. + /// PxBroadPhaseCallback does not have a safe wrapper, using it requires use of [`physx_sys`]. unsafe fn set_broad_phase_callback(&mut self, callback: &mut PxBroadPhaseCallback) { PxScene_setBroadPhaseCallback_mut(self.as_mut_ptr(), callback); } /// # Safety - /// PxBroadPhaseCallback does not have a safe wrapper, using it requires use of physx_sys. + /// PxBroadPhaseCallback does not have a safe wrapper, using it requires use of [`physx_sys`]. unsafe fn get_broad_phase_callback(&self) -> &PxBroadPhaseCallback { &*PxScene_getBroadPhaseCallback(self.as_ptr()) } diff --git a/physx/src/simulation_event_callback.rs b/physx/src/simulation_event_callback.rs index 0310bced..3c515f0a 100644 --- a/physx/src/simulation_event_callback.rs +++ b/physx/src/simulation_event_callback.rs @@ -84,21 +84,16 @@ where OA: AdvanceCallback, { unsafe { - let (collision_callback, collision_user_data) = on_collide - .map(OC::into_cb_user_data) - .unwrap_or((None, null_mut())); - let (trigger_callback, trigger_user_data) = on_trigger - .map(OT::into_cb_user_data) - .unwrap_or((None, null_mut())); - let (constraint_break_callback, constraint_break_user_data) = on_constraint_break - .map(OCB::into_cb_user_data) - .unwrap_or((None, null_mut())); - let (wake_sleep_callback, wake_sleep_user_data) = on_wake_sleep - .map(OWS::into_cb_user_data) - .unwrap_or((None, null_mut())); - let (advance_callback, advance_user_data) = on_advance - .map(OA::into_cb_user_data) - .unwrap_or((None, null_mut())); + let (collision_callback, collision_user_data) = + on_collide.map_or((None, null_mut()), OC::into_cb_user_data); + let (trigger_callback, trigger_user_data) = + on_trigger.map_or((None, null_mut()), OT::into_cb_user_data); + let (constraint_break_callback, constraint_break_user_data) = + on_constraint_break.map_or((None, null_mut()), OCB::into_cb_user_data); + let (wake_sleep_callback, wake_sleep_user_data) = + on_wake_sleep.map_or((None, null_mut()), OWS::into_cb_user_data); + let (advance_callback, advance_user_data) = + on_advance.map_or((None, null_mut()), OA::into_cb_user_data); Owner::from_raw( create_simulation_event_callbacks(&SimulationEventCallbackInfo { @@ -239,8 +234,8 @@ trait ConstraintBreakCallbackRaw: ConstraintBreakCallback { } } -/// A trait for onWake() and onSleep() callbacks. Parametrized by the ArticulationLink, -/// RigidStatic, and RigidDynamic types of the scene it is in. +/// A trait for `onWake()` and `onSleep()` callbacks. Parametrized by the [`ArticulationLink`], +/// [`RigidStatic`], and [`RigidDynamic`] types of the scene it is in. pub trait WakeSleepCallback: Sized { fn on_wake_sleep(&mut self, actors: &[&ActorMap], is_waking: bool); } @@ -282,10 +277,10 @@ where } /// A trait for the Advance Callback. onAdvance() is called during simulation, so it must -/// be thread safe, and `self` is not mutable. Parametrized by the ArticulationLink -/// and RigidDynamic types of the scene it is in. +/// be thread safe, and `self` is not mutable. Parametrized by the `ArticulationLink` +/// and `RigidDynamic` types of the scene it is in. pub trait AdvanceCallback: Sized { - /// All actors with PxRigidBodyFlag::eENABLE_POSE_INTEGRATION_PREVIEW set will be passed into here + /// All actors with `PxRigidBodyFlag::eENABLE_POSE_INTEGRATION_PREVIEW` set will be passed into here /// once the simulate call has updated their position. fn on_advance(&self, actors: &[&RigidBodyMap], transforms: &[PxTransform]); } diff --git a/physx/src/visual_debugger.rs b/physx/src/visual_debugger.rs index 299925ac..5a8a9d2d 100644 --- a/physx/src/visual_debugger.rs +++ b/physx/src/visual_debugger.rs @@ -27,7 +27,7 @@ pub enum VisualDebuggerSceneFlag { TransmitConstraints = 4, } -/// Combines the Pvd and it's current PvdTransport, if there is one. +/// Combines the Pvd and it's current `PvdTransport`, if there is one. pub struct VisualDebugger { pvd: Owner, transport: Option>, @@ -44,7 +44,7 @@ unsafe impl Class for VisualDebugger { } impl VisualDebugger { - /// Create a new VisualDebugger instance, a utility class that + /// Create a new `VisualDebugger` instance, a utility class that /// combines the TCP setup and the Pvd into one object. The port /// default for the PVD program is port 5425, so it is suggested to use this /// unless you're explicitly changing the other one as well. @@ -85,7 +85,7 @@ impl VisualDebugger { Some(visual_debugger) } - /// Connect the Pvd to the PvdTransport. + /// Connect the Pvd to the `PvdTransport`. pub fn connect(&mut self, flags: PxPvdInstrumentationFlags) -> bool { if let Some(transport) = self.transport.as_mut() { self.pvd.connect(transport.as_mut(), flags) @@ -164,7 +164,7 @@ impl Drop for PvdTransport { } } -/// A new type wrapper for PxPvd. +/// A new type wrapper for `PxPvd`. #[repr(transparent)] pub struct Pvd { obj: physx_sys::PxPvd, @@ -220,7 +220,7 @@ impl Drop for Pvd { } } -/// A new type wrapper for PxPvdSceneClient. +/// A new type wrapper for `PxPvdSceneClient`. pub struct PvdSceneClient { obj: physx_sys::PxPvdSceneClient, }