Merge pull request #58 from ijackson/table

Fix handling of inline tables

Author: bkchr

.github/workflows/rust.yml

@@ -44,3 +44,12 @@ jobs:
     - uses: Swatinem/rust-cache@v2
     - name: Check formatting
       run: cargo fmt --all -- --check
+  clippy:
+    name: cargo clippy (forbidden methods)
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: dtolnay/rust-toolchain@stable
+      # We just use clippy to spot forbidden methods.
+      # We disable the default lint set which has much questionable output.
+      - run: cargo clippy --all -- -A clippy::all -D clippy::disallowed_methods

clippy.toml

@@ -0,0 +1,5 @@
+disallowed-methods = [
+    # Avoid a repeat of https://github.com/bkchr/proc-macro-crate/issues/57
+    { path = "toml_edit::Item::as_table", reason = "Use .as_table_like instead!" },
+    { path = "toml_edit::Item::is_table", reason = "Use .is_table_like instead!" },
+]

src/lib.rs

@@ -94,7 +94,7 @@ use std::{
     time::SystemTime,
 };
 
-use toml_edit::{DocumentMut, Item, Table, TomlError};
+use toml_edit::{DocumentMut, Item, TableLike, TomlError};
 
 /// Error type used by this crate.
 pub enum Error {
@@ -308,6 +308,7 @@ fn extract_workspace_dependencies(
 ) -> Result<BTreeMap<String, String>, Error> {
     Ok(workspace_dep_tables(&workspace_toml)
         .into_iter()
+        .map(|t| t.iter())
         .flatten()
         .map(move |(dep_name, dep_value)| {
             let pkg_name = dep_value.get("package").and_then(|i| i.as_str()).unwrap_or(dep_name);
@@ -318,10 +319,10 @@ fn extract_workspace_dependencies(
 }
 
 /// Return an iterator over all `[workspace.dependencies]`
-fn workspace_dep_tables(cargo_toml: &DocumentMut) -> Option<&Table> {
+fn workspace_dep_tables(cargo_toml: &DocumentMut) -> Option<&dyn TableLike> {
     cargo_toml
         .get("workspace")
-        .and_then(|w| w.as_table()?.get("dependencies")?.as_table())
+        .and_then(|w| w.as_table_like()?.get("dependencies")?.as_table_like())
 }
 
 /// Make sure that the given crate name is a valid rust identifier.
@@ -354,27 +355,29 @@ fn extract_crate_names(
         (name.to_string(), cr)
     });
 
-    let dep_tables = dep_tables(cargo_toml).chain(target_dep_tables(cargo_toml));
-    let dep_pkgs = dep_tables.flatten().filter_map(move |(dep_name, dep_value)| {
-        let pkg_name = dep_value.get("package").and_then(|i| i.as_str()).unwrap_or(dep_name);
+    let dep_tables = dep_tables(cargo_toml.as_table()).chain(target_dep_tables(cargo_toml));
+    let dep_pkgs =
+        dep_tables.map(|t| t.iter()).flatten().filter_map(move |(dep_name, dep_value)| {
+            let pkg_name = dep_value.get("package").and_then(|i| i.as_str()).unwrap_or(dep_name);
 
-        // We already handle this via `root_pkg` above.
-        if package_name.as_ref().map_or(false, |n| *n == pkg_name) {
-            return None
-        }
+            // We already handle this via `root_pkg` above.
+            if package_name.as_ref().map_or(false, |n| *n == pkg_name) {
+                return None
+            }
 
-        // Check if this is a workspace dependency.
-        let workspace = dep_value.get("workspace").and_then(|w| w.as_bool()).unwrap_or_default();
+            // Check if this is a workspace dependency.
+            let workspace =
+                dep_value.get("workspace").and_then(|w| w.as_bool()).unwrap_or_default();
 
-        let pkg_name = workspace
-            .then(|| workspace_dependencies.get(pkg_name).map(|p| p.as_ref()))
-            .flatten()
-            .unwrap_or(pkg_name);
+            let pkg_name = workspace
+                .then(|| workspace_dependencies.get(pkg_name).map(|p| p.as_ref()))
+                .flatten()
+                .unwrap_or(pkg_name);
 
-        let cr = FoundCrate::Name(sanitize_crate_name(dep_name));
+            let cr = FoundCrate::Name(sanitize_crate_name(dep_name));
 
-        Some((pkg_name.to_owned(), cr))
-    });
+            Some((pkg_name.to_owned(), cr))
+        });
 
     Ok(root_pkg.into_iter().chain(dep_pkgs).collect())
 }
@@ -383,18 +386,25 @@ fn extract_package_name(cargo_toml: &DocumentMut) -> Option<&str> {
     cargo_toml.get("package")?.get("name")?.as_str()
 }
 
-fn target_dep_tables(cargo_toml: &DocumentMut) -> impl Iterator<Item = &Table> {
-    cargo_toml.get("target").into_iter().filter_map(Item::as_table).flat_map(|t| {
-        t.iter().map(|(_, value)| value).filter_map(Item::as_table).flat_map(dep_tables)
-    })
+fn target_dep_tables(cargo_toml: &DocumentMut) -> impl Iterator<Item = &dyn TableLike> {
+    cargo_toml
+        .get("target")
+        .into_iter()
+        .filter_map(Item::as_table_like)
+        .flat_map(|t| {
+            t.iter()
+                .map(|(_, value)| value)
+                .filter_map(Item::as_table_like)
+                .flat_map(dep_tables)
+        })
 }
 
-fn dep_tables(table: &Table) -> impl Iterator<Item = &Table> {
+fn dep_tables(table: &dyn TableLike) -> impl Iterator<Item = &dyn TableLike> {
     table
         .get("dependencies")
         .into_iter()
         .chain(table.get("dev-dependencies"))
-        .filter_map(Item::as_table)
+        .filter_map(Item::as_table_like)
 }
 
 #[cfg(test)]
@@ -438,6 +448,16 @@ mod tests {
         Ok(Some(FoundCrate::Name(name))) if name == "my_crate"
     }
 
+    // forbidding toml_edit::Item::as_table ought to mean this is OK, but let's have a test too
+    create_test! {
+        deps_with_crate_inline_table,
+        r#"
+            dependencies = { my_crate = "0.1" }
+        "#,
+        "",
+        Ok(Some(FoundCrate::Name(name))) if name == "my_crate"
+    }
+
     create_test! {
         dev_deps_with_crate,
         r#"