Make clippy happy and add to CI

* Add clippy to CI pipeline - might as well let CI do its magic :) * Applied a number of fixes suggested by clippy -- tests seem to still pass, and they do make sense: * `&Vec<Coordinate<f64>>` -> `&[Coordinate<f64>]` * switch to implicit returns * `&String` -> `&str` * many boolean `assert_eq!(true, ...)` -> `assert!(...)` -- code much more compact and easy to read * `points.iter().copied().collect()` -> `points.to_vec()` * a few useless clones on copyable objects * Allow excessive float precision in tests (complex topic, didn't want it to trigger all the time) * Bypass a bug in rustc clippy suggestion - that code might be fixed separately if desired
thehappycheese · Feb 22, 2022 · 15e1609 · 15e1609
1 parent c197141
commit 15e1609
Show file tree

Hide file tree

Showing 12 changed files with 61 additions and 89 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -52,6 +52,7 @@ jobs:
       - name: Checkout repository
         uses: actions/checkout@v2
       - run: cargo check --all-targets --no-default-features
+      - run: cargo clippy --all-targets --no-default-features -- -Dwarnings
       - run: cargo test --all-features
 
   geo:
@@ -78,6 +79,7 @@ jobs:
       - name: Checkout repository
         uses: actions/checkout@v2
       - run: cargo check --all-targets --no-default-features
+      - run: cargo clippy --all-targets --no-default-features -- -Dwarnings
       # we don't want to test `proj-network` because it only enables the `proj` feature
       - run: cargo test --features "use-proj use-serde"
 
@@ -105,6 +107,7 @@ jobs:
       - name: Checkout repository
         uses: actions/checkout@v2
       - run: cargo check --all-targets
+      - run: cargo clippy --all-targets -- -Dwarnings
       - run: cargo test
 
   geo_fuzz:

diff --git a/geo/benches/vincenty_distance.rs b/geo/benches/vincenty_distance.rs
@@ -7,7 +7,7 @@ use geo::prelude::*;
 fn criterion_benchmark(c: &mut criterion::Criterion) {
     c.bench_function("vincenty distance f32", |bencher| {
         let a = geo::Point::<f32>::new(17.107558, 48.148636);
-        let b = geo::Point::<f32>::new(16.372477, 48.208810);
+        let b = geo::Point::<f32>::new(16.372477, 48.20881);
 
         bencher.iter(|| {
             let _ = criterion::black_box(

diff --git a/geo/examples/concavehull-usage.rs b/geo/examples/concavehull-usage.rs
@@ -5,7 +5,7 @@ use geo_types::MultiPoint;
 use std::fs::File;
 use std::io::Write;
 
-fn generate_polygon_str(coords: &Vec<Coordinate<f64>>) -> String {
+fn generate_polygon_str(coords: &[Coordinate<f64>]) -> String {
     let mut points_str = String::from("");
     for coord in coords {
         points_str.push_str(format!("{},{} ", coord.x, coord.y).as_ref());
@@ -16,21 +16,21 @@ fn generate_polygon_str(coords: &Vec<Coordinate<f64>>) -> String {
     );
 }
 
-fn generate_consecutive_circles(coords: &Vec<Coordinate<f64>>) -> String {
+fn generate_consecutive_circles(coords: &[Coordinate<f64>]) -> String {
     let mut circles_str = String::from("");
     for coord in coords {
         circles_str.push_str(
             format!("<circle cx=\"{}\" cy=\"{}\" r=\"1\"/>\n", coord.x, coord.y).as_ref(),
         );
     }
-    return circles_str;
+    circles_str
 }
 
-fn produce_file_content(start_str: &String, mid_str: String) -> String {
-    let mut overall_string = start_str.clone();
+fn produce_file_content(start_str: &str, mid_str: String) -> String {
+    let mut overall_string = start_str.to_string();
     overall_string.push_str(mid_str.as_ref());
     overall_string.push_str("</svg>");
-    return overall_string;
+    overall_string
 }
 
 //Move the points such that they're clustered around the center of the image

diff --git a/geo/src/algorithm/closest_point.rs b/geo/src/algorithm/closest_point.rs
@@ -184,6 +184,7 @@ impl<F: GeoFloat> ClosestPoint<F> for Geometry<F> {
 
 #[cfg(test)]
 mod tests {
+    #![allow(clippy::excessive_precision)]
     use super::*;
     use crate::{point, polygon};
 

diff --git a/geo/src/algorithm/concave_hull.rs b/geo/src/algorithm/concave_hull.rs
@@ -150,6 +150,11 @@ where
                 .locate_within_distance(closest_point, search_dist)
                 .peekable();
             let peeked_edge = edges_nearby_point.peek();
+
+            // Clippy is having an issue here. It might be a valid suggestion,
+            // but the automatic clippy fix breaks the code, so may need to be done by hand.
+            // See https://github.com/rust-lang/rust/issues/94241
+            #[allow(clippy::manual_map)]
             let closest_edge_option = match peeked_edge {
                 None => None,
                 Some(&edge) => Some(edges_nearby_point.fold(*edge, |acc, candidate| {

diff --git a/geo/src/algorithm/contains/mod.rs b/geo/src/algorithm/contains/mod.rs
@@ -88,7 +88,7 @@ mod test {
             .into(),
             vec![],
         );
-        assert_eq!(!v.contains(&rect), true);
+        assert!(!v.contains(&rect));
     }
     #[test]
     // V contains rect because all its vertices are contained, and none of its edges intersect with V's boundaries
@@ -117,7 +117,7 @@ mod test {
             .into(),
             vec![],
         );
-        assert_eq!(v.contains(&rect), true);
+        assert!(v.contains(&rect));
     }
     #[test]
     // LineString is fully contained
@@ -127,7 +127,7 @@ mod test {
             vec![],
         );
         let ls = LineString::from(vec![(3.0, 0.5), (3.0, 3.5)]);
-        assert_eq!(poly.contains(&ls), true);
+        assert!(poly.contains(&ls));
     }
     /// Tests: Point in LineString
     #[test]
@@ -314,8 +314,8 @@ mod test {
             Coordinate { x: -10., y: -20. },
             Coordinate { x: 10., y: 20. },
         );
-        assert_eq!(true, bounding_rect_xl.contains(&bounding_rect_sm));
-        assert_eq!(false, bounding_rect_sm.contains(&bounding_rect_xl));
+        assert!(bounding_rect_xl.contains(&bounding_rect_sm));
+        assert!(!bounding_rect_sm.contains(&bounding_rect_xl));
     }
     #[test]
     fn point_in_line_test() {
@@ -349,11 +349,11 @@ mod test {
     fn linestring_in_line_test() {
         let line = Line::from([(0, 10), (30, 40)]);
         // linestring0 in line
-        let linestring0 = LineString::from(vec![(01, 11), (10, 20), (15, 25)]);
+        let linestring0 = LineString::from(vec![(1, 11), (10, 20), (15, 25)]);
         // linestring1 starts and ends in line, but wanders in the middle
-        let linestring1 = LineString::from(vec![(01, 11), (20, 20), (15, 25)]);
+        let linestring1 = LineString::from(vec![(1, 11), (20, 20), (15, 25)]);
         // linestring2 is co-linear, but extends beyond line
-        let linestring2 = LineString::from(vec![(01, 11), (10, 20), (40, 50)]);
+        let linestring2 = LineString::from(vec![(1, 11), (10, 20), (40, 50)]);
         // no part of linestring3 is contained in line
         let linestring3 = LineString::from(vec![(11, 11), (20, 20), (25, 25)]);
         // a linestring with singleton interior on the boundary of the line

diff --git a/geo/src/algorithm/convex_hull/mod.rs b/geo/src/algorithm/convex_hull/mod.rs
@@ -116,7 +116,7 @@ where
 
     // Remove repeated points unless collinear points
     // are to be included.
-    let mut ls: Vec<Coordinate<T>> = points.iter().copied().collect();
+    let mut ls: Vec<Coordinate<T>> = points.to_vec();
     if !include_on_hull {
         ls.dedup();
     }

diff --git a/geo/src/algorithm/haversine_distance.rs b/geo/src/algorithm/haversine_distance.rs
@@ -58,6 +58,7 @@ where
 
 #[cfg(test)]
 mod test {
+    #![allow(clippy::excessive_precision)]
     use crate::algorithm::haversine_distance::HaversineDistance;
     use crate::Point;
 

diff --git a/geo/src/algorithm/intersects/mod.rs b/geo/src/algorithm/intersects/mod.rs
@@ -370,10 +370,10 @@ mod test {
         let bounding_rect_s2 =
             Rect::new(Coordinate { x: 0., y: 0. }, Coordinate { x: 20., y: 30. });
         // confirmed using GEOS
-        assert_eq!(true, bounding_rect_xl.intersects(&bounding_rect_sm));
-        assert_eq!(true, bounding_rect_sm.intersects(&bounding_rect_xl));
-        assert_eq!(true, bounding_rect_sm.intersects(&bounding_rect_s2));
-        assert_eq!(true, bounding_rect_s2.intersects(&bounding_rect_sm));
+        assert!(bounding_rect_xl.intersects(&bounding_rect_sm));
+        assert!(bounding_rect_sm.intersects(&bounding_rect_xl));
+        assert!(bounding_rect_sm.intersects(&bounding_rect_s2));
+        assert!(bounding_rect_s2.intersects(&bounding_rect_sm));
     }
     #[test]
     fn rect_intersection_consistent_with_poly_intersection_test() {
@@ -388,65 +388,29 @@ mod test {
         let bounding_rect_s2 =
             Rect::new(Coordinate { x: 0., y: 0. }, Coordinate { x: 20., y: 30. });
 
-        assert_eq!(
-            true,
-            bounding_rect_xl.to_polygon().intersects(&bounding_rect_sm)
-        );
-        assert_eq!(
-            true,
-            bounding_rect_xl.intersects(&bounding_rect_sm.to_polygon())
-        );
-        assert_eq!(
-            true,
-            bounding_rect_xl
-                .to_polygon()
-                .intersects(&bounding_rect_sm.to_polygon())
-        );
+        assert!(bounding_rect_xl.to_polygon().intersects(&bounding_rect_sm));
+        assert!(bounding_rect_xl.intersects(&bounding_rect_sm.to_polygon()));
+        assert!(bounding_rect_xl
+            .to_polygon()
+            .intersects(&bounding_rect_sm.to_polygon()));
 
-        assert_eq!(
-            true,
-            bounding_rect_sm.to_polygon().intersects(&bounding_rect_xl)
-        );
-        assert_eq!(
-            true,
-            bounding_rect_sm.intersects(&bounding_rect_xl.to_polygon())
-        );
-        assert_eq!(
-            true,
-            bounding_rect_sm
-                .to_polygon()
-                .intersects(&bounding_rect_xl.to_polygon())
-        );
+        assert!(bounding_rect_sm.to_polygon().intersects(&bounding_rect_xl));
+        assert!(bounding_rect_sm.intersects(&bounding_rect_xl.to_polygon()));
+        assert!(bounding_rect_sm
+            .to_polygon()
+            .intersects(&bounding_rect_xl.to_polygon()));
 
-        assert_eq!(
-            true,
-            bounding_rect_sm.to_polygon().intersects(&bounding_rect_s2)
-        );
-        assert_eq!(
-            true,
-            bounding_rect_sm.intersects(&bounding_rect_s2.to_polygon())
-        );
-        assert_eq!(
-            true,
-            bounding_rect_sm
-                .to_polygon()
-                .intersects(&bounding_rect_s2.to_polygon())
-        );
+        assert!(bounding_rect_sm.to_polygon().intersects(&bounding_rect_s2));
+        assert!(bounding_rect_sm.intersects(&bounding_rect_s2.to_polygon()));
+        assert!(bounding_rect_sm
+            .to_polygon()
+            .intersects(&bounding_rect_s2.to_polygon()));
 
-        assert_eq!(
-            true,
-            bounding_rect_s2.to_polygon().intersects(&bounding_rect_sm)
-        );
-        assert_eq!(
-            true,
-            bounding_rect_s2.intersects(&bounding_rect_sm.to_polygon())
-        );
-        assert_eq!(
-            true,
-            bounding_rect_s2
-                .to_polygon()
-                .intersects(&bounding_rect_sm.to_polygon())
-        );
+        assert!(bounding_rect_s2.to_polygon().intersects(&bounding_rect_sm));
+        assert!(bounding_rect_s2.intersects(&bounding_rect_sm.to_polygon()));
+        assert!(bounding_rect_s2
+            .to_polygon()
+            .intersects(&bounding_rect_sm.to_polygon()));
     }
     #[test]
     fn point_intersects_line_test() {
@@ -589,9 +553,9 @@ mod test {
             Coordinate { x: 10., y: 20. },
             Coordinate { x: 20., y: -10. },
         );
-        let geom = Geometry::Point(pt.clone());
+        let geom = Geometry::Point(pt);
         let gc = GeometryCollection(vec![geom.clone()]);
-        let multi_point = MultiPoint(vec![pt.clone()]);
+        let multi_point = MultiPoint(vec![pt]);
         let multi_ls = MultiLineString(vec![ls.clone()]);
         let multi_poly = MultiPolygon(vec![poly.clone()]);
 

diff --git a/geo/src/algorithm/line_intersection.rs b/geo/src/algorithm/line_intersection.rs
@@ -299,6 +299,7 @@ fn proper_intersection<F: GeoFloat>(p: Line<F>, q: Line<F>) -> Coordinate<F> {
 
 #[cfg(test)]
 mod test {
+    #![allow(clippy::excessive_precision)]
     use super::*;
 
     /// Based on JTS test `testCentralEndpointHeuristicFailure`

diff --git a/geo/src/algorithm/rotate.rs b/geo/src/algorithm/rotate.rs
@@ -332,6 +332,7 @@ where
 
 #[cfg(test)]
 mod test {
+    #![allow(clippy::excessive_precision)]
     use super::*;
     use crate::{line_string, point, polygon, Coordinate, Point};
     use approx::assert_relative_eq;

diff --git a/geo/src/algorithm/simplifyvw.rs b/geo/src/algorithm/simplifyvw.rs
@@ -704,13 +704,13 @@ mod test {
         let c = point!(x: 3., y: 3.);
         let d = point!(x: 1., y: 1.);
         // cw + ccw
-        assert_eq!(cartesian_intersect(a, b, c, d), true);
+        assert!(cartesian_intersect(a, b, c, d));
         // ccw + ccw
-        assert_eq!(cartesian_intersect(b, a, c, d), true);
+        assert!(cartesian_intersect(b, a, c, d));
         // cw + cw
-        assert_eq!(cartesian_intersect(a, b, d, c), true);
+        assert!(cartesian_intersect(a, b, d, c));
         // ccw + cw
-        assert_eq!(cartesian_intersect(b, a, d, c), true);
+        assert!(cartesian_intersect(b, a, d, c));
     }
     #[test]
     fn simple_vwp_test() {
@@ -814,7 +814,7 @@ mod test {
             initial_min: 2,
             min_points: 4,
         };
-        let simplified = vwp_wrapper(gt, &points_ls.into(), None, &0.0005);
+        let simplified = vwp_wrapper(gt, &points_ls, None, &0.0005);
         assert_eq!(simplified[0].len(), 3278);
     }
 
@@ -843,12 +843,8 @@ mod test {
     }
     #[test]
     fn visvalingam_test_two_point_linestring() {
-        let mut vec = Vec::new();
-        vec.push(Point::new(0.0, 0.0));
-        vec.push(Point::new(27.8, 0.1));
-        let mut compare = Vec::new();
-        compare.push(Coordinate::from((0.0, 0.0)));
-        compare.push(Coordinate::from((27.8, 0.1)));
+        let vec = vec![Point::new(0.0, 0.0), Point::new(27.8, 0.1)];
+        let compare = vec![Coordinate::from((0.0, 0.0)), Coordinate::from((27.8, 0.1))];
         let simplified = visvalingam(&LineString::from(vec), &1.0);
         assert_eq!(simplified, compare);
     }