Auto merge of #6416 - alexcrichton:idioms, r=dwijnand

Migrate to some Rust 2018 idioms

Run a few lints, hand-edit a few things, otherwise try to help push Cargo into the 2018 edition!

Cargo.toml

@@ -37,19 +37,18 @@ glob = "0.2.11"
 hex = "0.3"
 home = "0.3"
 ignore = "0.4"
-lazy_static = "1.0.0"
+lazy_static = "1.2.0"
 jobserver = "0.1.11"
 lazycell = "1.2.0"
 libc = "0.2"
-log = "0.4"
+log = "0.4.6"
 libgit2-sys = "0.7.9"
 num_cpus = "1.0"
 opener = "0.3.0"
 rustfix = "0.4.2"
 same-file = "1"
 semver = { version = "0.9.0", features = ["serde"] }
-serde = "1.0"
-serde_derive = "1.0"
+serde = { version = "1.0.82", features = ['derive'] }
 serde_ignored = "0.0.4"
 serde_json = { version = "1.0.30", features = ["raw_value"] }
 shell-escape = "0.1.4"

src/bin/cargo/commands/locate_project.rs

@@ -1,6 +1,7 @@
 use crate::command_prelude::*;
 
 use cargo::print_json;
+use serde::Serialize;
 
 pub fn cli() -> App {
     subcommand("locate-project")

src/bin/cargo/main.rs

@@ -1,18 +1,9 @@
-#![cfg_attr(feature = "cargo-clippy", allow(clippy::too_many_arguments))] // large project
-#![cfg_attr(feature = "cargo-clippy", allow(clippy::redundant_closure))] // there's a false positive
+#![warn(rust_2018_idioms)] // while we're getting used to 2018
+#![allow(clippy::too_many_arguments)] // large project
+#![allow(clippy::redundant_closure)] // there's a false positive
 
-use cargo;
-
-#[cfg(not(feature = "pretty-env-logger"))]
-extern crate env_logger;
-#[cfg(feature = "pretty-env-logger")]
-extern crate pretty_env_logger;
 #[macro_use]
 extern crate failure;
-use git2_curl;
-
-#[macro_use]
-extern crate serde_derive;
 
 use std::collections::BTreeSet;
 use std::env;

src/cargo/core/compiler/build_context/mod.rs

@@ -3,6 +3,8 @@ use std::env;
 use std::path::{Path, PathBuf};
 use std::str;
 
+use log::debug;
+
 use crate::core::profiles::Profiles;
 use crate::core::{Dependency, Workspace};
 use crate::core::{PackageId, PackageSet, Resolve};
@@ -158,7 +160,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
         self.build_config.jobs
     }
 
