From c6066d7e6fcdda83760ff4c3b0435fc04b88fc91 Mon Sep 17 00:00:00 2001 From: Andrew Gazelka Date: Mon, 20 Oct 2025 18:10:09 -0700 Subject: [PATCH] workspace depencies --- clippy_lints/src/cargo/mod.rs | 43 +++++++ .../src/cargo/workspace_dependencies.rs | 119 ++++++++++++++++++ test_toml_parse.rs | 57 +++++++++ .../workspace_dependencies/fail/Cargo.toml | 27 ++++ .../workspace_dependencies/fail/src/main.rs | 1 + .../no_workspace_deps/Cargo.toml | 14 +++ .../no_workspace_deps/src/main.rs | 1 + .../workspace_dependencies/pass/Cargo.toml | 32 +++++ .../workspace_dependencies/pass/src/main.rs | 1 + tests/ui/explicit_write_in_test.stderr | 0 10 files changed, 295 insertions(+) create mode 100644 clippy_lints/src/cargo/workspace_dependencies.rs create mode 100644 test_toml_parse.rs create mode 100644 tests/ui-cargo/workspace_dependencies/fail/Cargo.toml create mode 100644 tests/ui-cargo/workspace_dependencies/fail/src/main.rs create mode 100644 tests/ui-cargo/workspace_dependencies/no_workspace_deps/Cargo.toml create mode 100644 tests/ui-cargo/workspace_dependencies/no_workspace_deps/src/main.rs create mode 100644 tests/ui-cargo/workspace_dependencies/pass/Cargo.toml create mode 100644 tests/ui-cargo/workspace_dependencies/pass/src/main.rs delete mode 100644 tests/ui/explicit_write_in_test.stderr diff --git a/clippy_lints/src/cargo/mod.rs b/clippy_lints/src/cargo/mod.rs index 60371dcd7715..1682cb301a4b 100644 --- a/clippy_lints/src/cargo/mod.rs +++ b/clippy_lints/src/cargo/mod.rs @@ -3,6 +3,7 @@ mod feature_name; mod lint_groups_priority; mod multiple_crate_versions; mod wildcard_dependencies; +mod workspace_dependencies; use cargo_metadata::MetadataCommand; use clippy_config::Conf; @@ -213,6 +214,46 @@ declare_clippy_lint! { "a lint group in `Cargo.toml` at the same priority as a lint" } +declare_clippy_lint! { + /// ### What it does + /// Checks that dependencies defined in `[workspace.dependencies]` are used with + /// `workspace = true` in package dependency declarations instead of specifying + /// version, git, or path information directly. + /// + /// ### Why is this bad? + /// When using workspace dependencies, all version information should be centralized + /// in the workspace's `Cargo.toml` to ensure consistency across all crates in the + /// workspace. Specifying version information in individual packages defeats this + /// purpose and can lead to version mismatches. + /// + /// ### Example + /// ```toml + /// # In workspace Cargo.toml + /// [workspace.dependencies] + /// serde = "1.0" + /// + /// # In package Cargo.toml (bad) + /// [dependencies] + /// serde = "1.0" + /// ``` + /// Use instead: + /// ```toml + /// # In workspace Cargo.toml + /// [workspace.dependencies] + /// serde = "1.0" + /// + /// # In package Cargo.toml (good) + /// [dependencies] + /// serde = { workspace = true } + /// # or with features + /// serde = { workspace = true, features = ["derive"] } + /// ``` + #[clippy::version = "1.84.0"] + pub WORKSPACE_DEPENDENCIES, + cargo, + "dependencies defined in workspace should use `workspace = true`" +} + pub struct Cargo { allowed_duplicate_crates: FxHashSet, ignore_publish: bool, @@ -225,6 +266,7 @@ impl_lint_pass!(Cargo => [ MULTIPLE_CRATE_VERSIONS, WILDCARD_DEPENDENCIES, LINT_GROUPS_PRIORITY, + WORKSPACE_DEPENDENCIES, ]); impl Cargo { @@ -247,6 +289,7 @@ impl LateLintPass<'_> for Cargo { static WITH_DEPS_LINTS: &[&Lint] = &[MULTIPLE_CRATE_VERSIONS]; lint_groups_priority::check(cx); + workspace_dependencies::check(cx); if !NO_DEPS_LINTS .iter() diff --git a/clippy_lints/src/cargo/workspace_dependencies.rs b/clippy_lints/src/cargo/workspace_dependencies.rs new file mode 100644 index 000000000000..fe9f60a1fd11 --- /dev/null +++ b/clippy_lints/src/cargo/workspace_dependencies.rs @@ -0,0 +1,119 @@ +use clippy_utils::diagnostics::span_lint; +use rustc_data_structures::fx::FxHashSet; +use rustc_lint::LateContext; +use rustc_span::{BytePos, Pos, SourceFile, Span, SyntaxContext, DUMMY_SP}; +use std::ops::Range; +use std::path::Path; +use toml::de::{DeTable, DeValue}; + +use super::WORKSPACE_DEPENDENCIES; + +fn toml_span(range: Range, file: &SourceFile) -> Span { + Span::new( + file.start_pos + BytePos::from_usize(range.start), + file.start_pos + BytePos::from_usize(range.end), + SyntaxContext::root(), + None, + ) +} + +fn is_workspace_dep(value: &DeValue<'_>) -> bool { + match value { + DeValue::Table(tbl) => { + if let Some(workspace) = tbl.get("workspace") { + if let DeValue::Boolean(b) = workspace.get_ref() { + return *b; + } + } + false + } + _ => false, + } +} + +fn has_inline_version_info(value: &DeValue<'_>) -> bool { + match value { + DeValue::String(_) => true, // e.g., serde = "1.0" + DeValue::Table(tbl) => { + // Check if it has version, git, or path fields (but not workspace = true) + if is_workspace_dep(value) { + return false; + } + tbl.contains_key("version") || tbl.contains_key("git") || tbl.contains_key("path") + } + _ => false, + } +} + +fn get_workspace_deps(cargo_toml: &DeTable<'_>) -> FxHashSet { + let mut workspace_deps = FxHashSet::default(); + + if let Some(workspace) = cargo_toml.get("workspace") + && let Some(workspace_tbl) = workspace.get_ref().as_table() + && let Some(dependencies) = workspace_tbl.get("dependencies") + && let Some(deps_tbl) = dependencies.get_ref().as_table() + { + for dep_name in deps_tbl.keys() { + workspace_deps.insert(dep_name.get_ref().to_string()); + } + } + + workspace_deps +} + +fn check_dependencies( + cx: &LateContext<'_>, + deps_tbl: &DeTable<'_>, + workspace_deps: &FxHashSet, + _file: &SourceFile, + section_name: &str, +) { + for (dep_name, dep_value) in deps_tbl { + let name = dep_name.get_ref().as_ref(); + + if workspace_deps.contains(name) && has_inline_version_info(dep_value.get_ref()) { + span_lint( + cx, + WORKSPACE_DEPENDENCIES, + DUMMY_SP, + format!("dependency `{name}` is defined in workspace but not using `workspace = true` in {section_name}"), + ); + } + } +} + +pub fn check(cx: &LateContext<'_>) { + if let Ok(file) = cx.tcx.sess.source_map().load_file(Path::new("Cargo.toml")) + && let Some(src) = file.src.as_deref() + && let Ok(cargo_toml) = DeTable::parse(src) + { + // First, collect all workspace dependencies + let workspace_deps = get_workspace_deps(cargo_toml.get_ref()); + + // If there are no workspace dependencies, nothing to check + if workspace_deps.is_empty() { + return; + } + + // Check [dependencies] + if let Some(dependencies) = cargo_toml.get_ref().get("dependencies") + && let Some(deps_tbl) = dependencies.get_ref().as_table() + { + check_dependencies(cx, deps_tbl, &workspace_deps, &file, "[dependencies]"); + } + + // Check [dev-dependencies] + if let Some(dev_dependencies) = cargo_toml.get_ref().get("dev-dependencies") + && let Some(dev_deps_tbl) = dev_dependencies.get_ref().as_table() + { + check_dependencies(cx, dev_deps_tbl, &workspace_deps, &file, "[dev-dependencies]"); + } + + // Check [build-dependencies] + if let Some(build_dependencies) = cargo_toml.get_ref().get("build-dependencies") + && let Some(build_deps_tbl) = build_dependencies.get_ref().as_table() + { + check_dependencies(cx, build_deps_tbl, &workspace_deps, &file, "[build-dependencies]"); + } + } +} diff --git a/test_toml_parse.rs b/test_toml_parse.rs new file mode 100644 index 000000000000..e392e9ed4c9a --- /dev/null +++ b/test_toml_parse.rs @@ -0,0 +1,57 @@ +use toml::de::DeTable; + +fn main() { + let content = r#" +[package] +name = "workspace_dependencies" +version = "0.1.0" +publish = false + +[workspace] +members = [] + +[workspace.dependencies] +serde = "1.0" +tokio = { version = "1.0", features = ["full"] } + +[dependencies] +serde = "1.0" +tokio = { version = "1.0", features = ["rt"] } +"#; + + match DeTable::parse(content) { + Ok(cargo_toml) => { + println!("Parsed successfully!"); + + // Check workspace.dependencies + if let Some(workspace) = cargo_toml.get_ref().get("workspace") { + println!("Found workspace"); + if let Some(workspace_tbl) = workspace.get_ref().as_table() { + if let Some(deps) = workspace_tbl.get("dependencies") { + println!("Found workspace.dependencies"); + if let Some(deps_tbl) = deps.get_ref().as_table() { + println!("Workspace dependencies:"); + for (name, _) in deps_tbl { + println!(" - {}", name.get_ref()); + } + } + } + } + } + + // Check dependencies + if let Some(deps) = cargo_toml.get_ref().get("dependencies") { + println!("Found dependencies"); + if let Some(deps_tbl) = deps.get_ref().as_table() { + println!("Package dependencies:"); + for (name, value) in deps_tbl { + println!(" - {} = {:?}", name.get_ref(), value.get_ref()); + } + } + } + } + Err(e) => { + println!("Parse error: {}", e); + } + } +} diff --git a/tests/ui-cargo/workspace_dependencies/fail/Cargo.toml b/tests/ui-cargo/workspace_dependencies/fail/Cargo.toml new file mode 100644 index 000000000000..5568b7ef52f7 --- /dev/null +++ b/tests/ui-cargo/workspace_dependencies/fail/Cargo.toml @@ -0,0 +1,27 @@ +[package] +name = "workspace_dependencies" +version = "0.1.0" +publish = false +edition = "2021" + +[workspace] +members = [] + +[workspace.dependencies] +serde = "1.0" +tokio = { version = "1.0", features = ["full"] } +regex = "1.5" + +[dependencies] +# Bad: specifying version directly when workspace has it +serde = "1.0" +# Bad: using table form with version +tokio = { version = "1.0", features = ["rt"] } + +[dev-dependencies] +# Bad: dev-dependency with inline version +regex = "1.5" + +[build-dependencies] +# Bad: build-dependency with inline version +serde = "1.0" diff --git a/tests/ui-cargo/workspace_dependencies/fail/src/main.rs b/tests/ui-cargo/workspace_dependencies/fail/src/main.rs new file mode 100644 index 000000000000..f328e4d9d04c --- /dev/null +++ b/tests/ui-cargo/workspace_dependencies/fail/src/main.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/tests/ui-cargo/workspace_dependencies/no_workspace_deps/Cargo.toml b/tests/ui-cargo/workspace_dependencies/no_workspace_deps/Cargo.toml new file mode 100644 index 000000000000..13d274901d3d --- /dev/null +++ b/tests/ui-cargo/workspace_dependencies/no_workspace_deps/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "workspace_dependencies" +version = "0.1.0" +publish = false +edition = "2021" + +[workspace] +members = [] + +# No workspace.dependencies section, so no warnings should be emitted + +[dependencies] +serde = "1.0" +tokio = { version = "1.0", features = ["rt"] } diff --git a/tests/ui-cargo/workspace_dependencies/no_workspace_deps/src/main.rs b/tests/ui-cargo/workspace_dependencies/no_workspace_deps/src/main.rs new file mode 100644 index 000000000000..f328e4d9d04c --- /dev/null +++ b/tests/ui-cargo/workspace_dependencies/no_workspace_deps/src/main.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/tests/ui-cargo/workspace_dependencies/pass/Cargo.toml b/tests/ui-cargo/workspace_dependencies/pass/Cargo.toml new file mode 100644 index 000000000000..8656b2c32608 --- /dev/null +++ b/tests/ui-cargo/workspace_dependencies/pass/Cargo.toml @@ -0,0 +1,32 @@ +[package] +name = "workspace_dependencies" +version = "0.1.0" +publish = false +edition = "2021" + +[workspace] +members = [] + +[workspace.dependencies] +serde = "1.0" +tokio = { version = "1.0", features = ["full"] } +regex = "1.5" +log = "0.4" + +[dependencies] +# Good: using workspace = true +serde = { workspace = true } +# Good: using workspace = true with additional features +tokio = { workspace = true, features = ["rt"] } +# Good: dependency not in workspace can have inline version +rand = "0.8" + +[dev-dependencies] +# Good: using workspace = true +regex = { workspace = true } +# Good: dependency not in workspace +criterion = "0.5" + +[build-dependencies] +# Good: using workspace = true +log = { workspace = true } diff --git a/tests/ui-cargo/workspace_dependencies/pass/src/main.rs b/tests/ui-cargo/workspace_dependencies/pass/src/main.rs new file mode 100644 index 000000000000..f328e4d9d04c --- /dev/null +++ b/tests/ui-cargo/workspace_dependencies/pass/src/main.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/tests/ui/explicit_write_in_test.stderr b/tests/ui/explicit_write_in_test.stderr deleted file mode 100644 index e69de29bb2d1..000000000000