Skip to content

Commit

Permalink
Clippy improvements (#4665)
Browse files Browse the repository at this point in the history
# Objective

Follow up to my previous MR #3718 to add new clippy warnings to bevy:

- [x] [~~option_if_let_else~~](https://rust-lang.github.io/rust-clippy/master/#option_if_let_else) (reverted)
- [x] [redundant_else](https://rust-lang.github.io/rust-clippy/master/#redundant_else)
- [x] [match_same_arms](https://rust-lang.github.io/rust-clippy/master/#match_same_arms)
- [x] [semicolon_if_nothing_returned](https://rust-lang.github.io/rust-clippy/master/#semicolon_if_nothing_returned)
- [x] [explicit_iter_loop](https://rust-lang.github.io/rust-clippy/master/#explicit_iter_loop)
- [x] [map_flatten](https://rust-lang.github.io/rust-clippy/master/#map_flatten)

There is one commit per clippy warning, and the matching flags are added to the CI execution.

To test the CI execution you may run `cargo run -p ci -- clippy` at the root.

I choose the add the flags in the `ci` tool crate to avoid having them in every `lib.rs` but I guess it could become an issue with suprise warnings coming up after a commit/push


Co-authored-by: Carter Anderson <mcanders1@gmail.com>
  • Loading branch information
ManevilleF and cart committed May 31, 2022
1 parent e543941 commit f000c2b
Show file tree
Hide file tree
Showing 56 changed files with 302 additions and 703 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_animation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ pub fn animation_player(
match &curve.keyframes {
Keyframes::Rotation(keyframes) => transform.rotation = keyframes[0],
Keyframes::Translation(keyframes) => {
transform.translation = keyframes[0]
transform.translation = keyframes[0];
}
Keyframes::Scale(keyframes) => transform.scale = keyframes[0],
}
Expand Down
14 changes: 7 additions & 7 deletions crates/bevy_app/src/plugin_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl PluginGroupBuilder {
/// Consumes the [`PluginGroupBuilder`] and [builds](Plugin::build) the contained [`Plugin`]s
/// in the order specified.
pub fn finish(self, app: &mut App) {
for ty in self.order.iter() {
for ty in &self.order {
if let Some(entry) = self.plugins.get(ty) {
if entry.enabled {
debug!("added plugin: {}", entry.plugin.name());
Expand Down Expand Up @@ -173,7 +173,7 @@ mod tests {
std::any::TypeId::of::<PluginB>(),
std::any::TypeId::of::<PluginC>(),
]
)
);
}

#[test]
Expand All @@ -190,7 +190,7 @@ mod tests {
std::any::TypeId::of::<PluginC>(),
std::any::TypeId::of::<PluginB>(),
]
)
);
}

#[test]
Expand All @@ -207,7 +207,7 @@ mod tests {
std::any::TypeId::of::<PluginC>(),
std::any::TypeId::of::<PluginB>(),
]
)
);
}

#[test]
Expand All @@ -225,7 +225,7 @@ mod tests {
std::any::TypeId::of::<PluginC>(),
std::any::TypeId::of::<PluginB>(),
]
)
);
}

#[test]
Expand All @@ -243,7 +243,7 @@ mod tests {
std::any::TypeId::of::<PluginC>(),
std::any::TypeId::of::<PluginB>(),
]
)
);
}

