Skip to content

Commit fe7e349

Browse files
brawerclaude
andauthored
Fix all clippy warnings (#578)
* Fix all clippy warnings Addresses clippy errors so that `cargo clippy --locked --all-targets -- -D warnings` passes cleanly: - assert_eq!(x, true/false) -> assert!(x) / assert!(!x) in tests - Remove needless borrows (&path.path(), &geojson, etc.) - Change `const TEST_X: LazyLock<...>` to `static` (interior mutability in const), which also fixes downstream borrow_interior_mutable_const warnings - Replace `x == None` with `x.is_none()` - Replace manual Option::map(|x| x.clone()) with .cloned() - Elide redundant lifetime in atp::mod::tags - Remove redundant `use predicates;` import - Fix inconsistent digit grouping (11_000_000_0 -> 110_000_000, etc.) - Replace assert!(false, ...) with panic!(...) - Replace .into_iter() on array ref with .iter() - Remove redundant & in format! argument Also ran cargo fmt to keep formatting consistent. src/pipeline/osm/id_tagging_schema.rs is excluded here: it's generated by scripts/generate_id_tagging_schema.py, so its remaining clippy warnings (bool_assert_comparison) will be fixed by updating the generator in a separate PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Group positive/negative cases in test_is_wikidata_id Per review feedback: visually separate the wikidata-key assertions into a "true" block and a "false" block within the same test, instead of interleaving them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Group positive/negative cases in remaining bool assertions Per review feedback: visually separate the "should be present" assertions from the "should be absent" ones, in the same tests, for src/tables/u64_set.rs (test_contains, test_create, writer::tests::test_create, writer::tests::test_create_single_value) and src/coverage.rs (contains_wikidata_item test). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent a795b6b commit fe7e349

14 files changed

Lines changed: 120 additions & 139 deletions

File tree

src/atp/fetch.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ mod tests {
144144
let client = test_client(&server);
145145
let progress = MultiProgress::with_draw_target(ProgressDrawTarget::hidden());
146146
let workdir = TempDir::new()?;
147-
let path = fetch_atp(&mock_history_url, &client, &progress, &workdir.path()).await?;
147+
let path = fetch_atp(&mock_history_url, &client, &progress, workdir.path()).await?;
148148
mock_history.assert_async().await;
149149
mock_atp_data.assert_async().await;
150150

src/atp/mod.rs

Lines changed: 39 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ mod tests {
388388

389389
#[test]
390390
fn test_find_point_for_line_string() {
391-
let pt = find_point(&BICYCLE_ROAD).unwrap();
391+
let pt = find_point(BICYCLE_ROAD).unwrap();
392392
assert!((pt.x() - 7.4593195).abs() < 1e-6);
393393
assert!((pt.y() - 46.9423753).abs() < 1e-6);
394394
}
@@ -413,12 +413,12 @@ mod tests {
413413
]
414414
}
415415
}"#;
416-
let pt = find_point(&geojson).unwrap();
416+
let pt = find_point(geojson).unwrap();
417417
assert!((pt.x() - -72.4474).abs() < 1e-3);
418418
assert!((pt.y() - 25.3935).abs() < 1e-3);
419419
}
420420

