From 6c231608cb67f9f9307b6307f89c54c05410e815 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 26 Jun 2014 22:53:05 -0700 Subject: [PATCH] Warn about unused TOML keys Closes #27 --- libs/toml-rs | 2 +- src/cargo/core/manifest.rs | 10 ++++++++ src/cargo/core/shell.rs | 6 ++++- src/cargo/ops/cargo_compile.rs | 4 +++ src/cargo/util/toml.rs | 47 ++++++++++++++++++++++++++++------ tests/support/mod.rs | 2 +- tests/test_cargo_compile.rs | 44 +++++++++++++++++++++++++++++++ 7 files changed, 104 insertions(+), 11 deletions(-) diff --git a/libs/toml-rs b/libs/toml-rs index 7ba80c5ac4a..713816102b1 160000 --- a/libs/toml-rs +++ b/libs/toml-rs @@ -1 +1 @@ -Subproject commit 7ba80c5ac4a3f6bc3801bb4ac86fd65a569a07ba +Subproject commit 713816102b1cb61f45d2306f38e3d95dfdcc8ae1 diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index f9b4c0e1840..ffd1e6eee98 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -20,6 +20,7 @@ pub struct Manifest { target_dir: Path, sources: Vec, build: Option, + unused_keys: Vec, } impl Show for Manifest { @@ -192,6 +193,7 @@ impl Manifest { target_dir: target_dir.clone(), sources: sources, build: build, + unused_keys: Vec::new(), } } @@ -234,6 +236,14 @@ impl Manifest { pub fn get_build<'a>(&'a self) -> Option<&'a str> { self.build.as_ref().map(|s| s.as_slice()) } + + pub fn add_unused_key(&mut self, s: String) { + self.unused_keys.push(s) + } + + pub fn get_unused_keys<'a>(&'a self) -> &'a [String] { + self.unused_keys.as_slice() + } } impl Target { diff --git a/src/cargo/core/shell.rs b/src/cargo/core/shell.rs index 73f91587cf9..32ca19934c6 100644 --- a/src/cargo/core/shell.rs +++ b/src/cargo/core/shell.rs @@ -1,6 +1,6 @@ use term; use term::{Terminal,color}; -use term::color::{Color, BLACK, RED, GREEN}; +use term::color::{Color, BLACK, RED, GREEN, YELLOW}; use term::attr::{Attr, Bold}; use std::io::{IoResult, stderr}; use std::fmt::Show; @@ -63,6 +63,10 @@ impl MultiShell { pub fn error(&mut self, message: T) -> IoResult<()> { self.err().say(message, RED) } + + pub fn warn(&mut self, message: T) -> IoResult<()> { + self.err().say(message, YELLOW) + } } pub type ShellCallback<'a> = |&mut Shell|:'a -> IoResult<()>; diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 9e8b4d4b053..f8a1b9e231b 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -43,6 +43,10 @@ pub fn compile(manifest_path: &Path, update: bool, let package = try!(source.get_root_package()); debug!("loaded package; package={}", package); + for key in package.get_manifest().get_unused_keys().iter() { + try!(shell.warn(format!("unused manifest key: {}", key))); + } + let override_ids = try!(source_ids_from_config()); let source_ids = package.get_source_ids(); diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index 784311f9ce2..e220a60d5ba 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -22,9 +22,37 @@ pub fn to_manifest(contents: &[u8], manifest\n\n{}", e))) }; - toml_manifest.to_manifest(source_id).map_err(|err| { + let pair = try!(toml_manifest.to_manifest(source_id).map_err(|err| { human(format!("Cargo.toml is not a valid manifest\n\n{}", err)) - }) + })); + let (mut manifest, paths) = pair; + match d.toml { + Some(ref toml) => add_unused_keys(&mut manifest, toml, "".to_string()), + None => {} + } + return Ok((manifest, paths)); + + fn add_unused_keys(m: &mut Manifest, toml: &toml::Value, key: String) { + match *toml { + toml::Table(ref table) => { + for (k, v) in table.iter() { + add_unused_keys(m, v, if key.len() == 0 { + k.clone() + } else { + key + "." + k.as_slice() + }) + } + } + toml::Array(ref arr) => { + for v in arr.iter() { + add_unused_keys(m, v, key.clone()); + } + } + _ => m.add_unused_key(key), + } + } + + } pub fn parse(toml: &str, file: &str) -> CargoResult { @@ -126,9 +154,9 @@ impl TomlManifest { (Some(string.clone()), SourceId::for_central()) }, DetailedDep(ref details) => { - let reference = details.branch.as_ref().map(|b| b.clone()) - .or_else(|| details.tag.as_ref().map(|t| t.clone())) - .or_else(|| details.rev.as_ref().map(|t| t.clone())) + let reference = details.branch.clone() + .or_else(|| details.tag.clone()) + .or_else(|| details.rev.clone()) .unwrap_or_else(|| "master".to_str()); let new_source_id = match details.git { @@ -161,11 +189,14 @@ impl TomlManifest { } let project = self.project.as_ref().or_else(|| self.package.as_ref()); - let project = try!(project.require(|| human("No `package` or `project` section found."))); + let project = try!(project.require(|| { + human("No `package` or `project` section found.") + })); + let pkgid = try!(project.to_package_id(source_id.get_location())); + let summary = Summary::new(&pkgid, deps.as_slice()); Ok((Manifest::new( - &Summary::new(&try!(project.to_package_id(source_id.get_location())), - deps.as_slice()), + &summary, targets.as_slice(), &Path::new("target"), sources, diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 39f5eef86a4..2a7bc40575c 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -360,7 +360,7 @@ pub fn escape_path(p: &Path) -> String { pub fn basic_bin_manifest(name: &str) -> String { format!(r#" - [project] + [package] name = "{}" version = "0.5.0" diff --git a/tests/test_cargo_compile.rs b/tests/test_cargo_compile.rs index 168603d22f9..f7497360ae5 100644 --- a/tests/test_cargo_compile.rs +++ b/tests/test_cargo_compile.rs @@ -559,3 +559,47 @@ test!(many_crate_types { assert!(file0.ends_with(os::consts::DLL_SUFFIX) || file1.ends_with(os::consts::DLL_SUFFIX)); }) + +test!(unused_keys { + let mut p = project("foo"); + p = p + .file("Cargo.toml", r#" + [project] + + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + bulid = "foo" + + [[lib]] + + name = "foo" + "#) + .file("src/foo.rs", r#" + pub fn foo() {} + "#); + assert_that(p.cargo_process("cargo-build"), + execs().with_status(0) + .with_stderr("unused manifest key: project.bulid\n")); + + let mut p = project("bar"); + p = p + .file("Cargo.toml", r#" + [project] + + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + + [[lib]] + + name = "foo" + build = "foo" + "#) + .file("src/foo.rs", r#" + pub fn foo() {} + "#); + assert_that(p.cargo_process("cargo-build"), + execs().with_status(0) + .with_stderr("unused manifest key: lib.build\n")); +})