#[test]
Expand All @@ -261,6 +261,6 @@ mod tests {
std::any::TypeId::of::<PluginC>(),
std::any::TypeId::of::<PluginB>(),
]
)
);
}
}
3 changes: 1 addition & 2 deletions crates/bevy_asset/src/debug_asset_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,7 @@ pub(crate) fn sync_debug_assets<T: Asset + Clone>(
let (changed_shaders, handle_map, debug_assets) = state.get_mut(world);
for changed in changed_shaders.iter_current_update_events() {
let debug_handle = match changed {
AssetEvent::Created { handle } => handle,
AssetEvent::Modified { handle } => handle,
AssetEvent::Created { handle } | AssetEvent::Modified { handle } => handle,
AssetEvent::Removed { .. } => continue,
};
if let Some(handle) = handle_map.handles.get(debug_handle) {
Expand Down
8 changes: 3 additions & 5 deletions crates/bevy_audio/src/audio_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,15 @@ where
Source: Asset + Decodable,
{
fn play_source(&self, audio_source: &Source, repeat: bool) -> Option<Sink> {
if let Some(stream_handle) = &self.stream_handle {
self.stream_handle.as_ref().map(|stream_handle| {
let sink = Sink::try_new(stream_handle).unwrap();
if repeat {
sink.append(audio_source.decoder().repeat_infinite());
} else {
sink.append(audio_source.decoder());
}
Some(sink)
} else {
None
}
sink
})
}

fn try_play_queued(
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ impl std::fmt::Debug for ComponentDescriptor {
impl ComponentDescriptor {
// SAFETY: The pointer points to a valid value of type `T` and it is safe to drop this value.
unsafe fn drop_ptr<T>(x: OwningPtr<'_>) {
x.drop_as::<T>()
x.drop_as::<T>();
}

/// Create a new `ComponentDescriptor` for the type `T`.
Expand Down
17 changes: 8 additions & 9 deletions crates/bevy_ecs/src/query/access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,12 +294,11 @@ impl<T: SparseSetIndex> FilteredAccessSet<T> {
pub fn is_compatible(&self, other: &FilteredAccessSet<T>) -> bool {
if self.combined_access.is_compatible(other.combined_access()) {
return true;
} else {
for filtered in self.filtered_accesses.iter() {
for other_filtered in other.filtered_accesses.iter() {
if !filtered.is_compatible(other_filtered) {
return false;
}
}
for filtered in &self.filtered_accesses {
for other_filtered in &other.filtered_accesses {
if !filtered.is_compatible(other_filtered) {
return false;
}
}
}
Expand All @@ -312,8 +311,8 @@ impl<T: SparseSetIndex> FilteredAccessSet<T> {
// if the unfiltered access is incompatible, must check each pair
let mut conflicts = HashSet::new();
if !self.combined_access.is_compatible(other.combined_access()) {
for filtered in self.filtered_accesses.iter() {
for other_filtered in other.filtered_accesses.iter() {
for filtered in &self.filtered_accesses {
for other_filtered in &other.filtered_accesses {
conflicts.extend(filtered.get_conflicts(other_filtered).into_iter());
}
}
Expand All @@ -326,7 +325,7 @@ impl<T: SparseSetIndex> FilteredAccessSet<T> {
// if the unfiltered access is incompatible, must check each pair
let mut conflicts = HashSet::new();
if !self.combined_access.is_compatible(filtered_access.access()) {
for filtered in self.filtered_accesses.iter() {
for filtered in &self.filtered_accesses {
conflicts.extend(filtered.get_conflicts(filtered_access).into_iter());
}
}
Expand Down
2 changes: 2 additions & 0 deletions crates/bevy_ecs/src/query/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ pub struct With<T>(PhantomData<T>);
impl<T: Component> WorldQuery for With<T> {
type State = WithState<T>;

#[allow(clippy::semicolon_if_nothing_returned)]
fn shrink<'wlong: 'wshort, 'wshort>(
item: super::QueryItem<'wlong, Self>,
) -> super::QueryItem<'wshort, Self> {
Expand Down Expand Up @@ -186,6 +187,7 @@ pub struct Without<T>(PhantomData<T>);
impl<T: Component> WorldQuery for Without<T> {
type State = WithoutState<T>;

#[allow(clippy::semicolon_if_nothing_returned)]
fn shrink<'wlong: 'wshort, 'wshort>(
item: super::QueryItem<'wlong, Self>,
) -> super::QueryItem<'wshort, Self> {
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/query/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery, const K: usize> QueryCombinationIter<

let ptr = values.as_mut_ptr().cast::<QueryItem<'w, Q>>();
for (offset, cursor) in self.cursors.iter_mut().enumerate() {
ptr.add(offset).write(cursor.peek_last().unwrap())
ptr.add(offset).write(cursor.peek_last().unwrap());
}

Some(values.assume_init())
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ mod tests {
)
})
.collect::<Vec<_>>();
assert_eq!(custom_param_data, normal_data)
assert_eq!(custom_param_data, normal_data);
}

{
Expand Down
7 changes: 3 additions & 4 deletions crates/bevy_ecs/src/schedule/stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -925,10 +925,9 @@ impl Stage for SystemStage {
}
}
match criteria.should_run {
ShouldRun::Yes => {
run_system_loop = true;
}
ShouldRun::YesAndCheckAgain | ShouldRun::NoAndCheckAgain => {
ShouldRun::Yes
| ShouldRun::YesAndCheckAgain
| ShouldRun::NoAndCheckAgain => {
run_system_loop = true;
}
ShouldRun::No => (),
Expand Down
9 changes: 2 additions & 7 deletions crates/bevy_ecs/src/schedule/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,8 @@ where
let pred_clone = pred.clone();
(move |state: Res<State<T>>, mut is_in_stack: Local<bool>| match &state.transition {
Some(StateTransition::Entering(ref relevant, _))
| Some(StateTransition::ExitingToResume(_, ref relevant)) => {
if relevant == &pred {
*is_in_stack = !*is_in_stack;
}
false
}
Some(StateTransition::ExitingFull(_, ref relevant)) => {
| Some(StateTransition::ExitingToResume(_, ref relevant))
| Some(StateTransition::ExitingFull(_, ref relevant)) => {
if relevant == &pred {
*is_in_stack = !*is_in_stack;
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/storage/blob_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ mod tests {

// SAFETY: The pointer points to a valid value of type `T` and it is safe to drop this value.
unsafe fn drop_ptr<T>(x: OwningPtr<'_>) {
x.drop_as::<T>()
x.drop_as::<T>();
}

/// # Safety
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_ecs/src/system/function_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,6 @@ where
world_id: Option<WorldId>,
archetype_generation: ArchetypeGeneration,
// NOTE: PhantomData<fn()-> T> gives this safe Send/Sync impls
#[allow(clippy::type_complexity)]
marker: PhantomData<fn() -> (In, Out, Marker)>,
}

Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/system/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> {
f,
self.last_change_tick,
self.change_tick,
)
);
};
}

Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1445,11 +1445,11 @@ unsafe impl<S: SystemParamState, P: SystemParam + 'static> SystemParamState
}

fn new_archetype(&mut self, archetype: &Archetype, system_meta: &mut SystemMeta) {
self.0.new_archetype(archetype, system_meta)
self.0.new_archetype(archetype, system_meta);
}

fn apply(&mut self, world: &mut World) {
self.0.apply(world)
self.0.apply(world);
}
}

Expand Down
40 changes: 15 additions & 25 deletions crates/bevy_gltf/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,55 +613,45 @@ fn load_material(material: &Material, load_context: &mut LoadContext) -> Handle<
let pbr = material.pbr_metallic_roughness();

let color = pbr.base_color_factor();
let base_color_texture = if let Some(info) = pbr.base_color_texture() {
let base_color_texture = pbr.base_color_texture().map(|info| {
// TODO: handle info.tex_coord() (the *set* index for the right texcoords)
let label = texture_label(&info.texture());
let path = AssetPath::new_ref(load_context.path(), Some(&label));
Some(load_context.get_handle(path))
} else {
None
};
load_context.get_handle(path)
});

let normal_map_texture: Option<Handle<Image>> =
if let Some(normal_texture) = material.normal_texture() {
material.normal_texture().map(|normal_texture| {
// TODO: handle normal_texture.scale
// TODO: handle normal_texture.tex_coord() (the *set* index for the right texcoords)
let label = texture_label(&normal_texture.texture());
let path = AssetPath::new_ref(load_context.path(), Some(&label));
Some(load_context.get_handle(path))
} else {
None
};
load_context.get_handle(path)
});

let metallic_roughness_texture = if let Some(info) = pbr.metallic_roughness_texture() {
let metallic_roughness_texture = pbr.metallic_roughness_texture().map(|info| {
// TODO: handle info.tex_coord() (the *set* index for the right texcoords)
let label = texture_label(&info.texture());
let path = AssetPath::new_ref(load_context.path(), Some(&label));
Some(load_context.get_handle(path))
} else {
None
};
load_context.get_handle(path)
});

let occlusion_texture = if let Some(occlusion_texture) = material.occlusion_texture() {
let occlusion_texture = material.occlusion_texture().map(|occlusion_texture| {
// TODO: handle occlusion_texture.tex_coord() (the *set* index for the right texcoords)
// TODO: handle occlusion_texture.strength() (a scalar multiplier for occlusion strength)
let label = texture_label(&occlusion_texture.texture());
let path = AssetPath::new_ref(load_context.path(), Some(&label));
Some(load_context.get_handle(path))
} else {
None
};
load_context.get_handle(path)
});

let emissive = material.emissive_factor();
let emissive_texture = if let Some(info) = material.emissive_texture() {
let emissive_texture = material.emissive_texture().map(|info| {
// TODO: handle occlusion_texture.tex_coord() (the *set* index for the right texcoords)
// TODO: handle occlusion_texture.strength() (a scalar multiplier for occlusion strength)
let label = texture_label(&info.texture());
let path = AssetPath::new_ref(load_context.path(), Some(&label));
Some(load_context.get_handle(path))
} else {
None
};
load_context.get_handle(path)
});

load_context.set_labeled_asset(
&material_label,
Expand Down
5 changes: 2 additions & 3 deletions crates/bevy_pbr/src/light.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,7 @@ impl ClusterConfig {

fn first_slice_depth(&self) -> f32 {
match self {
ClusterConfig::None => 0.0,
ClusterConfig::Single => 0.0,
ClusterConfig::None | ClusterConfig::Single => 0.0,
ClusterConfig::XYZ { z_config, .. } | ClusterConfig::FixedZ { z_config, .. } => {
z_config.first_slice_depth
}
Expand Down Expand Up @@ -880,7 +879,7 @@ pub(crate) fn assign_lights_to_clusters(

let inverse_projection = camera.projection_matrix.inverse();

for lights in clusters.lights.iter_mut() {
for lights in &mut clusters.lights {
lights.entities.clear();
}
clusters.lights.resize_with(
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_pbr/src/pbr_material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ impl RenderAsset for StandardMaterial {
| TextureFormat::Rg16Unorm
| TextureFormat::Bc5RgUnorm
| TextureFormat::EacRg11Unorm => {
flags |= StandardMaterialFlags::TWO_COMPONENT_NORMAL_MAP
flags |= StandardMaterialFlags::TWO_COMPONENT_NORMAL_MAP;
}
_ => {}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_pbr/src/render/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ pub fn prepare_skinned_meshes(
skinned_mesh_uniform
.buffer
.reserve(extracted_joints.buffer.len(), &render_device);
for joint in extracted_joints.buffer.iter() {
for joint in &extracted_joints.buffer {
skinned_mesh_uniform.buffer.push(*joint);
}
skinned_mesh_uniform
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_pbr/src/wireframe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ impl SpecializedMeshPipeline for WireframePipeline {
}

#[allow(clippy::too_many_arguments)]
#[allow(clippy::type_complexity)]
fn queue_wireframes(
opaque_3d_draw_functions: Res<DrawFunctions<Opaque3d>>,
render_meshes: Res<RenderAssets<Mesh>>,
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ptr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ impl<'a> OwningPtr<'a> {
/// Must point to a valid `T`.
#[inline]
pub unsafe fn drop_as<T>(self) {
self.as_ptr().cast::<T>().drop_in_place()
self.as_ptr().cast::<T>().drop_in_place();
}

/// Gets the underlying pointer, erasing the associated lifetime.
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_reflect/bevy_reflect_derive/src/type_uuid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub(crate) fn type_uuid_derive(input: proc_macro::TokenStream) -> proc_macro::To
ast.generics.type_params_mut().for_each(|param| {
param
.bounds
.push(syn::parse_quote!(#bevy_reflect_path::TypeUuid))
.push(syn::parse_quote!(#bevy_reflect_path::TypeUuid));
});

let (impl_generics, type_generics, where_clause) = &ast.generics.split_for_impl();
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_reflect/src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ pub fn array_hash<A: Array>(array: &A) -> Option<u64> {
std::any::Any::type_id(array).hash(&mut hasher);
array.len().hash(&mut hasher);
for value in array.iter() {
hasher.write_u64(value.reflect_hash()?)
hasher.write_u64(value.reflect_hash()?);
}
Some(hasher.finish())
}
Expand Down
Loading

0 comments on commit f000c2b

Please sign in to comment.