Update dependencies and fix warnings

All tests still pass. The `gecode-csv` update required a significant
bump to our `tokio` version.

Author: emk

Cargo.lock

@@ -8,3 +8,14 @@ This repository contains tools for manipulating CSV files, all written in Rust.
 - [`geocode-csv`](./geocode-csv): Geocode CSV files in bulk using the SmartyStreets API.
 - [`scrubcsv`](./scrubcsv): Turn messy, slightly corrupt CSV files into something clean and standardized.
 - [`hashcsv`](./hashcsv): Add a new column to a CSV file, containing a hash of the other columns. Useful for de-duplicating.
+
+## Current coding standards
+
+In general, this repository should contain standard modern Rust code, formatting using `cargo fmt` and the supplied settings. The code should have no warnings when run with `clippy`.
+
+These tools were written over several years, and they represent a history of Rust at Faraday. The following dependencies should be replaced if we get the chance:
+
+- `docopt`: Replace with `structopt`.
+- `error_chain` and `failure`: Replace with `anyhow` (plus `thiserror` if we need specific custom error types).
+
+In general, it's a good idea to update any older code to match the newest code.

README.md

@@ -13,14 +13,11 @@ repository = "https://github.com/faradayio/catcsv"
 cli_test_dir = "0.1.2"
 
 [dependencies]
-# Enable this by passing `--features "clippy"` to cargo.  Needs nightly Rust.
-clippy = { version = "0.0.*", optional = true }
-csv = "0.15.0"
-docopt = "0.7.0"
-env_logger = "0.4.2"
-error-chain = "0.10.0"
-log = "0.3.7"
-# Deprecated in favor of serde, but needed for doctopt:
-rustc-serialize = "0.3"
-snap = "0.2.1"
-walkdir = "1.0.7"
+csv = "1"
+docopt = "1"
+env_logger = "0.8"
+error-chain = "0.12.4"
+log = "0.4.14"
+serde = "1.0.123"
+snap = "1"
+walkdir = "2.3"

catcsv/Cargo.toml

@@ -5,40 +5,25 @@
 //! This replaces a 20-line shell script with two pages of Rust. But it has
 //! a much nicer UI and it handles the corner cases better.
 
-// `error_chain!` can recurse deeply
-#![recursion_limit = "1024"]
-// Enable clippy if we were asked to do so.
-#![cfg_attr(feature = "clippy", feature(plugin))]
-#![cfg_attr(feature = "clippy", plugin(clippy))]
-
-use csv;
-
-use env_logger;
-#[macro_use]
-extern crate error_chain;
-#[macro_use]
-extern crate log;
-
-use snap;
-use walkdir;
-
-use csv::ByteString;
+use csv::ByteRecord;
 use docopt::Docopt;
-use std::borrow::Cow;
+use error_chain::quick_main;
+use log::debug;
+use serde::Deserialize;
 use std::fs::File;
 use std::io::prelude::*;
 use std::io::{self, BufReader, BufWriter};
 use std::process;
