Auto merge of #2219 - matklad:encodable-audit, r=alexcrichton

@alexcrichton another preparation PR for #2196

I've removed obscure `metadata` field from `Target`. It is a breaking change (for read-manifest), but the field seemed cryptic, useless and untested :)

`Target` has a bunch of boolean fields:

```
    tested: bool,
    benched: bool,
    doc: bool,
    doctest: bool,
    harness: bool, // whether to use the test harness (--test)
    for_host: bool,
```

I guess they should not be included in serialized representation?

I will push commits for other `Encodable`s here.

Author: bors

src/cargo/core/dependency.rs

@@ -33,16 +33,29 @@ pub struct Dependency {
 
 
 #[derive(RustcEncodable)]
-struct SerializedDependency {
-    name: String,
-    req: String
+struct SerializedDependency<'a> {
+    name: &'a str,
+    source: &'a SourceId,
+    req: String,
+    kind: Kind,
+
+    optional: bool,
+    uses_default_features: bool,
+    features: &'a [String],
+    target: &'a Option<&'a str>,
 }
 
 impl Encodable for Dependency {
     fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
         SerializedDependency {
-            name: self.name().to_string(),
-            req: self.version_req().to_string()
+            name: self.name(),
+            source: &self.source_id(),
+            req: self.version_req().to_string(),
+            kind: self.kind(),
+            optional: self.is_optional(),
+            uses_default_features: self.uses_default_features(),
+            features: self.features(),
+            target: &self.only_for_platform(),
         }.encode(s)
     }
 }
@@ -54,6 +67,16 @@ pub enum Kind {
     Build,
 }
 
+impl Encodable for Kind {
+    fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
+        match *self {
+            Kind::Normal => None,
+            Kind::Development => Some("dev"),
+            Kind::Build => Some("build"),
+        }.encode(s)
+    }
+}
+
 impl DependencyInner {
     /// Attempt to create a `Dependency` from an entry in the manifest.
     pub fn parse(name: &str,
@@ -235,4 +258,3 @@ impl Dependency {
         self.inner.matches_id(id)
     }
 }
-

src/cargo/core/manifest.rs

@@ -43,25 +43,6 @@ pub struct ManifestMetadata {
     pub documentation: Option<String>,  // url
 }
 
-#[derive(RustcEncodable)]
-struct SerializedManifest<'a> {
-    name: String,
-    version: String,
-    dependencies: &'a [Dependency],
-    targets: Vec<Target>,
-}
-
-impl Encodable for Manifest {
-    fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
-        SerializedManifest {
-            name: self.summary.name().to_string(),
-            version: self.summary.version().to_string(),
-            dependencies: self.summary.dependencies(),
-            targets: self.targets.clone(),
-        }.encode(s)
-    }
-}
-
 #[derive(Debug, Clone, PartialEq, Eq, Hash, RustcEncodable, Copy)]
 pub enum LibKind {
     Lib,
@@ -93,7 +74,7 @@ impl LibKind {
     }
 }
 
