Add clippy to circleCI and fix all lints

.circleci/config.yml

@@ -29,6 +29,7 @@ jobs:
 workflows:
   default:
     jobs:
+      - lint
       - format
       - build
       - test:

src/description.rs

@@ -1,6 +1,6 @@
 use crate::entities::Describe;
 
-pub fn list_to_sentence(lst: &Vec<String>) -> String {
+pub fn list_to_sentence(lst: &[String]) -> String {
     let mut buf = String::with_capacity(
         lst.iter()
             .map(|e| e.len() + 2usize /* ", " */)
@@ -33,7 +33,7 @@ pub fn list_to_sentence(lst: &Vec<String>) -> String {
     buf
 }
 
-pub fn describe_list<A: Describe>(lst: &Vec<A>) -> String {
+pub fn describe_list<A: Describe>(lst: &[A]) -> String {
     list_to_sentence(
         &lst.iter().map(|e| e.description()).collect::<Vec<String>>(),
     )

src/display/mod.rs

@@ -33,6 +33,7 @@ impl<T: Draw> Draw for Box<T> {
 }
 
 pub trait DrawWithNeighbors: Positioned {
+    #[allow(clippy::borrowed_box)]
     fn do_draw_with_neighbors<'a, 'b>(
         &'a self,
         out: &'b mut Write,

src/display/viewport.rs

@@ -79,7 +79,8 @@ impl<W> Debug for Viewport<W> {
 
 impl<W: Write> Viewport<W> {
     /// Draw the given entity to the viewport at its position, if visible
-    pub fn draw<'a, T: DrawWithNeighbors>(
+    #[allow(clippy::borrowed_box)]
+    pub fn draw<T: DrawWithNeighbors>(
         &mut self,
         entity: &T,
         neighbors: &Neighbors<Vec<&Box<dyn Entity>>>,
@@ -165,19 +166,15 @@ impl<W: Write> Viewport<W> {
     }
 
     pub fn push_prompt_chr(&mut self, chr: char) -> io::Result<()> {
-        match self.cursor_state {
+        if let CursorState::Prompt(pos) = self.cursor_state {
-            CursorState::Prompt(pos) => {
             write!(self, "{}", chr)?;
             self.cursor_state = CursorState::Prompt(pos + Direction::Right);
         }
-            _ => {}
-        }
         Ok(())
     }
 
     pub fn pop_prompt_chr(&mut self) -> io::Result<()> {
-        match self.cursor_state {
+        if let CursorState::Prompt(pos) = self.cursor_state {
-            CursorState::Prompt(pos) => {
             let new_pos = pos + Direction::Left;
             write!(
                 self,
@@ -187,8 +184,6 @@ impl<W: Write> Viewport<W> {
             )?;
             self.cursor_state = CursorState::Prompt(new_pos);
         }
-            _ => {}
-        }
         Ok(())
     }
 

src/entities/character.rs

@@ -30,7 +30,7 @@ impl Character {
         1
     }
 
-    pub fn name<'a>(&'a self) -> &'a str {
+    pub fn name(&self) -> &str {
         self.o_name
             .as_ref()
             .expect("Character name not initialized")

src/entities/creature.rs

@@ -44,7 +44,7 @@ impl Creature {
 
     /// Returns true if this creature has died
     pub fn dead(&self) -> bool {
-        self.hitpoints <= 0
+        self.hitpoints == 0
     }
 }
 

src/entities/entity.rs

@@ -13,7 +13,7 @@ pub trait Identified<ID>: Debug {
 
     fn id(&self) -> ID {
         self.opt_id()
-            .expect(format!("Entity ({:?}) is not in the game", self).as_str())
+            .unwrap_or_else(|| panic!("Entity ({:?}) is not in the game", self))
     }
 }
 

src/entities/raws.rs

@@ -22,8 +22,8 @@ lazy_static! {
 pub fn raw(name: &'static str) -> &'static EntityRaw<'static> {
     RAWS_BY_NAME
         .get(name)
-        .map(|e| *e)
+        .copied()
-        .expect(format!("Raw not found: {}", name).as_str())
+        .unwrap_or_else(|| panic!("Raw not found: {}", name))
 }
 
 #[cfg(test)]

src/game.rs

@@ -201,10 +201,10 @@ impl<'a> Game<'a> {
         if !pos.within(self.viewport.inner) {
             Some(Collision::Stop)
         } else {
-            if self.creatures_at(pos).len() > 0 {
+            if self.creatures_at(pos).is_empty() {
-                Some(Collision::Combat)
-            } else {
                 None
+            } else {
+                Some(Collision::Combat)
             }
         }
     }
@@ -271,7 +271,7 @@ impl<'a> Game<'a> {
             entities.retain(|e| e.id() != self.character_entity_id);
         }
 
-        if entities.len() == 0 {
+        if entities.is_empty() {
             match mode {
                 Walk => return Ok(()),
                 Look => {
@@ -284,7 +284,10 @@ impl<'a> Game<'a> {
         }
 
         let descriptions = list_to_sentence(
-            &entities.iter().map(|e| e.description()).collect(),
+            &entities
+                .iter()
+                .map(|e| e.description())
+                .collect::<Vec<String>>(),
         );
 
         self.say(
@@ -371,9 +374,9 @@ impl<'a> Game<'a> {
     }
 
     fn expect_creature(&self, creature_id: EntityID) -> &Creature {
-        self.creature(creature_id).expect(
+        self.creature(creature_id).unwrap_or_else(|| {
-            format!("Creature ID went away: {:?}", creature_id).as_str(),
+            panic!("Creature ID went away: {:?}", creature_id)
-        )
+        })
     }
 
     fn mut_creature(&mut self, creature_id: EntityID) -> Option<&mut Creature> {
@@ -383,9 +386,9 @@ impl<'a> Game<'a> {
     }
 
     fn expect_mut_creature(&mut self, creature_id: EntityID) -> &mut Creature {
-        self.mut_creature(creature_id).expect(
+        self.mut_creature(creature_id).unwrap_or_else(|| {
-            format!("Creature ID went away: {:?}", creature_id).as_str(),
+            panic!("Creature ID went away: {:?}", creature_id)
-        )
+        })
     }
 
     fn attack(&mut self, creature_id: EntityID) -> io::Result<()> {
@@ -411,14 +414,19 @@ impl<'a> Game<'a> {
 
     fn attack_at(&mut self, pos: Position) -> io::Result<()> {
         let creatures = self.creatures_at(pos);
-        if creatures.len() == 1 {
+        match creatures.len() {
+            0 => Ok(()),
+            1 => {
                 let creature = creatures.get(0).unwrap();
-            self.attack(creature.id())
+                let creature_id = creature.id();
-        } else {
+                self.attack(creature_id)
+            }
+            _ => {
                 // TODO prompt with a menu of creatures to combat
                 unimplemented!()
             }
         }
+    }
 
     fn flush_promises(&mut self) {
         unsafe {
@@ -485,10 +493,9 @@ impl<'a> Game<'a> {
                         None => (),
                     }
 
-                    match old_position {
+                    if let Some(old_pos) = old_position {
-                        Some(old_pos) => {
                         let character = self.character();
-                            let char_pos = character.position.clone();
+                        let char_pos = character.position;
                         self.viewport.game_cursor_position = char_pos;
                         self.viewport.clear(old_pos)?;
                         self.draw_entities_at(old_pos)?;
@@ -497,11 +504,11 @@ impl<'a> Game<'a> {
                             char_pos,
                             EntityDescriptionMode::Walk,
                         )?;
-                            self.tick(self.character().speed().tiles_to_ticks(
+                        self.tick(
+                            self.character().speed().tiles_to_ticks(
                                 (old_pos - char_pos).as_tiles(),
-                            ));
+                            ),
-                        }
+                        );
-                        None => (),
                     }
                 }
 

src/level_gen/cave_automata.rs

@@ -53,12 +53,12 @@ impl Default for Params {
 }
 
 pub fn generate<R: Rng + ?Sized>(
-    dimensions: &Dimensions,
+    dimensions: Dimensions,
     params: &Params,
     rand: &mut R,
 ) -> Vec<Vec<bool>> {
     let mut cells =
-        rand_initialize(&dimensions, rand, params.chance_to_start_alive);
+        rand_initialize(dimensions, rand, params.chance_to_start_alive);
     for _ in 0..params.steps {
         step_automata(&mut cells, dimensions, params);
     }
@@ -70,7 +70,7 @@ pub fn generate<R: Rng + ?Sized>(
 
 fn step_automata(
     cells: &mut Vec<Vec<bool>>,
-    dimensions: &Dimensions,
+    dimensions: Dimensions,
     params: &Params,
 ) {
     let orig_cells = (*cells).clone();
@@ -83,20 +83,18 @@ fn step_automata(
                 } else {
                     cells[x][y] = true;
                 }
-            } else {
+            } else if nbs > params.birth_limit {
-                if nbs > params.birth_limit {
                 cells[x][y] = true;
             } else {
                 cells[x][y] = false;
             }
         }
     }
-    }
 }
 
 const COUNT_EDGES_AS_NEIGHBORS: bool = true;
 
-fn num_alive_neighbors(cells: &Vec<Vec<bool>>, x: i32, y: i32) -> i32 {
+fn num_alive_neighbors(cells: &[Vec<bool>], x: i32, y: i32) -> i32 {
     let mut count = 0;
     for i in -1..2 {
         for j in -1..2 {
@@ -107,15 +105,14 @@ fn num_alive_neighbors(cells: &Vec<Vec<bool>>, x: i32, y: i32) -> i32 {
             let neighbor_x = x + i;
             let neighbor_y = y + j;
 
-            if COUNT_EDGES_AS_NEIGHBORS
+            if (COUNT_EDGES_AS_NEIGHBORS
                 && (neighbor_x < 0
                     || neighbor_y < 0
                     || neighbor_x >= (cells.len() as i32)
-                    || neighbor_y >= (cells[0].len()) as i32)
+                    || neighbor_y >= (cells[0].len()) as i32))
+                || cells[neighbor_x as usize][neighbor_y as usize]
             {
                 count += 1;
-            } else if cells[neighbor_x as usize][neighbor_y as usize] {
-                count += 1;
             }
         }
     }

src/level_gen/mod.rs

@@ -68,7 +68,7 @@ pub fn draw_level<W: io::Write>(
     level: Vec<Vec<bool>>,
     out: &mut W,
 ) -> io::Result<()> {
-    if level.len() == 0 {
+    if level.is_empty() {
         return Ok(());
     }
 

src/level_gen/util.rs

@@ -1,7 +1,7 @@
 use crate::types::Dimensions;
 use rand::{distributions, Rng};
 
-pub fn falses(dims: &Dimensions) -> Vec<Vec<bool>> {
+pub fn falses(dims: Dimensions) -> Vec<Vec<bool>> {
     let mut ret = Vec::with_capacity(dims.h as usize);
     for _ in 0..dims.h {
         let mut row = Vec::with_capacity(dims.w as usize);
@@ -16,7 +16,7 @@ pub fn falses(dims: &Dimensions) -> Vec<Vec<bool>> {
 /// Randomly initialize a 2-dimensional boolean vector of the given
 /// `Dimensions`, using the given random number generator and alive chance
 pub fn rand_initialize<R: Rng + ?Sized>(
-    dims: &Dimensions,
+    dims: Dimensions,
     rng: &mut R,
     alive_chance: f64,
 ) -> Vec<Vec<bool>> {
@@ -40,9 +40,9 @@ pub fn fill_outer_edges(level: &mut Vec<Vec<bool>>) {
     }
     let ymax = level[0].len();
 
-    for x in 0..xmax {
+    for row in level.iter_mut() {
-        level[x][0] = true;
+        row[0] = true;
-        level[x][ymax - 1] = true;
+        row[ymax - 1] = true;
     }
 
     for y in 0..level[0].len() {

src/main.rs

@@ -90,7 +90,7 @@ fn generate_level<'a, W: io::Write>(
     let level = match params.value_of("generator") {
         None => panic!("Must supply a generator with --generator"),
         Some("cave_automata") => level_gen::cave_automata::generate(
-            &dimensions,
+            dimensions,
             &level_gen::cave_automata::Params::from_matches(params),
             &mut rand,
         ),

src/messages.rs

@@ -35,7 +35,7 @@ impl<'a> NestedMap<'a> {
     fn lookup(&'a self, path: &str) -> Option<&'a Message<'a>> {
         use NestedMap::*;
         let leaf =
-            path.split(".")
+            path.split('.')
                 .fold(Some(self), |current, key| match current {
                     Some(Nested(m)) => m.get(key),
                     _ => None,

src/types/collision.rs

@@ -1,4 +1,5 @@
 /// Describes a kind of game collision
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Collision {
     /// Stop moving - you can't move there!
     Stop,

src/types/mod.rs

@@ -1,3 +1,6 @@
+#![allow(clippy::unit_arg)]
+#![allow(clippy::identity_conversion)]
+
 use std::cmp::max;
 use std::cmp::Ordering;
 use std::ops;
@@ -139,7 +142,7 @@ impl Position {
 
     /// Returns a sequence of ASCII escape characters for moving the cursor to
     /// this Position
-    pub fn cursor_goto(&self) -> cursor::Goto {
+    pub fn cursor_goto(self) -> cursor::Goto {
         // + 1 because Goto is 1-based, but position is 0-based
         cursor::Goto(self.x as u16 + 1, self.y as u16 + 1)
     }
@@ -147,7 +150,7 @@ impl Position {
     /// Converts this position to the number of `Tiles` away from the origin it
     /// represents. Usually done after subtracting two positions. Gives distance
     /// as the crow flies
-    pub fn as_tiles(&self) -> Tiles {
+    pub fn as_tiles(self) -> Tiles {
         Tiles(max(self.x.abs(), self.y.abs()).into())
     }
 }
@@ -179,6 +182,7 @@ impl PartialOrd for Position {
 /// let right_pos = pos + Direction::Right
 /// assert_eq!(right_pos, Position { x: 0, y: 10 })
 /// ```
+#[allow(clippy::suspicious_arithmetic_impl)]
 impl ops::Add<Direction> for Position {
     type Output = Position;
     fn add(self, dir: Direction) -> Position {
@@ -362,7 +366,7 @@ impl Speed {
     /// Returns the number of tiles that would be moved in the given number of
     /// ticks at this speed
     pub fn ticks_to_tiles(self, ticks: Ticks) -> Tiles {
-        Tiles(ticks.0 as f32 / self.0 as f32)
+        Tiles(f32::from(ticks.0) / self.0 as f32)
     }
 
     /// Returns the number of ticks required to move the given number of tiles