+use std::{borrow::Cow, io::Cursor};
 use walkdir::WalkDir;
 
 use crate::errors::*;
 
 /// A module to hold `Error`, etc., types generated by `error-chain`.
 mod errors {
-    use csv;
+    use error_chain::error_chain;
     use std::io;
     use std::path::PathBuf;
-    use walkdir;
 
     error_chain! {
         foreign_links {
@@ -60,7 +45,7 @@ mod errors {
 // delegate the real work to `run`.
 quick_main!(run);
 
-const USAGE: &'static str = "
+const USAGE: &str = "
 catcsv - Combine many CSV files into one
 
 Usage:
@@ -78,7 +63,7 @@ If passed a directory, this will recurse over all files in that directory.
 ";
 
 /// Our command-line arguments.
-#[derive(Debug, RustcDecodable)]
+#[derive(Debug, Deserialize)]
 struct Args {
     /// Should we show the version of the program and exit?
     flag_version: bool,
@@ -88,10 +73,10 @@ struct Args {
 
 /// Our real `main` function that parses arguments and figures out what to do.
 fn run() -> Result<()> {
-    env_logger::init().expect("could not initialize log subsystem");
+    env_logger::init();
 
     let args: Args = Docopt::new(USAGE)
-        .and_then(|dopt| dopt.decode())
+        .and_then(|dopt| dopt.deserialize())
         .unwrap_or_else(|e| e.exit());
     debug!("{:?}", args);
 
@@ -108,7 +93,7 @@ fn run() -> Result<()> {
 
     // Iterate over our arguments.  We do this without using recursion, mostly
     // to see how that looks in Rust.
-    let mut first_headers: Option<Vec<ByteString>> = None;
+    let mut first_headers: Option<ByteRecord> = None;
     let mut files_processed: u64 = 0;
     for input in &args.arg_input_file_or_dir {
         for entry in WalkDir::new(input).follow_links(true) {
@@ -132,7 +117,7 @@ fn run() -> Result<()> {
                 } else if filename.ends_with(".csv.sz") {
                     debug!("Processing as *.csv.sz");
                     let file = File::open(path).chain_err(&mkerr)?;
-                    let mut decompressed = snap::Reader::new(file);
+                    let mut decompressed = snap::read::FrameDecoder::new(file);
                     output_csv(&mut decompressed, &mut first_headers, &mut out)
                         .chain_err(&mkerr)?;
                 } else {
@@ -159,7 +144,7 @@ fn run() -> Result<()> {
 /// Output the specified CSV data to stand
 fn output_csv(
     file: &mut dyn Read,
-    first_headers: &mut Option<Vec<ByteString>>,
+    first_headers: &mut Option<ByteRecord>,
     output: &mut dyn Write,
 ) -> Result<()> {
     // Force buffered input for a big performance boost and so we can
@@ -171,10 +156,11 @@ fn output_csv(
     // NOTE: This will fail if there are escaped newlines in the header line.
     let mut first_line = String::new();
     input.read_line(&mut first_line)?;
-    let mut rdr = csv::Reader::from_string(first_line.clone());
+    let mut cursor = Cursor::new(&first_line);
+    let mut rdr = csv::Reader::from_reader(&mut cursor);
 
     // Get our header line only.
-    let headers = rdr.byte_headers()?;
+    let headers = rdr.byte_headers()?.to_owned();
 
     // If this is the first set of headers we've found, save them.  If not,
     // make sure that the headers match between files.

catcsv/src/main.rs

@@ -7,9 +7,9 @@ license = "MIT/Apache-2.0"
 
 [dependencies]
 csv = "1.0.2"
-env_logger = "0.5.13"
+env_logger = "0.8.3"
 failure = "0.1.3"
 humansize = "1"
-humantime = "1"
+humantime = "2"
 log = "0.4.6"
-structopt = "0.2.12"
+structopt = "0.3.21"

fixed2csv/Cargo.toml

@@ -1,12 +1,7 @@
-use csv;
-
-#[macro_use]
-extern crate log;
-use structopt;
-
 use failure::Error;
 use humansize::{file_size_opts, FileSize};
 use humantime::format_duration;
+use log::debug;
 use std::{
     cmp::min,
     io::{prelude::*, stdin, stdout, BufReader},
@@ -95,8 +90,10 @@ fn extract_fields(
             // Get our column.
             let mut field = &line[min(offset, end)..min(offset + width, end)];
 
-            // Strip spaces and add to our output record.
-            while field.len() > 0 && field[field.len() - 1] == b' ' {
+            // Strip spaces and add to our output record. We do this the hard
+            // way because there's no stable API on `&[u8]` that works like
+            // `trim_end_matches`.
+            while !field.is_empty() && field[field.len() - 1] == b' ' {
                 field = &field[..field.len() - 1];
             }
             record.push_field(field);

fixed2csv/src/main.rs

@@ -14,16 +14,14 @@ homepage = "http://blog.faraday.io/geochunk-fast-intelligent-splitting-for-piles
 cli_test_dir = "0.1.2"
 
 [dependencies]
-# Enable this by passing `--features "clippy"` to cargo.  Needs nightly Rust.
-clippy = { version = "0.0.*", optional = true }
 csv = "1"
-docopt = "0.8"
-env_logger = "0.4"
-error-chain = "0.11"
-lazy_static = "0.2"
-log = "0.3.7"
-regex = "0.2"
+docopt = "1.1"
+env_logger = "0.8.3"
+error-chain = "0.12.4"
+lazy_static = "1.4"
+log = "0.4.14"
+regex = "1.4.3"
 serde = "1.0"
 serde_derive = "1.0"
-snap = "0.2.1"
-walkdir = "1.0.7"
+snap = "1.0.4"
+walkdir = "2.3.1"

geochunk/Cargo.toml

@@ -1,6 +1,5 @@
 //! A module to hold `Error`, etc., types generated by `error-chain`.
 
-use csv;
 use std::io;
 
 error_chain! {

geochunk/src/errors.rs

@@ -1,20 +1,10 @@
-// `error_chain!` can recurse deeply
-#![recursion_limit = "1024"]
-// Enable clippy if we were asked to do so.
-#![cfg_attr(feature = "clippy", feature(plugin))]
-#![cfg_attr(feature = "clippy", plugin(clippy))]
-
-use env_logger;
 #[macro_use]
 extern crate error_chain;
 #[macro_use]
 extern crate lazy_static;
 #[macro_use]
 extern crate log;
 
-#[macro_use]
-extern crate serde_derive;
-
 use docopt::Docopt;
 use serde::de::Error as DeError;
 use serde::{Deserialize, Deserializer};
@@ -52,7 +42,7 @@ impl<'de> Deserialize<'de> for ChunkType {
     }
 }
 
-const USAGE: &'static str = "
+const USAGE: &str = "
 geochunk - Partition data sets by estimated population.
 
 Usage:
@@ -89,7 +79,7 @@ quick_main!(run);
 
 /// Our actual `main` function, called by the `quick_main!` macro above.
 fn run() -> Result<()> {
-    env_logger::init().expect("Could not initialize logging");
+    env_logger::init();
     let args: Args = Docopt::new(USAGE)
         .and_then(|d| d.deserialize())
         .unwrap_or_else(|e| e.exit());

geochunk/src/main.rs

@@ -1,8 +1,5 @@
 //! Support for chunks based on 2010 census population data.
 
-use csv;
-#[cfg(test)]
-use env_logger;
 use regex::Regex;
 use std::collections::hash_map::Entry;
 use std::collections::HashMap;
@@ -35,8 +32,8 @@ impl Classifier {
             &mut chunk_id_for_prefix,
         );
         Classifier {
-            target_population: target_population,
-            chunk_id_for_prefix: chunk_id_for_prefix,
+            target_population,
+            chunk_id_for_prefix,
         }
     }
 
@@ -142,7 +139,7 @@ impl Classifier {
 
 #[test]
 fn classifies_sample_zip_codes_as_expected() {
-    let _ = env_logger::init();
+    let _ = env_logger::try_init();
     let classifier = Classifier::new(250000);
     assert_eq!(classifier.chunk_for("01000").unwrap(), "010_0");
     assert_eq!(classifier.chunk_for("07720").unwrap(), "077_1");
@@ -151,7 +148,7 @@ fn classifies_sample_zip_codes_as_expected() {
 
 #[test]
 fn does_not_assign_geochunks_to_missing_or_invalid_zips() {
-    let _ = env_logger::init();
+    let _ = env_logger::try_init();
     let classifier = Classifier::new(250000);
     assert!(classifier.chunk_for("").is_none());
     assert!(classifier.chunk_for("0").is_none());
@@ -160,7 +157,7 @@ fn does_not_assign_geochunks_to_missing_or_invalid_zips() {
 
 #[test]
 fn does_not_panic_on_corner_cases() {
-    let _ = env_logger::init();
+    let _ = env_logger::try_init();
     let classifier = Classifier::new(250000);
     // I don't actually care whether or not this is mapped to a geochunk or
     // not, because we don't try to do detailed validation until _after_
@@ -173,7 +170,7 @@ type PrefixPopulationMaps = [HashMap<String, u64>; ZIP_CODE_LENGTH + 1];
 
 /// Directly include our zip code population data in our application binary
 /// for ease of distribution and packaging.
-const ZIP_POPULATION_CSV: &'static str = include_str!("zip2010.csv");
+const ZIP_POPULATION_CSV: &str = include_str!("zip2010.csv");
 
 /// The population associated with a zip code prefix.
 struct PrefixPopulation {
@@ -206,7 +203,7 @@ impl PrefixPopulation {
             }
         }
 
-        PrefixPopulation { maps: maps }
+        PrefixPopulation { maps }
     }
 
     /// Look up the population of a zip code prefix.  Calling this function

geochunk/src/zip2010.rs

@@ -16,18 +16,19 @@ cli_test_dir = "0.1.7"
 [dependencies]
 common_failures = "0.1.1"
 csv = "1.0.7"
-env_logger = "0.7.1"
+env_logger = "0.8.3"
 failure = "0.1.5"
 futures = "0.3.4"
-hyper = "0.13.4"
-hyper-tls = "0.4.1"
+hyper = { version = "0.14.4", features = ["client", "http2", "stream"] }
+hyper-tls = "0.5.0"
 log = "0.4.6"
 serde = { version = "1.0.92", features = ["derive"] }
 # IMPORTANT: We require `preserve_order` to correctly handle "structure" JSON'
 # specifications in a way that puts the right data in the right output columns.
 serde_json = { version = "1.0.39", features = ["preserve_order"] }
 structopt = "0.3.4"
-strum = "0.18.0"
-strum_macros = "0.18.0"
-tokio = { version = "0.2.13", features = ["io-util", "stream", "sync"] }
+strum = "0.20.0"
+strum_macros = "0.20.1"
+tokio = { version = "1.2.0", features = ["io-util", "rt-multi-thread", "sync"] }
+tokio-stream = "0.1.3"
 url = "2.1.1"

geocode-csv/Cargo.toml

@@ -60,7 +60,7 @@ impl ColumnKeyOrKeys<usize> {
                         // Already there, so ignore it. This appears in a lot of
                         // real-world databases, for some reason.
                     } else {
-                        extracted.push_str(" ");
+                        extracted.push(' ');
                         extracted.push_str(s);
                     }
                 }
@@ -110,9 +110,9 @@ pub struct AddressColumnKeys<K: Default + Eq> {
 
 impl AddressColumnKeys<usize> {
     /// Given a CSV row, extract an `Address` value to send to SmartyStreets.
-    pub fn extract_address_from_record<'a>(
+    pub fn extract_address_from_record(
         &self,
-        record: &'a StringRecord,
+        record: &StringRecord,
     ) -> Result<Address> {
         Ok(Address {
             street: self.street.extract_from_record(record)?.into_owned(),
@@ -201,7 +201,7 @@ impl<Key: Default + Eq> AddressColumnSpec<Key> {
             .map(|k| &k[..])
             .collect::<Vec<_>>();
         // Do not remove this `sort`!
-        prefixes.sort();
+        prefixes.sort_unstable();
         prefixes
     }
 

geocode-csv/src/addresses.rs

@@ -18,7 +18,7 @@ where
     T: Send + 'static,
 {
     // Spawn a worker thread outside our thread pool to do the actual work.
-    let (mut sender, mut receiver) = mpsc::channel(1);
+    let (sender, mut receiver) = mpsc::channel(1);
     let thr = thread::Builder::new().name(thread_name);
     let handle = thr
         .spawn(move || {