Skip to content
Draft
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
43 changes: 43 additions & 0 deletions clippy_lints/src/cargo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String>,
ignore_publish: bool,
Expand All @@ -225,6 +266,7 @@ impl_lint_pass!(Cargo => [
MULTIPLE_CRATE_VERSIONS,
WILDCARD_DEPENDENCIES,
LINT_GROUPS_PRIORITY,
WORKSPACE_DEPENDENCIES,
]);

impl Cargo {
Expand All @@ -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()
Expand Down
119 changes: 119 additions & 0 deletions clippy_lints/src/cargo/workspace_dependencies.rs
Original file line number Diff line number Diff line change
@@ -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<usize>, 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<String> {
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<String>,
_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]");
}
}
}
57 changes: 57 additions & 0 deletions test_toml_parse.rs
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
27 changes: 27 additions & 0 deletions tests/ui-cargo/workspace_dependencies/fail/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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"
1 change: 1 addition & 0 deletions tests/ui-cargo/workspace_dependencies/fail/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}
14 changes: 14 additions & 0 deletions tests/ui-cargo/workspace_dependencies/no_workspace_deps/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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"] }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}
32 changes: 32 additions & 0 deletions tests/ui-cargo/workspace_dependencies/pass/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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 }
1 change: 1 addition & 0 deletions tests/ui-cargo/workspace_dependencies/pass/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}
Empty file.
Loading