Move from rust-toml to toml-rs

This commit should greatly improve all error messages related to decoding
Cargo.toml.

.gitmodules

@@ -1,9 +1,9 @@
-[submodule "libs/rust-toml"]
-	path = libs/rust-toml
-	url = https://github.com/wycats/rust-toml
 [submodule "libs/hammer.rs"]
 	path = libs/hammer.rs
 	url = https://github.com/wycats/hammer.rs.git
 [submodule "libs/hamcrest-rust"]
 	path = libs/hamcrest-rust
 	url = https://github.com/carllerche/hamcrest-rust/
+[submodule "libs/toml-rs"]
+	path = libs/toml-rs
+	url = https://github.com/alexcrichton/toml-rs

Makefile

@@ -11,8 +11,8 @@ BINS = cargo \
 
 SRC = $(shell find src -name '*.rs' -not -path 'src/bin*')
 
-DEPS = -L libs/hammer.rs/target -L libs/rust-toml/lib
-TOML = libs/rust-toml/lib/$(shell rustc --crate-file-name libs/rust-toml/src/toml/lib.rs)
+DEPS = -L libs/hammer.rs/target -L libs/toml-rs/build
+TOML = libs/toml-rs/lib/$(shell rustc --crate-file-name libs/toml-rs/src/toml.rs)
 HAMMER = libs/hammer.rs/target/$(shell rustc --crate-type=lib --crate-file-name libs/hammer.rs/src/hammer.rs)
 HAMCREST = libs/hamcrest-rust/target/libhamcrest.timestamp
 LIBCARGO = target/libcargo.timestamp
