Rust: Bump deps & 2021 edition, reexports, clippy (#282)

* Rust: Bump deps & 2021 edition, reexports, clippy

* Re-export the entire geozero because it has a number of other used traits like Error, so reexporting partial geozero creates more problem than solves
* Switch all crates to 2021 edition
* Cleanup crate.toml - we may want to switch to a proper workspace here
* bump all dependency versions to latest
* Address some clippy warnings

* Some Clippy fixes

```
cargo clippy --fix
cargo clippy --workspace --allow-dirty --fix --benches --tests --bins -- -A clippy::all -W clippy::uninlined_format_args
```

* A lot more clippy fixes

* rename internal `to_feature` because it breaks Rust naming expectations
* add a safety docs to `FgbReader::open_unchecked` -- please doublecheck that it is correct
* fix a lot of `.write(...)` to `write_all(...)` - because write does not guarantee how many bytes it will write
* removed a few lifetimes - clippy thinks they are not needed
* `&Vec<_>` -> `&[_]`
* Optimized `PackedRTree::build` iter

* cleanup formats

* Spelling, readme, unneeded clarifiers

* Optimize read_coordinates

* Update file_reader.rs

src/rust/CHANGELOG.md

@@ -93,7 +93,7 @@
 ## [0.3.1] - 2020-04-05
 
 - Rust FlatGeobuf reading via HTTP (#49)
-- Add Rust docs URL and update READMEs (#48)
+- Add Rust docs URL and update README files (#48)
 
 ## [0.3.0] - 2020-03-20
 

src/rust/Cargo.toml

@@ -2,8 +2,8 @@
 name = "flatgeobuf"
 version = "3.26.1"
 authors = ["Pirmin Kalberer <pka@sourcepole.ch>"]
-edition = "2018"
-description = "FlatGeobuf for Rust."
+edition = "2021"
+description = "FlatGeobuf for Rust"
 homepage = "https://flatgeobuf.org/"
 repository = "https://github.com/flatgeobuf/flatgeobuf/tree/master/src/rust"
 readme = "README.md"
@@ -16,25 +16,25 @@ default = ["http"]
 http = ["http-range-client", "bytes"]
 
 [dependencies]
-flatbuffers = "23.1.21"
+flatbuffers = "23.5.26"
 byteorder = "1.4.3"
 geozero = { version = "0.10.0", default-features = false }
 http-range-client = { version = "0.6.0", optional = true }
 bytes = { version = "1.4.0", optional = true }
-log = "0.4.17"
+log = "0.4.19"
 fallible-streaming-iterator = "0.1.9"
-tempfile = "3.5.0"
+tempfile = "3.7.1"
 
 [dev-dependencies]
 geozero = { version = "0.10.0", default-features = true }
 seek_bufread = "1.2.2"
 rand = "0.8.5"
 hex = "0.4.3"
-criterion = "0.4.0"
-tokio = { version = "1.28.1", default-features = false, features = ["macros"] }
+criterion = "0.5.1"
+tokio = { version = "1.30.0", default-features = false, features = ["macros"] }
 # One test needs SSL support; just use the default system bindings for that.
 reqwest = { version = "0.11.18", default-features = true }
-geo-types = "0.7.9"
+geo-types = "0.7.11"
 
 [[bench]]
 name = "read"
@@ -49,4 +49,3 @@ harness = false
 
 [package.metadata.docs.rs]
 all-features = true
-

src/rust/README.md

@@ -12,26 +12,30 @@ circular interpolations as defined by SQL-MM Part 3.
 ```rust
 use flatgeobuf::*;
 
-let mut filein = BufReader::new(File::open("countries.fgb")?);
-let mut fgb = FgbReader::open(&mut filein)?.select_all()?;
-while let Some(feature) = fgb.next()? {
-    println!("{}", feature.property::<String>("name").unwrap());
-    println!("{}", feature.to_json()?);
+fn main() {
+    let mut filein = BufReader::new(File::open("countries.fgb")?);
+    let mut fgb = FgbReader::open(&mut filein)?.select_all()?;
+    while let Some(feature) = fgb.next()? {
+        println!("{}", feature.property::<String>("name").unwrap());
+        println!("{}", feature.to_json()?);
+    }
 }
 ```
 
 With async HTTP client:
 ```rust
 use flatgeobuf::*;
 
-let mut fgb = HttpFgbReader::open("https://flatgeobuf.org/test/data/countries.fgb")
-    .await?;
-    .select_bbox(8.8, 47.2, 9.5, 55.3)
-    .await?;
-while let Some(feature) = fgb.next().await? {
-    let props = feature.properties()?;
-    println!("{}", props["name"]);
-    println!("{}", feature.to_wkt()?);
+async fn process() {
+    let mut fgb = HttpFgbReader::open("https://flatgeobuf.org/test/data/countries.fgb")
+        .await?
+        .select_bbox(8.8, 47.2, 9.5, 55.3)
+        .await?;
+    while let Some(feature) = fgb.next().await? {
+        let props = feature.properties()?;
+        println!("{}", props["name"]);
+        println!("{}", feature.to_wkt()?);
+    }
 }
 ```
 

src/rust/benches/geojson.rs

@@ -52,18 +52,18 @@ fn fgb_bbox_to_geojson_dev_null() -> Result<()> {
 }
 
 fn criterion_benchmark(c: &mut Criterion) {
-    c.bench_function("fgb_to_geojson", |b| b.iter(|| fgb_to_geojson()));
+    c.bench_function("fgb_to_geojson", |b| b.iter(fgb_to_geojson));
     c.bench_function("fgb_to_geojson_unchecked", |b| {
-        b.iter(|| fgb_to_geojson_unchecked())
+        b.iter(fgb_to_geojson_unchecked)
     });
     c.bench_function("fgb_to_geojson_dev_null", |b| {
-        b.iter(|| fgb_to_geojson_dev_null())
+        b.iter(fgb_to_geojson_dev_null)
     });
     c.bench_function("fgb_to_geojson_dev_null_unchecked", |b| {
-        b.iter(|| fgb_to_geojson_dev_null_unchecked())
+        b.iter(fgb_to_geojson_dev_null_unchecked)
     });
     c.bench_function("fgb_bbox_to_geojson_dev_null", |b| {
-        b.iter(|| fgb_bbox_to_geojson_dev_null())
+        b.iter(fgb_bbox_to_geojson_dev_null)
     });
 }
 

src/rust/benches/read.rs

@@ -65,14 +65,14 @@ fn read_bbox_seq() -> Result<()> {
 // }
 
 fn criterion_benchmark(c: &mut Criterion) {
-    c.bench_function("read_fgb", |b| b.iter(|| read_fgb()));
+    c.bench_function("read_fgb", |b| b.iter(read_fgb));
     c.bench_function("read_fgb_process_geom", |b| {
-        b.iter(|| read_fgb_process_geom())
+        b.iter(read_fgb_process_geom)
     });
-    c.bench_function("read_fgb_unchecked", |b| b.iter(|| read_fgb_unchecked()));
-    c.bench_function("read_fgb_seq", |b| b.iter(|| read_fgb_seq()));
-    c.bench_function("read_bbox", |b| b.iter(|| read_bbox()));
-    c.bench_function("read_bbox_seq", |b| b.iter(|| read_bbox_seq()));
+    c.bench_function("read_fgb_unchecked", |b| b.iter(read_fgb_unchecked));
+    c.bench_function("read_fgb_seq", |b| b.iter(read_fgb_seq));
+    c.bench_function("read_bbox", |b| b.iter(read_bbox));
+    c.bench_function("read_bbox_seq", |b| b.iter(read_bbox_seq));
     // c.bench_function("select_bbox", move |b| {
     //     b.iter_with_setup(
     //         || read_header("../../test/data/countries.fgb").unwrap(),

src/rust/fgbutil/Cargo.toml

@@ -1,14 +1,13 @@
+# Prevent this from interfering with workspaces
+[workspace]
+members = ["."]
+
 [package]
 name = "fgbutil"
 version = "0.1.0"
-edition = "2018"
+edition = "2021"
 
 [dependencies]
-clap = { version = "3.1.0", features = ["derive"] }
-geozero = { version = "0.9.5", default-features = true }
-
-[dependencies.flatgeobuf]
-path = ".."
-
-[workspace]
-members = ["."]
+clap = { version = "4.3.21", features = ["derive"] }
+flatgeobuf = { path = ".." }
+geozero = "0.10.0"

src/rust/fgbutil/src/main.rs

@@ -1,12 +1,12 @@
 use clap::{ArgEnum, Args, Parser, Subcommand};
 use flatgeobuf::*;
 use geozero::error::Result;
-use geozero::geojson::GeoJsonWriter;
 use geozero::geojson::read_geojson_fc;
+use geozero::geojson::GeoJsonWriter;
 use geozero::FeatureProcessor;
 use geozero::GeozeroDatasource;
 use std::fs::File;
-use std::io::{Read, BufRead, BufReader, BufWriter, Write};
+use std::io::{BufRead, BufReader, BufWriter, Read, Write};
 
 pub struct GeoJsonReaderStream<'a, R: Read>(pub &'a mut R);
 
@@ -135,7 +135,7 @@ fn transform(
 fn info(mut input: impl BufRead, args: &Info) -> Result<()> {
     let mut fgb = FgbReader::open(&mut input)?;
 
-    println!("{:#?}", &fgb.header());
+    println!("{:#?}", fgb.header());
 
     if args.index {
         fgb.process_index(&mut GeoJsonWriter::new(&mut std::io::stdout()))?;
@@ -151,7 +151,7 @@ fn info(mut input: impl BufRead, args: &Info) -> Result<()> {
         let mut n = 0;
         while let Some(feature) = fgb.next()? {
             if n == fno {
-                println!("{:#?}", &feature.fbs_feature());
+                println!("{:#?}", feature.fbs_feature());
                 break;
             }
             n += 1;

src/rust/fuzz/Cargo.toml

@@ -1,22 +1,20 @@
+# Prevent this from interfering with workspaces
+[workspace]
+members = ["."]
+
 [package]
 name = "flatgeobuf-fuzz"
 version = "0.0.0"
 authors = ["Automatically generated"]
 publish = false
-edition = "2018"
+edition = "2021"
 
 [package.metadata]
 cargo-fuzz = true
 
 [dependencies]
-libfuzzer-sys = "0.3"
-
-[dependencies.flatgeobuf]
-path = ".."
-
-# Prevent this from interfering with workspaces
-[workspace]
-members = ["."]
+libfuzzer-sys = "0.4.6"
+flatgeobuf = { path = ".." }
 
 [[bin]]
 name = "read"

src/rust/src/feature_writer.rs

@@ -128,8 +128,8 @@ impl<'a> FeatureWriter<'a> {
                 }
                 _ => {
                     return Err(GeozeroError::Geometry(format!(
-                        "Cannot mix geometry types - expected type `{:?}`, actual type `{:?}`",
-                        self.dataset_type, geometry_type
+                        "Cannot mix geometry types - expected type `{:?}`, actual type `{geometry_type:?}`",
+                        self.dataset_type
                     )));
                 }
             }
@@ -156,22 +156,22 @@ impl<'a> FeatureWriter<'a> {
             }
             _ => Some(to_fb_vector!(self, ends)),
         };
-        let z = if self.z.len() > 0 {
+        let z = if !self.z.is_empty() {
             Some(to_fb_vector!(self, z))
         } else {
             None
         };
-        let m = if self.m.len() > 0 {
+        let m = if !self.m.is_empty() {
             Some(to_fb_vector!(self, m))
         } else {
             None
         };
-        let t = if self.t.len() > 0 {
+        let t = if !self.t.is_empty() {
             Some(to_fb_vector!(self, t))
         } else {
             None
         };
-        let tm = if self.tm.len() > 0 {
+        let tm = if !self.tm.is_empty() {
             Some(to_fb_vector!(self, tm))
         } else {
             None
@@ -191,8 +191,8 @@ impl<'a> FeatureWriter<'a> {
         );
         self.parts.push(g);
     }
-    pub(crate) fn to_feature(&mut self) -> Vec<u8> {
-        let g = if self.parts.len() == 0 {
+    pub(crate) fn finish_to_feature(&mut self) -> Vec<u8> {
+        let g = if self.parts.is_empty() {
             self.finish_part();
             self.parts.pop().expect("push in finish_part")
         } else {
@@ -501,7 +501,7 @@ mod test {
         let mut out: Vec<u8> = Vec::new();
         let feat = FgbFeature {
             header_buf: header(fgb_writer.dataset_type),
-            feature_buf: fgb_writer.to_feature(),
+            feature_buf: fgb_writer.finish_to_feature(),
         };
         // dbg!(&feat.fbs_feature());
         feat.process(&mut GeoJsonWriter::new(&mut out), 0)?;
@@ -515,7 +515,7 @@ mod test {
         let mut out: Vec<u8> = Vec::new();
         let f = FgbFeature {
             header_buf: header(geometry_type),
-            feature_buf: fgb_writer.to_feature(),
+            feature_buf: fgb_writer.finish_to_feature(),
         };
         let mut json_writer = GeoJsonWriter::new(&mut out);
         json_writer.dims.z = with_z;
@@ -542,7 +542,7 @@ mod test {
 
         let geojson = r#"{"type": "MultiPoint", "coordinates": [[1,1],[2,2]]}"#;
         assert_eq!(
-            str::from_utf8(&json_to_fbg_to_json(&geojson, GeometryType::MultiPoint)).unwrap(),
+            str::from_utf8(&json_to_fbg_to_json(geojson, GeometryType::MultiPoint)).unwrap(),
             geojson
         );
 
@@ -572,7 +572,7 @@ mod test {
         let geojson = r#"{"type": "LineString", "coordinates": [[1,1,10],[2,2,20]]}"#;
         assert_eq!(
             str::from_utf8(&json_to_fbg_to_json_n(
-                &geojson,
+                geojson,
                 GeometryType::LineString,
                 true
             ))
@@ -634,7 +634,7 @@ mod test {
         let mut out: Vec<u8> = Vec::new();
         let feat = FgbFeature {
             header_buf: header(fgb_writer.dataset_type),
-            feature_buf: fgb_writer.to_feature(),
+            feature_buf: fgb_writer.finish_to_feature(),
         };
         assert_eq!(
             fgb_writer.bbox,

src/rust/src/file_reader.rs

@@ -39,6 +39,11 @@ impl<'a, R: Read> FgbReader<'a, R, Initial> {
     }
 
     /// Open dataset by reading the header information without FlatBuffers verification
+    ///
+    /// # Safety
+    /// This method is unsafe because it does not verify the FlatBuffers header.
+    /// It is still safe from the Rust safety guarantees perspective, but it may cause
+    /// undefined behavior if the FlatBuffers header is invalid.
     pub unsafe fn open_unchecked(reader: &'a mut R) -> Result<FgbReader<'a, R, Open>> {
         Self::read_header(reader, false)
     }
@@ -317,8 +322,8 @@ impl<'a, T: Read + Seek> GeozeroDatasource for FgbReader<'a, T, FeaturesSelected
 /// # }
 /// ```
 impl<'a, R: Read> FallibleStreamingIterator for FgbReader<'a, R, FeaturesSelected> {
-    type Error = GeozeroError;
     type Item = FgbFeature;
+    type Error = GeozeroError;
 
     fn advance(&mut self) -> Result<()> {
         if self.advance_finished() {
@@ -368,8 +373,8 @@ impl<'a, R: Read> FallibleStreamingIterator for FgbReader<'a, R, FeaturesSelecte
 /// # }
 /// ```
 impl<'a, R: Read + Seek> FallibleStreamingIterator for FgbReader<'a, R, FeaturesSelectedSeek> {
-    type Error = GeozeroError;
     type Item = FgbFeature;
+    type Error = GeozeroError;
 
     fn advance(&mut self) -> Result<()> {
         if self.advance_finished() {
@@ -412,11 +417,9 @@ impl<'a, R: Read, State> FgbReader<'a, R, State> {
 
     fn read_feature(&mut self) -> Result<()> {
         // Read feature size if not already read in select_all or select_bbox
-        if self.cur_pos != 4 {
-            if self.read_feature_size() {
-                self.finished = true;
-                return Ok(());
-            }
+        if self.cur_pos != 4 && self.read_feature_size() {
+            self.finished = true;
+            return Ok(());
         }
         let sbuf = &self.fbs.feature_buf;
         let feature_size = u32::from_le_bytes([sbuf[0], sbuf[1], sbuf[2], sbuf[3]]) as usize;