diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 6d77c77..12b532e 100644 --- a/.github/workflows/rust.yml +++ b/.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 diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 0000000..679700c --- /dev/null +++ b/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!" }, +] diff --git a/src/lib.rs b/src/lib.rs index 020d315..ff7e959 100644 --- a/src/lib.rs +++ b/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, 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 { - 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 { + 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 { +fn dep_tables(table: &dyn TableLike) -> impl Iterator { 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#"