Fixes a bunch of clippy warnings, adds clippy to CI

.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

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(_) => {}

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<dyn DamageLibrary> {
-        &self.damage_calculator
+    pub fn damage_calculator(&self) -> &dyn DamageLibrary {
+        self.damage_calculator.deref()
     }
-    pub fn misc_library(&self) -> &Box<dyn MiscLibrary<'static>> {
-        &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<Option<Box<dyn Script>>> {
@@ -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 {},

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 {

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<HistoryHolder> {
-        &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)

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,

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)

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)]

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

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(),

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,

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();
             }
         }

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<LevelInt, Vec<StringKey>>,
     distinct_level_moves: Vec<StringKey>,
@@ -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());
         }
     }

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::<f32>().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
     );