From c99b1bf8d92a351617fa831edc9a9fadeb4d3cba Mon Sep 17 00:00:00 2001 From: Deukhoofd Date: Sat, 18 Jun 2022 16:06:54 +0200 Subject: [PATCH] Fixes a bunch of clippy warnings, adds clippy to CI --- .drone.yml | 7 +++++++ src/dynamic_data/flow/turn_runner.rs | 4 ++-- src/dynamic_data/libraries/dynamic_library.rs | 11 ++++++----- src/dynamic_data/libraries/misc_library.rs | 6 ++++++ src/dynamic_data/models/battle.rs | 4 ++-- src/dynamic_data/models/battle_side.rs | 4 ++-- src/dynamic_data/script_handling/script_set.rs | 2 +- src/lib.rs | 1 + src/static_data/libraries/species_library.rs | 4 ++-- src/static_data/libraries/static_data.rs | 4 ++-- src/static_data/moves/move_data.rs | 4 ++-- src/static_data/natures.rs | 2 +- src/static_data/species_data/learnable_moves.rs | 4 ++-- tests/common/library_loader.rs | 12 ++++++------ 14 files changed, 42 insertions(+), 27 deletions(-) diff --git a/.drone.yml b/.drone.yml index 3272d05..1bd7ecd 100644 --- a/.drone.yml +++ b/.drone.yml @@ -44,3 +44,10 @@ steps: - cargo fmt --all -- --check depends_on: - test-debug-linux + - name: check-clippy + image: deukhoofd/linux64builder + failure: ignore + commands: + - cargo clippy --all-targets --all-features -- -D warnings + depends_on: + - test-debug-linux diff --git a/src/dynamic_data/flow/turn_runner.rs b/src/dynamic_data/flow/turn_runner.rs index a41f9c6..7b807a3 100644 --- a/src/dynamic_data/flow/turn_runner.rs +++ b/src/dynamic_data/flow/turn_runner.rs @@ -75,11 +75,11 @@ impl<'own, 'library> Battle<'own, 'library> { return Ok(()); } } - if !self.can_use(&choice) { + if !self.can_use(choice) { return Ok(()); } match choice { - TurnChoice::Move(..) => self.execute_move_choice(&choice)?, + TurnChoice::Move(..) => self.execute_move_choice(choice)?, TurnChoice::Item(_) => {} TurnChoice::Switch(_) => {} TurnChoice::Flee(_) => {} diff --git a/src/dynamic_data/libraries/dynamic_library.rs b/src/dynamic_data/libraries/dynamic_library.rs index 9b5b79d..17d10f1 100644 --- a/src/dynamic_data/libraries/dynamic_library.rs +++ b/src/dynamic_data/libraries/dynamic_library.rs @@ -7,6 +7,7 @@ use crate::dynamic_data::script_handling::script::Script; use crate::static_data::items::item::Item; use crate::static_data::libraries::static_data::StaticData; use crate::{PkmnResult, StringKey}; +use std::ops::Deref; #[derive(Debug)] pub struct DynamicLibrary { @@ -41,11 +42,11 @@ impl DynamicLibrary { pub fn stat_calculator(&self) -> &BattleStatCalculator { &self.stat_calculator } - pub fn damage_calculator(&self) -> &Box { - &self.damage_calculator + pub fn damage_calculator(&self) -> &dyn DamageLibrary { + self.damage_calculator.deref() } - pub fn misc_library(&self) -> &Box> { - &self.misc_library + pub fn misc_library(&self) -> &dyn MiscLibrary<'static> { + self.misc_library.deref() } pub fn load_script(&self, _category: ScriptCategory, _key: &StringKey) -> PkmnResult>> { @@ -64,7 +65,7 @@ pub mod test { use crate::dynamic_data::libraries::misc_library::Gen7MiscLibrary; use crate::static_data::libraries::static_data; - pub fn build<'library>() -> DynamicLibrary { + pub fn build() -> DynamicLibrary { DynamicLibrary { static_data: static_data::test::build(), stat_calculator: BattleStatCalculator {}, diff --git a/src/dynamic_data/libraries/misc_library.rs b/src/dynamic_data/libraries/misc_library.rs index 41b4e1c..b82ffa9 100644 --- a/src/dynamic_data/libraries/misc_library.rs +++ b/src/dynamic_data/libraries/misc_library.rs @@ -64,6 +64,12 @@ impl<'library> Gen7MiscLibrary<'library> { } } +impl<'library> Default for Gen7MiscLibrary<'library> { + fn default() -> Self { + Self::new() + } +} + impl<'library> Drop for Gen7MiscLibrary<'library> { fn drop(&mut self) { unsafe { diff --git a/src/dynamic_data/models/battle.rs b/src/dynamic_data/models/battle.rs index 51dc327..4b97177 100644 --- a/src/dynamic_data/models/battle.rs +++ b/src/dynamic_data/models/battle.rs @@ -121,8 +121,8 @@ impl<'own, 'library> Battle<'own, 'library> { pub fn event_hook(&self) -> &EventHook { &self.event_hook } - pub fn history_holder(&self) -> &Box { - &self.history_holder + pub fn history_holder(&self) -> &HistoryHolder { + self.history_holder.deref() } pub fn current_turn(&self) -> u32 { self.current_turn.load(Ordering::Relaxed) diff --git a/src/dynamic_data/models/battle_side.rs b/src/dynamic_data/models/battle_side.rs index fa13f35..da3a824 100644 --- a/src/dynamic_data/models/battle_side.rs +++ b/src/dynamic_data/models/battle_side.rs @@ -155,9 +155,9 @@ impl<'own, 'library> BattleSide<'own, 'library> { battle.event_hook().trigger(Event::Switch { side_index: self.index, index, - pokemon: Some(&pokemon), + pokemon: Some(pokemon), }); - script_hook!(on_switch_in, pokemon, &pokemon); + script_hook!(on_switch_in, pokemon, pokemon); } else { self.battle().event_hook().trigger(Event::Switch { side_index: self.index, diff --git a/src/dynamic_data/script_handling/script_set.rs b/src/dynamic_data/script_handling/script_set.rs index ca48fe4..031ca79 100644 --- a/src/dynamic_data/script_handling/script_set.rs +++ b/src/dynamic_data/script_handling/script_set.rs @@ -35,7 +35,7 @@ impl ScriptSet { if let Some(script) = script { let name = script.name().clone(); let arc = ScriptContainer::new(script); - self.scripts.insert(name, arc.clone()); + self.scripts.insert(name, arc); Ok(Some(self.scripts.last().unwrap().1.clone())) } else { Ok(None) diff --git a/src/lib.rs b/src/lib.rs index 95dd6ed..a548434 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,6 @@ // The too many arguments is annoying, especially for when we create constructors, disable. #![allow(clippy::too_many_arguments, clippy::needless_range_loop)] +#![allow(clippy::not_unsafe_ptr_arg_deref)] #![feature(test)] #![feature(bench_black_box)] #![feature(let_chains)] diff --git a/src/static_data/libraries/species_library.rs b/src/static_data/libraries/species_library.rs index 58e802f..1661925 100644 --- a/src/static_data/libraries/species_library.rs +++ b/src/static_data/libraries/species_library.rs @@ -42,7 +42,7 @@ pub mod tests { use crate::static_data::StaticStatisticSet; use hashbrown::HashSet; - fn build_species<'a>() -> Species { + fn build_species() -> Species { Species::new( 0, &"foo".into(), @@ -65,7 +65,7 @@ pub mod tests { ) } - pub fn build<'a>() -> SpeciesLibrary { + pub fn build() -> SpeciesLibrary { let mut lib = SpeciesLibrary::new(1); let species = build_species(); // Borrow as mut so we can insert diff --git a/src/static_data/libraries/static_data.rs b/src/static_data/libraries/static_data.rs index 356148d..6f033b4 100644 --- a/src/static_data/libraries/static_data.rs +++ b/src/static_data/libraries/static_data.rs @@ -76,7 +76,7 @@ impl StaticData { pub fn abilities(&self) -> &AbilityLibrary { &self.abilities } - pub fn abilities_mut<'a>(&'a mut self) -> &'a mut AbilityLibrary { + pub fn abilities_mut(&mut self) -> &mut AbilityLibrary { &mut self.abilities } } @@ -90,7 +90,7 @@ pub mod test { }; use crate::static_data::natures; - pub fn build<'a>() -> StaticData { + pub fn build() -> StaticData { StaticData { settings: LibrarySettings::new(100), species: species_library::tests::build(), diff --git a/src/static_data/moves/move_data.rs b/src/static_data/moves/move_data.rs index 74fbc64..946e501 100644 --- a/src/static_data/moves/move_data.rs +++ b/src/static_data/moves/move_data.rs @@ -5,7 +5,7 @@ use hashbrown::HashSet; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -#[derive(Copy, Clone, PartialEq, Debug)] +#[derive(Copy, Clone, PartialEq, Eq, Debug)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "serde", serde(rename_all = "snake_case"))] pub enum MoveCategory { @@ -14,7 +14,7 @@ pub enum MoveCategory { Status = 2, } -#[derive(Copy, Clone, PartialEq, Debug)] +#[derive(Copy, Clone, PartialEq, Eq, Debug)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub enum MoveTarget { Adjacent = 0, diff --git a/src/static_data/natures.rs b/src/static_data/natures.rs index 263d2fc..4fe8885 100644 --- a/src/static_data/natures.rs +++ b/src/static_data/natures.rs @@ -68,7 +68,7 @@ impl NatureLibrary { for kv in &self.map { // As natures can't be copied, and should always be the same reference as the value // in the map, we just compare by reference. - if (kv.1 as *const Nature) == (nature as *const Nature) { + if std::ptr::eq(kv.1, nature) { return kv.0.clone(); } } diff --git a/src/static_data/species_data/learnable_moves.rs b/src/static_data/species_data/learnable_moves.rs index 6e5ceb5..e30d8a1 100644 --- a/src/static_data/species_data/learnable_moves.rs +++ b/src/static_data/species_data/learnable_moves.rs @@ -3,7 +3,7 @@ use crate::StringKey; use hashbrown::hash_map::Entry::{Occupied, Vacant}; use hashbrown::HashMap; -#[derive(Default, PartialEq, Debug)] +#[derive(Default, PartialEq, Eq, Debug)] pub struct LearnableMoves { learned_by_level: HashMap>, distinct_level_moves: Vec, @@ -23,7 +23,7 @@ impl LearnableMoves { self.learned_by_level.insert(level, vec![m.clone()]); } } - if !self.distinct_level_moves.contains(&m) { + if !self.distinct_level_moves.contains(m) { self.distinct_level_moves.push(m.clone()); } } diff --git a/tests/common/library_loader.rs b/tests/common/library_loader.rs index ceb738e..ea92d57 100644 --- a/tests/common/library_loader.rs +++ b/tests/common/library_loader.rs @@ -46,13 +46,13 @@ pub fn load_types(path: &String, type_library: &mut TypeLibrary) { .unwrap(); let headers = reader.headers().unwrap(); for header in headers.iter().skip(1) { - type_library.register_type(&StringKey::new(header.clone())); + type_library.register_type(&StringKey::new(header)); } for record in reader.records() { let record = record.unwrap(); let offensive_type = record.get(0).unwrap(); - let offensive_type_id = type_library.get_type_id(&StringKey::new(offensive_type.clone())); + let offensive_type_id = type_library.get_type_id(&StringKey::new(offensive_type)); for (i, v) in record.iter().skip(1).enumerate() { let effectiveness = v.parse::().unwrap(); @@ -166,7 +166,7 @@ pub fn load_moves(path: &String, lib: &mut StaticData) { let data = json.as_object().unwrap().get("data").unwrap().as_array().unwrap(); for move_data in data { let move_data = move_data.as_object().unwrap(); - let move_name = StringKey::new(move_data["name"].as_str().unwrap().clone()); + let move_name = StringKey::new(move_data["name"].as_str().unwrap()); let move_type = StringKey::new(move_data["type"].as_str().unwrap()); let move_type_id = lib.types().get_type_id(&move_type); let move_category = serde_json::from_value(move_data["category"].clone()).unwrap(); @@ -188,7 +188,7 @@ pub fn load_moves(path: &String, lib: &mut StaticData) { } } - SecondaryEffect::new(chance, StringKey::new(v["name"].as_str().unwrap().clone()), parameters) + SecondaryEffect::new(chance, StringKey::new(v["name"].as_str().unwrap()), parameters) } else { SecondaryEffect::empty() }; @@ -227,7 +227,7 @@ pub fn load_species(path: &String, library: &mut StaticData) { let o = json.as_object().unwrap(); for (key, value) in o { - if key.starts_with("$") { + if key.starts_with('$') { continue; } let name = StringKey::new(key); @@ -393,7 +393,7 @@ fn test_type_library_loaded() { assert_eq!( lib.get_effectiveness( lib.get_type_id(&StringKey::new("fire")), - &vec![lib.get_type_id(&StringKey::new("grass"))], + &[lib.get_type_id(&StringKey::new("grass"))], ), 2.0 );