Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn about unused TOML keys #87

Merged
merged 1 commit into from
Jun 30, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"));
})