Merge georust#740

740: Make clippy happy and add it to CI r=michaelkirk a=nyurik * 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 * Made a tiny improvement to `geo/examples/concavehull-usage.rs` by switching `String` to `&str` - [x] I agree to follow the project's [code of conduct](https://github.com/georust/geo/blob/master/CODE_OF_CONDUCT.md). - ~I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users.~ --- fixes georust#558 Co-authored-by: Yuri Astrakhan <YuriAstrakhan@gmail.com>
nolotz · Feb 22, 2022 · 87dd612 · 87dd612
2 parents efdb9e1 + a0529a9
commit 87dd612
Show file tree

Hide file tree

Showing 13 changed files with 90 additions and 107 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -21,6 +21,7 @@ jobs:
     name: ci result
     runs-on: ubuntu-latest
     needs:
+      - clippy
       - geo_types
       - geo
       - geo_postgis
@@ -34,6 +35,23 @@ jobs:
         if: "!success()"
         run: exit 1
 
+  clippy:
+    name: clippy
+    runs-on: ubuntu-latest
+    if: "!contains(github.event.head_commit.message, '[skip ci]')"
+    strategy:
+      matrix:
+        container_image:
+          # Use the latest stable clippy. No need for older versions.
+          - "georust/geo-ci:rust-1.58"
+    container:
+      image: ${{ matrix.container_image }}
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v2
+      - run: rustup component add clippy
+      - run: cargo clippy --all-features --all-targets -- -Dwarnings
+
   geo_types:
     name: geo-types
     runs-on: ubuntu-latest

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();
-    overall_string.push_str(mid_str.as_ref());
+fn produce_file_content(start_str: &str, mid_str: &str) -> String {
+    let mut overall_string = start_str.to_string();
+    overall_string.push_str(mid_str);
     overall_string.push_str("</svg>");
-    return overall_string;
+    overall_string
 }
 
 //Move the points such that they're clustered around the center of the image
@@ -73,9 +73,9 @@ fn main() -> std::io::Result<()> {
     let convex_polygon_str = generate_polygon_str(&convex.exterior().0);
     let v_coords = map_points_to_coords(multipoint.0);
     let circles_str = generate_consecutive_circles(&v_coords);
-    let points_str = produce_file_content(&svg_file_string, circles_str);
-    let concave_hull_str = produce_file_content(&svg_file_string, concave_polygon_str);
-    let convex_hull_str = produce_file_content(&svg_file_string, convex_polygon_str);
+    let points_str = produce_file_content(&svg_file_string, &circles_str);
+    let concave_hull_str = produce_file_content(&svg_file_string, &concave_polygon_str);
+    let convex_hull_str = produce_file_content(&svg_file_string, &convex_polygon_str);
 
     points_file.write_all(points_str.as_ref())?;
     concave_hull_file.write_all(concave_hull_str.as_ref())?;

diff --git a/geo/src/algorithm/closest_point.rs b/geo/src/algorithm/closest_point.rs
@@ -277,7 +277,7 @@ mod tests {
     #[test]
     fn polygon_without_rings_and_point_outside_is_same_as_linestring() {
         let poly = holy_polygon();
-        let p = Point::new(1000.0, 12345.6789);
+        let p = Point::new(1000.0, 12345.678);
         assert!(
             !poly.exterior().contains(&p),
             "`p` should be outside the 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
@@ -98,8 +98,8 @@ mod test {
     #[test]
     fn distance3_test_f32() {
         // this input comes from issue #100
-        let a = Point::<f32>::new(-77.036585, 38.897448);
-        let b = Point::<f32>::new(-77.009080, 38.889825);
+        let a = Point::<f32>::new(-77.03658, 38.89745);
+        let b = Point::<f32>::new(-77.00908, 38.889825);
         assert_relative_eq!(a.haversine_distance(&b), 2526.8354_f32, epsilon = 1.0e-6);
     }
 }
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
@@ -538,7 +538,7 @@ mod test {
         let line_1 = Line::new(
             Coordinate {
                 x: 2089426.5233462777,
-                y: 1180182.3877339689,
+                y: 1180182.387733969,
             },
             Coordinate {
                 x: 2085646.6891757075,
@@ -551,7 +551,7 @@ mod test {
                 y: 1997547.0560044837,
             },
             Coordinate {
-                x: 2259977.3672235999,
+                x: 2259977.3672236,
                 y: 483675.17050843034,
             },
         );

diff --git a/geo/src/algorithm/map_coords.rs b/geo/src/algorithm/map_coords.rs
@@ -836,7 +836,7 @@ mod test {
         use proj::{Coord, Proj, ProjError};
         let from = "EPSG:4326";
         let to = "EPSG:2230";
-        let to_feet = Proj::new_known_crs(&from, &to, None).unwrap();
+        let to_feet = Proj::new_known_crs(from, to, None).unwrap();
 
         let f = |x: f64, y: f64| -> Result<_, ProjError> {
             let shifted = to_feet.convert((x, y))?;

diff --git a/geo/src/algorithm/rotate.rs b/geo/src/algorithm/rotate.rs
@@ -656,16 +656,16 @@ mod test {
         let expected_centroid: MultiPolygon<f64> = vec![
             polygon![
                 (x: 0., y: 0.),
-                (x: 7.0710678118654755, y: 7.0710678118654746),
-                (x: 0., y: 14.1421356237309510),
-                (x: -7.0710678118654746, y: 7.0710678118654755),
+                (x: 7.0710678118654755, y: 7.071067811865475),
+                (x: 0., y: 14.142135623730951),
+                (x: -7.071067811865475, y: 7.0710678118654755),
                 (x: 0., y: 0.),
             ],
             polygon![
                 (x: 0., y: 0.),
-                (x: -7.0710678118654755, y: -7.0710678118654746),
-                (x: 0., y: -14.1421356237309510),
-                (x: 7.0710678118654746, y: -7.0710678118654755),
+                (x: -7.0710678118654755, y: -7.071067811865475),
+                (x: 0., y: -14.142135623730951),
+                (x: 7.071067811865475, y: -7.0710678118654755),
                 (x: 0., y: 0.),
             ],
         ]
@@ -727,11 +727,11 @@ mod test {
                 (x: -0.1016007672888048, y: 3.05186627999456),
             ],
             polygon![
-                (x: 8.591731669312811, y: 0.7224948740718733),
+                (x: 8.59173166931281, y: 0.7224948740718733),
                 (x: 10.52358332189095, y: 0.2048567838668318),
                 (x: 13.37059281801868, y: 10.83004087304658),
                 (x: 11.43874116544054, y: 11.34767896325162),
-                (x: 8.591731669312811, y: 0.7224948740718733),
+                (x: 8.59173166931281, y: 0.7224948740718733),
             ],
         ]
         .into();