@@ -25,8 +25,8 @@ all: $(BIN_TARGETS)
 $(HAMMER): $(wildcard libs/hammer.rs/src/*.rs)
 	$(MAKE) -C libs/hammer.rs
 
-$(TOML): $(wildcard libs/rust-toml/src/toml/*.rs)
-	$(MAKE) -C libs/rust-toml
+$(TOML): $(wildcard libs/toml-rs/src/*.rs)
+	$(MAKE) -C libs/toml-rs
 
 $(HAMCREST): $(shell find libs/hamcrest-rust/src/hamcrest -name '*.rs')
 	$(MAKE) -C libs/hamcrest-rust
@@ -71,7 +71,7 @@ clean:
 distclean: clean
 	cd libs/hamcrest-rust && make clean
 	cd libs/hammer.rs && make clean
-	cd libs/rust-toml && make clean
+	cd libs/toml-rs && make clean
 
 # Setup phony tasks
 .PHONY: all clean distclean test test-unit test-integration libcargo

libs/rust-toml

@@ -1 +0,0 @@
-Subproject commit 3eaa0ee4c81aa49bdcc98cad448ef3a6386d62af

libs/toml-rs

@@ -0,0 +1 @@
+Subproject commit ed88ef0b8151277cc6a876f2daaeb1c63420717b

src/bin/cargo-verify-project.rs

@@ -1,10 +1,11 @@
 #![crate_id="cargo-verify-project"]
 
-extern crate toml = "github.com/mneumann/rust-toml#toml";
+extern crate toml;
 extern crate getopts;
 
-use std::os::{args,set_exit_status};
-use getopts::{reqopt,getopts};
+use std::io::File;
+use std::os::{args, set_exit_status};
+use getopts::{reqopt, getopts};
 
 /**
     cargo-verify-project --manifest=LOCATION
@@ -32,18 +33,17 @@ fn main() {
 
     let manifest = matches.opt_str("m").unwrap();
     let file = Path::new(manifest);
-
-    if !file.exists() {
-        fail("invalid", "not-found");
-        return;
-    }
-
-    match toml::parse_from_file(file.as_str().unwrap()) {
-        Err(_) => {
+    let contents = match File::open(&file).read_to_str() {
+        Ok(s) => s,
+        Err(e) => return fail("invalid", format!("error reading file: {}",
+                                                 e).as_slice())
+    };
+    match toml::Parser::new(contents.as_slice()).parse() {
+        None => {
             fail("invalid", "invalid-format");
             return;
         },
-        Ok(r) => r
+        Some(..) => {}
     };
 
     println!("{}", "{ \"success\": \"true\" }");

src/bin/cargo.rs

@@ -1,7 +1,6 @@
 #![feature(phase)]
 
 extern crate cargo;
-extern crate toml = "github.com/mneumann/rust-toml#toml";
 extern crate hammer;
 extern crate serialize;
 #[phase(plugin, link)]

src/cargo/lib.rs

@@ -8,7 +8,7 @@ extern crate term;
 extern crate url;
 extern crate serialize;
 extern crate semver;
-extern crate toml = "github.com/mneumann/rust-toml#toml";
+extern crate toml;
 
 #[phase(plugin, link)]
 extern crate hammer;

src/cargo/util/config.rs

@@ -5,6 +5,8 @@ use toml;
 use core::MultiShell;
 use util::{CargoResult, CargoError, ChainError, Require, internal, human};
 
+use cargo_toml = util::toml;
+
 pub struct Config<'a> {
     home_path: Path,
     shell: &'a mut MultiShell
@@ -164,11 +166,12 @@ fn walk_tree(pwd: &Path,
     Ok(())
 }
 
-fn extract_config(file: io::fs::File, key: &str) -> CargoResult<ConfigValue> {
-    let path = file.path().clone();
-    let mut buf = io::BufferedReader::new(file);
-    let root = try!(toml::parse_from_buffer(&mut buf));
-    let val = try!(root.lookup(key).require(|| internal("")));
+fn extract_config(mut file: io::fs::File, key: &str) -> CargoResult<ConfigValue> {
+    let contents = try!(file.read_to_str());
+    let toml = try!(cargo_toml::parse(contents.as_slice(),
+                                      file.path().filename_display()
+                                          .to_str().as_slice()));
+    let val = try!(toml.find_equiv(&key).require(|| internal("")));
 
     let v = match *val {
         toml::String(ref val) => String(val.clone()),
@@ -178,19 +181,16 @@ fn extract_config(file: io::fs::File, key: &str) -> CargoResult<ConfigValue> {
         _ => return Err(internal(""))
     };
 
-    Ok(ConfigValue{ value: v, path: vec!(path) })
+    Ok(ConfigValue{ value: v, path: vec![file.path().clone()] })
 }
 
-fn extract_all_configs(file: io::fs::File,
+fn extract_all_configs(mut file: io::fs::File,
                        map: &mut HashMap<String, ConfigValue>) -> CargoResult<()> {
     let path = file.path().clone();
-    let mut buf = io::BufferedReader::new(file);
-    let root = try!(toml::parse_from_buffer(&mut buf).chain_error(|| {
-        internal(format!("could not parse Toml manifest; path={}",
-                         path.display()))
-    }));
-
-    let table = try!(root.get_table().require(|| {
+    let contents = try!(file.read_to_str());
+    let file = path.filename_display().to_str();
+    let table = try!(cargo_toml::parse(contents.as_slice(),
+                                       file.as_slice()).chain_error(|| {
         internal(format!("could not parse Toml manifest; path={}",
                          path.display()))
     }));

src/cargo/util/toml.rs

@@ -1,8 +1,9 @@
-use toml;
-use url;
-use url::Url;
-use std::collections::HashMap;
 use serialize::Decodable;
+use std::collections::HashMap;
+use std::str;
+use toml;
+use url::Url;
+use url;
 
 use core::{SourceId,GitKind};
 use core::manifest::{LibKind,Lib};
@@ -11,84 +12,43 @@ use util::{CargoResult, Require, human};
 
 pub fn to_manifest(contents: &[u8],
                    source_id: &SourceId) -> CargoResult<(Manifest, Vec<Path>)> {
-    let root = try!(toml::parse_from_bytes(contents).map_err(|_| {
-        human("Cargo.toml is not valid Toml")
+    let contents = try!(str::from_utf8(contents).require(|| {
+        human("Cargo.toml is not valid UTF-8")
     }));
-    let toml = try!(toml_to_manifest(root).map_err(|_| {
-        human("Cargo.toml is not a valid manifest")
-    }));
-
-    toml.to_manifest(source_id)
-}
-
-fn toml_to_manifest(root: toml::Value) -> CargoResult<TomlManifest> {
-    fn decode<T: Decodable<toml::Decoder,toml::Error>>(root: &toml::Value,
-                                                       path: &str)
-        -> Result<T, toml::Error>
-    {
-        let root = match root.lookup(path) {
-            Some(val) => val,
-            None => return Err(toml::ParseError)
-        };
-        toml::from_toml(root.clone())
-    }
-
-    let project = try!(decode(&root, "project"));
-    let lib = decode(&root, "lib").ok();
-    let bin = decode(&root, "bin").ok();
-
-    let deps = root.lookup("dependencies");
-
-    let deps = match deps {
-        Some(deps) => {
-            let table = try!(deps.get_table().require(|| {
-                human("dependencies must be a table")
-            })).clone();
-
-            let mut deps: HashMap<String, TomlDependency> = HashMap::new();
-
-            for (k, v) in table.iter() {
-                match v {
-                    &toml::String(ref string) => {
-                        deps.insert(k.clone(), SimpleDep(string.clone()));
-                    },
-                    &toml::Table(ref table) => {
-                        let mut details = HashMap::<String, String>::new();
-
-                        for (k, v) in table.iter() {
-                            let v = try!(v.get_str().require(|| {
-                                human("dependency values must be string")
-                            }));
-
-                            details.insert(k.clone(), v.clone());
-                        }
-
-                        let version = try!(details.find_equiv(&"version")
-                                           .require(|| {
-                            human("dependencies must include a version")
-                        })).clone();
-
-                        deps.insert(k.clone(),
-                                    DetailedDep(DetailedTomlDependency {
-                            version: version,
-                            other: details
-                        }));
-                    },
-                    _ => ()
-                }
-            }
-
-            Some(deps)
-        },
-        None => None
+    let root = try!(parse(contents, "Cargo.toml"));
+    let mut d = toml::Decoder::new(toml::Table(root));
+    let toml_manifest: TomlManifest = match Decodable::decode(&mut d) {
+        Ok(t) => t,
+        Err(e) => return Err(human(format!("Cargo.toml is not a valid \
+                                            manifest\n\n{}", e)))
     };
 
-    Ok(TomlManifest {
-        project: box project,
-        lib: lib,
-        bin: bin,
-        dependencies: deps,
-    })
+    toml_manifest.to_manifest(source_id)
+}
+
+pub fn parse(toml: &str, file: &str) -> CargoResult<toml::Table> {
+    let mut parser = toml::Parser::new(toml.as_slice());
+    match parser.parse() {
+        Some(toml) => Ok(toml),
+        None => {
+            let mut error_str = format!("could not parse input TOML\n");
+            for error in parser.errors.iter() {
+                let (loline, locol) = parser.to_linecol(error.lo);
+                let (hiline, hicol) = parser.to_linecol(error.hi);
+                error_str.push_str(format!("{}:{}:{}{} {}",
+                                           file,
+                                           loline + 1, locol + 1,
+                                           if loline != hiline || locol != hicol {
+                                               format!("-{}:{}", hiline + 1,
+                                                       hicol + 1)
+                                           } else {
+                                               "".to_string()
+                                           },
+                                           error.desc).as_slice());
+            }
+            Err(human(error_str))
+        }
+    }
 }
 
 type TomlLibTarget = TomlTarget;
@@ -98,19 +58,20 @@ type TomlBinTarget = TomlTarget;
  * TODO: Make all struct fields private
  */
 
-#[deriving(Encodable,PartialEq,Clone,Show)]
+#[deriving(Encodable,Decodable,PartialEq,Clone,Show)]
 pub enum TomlDependency {
     SimpleDep(String),
     DetailedDep(DetailedTomlDependency)
 }
 
-#[deriving(Encodable,PartialEq,Clone,Show)]
+#[deriving(Encodable,Decodable,PartialEq,Clone,Show)]
 pub struct DetailedTomlDependency {
     version: String,
-    other: HashMap<String, String>
+    path: Option<String>,
+    git: Option<String>,
 }
 
-#[deriving(Encodable,PartialEq,Clone)]
+#[deriving(Encodable,Decodable,PartialEq,Clone)]
 pub struct TomlManifest {
     project: Box<TomlProject>,
     lib: Option<Vec<TomlLibTarget>>,
@@ -158,8 +119,7 @@ impl TomlManifest {
                             (string.clone(), SourceId::for_central())
                         },
                         DetailedDep(ref details) => {
-                            let new_source_id = details.other.find_equiv(&"git");
-                            let new_source_id = new_source_id.map(|git| {
+                            let new_source_id = details.git.as_ref().map(|git| {
                                 // TODO: Don't unwrap here
                                 let kind = GitKind("master".to_str());
                                 let url = url::from_str(git.as_slice()).unwrap();
@@ -168,7 +128,7 @@ impl TomlManifest {
                                 sources.push(source_id.clone());
                                 source_id
                             }).or_else(|| {
-                                details.other.find_equiv(&"path").map(|path| {
+                                details.path.as_ref().map(|path| {
                                     nested_paths.push(Path::new(path.as_slice()));
                                     source_id.clone()
                                 })

tests/test_cargo_compile.rs

@@ -47,7 +47,22 @@ test!(cargo_compile_with_invalid_manifest {
     assert_that(p.cargo_process("cargo-compile"),
         execs()
         .with_status(101)
-        .with_stderr("Cargo.toml is not a valid manifest\n"));
+        .with_stderr("Cargo.toml is not a valid manifest\n\n\
+                      expected a section for the key `project`\n"))
+})
+
+test!(cargo_compile_with_invalid_manifest2 {
+    let p = project("foo")
+        .file("Cargo.toml", r"
+            [project]
+            foo = bar
+        ");
+
+    assert_that(p.cargo_process("cargo-compile"),
+        execs()
+        .with_status(101)
+        .with_stderr("could not parse input TOML\n\
+                      Cargo.toml:3:19-3:20 expected a value\n"))
 })
 
 test!(cargo_compile_without_manifest {
@@ -250,9 +265,9 @@ test!(cargo_compile_with_nested_deps_longhand {
             version = "0.5.0"
             authors = ["wycats@example.com"]
 
-            [dependencies.bar]
+            [dependencies]
 
-            version = "0.5.0"
+            bar = "0.5.0"
 
             [[bin]]
 
@@ -473,7 +488,7 @@ test!(custom_build_in_dependency {
             authors = ["wycats@example.com"]
 
             [[bin]] name = "foo"
-            [dependencies.bar] version = "0.5.0"
+            [dependencies] bar = "0.5.0"
         "#)
         .file("src/foo.rs", r#"
             extern crate bar;

tests/test_cargo_compile_git_deps.rs

@@ -187,7 +187,8 @@ test!(recompilation {
                 version = "0.5.0"
                 authors = ["carlhuda@example.com"]
 
-                [[lib]] name = "bar"
+                [[lib]]
+                name = "bar"
             "#)
             .file("src/bar.rs", r#"
                 pub fn bar() {}

tests/test_cargo_compile_path_deps.rs

@@ -98,7 +98,7 @@ test!(no_rebuild_dependency {
             authors = ["wycats@example.com"]
 
             [[bin]] name = "foo"
-            [dependencies.bar] version = "0.5.0"
+            [dependencies] bar = "0.5.0"
         "#)
         .file("src/foo.rs", r#"
             extern crate bar;
@@ -152,8 +152,10 @@ test!(deep_dependencies_trigger_rebuild {
             version = "0.5.0"
             authors = ["wycats@example.com"]
 
-            [[bin]] name = "foo"
-            [dependencies.bar] version = "0.5.0"
+            [[bin]]
+            name = "foo"
+            [dependencies]
+            bar = "0.5.0"
         "#)
         .file("src/foo.rs", r#"
             extern crate bar;
@@ -166,8 +168,10 @@ test!(deep_dependencies_trigger_rebuild {
             version = "0.5.0"
             authors = ["wycats@example.com"]
 
-            [[lib]] name = "bar"
-            [dependencies.baz] version = "0.5.0"
+            [[lib]]
+            name = "bar"
+            [dependencies]
+            baz = "0.5.0"
         "#)
         .file("bar/src/bar.rs", r#"
             extern crate baz;
@@ -180,7 +184,8 @@ test!(deep_dependencies_trigger_rebuild {
             version = "0.5.0"
             authors = ["wycats@example.com"]
 
-            [[lib]] name = "baz"
+            [[lib]]
+            name = "baz"
         "#)
         .file("baz/src/baz.rs", r#"
             pub fn baz() {}
@@ -246,9 +251,11 @@ test!(no_rebuild_two_deps {
             version = "0.5.0"
             authors = ["wycats@example.com"]
 
-            [[bin]] name = "foo"
-            [dependencies.bar] version = "0.5.0"
-            [dependencies.baz] version = "0.5.0"
+            [[bin]]
+            name = "foo"
+            [dependencies]
+            bar = "0.5.0"
+            baz = "0.5.0"
         "#)
         .file("src/foo.rs", r#"
             extern crate bar;
@@ -261,8 +268,10 @@ test!(no_rebuild_two_deps {
             version = "0.5.0"
             authors = ["wycats@example.com"]
 
-            [[lib]] name = "bar"
-            [dependencies.baz] version = "0.5.0"
+            [[lib]]
+            name = "bar"
+            [dependencies]
+            baz = "0.5.0"
         "#)
         .file("bar/src/bar.rs", r#"
             pub fn bar() {}
@@ -274,7 +283,8 @@ test!(no_rebuild_two_deps {
             version = "0.5.0"
             authors = ["wycats@example.com"]
 
-            [[lib]] name = "baz"
+            [[lib]]
+            name = "baz"
         "#)
         .file("baz/src/baz.rs", r#"
             pub fn baz() {}