421-
fn tags<'a>(place: &'a Place) -> Vec<(&'a str, &'a str)> {
421+
fn tags(place: &Place) -> Vec<(&str, &str)> {
422422
place
423423
.tags
424424
.iter()
@@ -504,54 +504,42 @@ mod tests {
504504

505505
#[test]
506506
fn test_is_usable_for_osm() {
507-
assert_eq!(
508-
is_usable_for_osm(&make_dataset(&[("use:openstreetmap", "yes")])),
509-
true
510-
);
511-
assert_eq!(
512-
is_usable_for_osm(&make_dataset(&[("use:openstreetmap", "no")])),
513-
false
514-
);
515-
assert_eq!(
516-
is_usable_for_osm(&make_dataset(&[
517-
("license", "Creative Commons Zero"),
518-
("license:wikidata", "Q6938433"),
519-
("spider:lineage", "S_ATP_AGGREGATORS"),
520-
])),
521-
true
522-
);
523-
assert_eq!(
524-
is_usable_for_osm(&make_dataset(&[
525-
("license", "Creative Commons Zero"),
526-
("license:wikidata", "Q6938433"),
527-
("use:openstreetmap", "no"),
528-
])),
529-
false
530-
);
531-
assert_eq!(
532-
is_usable_for_osm(&make_dataset(&[("spider:lineage", "S_ATP_BRANDS")])),
533-
true
534-
);
535-
assert_eq!(
536-
is_usable_for_osm(&make_dataset(&[
537-
("license", "Creative Commons Attribution 4.0 International"),
538-
("license:wikidata", "Q20007257"),
539-
("spider:lineage", "S_ATP_BRANDS")
540-
])),
541-
false
542-
);
543-
assert_eq!(
544-
is_usable_for_osm(&make_dataset(&[
545-
("license", "Etalab Open License 2.0"),
546-
("license:wikidata", "Q80939351"),
547-
("spider:lineage", "S_ATP_GOVERNMENT")
548-
])),
549-
true
550-
);
551-
assert_eq!(
552-
is_usable_for_osm(&make_dataset(&[("spider:lineage", "S_ATP_AGGREGATORS")])),
553-
false
554-
);
507+
assert!(is_usable_for_osm(&make_dataset(&[(
508+
"use:openstreetmap",
509+
"yes"
510+
)])));
511+
assert!(!is_usable_for_osm(&make_dataset(&[(
512+
"use:openstreetmap",
513+
"no"
514+
)])));
515+
assert!(is_usable_for_osm(&make_dataset(&[
516+
("license", "Creative Commons Zero"),
517+
("license:wikidata", "Q6938433"),
518+
("spider:lineage", "S_ATP_AGGREGATORS"),
519+
])));
520+
assert!(!is_usable_for_osm(&make_dataset(&[
521+
("license", "Creative Commons Zero"),
522+
("license:wikidata", "Q6938433"),
523+
("use:openstreetmap", "no"),
524+
])));
525+
assert!(is_usable_for_osm(&make_dataset(&[(
526+
"spider:lineage",
527+
"S_ATP_BRANDS"
528+
)])));
529+
assert!(!is_usable_for_osm(&make_dataset(&[
530+
("license", "Creative Commons Attribution 4.0 International"),
531+
("license:wikidata", "Q20007257"),
532+
("spider:lineage", "S_ATP_BRANDS")
533+
])));
534+
assert!(is_usable_for_osm(&make_dataset(&[
535+
("license", "Etalab Open License 2.0"),
536+
("license:wikidata", "Q80939351"),
537+
("spider:lineage", "S_ATP_GOVERNMENT")
538+
])));
539+
assert!(!is_usable_for_osm(&make_dataset(&[(
540+
"spider:lineage",
541+
"S_ATP_AGGREGATORS"
542+
)])));
555543
}
556544

557545
fn make_dataset(tags: &[(&str, &str)]) -> geojson::FeatureCollection {

src/coverage.rs

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,13 @@ mod tests {
4040

4141
#[test]
4242
fn test_is_wikidata_id() {
43-
assert_eq!(is_wikidata_key("highway"), false);
44-
assert_eq!(is_wikidata_key("wikidata"), true);
45-
assert_eq!(is_wikidata_key("brand:wikidata"), true);
46-
assert_eq!(is_wikidata_key("network:wikidata"), true);
47-
assert_eq!(is_wikidata_key("operator:wikidata"), true);
48-
assert_eq!(is_wikidata_key("species:wikidata"), false);
43+
assert!(is_wikidata_key("wikidata"));
44+
assert!(is_wikidata_key("brand:wikidata"));
45+
assert!(is_wikidata_key("network:wikidata"));
46+
assert!(is_wikidata_key("operator:wikidata"));
47+
48+
assert!(!is_wikidata_key("highway"));
49+
assert!(!is_wikidata_key("species:wikidata"));
4950
}
5051

5152
#[test]
@@ -211,7 +212,7 @@ mod reader {
211212
let mut bytes = Vec::new();
212213
bytes.extend_from_slice(b"diffed-places coverage\0\0");
213214
bytes.extend_from_slice(&2_u64.to_le_bytes());
214-
assert!(Coverage::get_offset_size(b"some_key", &bytes, 1) == None);
215+
assert!(Coverage::get_offset_size(b"some_key", &bytes, 1).is_none());
215216
for (key, offset, size) in [(b"some_key", 80, 16), (b"otherkey", 83, 2)] {
216217
bytes.extend_from_slice(key as &[u8; 8]);
217218
bytes.extend_from_slice(&(offset as u64).to_le_bytes());
@@ -721,12 +722,13 @@ mod writer {
721722
);
722723
}
723724

724-
assert_eq!(cov.contains_wikidata_item(1), false);
725-
assert_eq!(cov.contains_wikidata_item(23), true);
726-
assert_eq!(cov.contains_wikidata_item(51), false);
727-
assert_eq!(cov.contains_wikidata_item(77), true);
728-
assert_eq!(cov.contains_wikidata_item(88), true);
729-
assert_eq!(cov.contains_wikidata_item(89), false);
725+
assert!(cov.contains_wikidata_item(23));
726+
assert!(cov.contains_wikidata_item(77));
727+
assert!(cov.contains_wikidata_item(88));
728+
729+
assert!(!cov.contains_wikidata_item(1));
730+
assert!(!cov.contains_wikidata_item(51));
731+
assert!(!cov.contains_wikidata_item(89));
730732

731733
Ok(())
732734
}

src/pipeline/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ mod tests {
7878
f7.as_file().set_modified(t7)?;
7979

8080
assert!(last_modified(&[]).is_err());
81-
assert!(last_modified(&[&Path::new("/no/such/file")]).is_err());
81+
assert!(last_modified(&[Path::new("/no/such/file")]).is_err());
8282

8383
assert_eq!(last_modified(&[f0.path()])?, t0);
8484
assert_eq!(last_modified(&[f0.path(), f2.path()])?, t2);

src/pipeline/osm/coords.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ mod tests {
6969
let workdir = TempDir::new()?;
7070
let keys_file = NamedTempFile::new()?;
7171
let data_file = NamedTempFile::new()?;
72-
build_tables(rx, &workdir.path(), &keys_file.path(), &data_file.path())?;
72+
build_tables(rx, workdir.path(), keys_file.path(), data_file.path())?;
7373

7474
let keys: Vec<u64> = {
7575
let mut buf = Vec::new();

src/pipeline/osm/filter.rs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,7 +1065,7 @@ pub mod filtered_file {
10651065
bytes.extend_from_slice(&0xdeadbeefcafefeed_u64.to_le_bytes()); // [32..40]: data[0]
10661066
bytes.extend_from_slice(&42_u64.to_le_bytes()); // [40..48]: data[1]
10671067
bytes.extend_from_slice(&2_u64.to_le_bytes()); // [48..56]: num_headers
1068-
assert!(FilteredFile::get_offset_size(b"some_key", &bytes, 1) == None);
1068+
assert!(FilteredFile::get_offset_size(b"some_key", &bytes, 1).is_none());
10691069
for (key, offset, size) in [(b"some_key", 88, 16), (b"otherkey", 91, 2)] {
10701070
bytes.extend_from_slice(key as &[u8; 8]);
10711071
bytes.extend_from_slice(&(offset as u64).to_le_bytes());
@@ -1120,7 +1120,7 @@ pub mod filtered_file {
11201120
}
11211121
file.sync_all()?;
11221122
}
1123-
writer.write_coords(&keys_path, &data.path())?;
1123+
writer.write_coords(&keys_path, data.path())?;
11241124
writer.close()?;
11251125
let ff = FilteredFile::open(tmp.path())?;
11261126
assert_eq!(
@@ -1195,12 +1195,12 @@ pub mod filtered_file {
11951195
writer.write_node_refs(&test_data_path("u64_le_0_2_7"))?;
11961196
writer.close()?;
11971197
let ff = FilteredFile::open(tmp.path())?;
1198-
assert_eq!(ff.has_node_ref(0), true);
1199-
assert_eq!(ff.has_node_ref(2), true);
1200-
assert_eq!(ff.has_node_ref(7), true);
1201-
assert_eq!(ff.has_node_ref(1), false);
1202-
assert_eq!(ff.has_node_ref(8), false);
1203-
assert_eq!(ff.has_node_ref(1234567890123456789), false);
1198+
assert!(ff.has_node_ref(0));
1199+
assert!(ff.has_node_ref(2));
1200+
assert!(ff.has_node_ref(7));
1201+
assert!(!ff.has_node_ref(1));
1202+
assert!(!ff.has_node_ref(8));
1203+
assert!(!ff.has_node_ref(1234567890123456789));
12041204
Ok(())
12051205
}
12061206

@@ -1211,12 +1211,12 @@ pub mod filtered_file {
12111211
writer.write_way_refs(&test_data_path("u64_le_0_2_7"))?;
12121212
writer.close()?;
12131213
let ff = FilteredFile::open(tmp.path())?;
1214-
assert_eq!(ff.has_way_ref(0), true);
1215-
assert_eq!(ff.has_way_ref(2), true);
1216-
assert_eq!(ff.has_way_ref(7), true);
1217-
assert_eq!(ff.has_way_ref(1), false);
1218-
assert_eq!(ff.has_way_ref(8), false);
1219-
assert_eq!(ff.has_way_ref(1234567890123456789), false);
1214+
assert!(ff.has_way_ref(0));
1215+
assert!(ff.has_way_ref(2));
1216+
assert!(ff.has_way_ref(7));
1217+
assert!(!ff.has_way_ref(1));
1218+
assert!(!ff.has_way_ref(8));
1219+
assert!(!ff.has_way_ref(1234567890123456789));
12201220
Ok(())
12211221
}
12221222

@@ -1230,8 +1230,8 @@ pub mod filtered_file {
12301230
assert_eq!(ff.get_coord(5), None);
12311231
assert_eq!(ff.feature_count(), 0);
12321232
assert_eq!(ff.feature_data(17123), None);
1233-
assert_eq!(ff.has_node_ref(123), false);
1234-
assert_eq!(ff.has_way_ref(789), false);
1233+
assert!(!ff.has_node_ref(123));
1234+
assert!(!ff.has_way_ref(789));
12351235
Ok(())
12361236
}
12371237

src/pipeline/osm/mod.rs

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -520,27 +520,15 @@ mod tests {
520520

521521
impl FeatureStore for MockFeatureStore {
522522
fn get_node(&self, id: u64) -> Option<Node> {
523-
if let Some(node) = self.nodes.get(&id) {
524-
Some(node.clone())
525-
} else {
526-
None
527-
}
523+
self.nodes.get(&id).cloned()
528524
}
529525

530526
fn get_way(&self, id: u64) -> Option<Way> {
531-
if let Some(way) = self.ways.get(&id) {
532-
Some(way.clone())
533-
} else {
534-
None
535-
}
527+
self.ways.get(&id).cloned()
536528
}
537529

538530
fn get_relation(&self, id: u64) -> Option<Relation> {
539-
if let Some(relation) = self.relations.get(&id) {
540-
Some(relation.clone())
541-
} else {
542-
None
543-
}
531+
self.relations.get(&id).cloned()
544532
}
545533

546534
fn node_count(&self) -> u64 {

src/pipeline/osm/old_assemble.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,8 @@ mod tests {
189189
changeset: NonZeroU64::new(999),
190190
version: NonZeroU32::new(7),
191191
tags: vec![String::from("shop"), String::from("supermarket")],
192-
lon_e7: 11_000_000_0,
193-
lat_e7: 12_000_000_0,
192+
lon_e7: 110_000_000,
193+
lat_e7: 120_000_000,
194194
}],
195195
vec![],
196196
vec![],
@@ -224,16 +224,16 @@ mod tests {
224224
changeset: NonZeroU64::new(1999),
225225
version: NonZeroU32::new(17),
226226
tags: vec![],
227-
lon_e7: 50_000_000_0,
228-
lat_e7: 20_000_000_0,
227+
lon_e7: 500_000_000,
228+
lat_e7: 200_000_000,
229229
},
230230
Node {
231231
id: 2,
232232
changeset: NonZeroU64::new(2999),
233233
version: NonZeroU32::new(27),
234234
tags: vec![],
235-
lon_e7: 70_000_000_0,
236-
lat_e7: 40_000_000_0,
235+
lon_e7: 700_000_000,
236+
lat_e7: 400_000_000,
237237
},
238238
],
239239
vec![Way {

src/pipeline/tiles.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ mod tests {
118118
.get_args()
119119
.map(|s| s.to_string_lossy().into_owned())
120120
.collect();
121-
let shop_layer_arg = format!("--named-layer=Shops:{}", &layers[1].path.display());
121+
let shop_layer_arg = format!("--named-layer=Shops:{}", layers[1].path.display());
122122
let workdir_abs = std::path::absolute(&workdir).expect("absolute path of workdir");
123123
assert_eq!(
124124
args,

src/places/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,8 @@ mod tests {
194194
vec![],
195195
)
196196
.unwrap();
197-
assert_eq!(a.eq(&b), false);
198-
assert_eq!(a.eq(&a), true);
197+
assert!(!a.eq(&b));
198+
assert!(a.eq(&a));
199199
assert_eq!(a.cmp(&b), a.s2_cell_id.cmp(&b.s2_cell_id));
200200
assert_eq!(a.partial_cmp(&b), a.s2_cell_id.partial_cmp(&b.s2_cell_id));
201201
}
@@ -217,7 +217,7 @@ mod tests {
217217
assert_eq!(p.x(), 7.4478123);
218218
assert_eq!(p.y(), 46.9479801);
219219
} else {
220-
assert!(false, "expected a point, got {:?}", shape);
220+
panic!("expected a point, got {:?}", shape);
221221
};
222222
}
223223
}

0 commit comments

Comments
 (0)