Merge branch 'clippy-fixup' into 'master'

fix various clippy issues

See merge request veloren/veloren!1137

.gitlab/CI/check.gitlab-ci.yml

@@ -11,6 +11,7 @@ code-quality:
   script:
     - ln -s /dockercache/cache-all target
     - cargo clippy -- -D warnings
+    - cargo clippy --tests -- -D warnings
     - cargo fmt --all -- --check
 
 security:

client/src/lib.rs

@@ -1,5 +1,4 @@
 #![deny(unsafe_code)]
-#![allow(clippy::option_map_unit_fn)]
 #![feature(label_break_value)]
 
 pub mod cmd;
@@ -98,8 +97,6 @@ pub struct CharacterList {
 
 impl Client {
     /// Create a new `Client`.
-    #[allow(clippy::cmp_owned)] // TODO: Pending review in #587
-    #[allow(clippy::or_fun_call)] // TODO: Pending review in #587
     pub fn new<A: Into<SocketAddr>>(addr: A, view_distance: Option<u32>) -> Result<Self, Error> {
         let client_state = ClientState::Connected;
         let mut postbox = PostBox::to(addr)?;
@@ -113,7 +110,7 @@ impl Client {
                 world_map: (map_size, world_map),
             } => {
                 // TODO: Display that versions don't match in Voxygen
-                if server_info.git_hash != common::util::GIT_HASH.to_string() {
+                if &server_info.git_hash != *common::util::GIT_HASH {
                     warn!(
                         "Server is running {}[{}], you are running {}[{}], versions might be \
                          incompatible!",
@@ -145,7 +142,7 @@ impl Client {
                         // Should not fail if the dimensions are correct.
                         let world_map =
                             image::ImageBuffer::from_raw(map_size.x, map_size.y, world_map_raw);
-                        world_map.ok_or(Error::Other("Server sent a bad world map image".into()))?
+                        world_map.ok_or_else(|| Error::Other("Server sent a bad world map image".into()))?
                     })
                     // Flip the image, since Voxygen uses an orientation where rotation from
                     // positive x axis to positive y axis is counterclockwise around the z axis.
@@ -492,7 +489,6 @@ impl Client {
 
     /// Execute a single client tick, handle input and update the game state by
     /// the given duration.
-    #[allow(clippy::manual_saturating_arithmetic)] // TODO: Pending review in #587
     pub fn tick(
         &mut self,
         inputs: ControllerInputs,
@@ -596,7 +592,7 @@ impl Client {
                 // 1 as a buffer so that if the player moves back in that direction the chunks
                 //   don't need to be reloaded
                 if (chunk_pos - key)
-                    .map(|e: i32| (e.abs() as u32).checked_sub(2).unwrap_or(0))
+                    .map(|e: i32| (e.abs() as u32).saturating_sub(2))
                     .magnitude_squared()
                     > view_distance.pow(2)
                 {

common/src/comp/inventory/slot.rs

@@ -384,7 +384,7 @@ mod tests {
         assert_eq!(sword, loadout.second_item);
 
         // Verify inventory
-        assert_eq!(inv.slots[0], Some(sword.clone().unwrap().item));
+        assert_eq!(inv.slots[0], Some(sword.unwrap().item));
         assert_eq!(inv.slots.len(), 1);
     }
 
@@ -459,7 +459,7 @@ mod tests {
 
         // The swap should return the sword
         assert_eq!(
-            Some(sword.clone().unwrap().item),
+            Some(sword.unwrap().item),
             loadout_remove(EquipSlot::Mainhand, &mut loadout,)
         );
 

common/src/region.rs

@@ -223,7 +223,6 @@ impl RegionMap {
 
     /// Finds the region where a given entity is located using a given position
     /// to speed up the search
-    #[allow(clippy::needless_range_loop)] // TODO: Pending review in #587
     pub fn find_region(&self, entity: specs::Entity, pos: Vec3<f32>) -> Option<Vec2<i32>> {
         let id = entity.id();
         // Compute key for most likely region
@@ -234,9 +233,9 @@ impl RegionMap {
                 return Some(key);
             } else {
                 // Check neighbors
-                for i in 0..8 {
+                for o in region.neighbors.iter() {
-                    if let Some(idx) = region.neighbors[i] {
+                    if let Some(idx) = o {
-                        let (key, region) = self.regions.get_index(idx).unwrap();
+                        let (key, region) = self.regions.get_index(*idx).unwrap();
                         if region.entities().contains(id) {
                             return Some(*key);
                         }
@@ -245,8 +244,8 @@ impl RegionMap {
             }
         } else {
             // Check neighbors
-            for i in 0..8 {
+            for o in &NEIGHBOR_OFFSETS {
-                let key = key + NEIGHBOR_OFFSETS[i];
+                let key = key + o;
                 if let Some(region) = self.regions.get(&key) {
                     if region.entities().contains(id) {
                         return Some(key);

common/src/spiral.rs

@@ -16,8 +16,6 @@ impl Spiral2d {
 impl Iterator for Spiral2d {
     type Item = Vec2<i32>;
 
-    #[allow(clippy::erasing_op)] // TODO: Pending review in #587
-    #[allow(clippy::identity_op)] // TODO: Pending review in #587
     fn next(&mut self) -> Option<Self::Item> {
         let layer_size = (self.layer * 8 + 4 * self.layer.min(1) - 4).max(1);
         if self.i >= layer_size {

network/src/channel.rs

@@ -139,7 +139,6 @@ impl Handshake {
             },
         };
 
-        #[allow(clippy::unit_arg)]
         match res {
             Ok(res) => {
                 let mut leftover_frames = vec![];
@@ -152,7 +151,7 @@ impl Handshake {
                 }
                 Ok((res.0, res.1, res.2, leftover_frames))
             },
-            Err(e) => Err(e),
+            Err(()) => Err(()),
         }
     }
 

network/src/lib.rs

@@ -1,5 +1,4 @@
 #![deny(unsafe_code)]
-#![allow(clippy::option_map_unit_fn)]
 #![cfg_attr(test, deny(rust_2018_idioms))]
 #![cfg_attr(test, deny(warnings))]
 #![feature(try_trait, const_if_match)]

network/src/message.rs

@@ -138,9 +138,9 @@ mod tests {
         assert_eq!(mb.data[0], 3);
         assert_eq!(mb.data[1], 0);
         assert_eq!(mb.data[7], 0);
-        assert_eq!(mb.data[8], 'a' as u8);
+        assert_eq!(mb.data[8], b'a');
         assert_eq!(mb.data[8], 97);
-        assert_eq!(mb.data[9], 'b' as u8);
+        assert_eq!(mb.data[9], b'b');
-        assert_eq!(mb.data[10], 'c' as u8);
+        assert_eq!(mb.data[10], b'c');
     }
 }

network/src/metrics.rs

@@ -353,8 +353,8 @@ mod tests {
     #[test]
     fn pid_cid_frame_cache() {
         let pid = Pid::fake(1);
-        let frame1 = Frame::Raw("Foo".as_bytes().to_vec());
+        let frame1 = Frame::Raw(b"Foo".to_vec());
-        let frame2 = Frame::Raw("Bar".as_bytes().to_vec());
+        let frame2 = Frame::Raw(b"Bar".to_vec());
         let metrics = NetworkMetrics::new(&pid).unwrap();
         let mut cache = PidCidFrameCache::new(metrics.frames_in_total, pid);
         let v1 = cache.with_label_values(1, &frame1);
@@ -377,8 +377,8 @@ mod tests {
     #[test]
     fn cid_frame_cache() {
         let pid = Pid::fake(1);
-        let frame1 = Frame::Raw("Foo".as_bytes().to_vec());
+        let frame1 = Frame::Raw(b"Foo".to_vec());
-        let frame2 = Frame::Raw("Bar".as_bytes().to_vec());
+        let frame2 = Frame::Raw(b"Bar".to_vec());
         let metrics = NetworkMetrics::new(&pid).unwrap();
         let mut cache = CidFrameCache::new(metrics.frames_wire_out_total, 1);
         let v1 = cache.with_label_values(&frame1);

network/src/prios.rs

@@ -327,6 +327,7 @@ mod tests {
     const SIZE: u64 = PrioManager::FRAME_DATA_SIZE;
     const USIZE: usize = PrioManager::FRAME_DATA_SIZE as usize;
 
+    #[allow(clippy::type_complexity)]
     fn mock_new() -> (
         PrioManager,
         Sender<(Prio, Sid, OutgoingMessage)>,
@@ -605,10 +606,10 @@ mod tests {
     fn gigantic_message() {
         let (mut mgr, msg_tx, _flush_tx) = mock_new();
         let mut data = vec![1; USIZE];
-        data.extend_from_slice(&vec![2; USIZE]);
+        data.extend_from_slice(&[2; USIZE]);
-        data.extend_from_slice(&vec![3; USIZE]);
+        data.extend_from_slice(&[3; USIZE]);
-        data.extend_from_slice(&vec![4; USIZE]);
+        data.extend_from_slice(&[4; USIZE]);
-        data.extend_from_slice(&vec![5; USIZE]);
+        data.extend_from_slice(&[5; USIZE]);
         let sid = Sid::new(2);
         msg_tx
             .send((16, sid, OutgoingMessage {
@@ -634,10 +635,10 @@ mod tests {
     fn gigantic_message_order() {
         let (mut mgr, msg_tx, _flush_tx) = mock_new();
         let mut data = vec![1; USIZE];
-        data.extend_from_slice(&vec![2; USIZE]);
+        data.extend_from_slice(&[2; USIZE]);
-        data.extend_from_slice(&vec![3; USIZE]);
+        data.extend_from_slice(&[3; USIZE]);
-        data.extend_from_slice(&vec![4; USIZE]);
+        data.extend_from_slice(&[4; USIZE]);
-        data.extend_from_slice(&vec![5; USIZE]);
+        data.extend_from_slice(&[5; USIZE]);
         let sid = Sid::new(2);
         msg_tx
             .send((16, sid, OutgoingMessage {
@@ -666,10 +667,10 @@ mod tests {
     fn gigantic_message_order_other_prio() {
         let (mut mgr, msg_tx, _flush_tx) = mock_new();
         let mut data = vec![1; USIZE];
-        data.extend_from_slice(&vec![2; USIZE]);
+        data.extend_from_slice(&[2; USIZE]);
-        data.extend_from_slice(&vec![3; USIZE]);
+        data.extend_from_slice(&[3; USIZE]);
-        data.extend_from_slice(&vec![4; USIZE]);
+        data.extend_from_slice(&[4; USIZE]);
-        data.extend_from_slice(&vec![5; USIZE]);
+        data.extend_from_slice(&[5; USIZE]);
         let sid = Sid::new(2);
         msg_tx
             .send((16, sid, OutgoingMessage {

network/src/types.rs

@@ -322,7 +322,7 @@ mod tests {
 
     #[test]
     fn frame_get_int() {
-        assert_eq!(Frame::get_int(&Frame::Raw("Foo".as_bytes().to_vec())), 7);
+        assert_eq!(Frame::get_int(&Frame::Raw(b"Foo".to_vec())), 7);
         assert_eq!(Frame::get_int(&Frame::Shutdown), 2);
     }
 

network/tests/integration.rs

@@ -101,11 +101,11 @@ fn failed_listen_on_used_ports() -> std::result::Result<(), Box<dyn std::error::
     let e2 = block_on(network2.listen(tcp1));
     match e1 {
         Err(NetworkError::ListenFailed(e)) if e.kind() == ErrorKind::AddrInUse => (),
-        _ => assert!(false),
+        _ => panic!(),
     };
     match e2 {
         Err(NetworkError::ListenFailed(e)) if e.kind() == ErrorKind::AddrInUse => (),
-        _ => assert!(false),
+        _ => panic!(),
     };
     Ok(())
 }
@@ -178,7 +178,7 @@ fn wrong_parse() {
 
     s1_a.send(1337).unwrap();
     match block_on(s1_b.recv::<String>()) {
-        Err(StreamError::DeserializeError(_)) => assert!(true),
+        Err(StreamError::DeserializeError(_)) => (),
-        _ => assert!(false, "this should fail, but it doesnt!"),
+        _ => panic!("this should fail, but it doesnt!"),
     }
 }

server-cli/src/main.rs

@@ -1,5 +1,4 @@
 #![deny(unsafe_code)]
-#![allow(clippy::option_map_unit_fn)]
 
 use common::clock::Clock;
 use server::{Event, Input, Server, ServerSettings};
@@ -10,7 +9,6 @@ use tracing_subscriber::{filter::LevelFilter, EnvFilter, FmtSubscriber};
 const TPS: u64 = 30;
 const RUST_LOG_ENV: &str = "RUST_LOG";
 
-#[allow(clippy::redundant_pattern_matching)] // TODO: Pending review in #587
 fn main() {
     // Init logging
     let filter = match std::env::var_os(RUST_LOG_ENV).map(|s| s.into_string()) {

server/src/cmd.rs

@@ -240,7 +240,6 @@ fn handle_jump(
     }
 }
 
-#[allow(clippy::useless_conversion)] // TODO: Pending review in #587
 fn handle_goto(
     server: &mut Server,
     client: EcsEntity,
@@ -294,7 +293,6 @@ fn handle_kill(
         .map(|s| s.health.set_to(0, reason));
 }
 
-#[allow(clippy::option_as_ref_deref)] // TODO: Pending review in #587
 fn handle_time(
     server: &mut Server,
     client: EcsEntity,
@@ -303,7 +301,7 @@ fn handle_time(
     action: &ChatCommand,
 ) {
     let time = scan_fmt_some!(&args, &action.arg_fmt(), String);
-    let new_time = match time.as_ref().map(|s| s.as_str()) {
+    let new_time = match time.as_deref() {
         Some("midnight") => NaiveTime::from_hms(0, 0, 0),
         Some("night") => NaiveTime::from_hms(20, 0, 0),
         Some("dawn") => NaiveTime::from_hms(5, 0, 0),
@@ -382,7 +380,6 @@ fn handle_health(
     }
 }
 
-#[allow(clippy::useless_conversion)] // TODO: Pending review in #587
 fn handle_alias(
     server: &mut Server,
     client: EcsEntity,
@@ -418,10 +415,8 @@ fn handle_alias(
             ecs.read_storage::<comp::Player>().get(target),
             old_alias_optional,
         ) {
-            let msg = ServerMsg::PlayerListUpdate(PlayerListUpdate::Alias(
+            let msg =
-                (*uid).into(),
+                ServerMsg::PlayerListUpdate(PlayerListUpdate::Alias(*uid, player.alias.clone()));
-                player.alias.clone(),
-            ));
             server.state.notify_registered_clients(msg);
 
             // Announce alias change if target has a Body.
@@ -440,8 +435,6 @@ fn handle_alias(
     }
 }
 
-#[allow(clippy::useless_conversion)] // TODO: Pending review in #587
-#[allow(clippy::useless_format)] // TODO: Pending review in #587
 fn handle_tp(
     server: &mut Server,
     client: EcsEntity,
@@ -497,8 +490,6 @@ fn handle_tp(
     }
 }
 
-#[allow(clippy::useless_conversion)] // TODO: Pending review in #587
-#[allow(clippy::redundant_clone)] // TODO: Pending review in #587
 fn handle_spawn(
     server: &mut Server,
     client: EcsEntity,
@@ -644,7 +635,6 @@ fn handle_build(
     }
 }
 
-#[allow(clippy::useless_conversion)] // TODO: Pending review in #587
 fn handle_help(
     server: &mut Server,
     client: EcsEntity,
@@ -1350,8 +1340,6 @@ fn handle_debug_column(
 }
 
 #[cfg(feature = "worldgen")]
-#[allow(clippy::blacklisted_name)] // TODO: Pending review in #587
-#[allow(clippy::useless_conversion)] // TODO: Pending review in #587
 fn handle_debug_column(
     server: &mut Server,
     client: EcsEntity,
@@ -1367,7 +1355,7 @@ fn handle_debug_column(
             e / sz as i32
         }); */
 
-        let foo = || {
+        let msg_generator = || {
             // let sim_chunk = sim.get(chunk_pos)?;
             let alt = sim.get_interpolated(wpos, |chunk| chunk.alt)?;
             let basement = sim.get_interpolated(wpos, |chunk| chunk.basement)?;
@@ -1416,7 +1404,7 @@ spawn_rate {:?} "#,
                 spawn_rate
             ))
         };
-        if let Some(s) = foo() {
+        if let Some(s) = msg_generator() {
             server.notify_client(client, ChatType::CommandInfo.server_msg(s));
         } else {
             server.notify_client(
@@ -1432,7 +1420,6 @@ spawn_rate {:?} "#,
     }
 }
 
-#[allow(clippy::or_fun_call)] // TODO: Pending review in #587
 fn find_target(
     ecs: &specs::World,
     opt_alias: Option<String>,
@@ -1443,7 +1430,9 @@ fn find_target(
             .join()
             .find(|(_, player)| player.alias == alias)
             .map(|(entity, _)| entity)
-            .ok_or(ChatType::CommandError.server_msg(format!("Player '{}' not found!", alias)))
+            .ok_or_else(|| {
+                ChatType::CommandError.server_msg(format!("Player '{}' not found!", alias))
+            })
     } else {
         Ok(fallback)
     }
@@ -1569,7 +1558,6 @@ fn handle_debug(
     }
 }
 
-#[allow(clippy::useless_conversion)] // TODO: Pending review in #587
 fn handle_remove_lights(
     server: &mut Server,
     client: EcsEntity,
@@ -1621,10 +1609,6 @@ fn handle_remove_lights(
     );
 }
 
-#[allow(clippy::chars_next_cmp)] // TODO: Pending review in #587
-#[allow(clippy::useless_conversion)] // TODO: Pending review in #587
-#[allow(clippy::or_fun_call)] // TODO: Pending review in #587
-#[allow(clippy::useless_format)] // TODO: Pending review in #587
 fn handle_sudo(
     server: &mut Server,
     client: EcsEntity,
@@ -1635,7 +1619,7 @@ fn handle_sudo(
     if let (Some(player_alias), Some(cmd), cmd_args) =
         scan_fmt_some!(&args, &action.arg_fmt(), String, String, String)
     {
-        let cmd_args = cmd_args.unwrap_or(String::from(""));
+        let cmd_args = cmd_args.unwrap_or_else(|| String::from(""));
         if let Ok(action) = cmd.parse() {
             let ecs = server.state.ecs();
             let entity_opt = (&ecs.entities(), &ecs.read_storage::<comp::Player>())

server/src/sys/terrain.rs

@@ -36,9 +36,6 @@ impl<'a> System<'a> for Sys {
         WriteStorage<'a, Client>,
     );
 
-    #[allow(clippy::useless_conversion)] // TODO: Pending review in #587
-    #[allow(clippy::manual_saturating_arithmetic)] // TODO: Pending review in #587
-    #[allow(clippy::or_fun_call)] // TODO: Pending review in #587
     fn run(
         &mut self,
         (
@@ -83,8 +80,8 @@ impl<'a> System<'a> for Sys {
                 // Subtract 2 from the offset before computing squared magnitude
                 // 1 since chunks need neighbors to be meshed
                 // 1 to act as a buffer if the player moves in that direction
-                let adjusted_dist_sqr = (Vec2::from(chunk_pos) - Vec2::from(key))
+                let adjusted_dist_sqr = (chunk_pos - key)
-                    .map(|e: i32| (e.abs() as u32).checked_sub(2).unwrap_or(0))
+                    .map(|e: i32| (e.abs() as u32).saturating_sub(2))
                     .magnitude_squared();
 
                 if adjusted_dist_sqr <= view_distance.pow(2) {
@@ -111,7 +108,7 @@ impl<'a> System<'a> for Sys {
                 }
 
                 let mut body = entity.body;
-                let name = entity.name.unwrap_or("Unnamed".to_string());
+                let name = entity.name.unwrap_or_else(|| "Unnamed".to_string());
                 let alignment = entity.alignment;
                 let main_tool = entity.main_tool;
                 let mut stats = comp::Stats::new(name, body);
@@ -398,8 +395,6 @@ impl<'a> System<'a> for Sys {
     }
 }
 
-#[allow(clippy::useless_conversion)] // TODO: Pending review in #587
-#[allow(clippy::manual_saturating_arithmetic)] // TODO: Pending review in #587
 pub fn chunk_in_vd(
     player_pos: Vec3<f32>,
     chunk_pos: Vec2<i32>,
@@ -408,8 +403,8 @@ pub fn chunk_in_vd(
 ) -> bool {
     let player_chunk_pos = terrain.pos_key(player_pos.map(|e| e as i32));
 
-    let adjusted_dist_sqr = Vec2::from(player_chunk_pos - chunk_pos)
+    let adjusted_dist_sqr = (player_chunk_pos - chunk_pos)
-        .map(|e: i32| (e.abs() as u32).checked_sub(2).unwrap_or(0))
+        .map(|e: i32| (e.abs() as u32).saturating_sub(2))
         .magnitude_squared();
 
     adjusted_dist_sqr <= vd.pow(2)

voxygen/tests/check_i18n_files.rs

@@ -81,15 +81,12 @@ fn generate_key_version<'a>(
         .map(|k| (k.to_owned(), LocalizationEntryState::new()))
         .collect();
     let mut to_process: HashSet<&String> = localization.string_map.keys().map(|k| k).collect();
-    let mut line_nb = 0;
-
     // Find key start lines
-    for line in std::str::from_utf8(file_blob.content())
+    for (line_nb, line) in std::str::from_utf8(file_blob.content())
         .expect("UTF-8 file")
         .split('\n')
+        .enumerate()
     {
-        line_nb += 1;
-
         let mut found_key = None;
 
         for key in to_process.iter() {

world/src/block/mod.rs

@@ -25,18 +25,14 @@ impl<'a> BlockGen<'a> {
         }
     }
 
-    #[allow(clippy::useless_conversion)] // TODO: Pending review in #587
     pub fn sample_column<'b>(
         column_gen: &ColumnGen<'a>,
         cache: &'b mut SmallCache<Option<ColumnSample<'a>>>,
         wpos: Vec2<i32>,
     ) -> Option<&'b ColumnSample<'a>> {
-        cache
+        cache.get(wpos, |wpos| column_gen.get(wpos)).as_ref()
-            .get(Vec2::from(wpos), |wpos| column_gen.get(wpos))
-            .as_ref()
     }
 
-    #[allow(clippy::useless_conversion)] // TODO: Pending review in #587
     fn get_cliff_height(
         column_gen: &ColumnGen<'a>,
         cache: &mut SmallCache<Option<ColumnSample<'a>>>,
@@ -47,11 +43,8 @@ impl<'a> BlockGen<'a> {
     ) -> f32 {
         close_cliffs.iter().fold(
             0.0f32,
-            |max_height, (cliff_pos, seed)| match Self::sample_column(
+            |max_height, (cliff_pos, seed)| match Self::sample_column(column_gen, cache, *cliff_pos)
-                column_gen,
+            {
-                cache,
-                Vec2::from(*cliff_pos),
-            ) {
                 Some(cliff_sample) if cliff_sample.is_cliffs && cliff_sample.spawn_rate > 0.5 => {
                     let cliff_pos3d = Vec3::from(*cliff_pos);
 
@@ -91,7 +84,6 @@ impl<'a> BlockGen<'a> {
         )
     }
 
-    #[allow(clippy::useless_conversion)] // TODO: Pending review in #587
     pub fn get_z_cache(&mut self, wpos: Vec2<i32>) -> Option<ZCache<'a>> {
         let BlockGen {
             column_cache,
@@ -109,8 +101,7 @@ impl<'a> BlockGen<'a> {
             .zip(structures.iter_mut())
             .for_each(|(close_structure, structure)| {
                 if let Some(st) = *close_structure {
-                    let st_sample =
+                    let st_sample = Self::sample_column(column_gen, column_cache, st.pos);
-                        Self::sample_column(column_gen, column_cache, Vec2::from(st.pos));
                     if let Some(st_sample) = st_sample {
                         let st_sample = st_sample.clone();
                         let st_info = match st.meta {
@@ -141,8 +132,6 @@ impl<'a> BlockGen<'a> {
         })
     }
 
-    #[allow(clippy::identity_op)] // TODO: Pending review in #587
-    #[allow(clippy::or_fun_call)] // TODO: Pending review in #587
     pub fn get_with_z_cache(
         &mut self,
         wpos: Vec3<i32>,
@@ -412,7 +401,7 @@ impl<'a> BlockGen<'a> {
             })
             .or(block);
 
-        Some(block.unwrap_or(Block::empty()))
+        Some(block.unwrap_or_else(Block::empty))
     }
 }
 
@@ -423,7 +412,6 @@ pub struct ZCache<'a> {
 }
 
 impl<'a> ZCache<'a> {
-    #[allow(clippy::unnecessary_mut_passed)] // TODO: Pending review in #587
     pub fn get_z_limits(&self, block_gen: &mut BlockGen) -> (f32, f32, f32) {
         let cave_depth =
             if self.sample.cave_xy.abs() > 0.9 && self.sample.water_level <= self.sample.alt {
@@ -436,7 +424,7 @@ impl<'a> ZCache<'a> {
         let min = min - 4.0;
 
         let cliff = BlockGen::get_cliff_height(
-            &mut block_gen.column_gen,
+            &block_gen.column_gen,
             &mut block_gen.column_cache,
             self.wpos.map(|e| e as f32),
             &self.sample.close_cliffs,
@@ -555,7 +543,6 @@ impl StructureInfo {
     }
 }
 
-#[allow(clippy::identity_op)] // TODO: Pending review in #587
 pub fn block_from_structure(
     sblock: StructureBlock,
     pos: Vec3<i32>,

world/src/sim/erosion.rs

@@ -27,7 +27,6 @@ pub type Computex8 = [Compute; 8];
 
 /// Compute the water flux at all chunks, given a list of chunk indices sorted
 /// by increasing height.
-#[allow(clippy::into_iter_on_ref)] // TODO: Pending review in #587
 pub fn get_drainage(newh: &[u32], downhill: &[isize], _boundary_len: usize) -> Box<[f32]> {
     // FIXME: Make the below work.  For now, we just use constant flux.
     // Initially, flux is determined by rainfall.  We currently treat this as the
@@ -40,7 +39,7 @@ pub fn get_drainage(newh: &[u32], downhill: &[isize], _boundary_len: usize) -> B
     // WORLD_SIZE.y) as f32);
     let base_flux = 1.0;
     let mut flux = vec![base_flux; WORLD_SIZE.x * WORLD_SIZE.y].into_boxed_slice();
-    newh.into_iter().rev().for_each(|&chunk_idx| {
+    newh.iter().rev().for_each(|&chunk_idx| {
         let chunk_idx = chunk_idx as usize;
         let downhill_idx = downhill[chunk_idx];
         if downhill_idx >= 0 {
@@ -52,7 +51,6 @@ pub fn get_drainage(newh: &[u32], downhill: &[isize], _boundary_len: usize) -> B
 
 /// Compute the water flux at all chunks for multiple receivers, given a list of
 /// chunk indices sorted by increasing height and weights for each receiver.
-#[allow(clippy::into_iter_on_ref)] // TODO: Pending review in #587
 pub fn get_multi_drainage(
     mstack: &[u32],
     mrec: &[u8],
@@ -69,7 +67,7 @@ pub fn get_multi_drainage(
     // there's no erosion anyway.
     let base_area = 1.0;
     let mut area = vec![base_area; WORLD_SIZE.x * WORLD_SIZE.y].into_boxed_slice();
-    mstack.into_iter().for_each(|&ij| {
+    mstack.iter().for_each(|&ij| {
         let ij = ij as usize;
         let wrec_ij = &mwrec[ij];
         let area_ij = area[ij];
@@ -219,8 +217,7 @@ impl RiverData {
             .unwrap_or(false)
     }
 
-    #[allow(clippy::len_zero)] // TODO: Pending review in #587
+    pub fn near_river(&self) -> bool { self.is_river() || !self.neighbor_rivers.is_empty() }
-    pub fn near_river(&self) -> bool { self.is_river() || self.neighbor_rivers.len() > 0 }
 
     pub fn near_water(&self) -> bool { self.near_river() || self.is_lake() || self.is_ocean() }
 }
@@ -228,7 +225,6 @@ impl RiverData {
 /// Draw rivers and assign them heights, widths, and velocities.  Take some
 /// liberties with the constant factors etc. in order to make it more likely
 /// that we draw rivers at all.
-#[allow(clippy::into_iter_on_ref)] // TODO: Pending review in #587
 pub fn get_rivers<F: fmt::Debug + Float + Into<f64>, G: Float + Into<f64>>(
     newh: &[u32],
     water_alt: &[F],
@@ -256,7 +252,7 @@ pub fn get_rivers<F: fmt::Debug + Float + Into<f64>, G: Float + Into<f64>>(
     // NOTE: This technically makes us discontinuous, so we should be cautious about
     // using this.
     let derivative_divisor = 1.0;
-    newh.into_iter().rev().for_each(|&chunk_idx| {
+    newh.iter().rev().for_each(|&chunk_idx| {
         let chunk_idx = chunk_idx as usize;
         let downhill_idx = downhill[chunk_idx];
         if downhill_idx < 0 {
@@ -601,10 +597,9 @@ fn get_max_slope(
                                 .max(dmin),
                         ),
             );
-            let max_slope = rock_strength * max_angle_range + min_max_angle;
             // NOTE: If you want to disable varying rock strength entirely, uncomment  this
             // line. let max_slope = 3.0.sqrt() / 3.0;
-            max_slope
+            rock_strength * max_angle_range + min_max_angle //max_slope
         })
         .collect::<Vec<_>>()
         .into_boxed_slice()
@@ -687,7 +682,6 @@ fn get_max_slope(
 ///     Prediction in Geomorphology, Geophysical Monograph 135.
 ///     Copyright 2003 by the American Geophysical Union
 ///     10.1029/135GM09
-#[allow(clippy::assign_op_pattern)] // TODO: Pending review in #587
 #[allow(clippy::many_single_char_names)]
 #[allow(clippy::too_many_arguments)]
 fn erode(
@@ -1620,7 +1614,7 @@ fn erode(
                 let mag_slope = dz / neighbor_distance;
                 if mag_slope >= max_slope {
                     let dtherm = 0.0;
-                    new_h_i = new_h_i - dtherm;
+                    new_h_i -= dtherm;
                     if new_h_i <= wh_j {
                         if compute_stats {
                             ncorr += 1;
@@ -1798,7 +1792,6 @@ pub fn fill_sinks<F: Float + Send + Sync>(
 /// - The adjacency list (stored in a single vector), indexed by the second
 ///   indirection vector.
 #[allow(clippy::filter_next)] // TODO: Pending review in #587
-#[allow(clippy::into_iter_on_ref)] // TODO: Pending review in #587
 pub fn get_lakes<F: Float>(
     h: impl Fn(usize) -> F,
     downhill: &mut [isize],
@@ -1839,7 +1832,7 @@ pub fn get_lakes<F: Float>(
     // First, find all the lakes.
     let mut lake_roots = Vec::with_capacity(downhill.len()); // Test
     (&*downhill)
-        .into_iter()
+        .iter()
         .enumerate()
         .filter(|(_, &dh_idx)| dh_idx < 0)
         .for_each(|(chunk_idx, &dh)| {
@@ -1888,7 +1881,7 @@ pub fn get_lakes<F: Float>(
     // with the starting index, resulting in a vector of slices.  As we go, we
     // replace each lake entry with its index in the new indirection buffer,
     // allowing
-    newh.into_iter().for_each(|&chunk_idx_| {
+    newh.iter().for_each(|&chunk_idx_| {
         let chunk_idx = chunk_idx_ as usize;
         let lake_idx_ = indirection_[chunk_idx];
         let lake_idx = lake_idx_ as usize;
@@ -2303,7 +2296,6 @@ pub fn mrec_downhill<'a>(
 /// * A bitmask representing which neighbors are downhill.
 /// * Stack order for multiple receivers (from top to bottom).
 /// * The weight for each receiver, for each node.
-#[allow(clippy::into_iter_on_ref)] // TODO: Pending review in #587
 #[allow(clippy::too_many_arguments)]
 #[allow(clippy::type_complexity)] // TODO: Pending review in #587
 pub fn get_multi_rec<F: fmt::Debug + Float + Sync + Into<Compute>>(
@@ -2356,7 +2348,7 @@ pub fn get_multi_rec<F: fmt::Debug + Float + Sync + Into<Compute>>(
     // or work out a more precise upper bound (since using nn * 2 * (maxh +
     // epsilon) makes f32 not work very well).
     let deltah = F::epsilon() + F::epsilon();
-    newh.into_iter().for_each(|&ijk| {
+    newh.iter().for_each(|&ijk| {
         let ijk = ijk as usize;
         let h_i = h(ijk);
         let ijr = downhill[ijk];

world/src/sim/mod.rs

@@ -304,8 +304,6 @@ pub struct WorldSim {
 }
 
 impl WorldSim {
-    #[allow(clippy::if_same_then_else)] // TODO: Pending review in #587
-    #[allow(clippy::let_and_return)] // TODO: Pending review in #587
     #[allow(clippy::unnested_or_patterns)] // TODO: Pending review in #587
 
     pub fn generate(seed: u32, opts: WorldOpts) -> Self {
@@ -973,7 +971,7 @@ impl WorldSim {
                 &rock_strength_nz,
                 // initial conditions
                 alt_func,
-                |posi| alt_func(posi) - if is_ocean_fn(posi) { 0.0 } else { 0.0 },
+                alt_func,
                 is_ocean_fn,
                 // empirical constants
                 uplift_fn,
@@ -1138,9 +1136,8 @@ impl WorldSim {
                 // Find the height of the pass into which our lake is flowing.
                 let pass_height_j = alt[neighbor_pass_idx as usize];
                 // Find the maximum of these two heights.
-                let pass_height = pass_height_i.max(pass_height_j);
                 // Use the pass height as the initial water altitude.
-                pass_height
+                pass_height_i.max(pass_height_j) /*pass_height*/
             };
             // Use the maximum of the pass height and chunk height as the parameter to
             // fill_sinks.
@@ -1322,7 +1319,6 @@ impl WorldSim {
     }
 
     /// Prepare the world for simulation
-    #[allow(clippy::useless_conversion)] // TODO: Pending review in #587
     pub fn seed_elements(&mut self) {
         let mut rng = self.rng.clone();
 
@@ -1452,10 +1448,9 @@ impl WorldSim {
                 const MAX_ITERS: usize = 64;
                 for _ in 0..MAX_ITERS {
                     let downhill_pos = match chunk.downhill {
-                        Some(downhill) => downhill
+                        Some(downhill) => {
-                            .map2(Vec2::from(TerrainChunkSize::RECT_SIZE), |e, sz: u32| {
+                            downhill.map2(TerrainChunkSize::RECT_SIZE, |e, sz: u32| e / (sz as i32))
-                                e / (sz as i32)
+                        },
-                            }),
                         None => return Some(pos),
                     };
 
@@ -1491,15 +1486,11 @@ impl WorldSim {
         }
     }
 
-    #[allow(clippy::useless_conversion)] // TODO: Pending review in #587
     pub fn get_gradient_approx(&self, chunk_pos: Vec2<i32>) -> Option<f32> {
         let a = self.get(chunk_pos)?;
         if let Some(downhill) = a.downhill {
-            let b = self.get(
+            let b =
-                downhill.map2(Vec2::from(TerrainChunkSize::RECT_SIZE), |e, sz: u32| {
+                self.get(downhill.map2(TerrainChunkSize::RECT_SIZE, |e, sz: u32| e / (sz as i32)))?;
-                    e / (sz as i32)
-                }),
-            )?;
             Some((a.alt - b.alt).abs() / TerrainChunkSize::RECT_SIZE.x as f32)
         } else {
             Some(0.0)
@@ -1510,13 +1501,10 @@ impl WorldSim {
         self.get_interpolated(wpos, |chunk| chunk.alt)
     }
 
-    #[allow(clippy::useless_conversion)] // TODO: Pending review in #587
     pub fn get_wpos(&self, wpos: Vec2<i32>) -> Option<&SimChunk> {
-        self.get(
+        self.get(wpos.map2(TerrainChunkSize::RECT_SIZE, |e, sz: u32| {
-            wpos.map2(Vec2::from(TerrainChunkSize::RECT_SIZE), |e, sz: u32| {
+            e.div_euclid(sz as i32)
-                e.div_euclid(sz as i32)
+        }))
-            }),
-        )
     }
 
     pub fn get_mut(&mut self, chunk_pos: Vec2<i32>) -> Option<&mut SimChunk> {
@@ -1711,13 +1699,12 @@ impl WorldSim {
         Some(z0 + z1 + z2 + z3)
     }
 
-    #[allow(clippy::useless_conversion)] // TODO: Pending review in #587
     pub fn get_nearest_path(&self, wpos: Vec2<i32>) -> Option<(f32, Vec2<f32>)> {
-        let chunk_pos = wpos.map2(Vec2::from(TerrainChunkSize::RECT_SIZE), |e, sz: u32| {
+        let chunk_pos = wpos.map2(TerrainChunkSize::RECT_SIZE, |e, sz: u32| {
             e.div_euclid(sz as i32)
         });
         let get_chunk_centre = |chunk_pos: Vec2<i32>| {
-            chunk_pos.map2(Vec2::from(TerrainChunkSize::RECT_SIZE), |e, sz: u32| {
+            chunk_pos.map2(TerrainChunkSize::RECT_SIZE, |e, sz: u32| {
                 e * sz as i32 + sz as i32 / 2
             })
         };