Skip to content

Commit 7ce7163

Browse files
committed
feat(lint): lint also workspace dependencies
This has a future performance if we have more lints over dependencies and version requirements: * Version requirments in `[workspace.dependencies]` are reparsed. * cfg key in target-specific dep table for each package manifests are reparsed.
1 parent 619492c commit 7ce7163

File tree

4 files changed

+202
-25
lines changed

4 files changed

+202
-25
lines changed

src/cargo/core/workspace.rs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ use cargo_util::paths;
3939
use cargo_util::paths::normalize_path;
4040
use cargo_util_schemas::manifest;
4141
use cargo_util_schemas::manifest::RustVersion;
42+
use cargo_util_schemas::manifest::TomlManifest;
4243
use cargo_util_schemas::manifest::{TomlDependency, TomlProfiles};
4344
use pathdiff::diff_paths;
4445

@@ -1297,7 +1298,13 @@ impl<'gctx> Workspace<'gctx> {
12971298
self.gctx,
12981299
)?;
12991300
check_im_a_teapot(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
1300-
implicit_minimum_version_req(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
1301+
implicit_minimum_version_req(
1302+
pkg.into(),
1303+
&path,
1304+
&cargo_lints,
1305+
&mut error_count,
1306+
self.gctx,
1307+
)?;
13011308
}
13021309

13031310
if error_count > 0 {
@@ -1333,7 +1340,13 @@ impl<'gctx> Workspace<'gctx> {
13331340
.unwrap_or(manifest::TomlToolLints::default());
13341341

13351342
if self.gctx.cli_unstable().cargo_lints {
1336-
// Calls to lint functions go in here
1343+
implicit_minimum_version_req(
1344+
self.root_maybe().into(),
1345+
self.root_manifest(),
1346+
&cargo_lints,
1347+
&mut error_count,
1348+
self.gctx,
1349+
)?;
13371350
}
13381351

13391352
// This is a short term hack to allow `blanket_hint_mostly_unused`
@@ -1988,6 +2001,13 @@ impl MaybePackage {
19882001
MaybePackage::Virtual(vm) => vm.unstable_features(),
19892002
}
19902003
}
2004+
2005+
pub fn normalized_toml(&self) -> &TomlManifest {
2006+
match self {
2007+
MaybePackage::Package(p) => p.manifest().normalized_toml(),
2008+
MaybePackage::Virtual(vm) => vm.normalized_toml(),
2009+
}
2010+
}
19912011
}
19922012

19932013
impl WorkspaceRootConfig {

src/cargo/util/lints/implicit_minimum_version_req.rs

Lines changed: 97 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use std::borrow::Cow;
12
use std::collections::HashMap;
23
use std::path::Path;
34

@@ -6,16 +7,16 @@ use annotate_snippets::Level;
67
use annotate_snippets::Patch;
78
use annotate_snippets::Snippet;
89
use cargo_platform::Platform;
10+
use cargo_util_schemas::manifest::TomlDependency;
911
use cargo_util_schemas::manifest::TomlToolLints;
1012
use toml::de::DeValue;
1113

1214
use crate::CargoResult;
1315
use crate::GlobalContext;
14-
use crate::core::Manifest;
15-
use crate::core::Package;
1616
use crate::util::OptVersionReq;
1717
use crate::util::lints::Lint;
1818
use crate::util::lints::LintLevel;
19+
use crate::util::lints::ManifestFor;
1920
use crate::util::lints::get_key_value;
2021
use crate::util::lints::rel_cwd_manifest_path;
2122

@@ -30,13 +31,12 @@ pub const LINT: Lint = Lint {
3031
};
3132

3233
pub fn implicit_minimum_version_req(
33-
pkg: &Package,
34+
manifest: ManifestFor<'_>,
3435
manifest_path: &Path,
3536
cargo_lints: &TomlToolLints,
3637
error_count: &mut usize,
3738
gctx: &GlobalContext,
3839
) -> CargoResult<()> {
39-
let manifest = pkg.manifest();
4040
let (lint_level, reason) = LINT.level(
4141
cargo_lints,
4242
manifest.edition(),
@@ -52,19 +52,24 @@ pub fn implicit_minimum_version_req(
5252
let manifest_path = rel_cwd_manifest_path(manifest_path, gctx);
5353
let target_key_for_platform = target_key_for_platform(&manifest);
5454

55-
for dep in manifest.dependencies().iter() {
55+
for dep in dep_entries(&manifest) {
5656
let version_req = dep.version_req();
5757
let Some(cmp) = get_implicit_req(&version_req) else {
5858
continue;
5959
};
6060

61-
let name_in_toml = dep.name_in_toml().as_str();
62-
let key_path =
63-
if let Some(cfg) = dep.platform().and_then(|p| target_key_for_platform.get(p)) {
64-
&["target", &cfg, dep.kind().kind_table(), name_in_toml][..]
65-
} else {
66-
&[dep.kind().kind_table(), name_in_toml][..]
67-
};
61+
let name_in_toml = dep.name_in_toml();
62+
let key_path = match dep {
63+
DepView::Workspace { .. } => &["workspace", "dependencies", name_in_toml][..],
64+
DepView::Real(dep) => {
65+
let dep_kind = dep.kind().kind_table();
66+
if let Some(cfg) = dep.platform().and_then(|p| target_key_for_platform.get(p)) {
67+
&["target", cfg, dep_kind, name_in_toml][..]
68+
} else {
69+
&[dep_kind, name_in_toml][..]
70+
}
71+
}
72+
};
6873

6974
let Some((_key, value)) = get_key_value(document, key_path) else {
7075
continue;
@@ -137,15 +142,84 @@ fn get_implicit_req(req: &OptVersionReq) -> Option<&semver::Comparator> {
137142
///
138143
/// This is only relevant for package dependencies.
139144
/// Workspace deps don't have target-specific variants, so an empty map is returned.
140-
fn target_key_for_platform(manifest: &Manifest) -> HashMap<Platform, String> {
141-
manifest
142-
.normalized_toml()
143-
.target
144-
.as_ref()
145-
.map(|map| {
146-
map.keys()
147-
.map(|k| (k.parse().expect("already parsed"), k.clone()))
148-
.collect()
149-
})
150-
.unwrap_or_default()
145+
fn target_key_for_platform(scope: &ManifestFor<'_>) -> HashMap<Platform, String> {
146+
match scope {
147+
ManifestFor::Package(package) => package
148+
.manifest()
149+
.normalized_toml()
150+
.target
151+
.as_ref()
152+
.map(|map| {
153+
map.keys()
154+
.map(|k| (k.parse().expect("already parsed"), k.clone()))
155+
.collect()
156+
})
157+
.unwrap_or_default(),
158+
ManifestFor::Workspace(_) => Default::default(),
159+
}
160+
}
161+
162+
/// A common view of dependency declarations
163+
/// for linting both workspace and package dependencies.
164+
enum DepView<'a> {
165+
/// Dependency from `[workspace.dependencies]`
166+
Workspace {
167+
name: &'a str,
168+
dep: &'a TomlDependency,
169+
},
170+
/// Dependency from package manifest
171+
Real(&'a crate::core::Dependency),
172+
}
173+
174+
impl DepView<'_> {
175+
fn name_in_toml(&self) -> &str {
176+
match self {
177+
DepView::Workspace { name, dep: _ } => name,
178+
DepView::Real(dep) => dep.name_in_toml().as_str(),
179+
}
180+
}
181+
182+
fn version_req(&self) -> Cow<'_, OptVersionReq> {
183+
match self {
184+
DepView::Workspace { name: _, dep } => {
185+
let opt = match dep {
186+
TomlDependency::Simple(ver) => semver::VersionReq::parse(ver),
187+
TomlDependency::Detailed(detailed) => {
188+
let Some(ver) = detailed.version.as_ref() else {
189+
return Cow::Owned(OptVersionReq::Any);
190+
};
191+
semver::VersionReq::parse(ver)
192+
}
193+
}
194+
.map(Into::into);
195+
Cow::Owned(opt.unwrap_or(OptVersionReq::Any))
196+
}
197+
DepView::Real(dep) => Cow::Borrowed(dep.version_req()),
198+
}
199+
}
200+
}
201+
202+
/// An iterator over dependencies to be linted.
203+
///
204+
/// * For package manifest, iterates over package dependencies of all kinds.
205+
/// * For workspace manifest, iterates over `[workspace.dependencies]` entries.
206+
fn dep_entries<'a>(scope: &'a ManifestFor<'_>) -> Box<dyn Iterator<Item = DepView<'a>> + 'a> {
207+
match scope {
208+
ManifestFor::Package(package) => Box::new(package.dependencies().iter().map(DepView::Real)),
209+
ManifestFor::Workspace(maybe_package) => {
210+
let toml = maybe_package.normalized_toml();
211+
let iter = toml
212+
.workspace
213+
.as_ref()
214+
.and_then(|ws| ws.dependencies.as_ref())
215+
.into_iter()
216+
.flat_map(|deps| {
217+
deps.iter().map(|(name, dep)| DepView::Workspace {
218+
name: name.as_str(),
219+
dep,
220+
})
221+
});
222+
Box::new(iter)
223+
}
224+
}
151225
}

src/cargo/util/lints/mod.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,56 @@ pub const LINTS: &[Lint] = &[
1919
UNKNOWN_LINTS,
2020
];
2121

22+
/// Scope at which a lint runs: package-level or workspace-level.
23+
pub enum ManifestFor<'a> {
24+
/// Lint runs for a specific package.
25+
Package(&'a Package),
26+
/// Lint runs for workspace-level config.
27+
Workspace(&'a MaybePackage),
28+
}
29+
30+
impl ManifestFor<'_> {
31+
fn edition(&self) -> Edition {
32+
match self {
33+
ManifestFor::Package(p) => p.manifest().edition(),
34+
ManifestFor::Workspace(p) => p.edition(),
35+
}
36+
}
37+
38+
fn unstable_features(&self) -> &crate::core::Features {
39+
match self {
40+
ManifestFor::Package(p) => p.manifest().unstable_features(),
41+
ManifestFor::Workspace(p) => p.unstable_features(),
42+
}
43+
}
44+
45+
fn document(&self) -> &toml::Spanned<toml::de::DeTable<'static>> {
46+
match self {
47+
ManifestFor::Package(p) => p.manifest().document(),
48+
ManifestFor::Workspace(p) => p.document(),
49+
}
50+
}
51+
52+
fn contents(&self) -> &str {
53+
match self {
54+
ManifestFor::Package(p) => p.manifest().contents(),
55+
ManifestFor::Workspace(p) => p.contents(),
56+
}
57+
}
58+
}
59+
60+
impl<'a> From<&'a MaybePackage> for ManifestFor<'a> {
61+
fn from(value: &'a MaybePackage) -> ManifestFor<'a> {
62+
ManifestFor::Workspace(value)
63+
}
64+
}
65+
66+
impl<'a> From<&'a Package> for ManifestFor<'a> {
67+
fn from(value: &'a Package) -> ManifestFor<'a> {
68+
ManifestFor::Package(value)
69+
}
70+
}
71+
2272
pub fn analyze_cargo_lints_table(
2373
pkg: &Package,
2474
path: &Path,

tests/testsuite/lints/implicit_minimum_version_req.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,17 @@ workspace = true
822822
p.cargo("check -Zcargo-lints")
823823
.masquerade_as_nightly_cargo(&["cargo-lints"])
824824
.with_stderr_data(str![[r#"
825+
[WARNING] dependency version requirement without an explicit minimum version
826+
--> Cargo.toml:7:7
827+
|
828+
7 | dep = "1"
829+
| ^^^ missing minor and patch components
830+
|
831+
[HELP] consider specifying a full `major.minor.patch` version requirement
832+
|
833+
7 | dep = "1.0.0"
834+
| ++++
835+
= [NOTE] `cargo::implicit_minimum_version_req` is set to `warn` in `[lints]`
825836
[UPDATING] `dummy-registry` index
826837
[LOCKING] 1 package to latest compatible version
827838
[DOWNLOADING] crates ...
@@ -868,6 +879,17 @@ edition = "2021"
868879
p.cargo("check -Zcargo-lints")
869880
.masquerade_as_nightly_cargo(&["cargo-lints"])
870881
.with_stderr_data(str![[r#"
882+
[WARNING] dependency version requirement without an explicit minimum version
883+
--> Cargo.toml:7:7
884+
|
885+
7 | dep = "1"
886+
| ^^^ missing minor and patch components
887+
|
888+
[HELP] consider specifying a full `major.minor.patch` version requirement
889+
|
890+
7 | dep = "1.0.0"
891+
| ++++
892+
= [NOTE] `cargo::implicit_minimum_version_req` is set to `warn` in `[lints]`
871893
[CHECKING] member v0.0.0 ([ROOT]/foo/member)
872894
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
873895
@@ -915,6 +937,17 @@ workspace = true
915937
p.cargo("check -Zcargo-lints")
916938
.masquerade_as_nightly_cargo(&["cargo-lints"])
917939
.with_stderr_data(str![[r#"
940+
[WARNING] dependency version requirement without an explicit minimum version
941+
--> Cargo.toml:7:7
942+
|
943+
7 | dep = "1"
944+
| ^^^ missing minor and patch components
945+
|
946+
[HELP] consider specifying a full `major.minor.patch` version requirement
947+
|
948+
7 | dep = "1.0.0"
949+
| ++++
950+
= [NOTE] `cargo::implicit_minimum_version_req` is set to `warn` in `[lints]`
918951
[WARNING] dependency version requirement without an explicit minimum version
919952
--> member/Cargo.toml:7:7
920953
|

0 commit comments

Comments
 (0)