-#[derive(Debug, Clone, Hash, PartialEq, RustcEncodable, Eq)]
+#[derive(Debug, Clone, Hash, PartialEq, Eq)]
 pub enum TargetKind {
     Lib(Vec<LibKind>),
     Bin,
@@ -103,6 +84,21 @@ pub enum TargetKind {
     CustomBuild,
 }
 
+impl Encodable for TargetKind {
+    fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
+        match *self {
+            TargetKind::Lib(ref kinds) => {
+                kinds.iter().map(|k| k.crate_type()).collect()
+            }
+            TargetKind::Bin => vec!["bin"],
+            TargetKind::Example => vec!["example"],
+            TargetKind::Test => vec!["test"],
+            TargetKind::CustomBuild => vec!["custom-build"],
+            TargetKind::Bench => vec!["bench"],
+        }.encode(s)
+    }
+}
+
 #[derive(RustcEncodable, RustcDecodable, Clone, PartialEq, Eq, Debug, Hash)]
 pub struct Profile {
     pub opt_level: u32,
@@ -145,31 +141,18 @@ pub struct Target {
 }
 
 #[derive(RustcEncodable)]
-pub struct SerializedTarget {
-    kind: Vec<&'static str>,
-    name: String,
-    src_path: String,
-    metadata: Option<Metadata>
+struct SerializedTarget<'a> {
+    kind: &'a TargetKind,
+    name: &'a str,
+    src_path: &'a str,
 }
 
 impl Encodable for Target {
     fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
-        let kind = match self.kind {
-            TargetKind::Lib(ref kinds) => {
-                kinds.iter().map(|k| k.crate_type()).collect()
-            }
-            TargetKind::Bin => vec!["bin"],
-            TargetKind::Example => vec!["example"],
-            TargetKind::Test => vec!["test"],
-            TargetKind::CustomBuild => vec!["custom-build"],
-            TargetKind::Bench => vec!["bench"],
-        };
-
         SerializedTarget {
-            kind: kind,
-            name: self.name.clone(),
-            src_path: self.src_path.display().to_string(),
-            metadata: self.metadata.clone()
+            kind: &self.kind,
+            name: &self.name,
+            src_path: &self.src_path.display().to_string(),
         }.encode(s)
     }
 }

src/cargo/core/package.rs

@@ -1,3 +1,4 @@
+use std::collections::HashMap;
 use std::fmt::{self, Formatter};
 use std::hash;
 use std::slice;
@@ -24,25 +25,30 @@ pub struct Package {
 
 #[derive(RustcEncodable)]
 struct SerializedPackage<'a> {
-    name: String,
-    version: String,
+    name: &'a str,
+    version: &'a str,
+    id: &'a PackageId,
+    source: &'a SourceId,
     dependencies: &'a [Dependency],
-    targets: Vec<Target>,
-    manifest_path: String,
+    targets: &'a [Target],
+    features: &'a HashMap<String, Vec<String>>,
+    manifest_path: &'a str,
 }
 
 impl Encodable for Package {
     fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
-        let manifest = self.manifest();
-        let summary = manifest.summary();
+        let summary = self.manifest.summary();
         let package_id = summary.package_id();
 
         SerializedPackage {
-            name: package_id.name().to_string(),
-            version: package_id.version().to_string(),
+            name: &package_id.name(),
+            version: &package_id.version().to_string(),
+            id: package_id,
+            source: summary.source_id(),
             dependencies: summary.dependencies(),
-            targets: manifest.targets().to_vec(),
-            manifest_path: self.manifest_path.display().to_string()
+            targets: &self.manifest.targets(),
+            features: summary.features(),
+            manifest_path: &self.manifest_path.display().to_string(),
         }.encode(s)
     }
 }

tests/test_cargo_read_manifest.rs

@@ -3,20 +3,26 @@ use hamcrest::{assert_that};
 
 fn setup() {}
 
+fn remove_all_whitespace(s: &str) -> String {
+    s.split_whitespace().collect()
+}
+
 fn read_manifest_output() -> String {
-    "\
-{\
-    \"name\":\"foo\",\
-    \"version\":\"0.5.0\",\
-    \"dependencies\":[],\
-    \"targets\":[{\
-        \"kind\":[\"bin\"],\
-        \"name\":\"foo\",\
-        \"src_path\":\"src[..]foo.rs\",\
-        \"metadata\":null\
-    }],\
-    \"manifest_path\":\"[..]Cargo.toml\"\
-}".into()
+    remove_all_whitespace(r#"
+{
+    "name":"foo",
+    "version":"0.5.0",
+    "id":"foo[..]0.5.0[..](path+file://[..]/foo)",
+    "source":null,
+    "dependencies":[],
+    "targets":[{
+        "kind":["bin"],
+        "name":"foo",
+        "src_path":"src[..]foo.rs"
+    }],
+    "features":{},
+    "manifest_path":"[..]Cargo.toml"
+}"#)
 }
 
 test!(cargo_read_manifest_path_to_cargo_toml_relative {