Skip to content

Commit

Permalink
Warn about unused TOML keys
Browse files Browse the repository at this point in the history
Closes #27
  • Loading branch information
alexcrichton committed Jun 28, 2014
1 parent d217303 commit 6c23160
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 11 deletions.
2 changes: 1 addition & 1 deletion libs/toml-rs
10 changes: 10 additions & 0 deletions src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub struct Manifest {
target_dir: Path,
sources: Vec<SourceId>,
build: Option<String>,
unused_keys: Vec<String>,
}

impl Show for Manifest {
Expand Down Expand Up @@ -192,6 +193,7 @@ impl Manifest {
target_dir: target_dir.clone(),
sources: sources,
build: build,
unused_keys: Vec::new(),
}
}

Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 5 additions & 1 deletion src/cargo/core/shell.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -63,6 +63,10 @@ impl MultiShell {
pub fn error<T: ToStr>(&mut self, message: T) -> IoResult<()> {
self.err().say(message, RED)
}

pub fn warn<T: ToStr>(&mut self, message: T) -> IoResult<()> {
self.err().say(message, YELLOW)
}
}

pub type ShellCallback<'a> = |&mut Shell|:'a -> IoResult<()>;
Expand Down
4 changes: 4 additions & 0 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
47 changes: 39 additions & 8 deletions src/cargo/util/toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<toml::Table> {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tests/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
44 changes: 44 additions & 0 deletions tests/test_cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
})

5 comments on commit 6c23160

@bors
Copy link
Contributor

@bors bors commented on 6c23160 Jun 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from wycats
at alexcrichton@6c23160

@bors
Copy link
Contributor

@bors bors commented on 6c23160 Jun 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging alexcrichton/cargo/toml-warnings = 6c23160 into auto-cargo

@bors
Copy link
Contributor

@bors bors commented on 6c23160 Jun 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alexcrichton/cargo/toml-warnings = 6c23160 merged ok, testing candidate = b3c7716

@bors
Copy link
Contributor

@bors bors commented on 6c23160 Jun 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 6c23160 Jun 30, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto-cargo = b3c7716

Please sign in to comment.