-    pub fn rustflags_args(&self, unit: &Unit) -> CargoResult<Vec<String>> {
+    pub fn rustflags_args(&self, unit: &Unit<'_>) -> CargoResult<Vec<String>> {
         env_args(
             self.config,
             &self.build_config.requested_target,
@@ -169,7 +171,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
         )
     }
 
-    pub fn rustdocflags_args(&self, unit: &Unit) -> CargoResult<Vec<String>> {
+    pub fn rustdocflags_args(&self, unit: &Unit<'_>) -> CargoResult<Vec<String>> {
         env_args(
             self.config,
             &self.build_config.requested_target,

src/cargo/core/compiler/build_context/target_info.rs

@@ -259,7 +259,7 @@ impl TargetInfo {
 fn parse_crate_type(
     crate_type: &str,
     error: &str,
-    lines: &mut str::Lines,
+    lines: &mut str::Lines<'_>,
 ) -> CargoResult<Option<(String, String)>> {
     let not_supported = error.lines().any(|line| {
         (line.contains("unsupported crate type") || line.contains("unknown crate type"))

src/cargo/core/compiler/build_plan.rs

@@ -7,14 +7,14 @@
 //! dependencies on other Invocations.
 
 use std::collections::BTreeMap;
+use std::path::PathBuf;
+
+use serde::Serialize;
 
 use super::context::OutputFile;
 use super::{CompileMode, Context, Kind, Unit};
 use crate::core::TargetKind;
 use crate::util::{internal, CargoResult, ProcessBuilder};
-use semver;
-use serde_json;
-use std::path::PathBuf;
 
 #[derive(Debug, Serialize)]
 struct Invocation {
@@ -45,7 +45,7 @@ struct SerializedBuildPlan {
 }
 
 impl Invocation {
-    pub fn new(unit: &Unit, deps: Vec<usize>) -> Invocation {
+    pub fn new(unit: &Unit<'_>, deps: Vec<usize>) -> Invocation {
         let id = unit.pkg.package_id();
         Invocation {
             package_name: id.name().to_string(),
@@ -109,7 +109,7 @@ impl BuildPlan {
         }
     }
 
-    pub fn add(&mut self, cx: &Context, unit: &Unit) -> CargoResult<()> {
+    pub fn add(&mut self, cx: &Context<'_, '_>, unit: &Unit<'_>) -> CargoResult<()> {
         let id = self.plan.invocations.len();
         self.invocation_map.insert(unit.buildkey(), id);
         let deps = cx

src/cargo/core/compiler/compilation.rs

@@ -256,7 +256,7 @@ fn pre_version_component(v: &Version) -> String {
     ret
 }
 
-fn target_runner(bcx: &BuildContext) -> CargoResult<Option<(PathBuf, Vec<String>)>> {
+fn target_runner(bcx: &BuildContext<'_, '_>) -> CargoResult<Option<(PathBuf, Vec<String>)>> {
     let target = bcx.target_triple();
 
     // try target.{}.runner

src/cargo/core/compiler/context/compilation_files.rs

@@ -6,6 +6,7 @@ use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
 use lazycell::LazyCell;
+use log::info;
 
 use super::{BuildContext, Context, FileFlavor, Kind, Layout, Unit};
 use crate::core::{TargetKind, Workspace};
@@ -15,7 +16,7 @@ use crate::util::{self, CargoResult};
 pub struct Metadata(u64);
 
 impl fmt::Display for Metadata {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{:016x}", self.0)
     }
 }
@@ -106,7 +107,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
 
     /// Get the short hash based only on the PackageId
     /// Used for the metadata when target_metadata returns None
-    pub fn target_short_hash(&self, unit: &Unit) -> String {
+    pub fn target_short_hash(&self, unit: &Unit<'_>) -> String {
         let hashable = unit.pkg.package_id().stable_hash(self.ws.root());
         util::short_hash(&hashable)
     }
@@ -148,7 +149,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
 
     /// Returns the directories where Rust crate dependencies are found for the
     /// specified unit.
-    pub fn deps_dir(&self, unit: &Unit) -> &Path {
+    pub fn deps_dir(&self, unit: &Unit<'_>) -> &Path {
         self.layout(unit.kind).deps()
     }
 
@@ -192,7 +193,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
     }
 
     /// Returns the bin stem for a given target (without metadata)
-    fn bin_stem(&self, unit: &Unit) -> String {
+    fn bin_stem(&self, unit: &Unit<'_>) -> String {
         if unit.target.allows_underscores() {
             unit.target.name().to_string()
         } else {

src/cargo/core/compiler/context/mod.rs

@@ -129,7 +129,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
         mut self,
         units: &[Unit<'a>],
         export_dir: Option<PathBuf>,
-        exec: &Arc<Executor>,
+        exec: &Arc<dyn Executor>,
     ) -> CargoResult<Compilation<'cfg>> {
         let mut queue = JobQueue::new(self.bcx);
         let mut plan = BuildPlan::new();
@@ -386,7 +386,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
         deps
     }
 
-    pub fn incremental_args(&self, unit: &Unit) -> CargoResult<Vec<String>> {
+    pub fn incremental_args(&self, unit: &Unit<'_>) -> CargoResult<Vec<String>> {
         // There's a number of ways to configure incremental compilation right
         // now. In order of descending priority (first is highest priority) we
         // have:
@@ -471,24 +471,25 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
 
     fn check_collistions(&self) -> CargoResult<()> {
         let mut output_collisions = HashMap::new();
-        let describe_collision = |unit: &Unit, other_unit: &Unit, path: &PathBuf| -> String {
-            format!(
-                "The {} target `{}` in package `{}` has the same output \
-                 filename as the {} target `{}` in package `{}`.\n\
-                 Colliding filename is: {}\n",
-                unit.target.kind().description(),
-                unit.target.name(),
-                unit.pkg.package_id(),
-                other_unit.target.kind().description(),
-                other_unit.target.name(),
-                other_unit.pkg.package_id(),
-                path.display()
-            )
-        };
+        let describe_collision =
+            |unit: &Unit<'_>, other_unit: &Unit<'_>, path: &PathBuf| -> String {
+                format!(
+                    "The {} target `{}` in package `{}` has the same output \
+                     filename as the {} target `{}` in package `{}`.\n\
+                     Colliding filename is: {}\n",
+                    unit.target.kind().description(),
+                    unit.target.name(),
+                    unit.pkg.package_id(),
+                    other_unit.target.kind().description(),
+                    other_unit.target.name(),
+                    other_unit.pkg.package_id(),
+                    path.display()
+                )
+            };
         let suggestion = "Consider changing their names to be unique or compiling them separately.\n\
             This may become a hard error in the future, see https://github.com/rust-lang/cargo/issues/6313";
-        let report_collision = |unit: &Unit,
-                                other_unit: &Unit,
+        let report_collision = |unit: &Unit<'_>,
+                                other_unit: &Unit<'_>,
                                 path: &PathBuf|
          -> CargoResult<()> {
             if unit.target.name() == other_unit.target.name() {
@@ -567,7 +568,7 @@ impl Links {
         }
     }
 
-    pub fn validate(&mut self, resolve: &Resolve, unit: &Unit) -> CargoResult<()> {
+    pub fn validate(&mut self, resolve: &Resolve, unit: &Unit<'_>) -> CargoResult<()> {
         if !self.validated.insert(unit.pkg.package_id()) {
             return Ok(());
         }

src/cargo/core/compiler/context/unit_dependencies.rs

@@ -18,6 +18,8 @@
 use std::cell::RefCell;
 use std::collections::{HashMap, HashSet};
 
+use log::trace;
+
 use super::{BuildContext, CompileMode, Kind, Unit};
 use crate::core::dependency::Kind as DepKind;
 use crate::core::package::Downloads;
@@ -328,7 +330,7 @@ fn compute_deps_doc<'a, 'cfg, 'tmp>(
 
 fn maybe_lib<'a>(
     unit: &Unit<'a>,
-    bcx: &BuildContext,
+    bcx: &BuildContext<'_, '_>,
     unit_for: UnitFor,
 ) -> Option<(Unit<'a>, UnitFor)> {
     unit.pkg.targets().iter().find(|t| t.linkable()).map(|t| {
@@ -345,7 +347,10 @@ fn maybe_lib<'a>(
 /// script itself doesn't have any dependencies, so even in that case a unit
 /// of work is still returned. `None` is only returned if the package has no
 /// build script.
-fn dep_build_script<'a>(unit: &Unit<'a>, bcx: &BuildContext) -> Option<(Unit<'a>, UnitFor)> {
+fn dep_build_script<'a>(
+    unit: &Unit<'a>,
+    bcx: &BuildContext<'_, '_>,
+) -> Option<(Unit<'a>, UnitFor)> {
     unit.pkg
         .targets()
         .iter()
@@ -385,7 +390,7 @@ fn check_or_build_mode(mode: CompileMode, target: &Target) -> CompileMode {
 }
 
 fn new_unit<'a>(
-    bcx: &BuildContext,
+    bcx: &BuildContext<'_, '_>,
     pkg: &'a Package,
     target: &'a Target,
     unit_for: UnitFor,
@@ -418,7 +423,7 @@ fn new_unit<'a>(
 ///
 /// Here we take the entire `deps` map and add more dependencies from execution
 /// of one build script to execution of another build script.
-fn connect_run_custom_build_deps(state: &mut State) {
+fn connect_run_custom_build_deps(state: &mut State<'_, '_, '_>) {
     let mut new_deps = Vec::new();
 
     {

src/cargo/core/compiler/fingerprint.rs

@@ -5,9 +5,10 @@ use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex};
 
 use filetime::FileTime;
-use serde::de::{self, Deserialize};
+use log::{debug, info};
+use serde::de;
 use serde::ser;
-use serde_json;
+use serde::{Deserialize, Serialize};
 
 use crate::core::{Edition, Package};
 use crate::util;
@@ -495,7 +496,7 @@ fn calculate<'a, 'cfg>(
 // git/registry source, then the mtime of files may fluctuate, but they won't
 // change so long as the source itself remains constant (which is the
 // responsibility of the source)
-fn use_dep_info(unit: &Unit) -> bool {
+fn use_dep_info(unit: &Unit<'_>) -> bool {
     let path = unit.pkg.summary().source_id().is_path();
     !unit.mode.is_doc() && path
 }
@@ -683,7 +684,7 @@ fn compare_old_fingerprint(loc: &Path, new_fingerprint: &Fingerprint) -> CargoRe
     new_fingerprint.compare(&old_fingerprint)
 }
 
-fn log_compare(unit: &Unit, compare: &CargoResult<()>) {
+fn log_compare(unit: &Unit<'_>, compare: &CargoResult<()>) {
     let ce = match *compare {
         Ok(..) => return,
         Err(ref e) => e,
@@ -721,7 +722,7 @@ fn dep_info_mtime_if_fresh(pkg: &Package, dep_info: &Path) -> CargoResult<Option
     }
 }
 
-fn pkg_fingerprint(bcx: &BuildContext, pkg: &Package) -> CargoResult<String> {
+fn pkg_fingerprint(bcx: &BuildContext<'_, '_>, pkg: &Package) -> CargoResult<String> {
     let source_id = pkg.package_id().source_id();
     let sources = bcx.packages.sources();
 

src/cargo/core/compiler/job.rs

@@ -11,7 +11,7 @@ pub struct Job {
 /// Each proc should send its description before starting.
 /// It should send either once or close immediately.
 pub struct Work {
-    inner: Box<for<'a, 'b> FnBox<&'a JobState<'b>, CargoResult<()>> + Send>,
+    inner: Box<dyn for<'a, 'b> FnBox<&'a JobState<'b>, CargoResult<()>> + Send>,
 }
 
 trait FnBox<A, R> {
@@ -27,7 +27,7 @@ impl<A, R, F: FnOnce(A) -> R> FnBox<A, R> for F {
 impl Work {
     pub fn new<F>(f: F) -> Work
     where
-        F: FnOnce(&JobState) -> CargoResult<()> + Send + 'static,
+        F: FnOnce(&JobState<'_>) -> CargoResult<()> + Send + 'static,
     {
         Work { inner: Box::new(f) }
     }
@@ -36,7 +36,7 @@ impl Work {
         Work::new(|_| Ok(()))
     }
 
-    pub fn call(self, tx: &JobState) -> CargoResult<()> {
+    pub fn call(self, tx: &JobState<'_>) -> CargoResult<()> {
         self.inner.call_box(tx)
     }
 
@@ -56,7 +56,7 @@ impl Job {
 
     /// Consumes this job by running it, returning the result of the
     /// computation.
-    pub fn run(self, fresh: Freshness, state: &JobState) -> CargoResult<()> {
+    pub fn run(self, fresh: Freshness, state: &JobState<'_>) -> CargoResult<()> {
         match fresh {
             Fresh => self.fresh.call(state),
             Dirty => self.dirty.call(state),
@@ -65,7 +65,7 @@ impl Job {
 }
 
 impl fmt::Debug for Job {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "Job {{ ... }}")
     }
 }