Skip to content

Commit 998524f

Browse files
committed
feat: Add a basic linting system
1 parent 8bcecfe commit 998524f

File tree

8 files changed

+474
-6
lines changed

8 files changed

+474
-6
lines changed

Diff for: Cargo.lock

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ tempfile = "3.10.1"
9797
thiserror = "1.0.57"
9898
time = { version = "0.3", features = ["parsing", "formatting", "serde"] }
9999
toml = "0.8.10"
100-
toml_edit = { version = "0.22.7", features = ["serde"] }
100+
toml_edit = { version = "0.22.9", features = ["serde"] }
101101
tracing = "0.1.40" # be compatible with rustc_log: https://github.com/rust-lang/rust/blob/e51e98dde6a/compiler/rustc_log/Cargo.toml#L9
102102
tracing-chrome = "0.7.1"
103103
tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }

Diff for: src/cargo/core/workspace.rs

+30-1
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,12 @@ use crate::sources::{PathSource, CRATES_IO_INDEX, CRATES_IO_REGISTRY};
2424
use crate::util::edit_distance;
2525
use crate::util::errors::{CargoResult, ManifestError};
2626
use crate::util::interning::InternedString;
27+
use crate::util::lints::check_implicit_features;
2728
use crate::util::toml::{read_manifest, InheritableFields};
2829
use crate::util::{context::ConfigRelativePath, Filesystem, GlobalContext, IntoUrl};
2930
use cargo_util::paths;
3031
use cargo_util::paths::normalize_path;
32+
use cargo_util_schemas::manifest;
3133
use cargo_util_schemas::manifest::RustVersion;
3234
use cargo_util_schemas::manifest::{TomlDependency, TomlProfiles};
3335
use pathdiff::diff_paths;
@@ -1095,11 +1097,14 @@ impl<'gctx> Workspace<'gctx> {
10951097

10961098
pub fn emit_warnings(&self) -> CargoResult<()> {
10971099
for (path, maybe_pkg) in &self.packages.packages {
1100+
let path = path.join("Cargo.toml");
1101+
if let MaybePackage::Package(pkg) = maybe_pkg {
1102+
self.emit_lints(pkg, &path)?
1103+
}
10981104
let warnings = match maybe_pkg {
10991105
MaybePackage::Package(pkg) => pkg.manifest().warnings().warnings(),
11001106
MaybePackage::Virtual(vm) => vm.warnings().warnings(),
11011107
};
1102-
let path = path.join("Cargo.toml");
11031108
for warning in warnings {
11041109
if warning.is_critical {
11051110
let err = anyhow::format_err!("{}", warning.message);
@@ -1121,6 +1126,30 @@ impl<'gctx> Workspace<'gctx> {
11211126
Ok(())
11221127
}
11231128

1129+
pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
1130+
let mut error_count = 0;
1131+
let lints = pkg
1132+
.manifest()
1133+
.resolved_toml()
1134+
.lints
1135+
.clone()
1136+
.map(|lints| lints.lints)
1137+
.unwrap_or(manifest::TomlLints::default())
1138+
.get("cargo")
1139+
.cloned()
1140+
.unwrap_or(manifest::TomlToolLints::default());
1141+
1142+
check_implicit_features(pkg, &path, &lints, &mut error_count, self.gctx)?;
1143+
if error_count > 0 {
1144+
Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
1145+
"encountered {error_count} errors(s) while running lints"
1146+
))
1147+
.into())
1148+
} else {
1149+
Ok(())
1150+
}
1151+
}
1152+
11241153
pub fn set_target_dir(&mut self, target_dir: Filesystem) {
11251154
self.target_dir = Some(target_dir);
11261155
}

Diff for: src/cargo/util/lints.rs

+241
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
use crate::core::FeatureValue::Dep;
2+
use crate::core::{Edition, FeatureMap, FeatureValue, Package};
3+
use crate::util::interning::InternedString;
4+
use crate::{CargoResult, GlobalContext};
5+
use annotate_snippets::{Level, Renderer, Snippet};
6+
use cargo_util_schemas::manifest::{TomlLintLevel, TomlToolLints};
7+
use pathdiff::diff_paths;
8+
use std::collections::{BTreeMap, HashSet};
9+
use std::ops::Range;
10+
use std::path::Path;
11+
use toml_edit::ImDocument;
12+
13+
pub fn get_span(
14+
document: &ImDocument<String>,
15+
path: &[&str],
16+
get_value: bool,
17+
) -> Option<Range<usize>> {
18+
let mut table = document.as_item().as_table_like().unwrap();
19+
let mut iter = path.into_iter().peekable();
20+
while let Some(key) = iter.next() {
21+
let (key, item) = table.get_key_value(key).unwrap();
22+
if iter.peek().is_none() {
23+
return if get_value {
24+
item.span()
25+
} else {
26+
let leaf_decor = key.dotted_decor();
27+
let leaf_prefix_span = leaf_decor.prefix().and_then(|p| p.span());
28+
let leaf_suffix_span = leaf_decor.suffix().and_then(|s| s.span());
29+
if let (Some(leaf_prefix_span), Some(leaf_suffix_span)) =
30+
(leaf_prefix_span, leaf_suffix_span)
31+
{
32+
Some(leaf_prefix_span.start..leaf_suffix_span.end)
33+
} else {
34+
key.span()
35+
}
36+
};
37+
}
38+
if item.is_table_like() {
39+
table = item.as_table_like().unwrap();
40+
}
41+
if item.is_array() && iter.peek().is_some() {
42+
let array = item.as_array().unwrap();
43+
let next = iter.next().unwrap();
44+
return array.iter().find_map(|item| {
45+
if next == &item.to_string() {
46+
item.span()
47+
} else {
48+
None
49+
}
50+
});
51+
}
52+
}
53+
None
54+
}
55+
56+
fn manifest_path(path: &Path, gctx: &GlobalContext) -> String {
57+
diff_paths(path, gctx.cwd())
58+
.unwrap_or_else(|| path.to_path_buf())
59+
.display()
60+
.to_string()
61+
}
62+
63+
#[derive(Copy, Clone, Debug)]
64+
pub struct LintGroup {
65+
pub name: &'static str,
66+
pub default_level: LintLevel,
67+
pub desc: &'static str,
68+
pub edition_lint_opts: Option<(Edition, LintLevel)>,
69+
}
70+
71+
const RUST_2024_COMPATIBILITY: LintGroup = LintGroup {
72+
name: "rust-2024-compatibility",
73+
default_level: LintLevel::Allow,
74+
desc: "warn about compatibility with Rust 2024",
75+
edition_lint_opts: Some((Edition::Edition2024, LintLevel::Deny)),
76+
};
77+
78+
#[derive(Copy, Clone, Debug)]
79+
pub struct Lint {
80+
pub name: &'static str,
81+
pub desc: &'static str,
82+
pub groups: &'static [LintGroup],
83+
pub default_level: LintLevel,
84+
pub edition_lint_opts: Option<(Edition, LintLevel)>,
85+
}
86+
87+
impl Lint {
88+
pub fn level(&self, lints: &TomlToolLints, edition: Edition) -> LintLevel {
89+
let level = self
90+
.groups
91+
.iter()
92+
.map(|g| g.name)
93+
.chain(std::iter::once(self.name))
94+
.filter_map(|n| lints.get(n).map(|l| (n, l)))
95+
.max_by_key(|(n, l)| (l.priority(), std::cmp::Reverse(*n)));
96+
97+
match level {
98+
Some((_, toml_lint)) => toml_lint.level().into(),
99+
None => {
100+
if let Some((lint_edition, lint_level)) = self.edition_lint_opts {
101+
if edition >= lint_edition {
102+
lint_level
103+
} else {
104+
IMPLICIT_FEATURES.default_level
105+
}
106+
} else {
107+
IMPLICIT_FEATURES.default_level
108+
}
109+
}
110+
}
111+
}
112+
}
113+
114+
#[derive(Copy, Clone, Debug, PartialEq)]
115+
pub enum LintLevel {
116+
Allow,
117+
Warn,
118+
Deny,
119+
Forbid,
120+
}
121+
122+
impl LintLevel {
123+
pub fn to_diagnostic_level(self) -> Level {
124+
match self {
125+
LintLevel::Allow => Level::Note,
126+
LintLevel::Warn => Level::Warning,
127+
LintLevel::Deny => Level::Error,
128+
LintLevel::Forbid => Level::Error,
129+
}
130+
}
131+
}
132+
133+
impl Into<LintLevel> for TomlLintLevel {
134+
fn into(self) -> LintLevel {
135+
match self {
136+
TomlLintLevel::Allow => LintLevel::Allow,
137+
TomlLintLevel::Warn => LintLevel::Warn,
138+
TomlLintLevel::Deny => LintLevel::Deny,
139+
TomlLintLevel::Forbid => LintLevel::Forbid,
140+
}
141+
}
142+
}
143+
144+
const IMPLICIT_FEATURES: Lint = Lint {
145+
name: "implicit-features",
146+
desc: "warn about the use of unstable features",
147+
groups: &[RUST_2024_COMPATIBILITY],
148+
default_level: LintLevel::Allow,
149+
edition_lint_opts: Some((Edition::Edition2024, LintLevel::Deny)),
150+
};
151+
152+
pub fn check_implicit_features(
153+
pkg: &Package,
154+
path: &Path,
155+
lints: &TomlToolLints,
156+
error_count: &mut usize,
157+
gctx: &GlobalContext,
158+
) -> CargoResult<()> {
159+
let lint_level = IMPLICIT_FEATURES.level(lints, pkg.manifest().edition());
160+
if lint_level == LintLevel::Allow {
161+
return Ok(());
162+
}
163+
164+
let manifest = pkg.manifest();
165+
let features: BTreeMap<InternedString, Vec<InternedString>> = manifest
166+
.resolved_toml()
167+
.features()
168+
.map_or(BTreeMap::new(), |f| {
169+
f.iter()
170+
.map(|(k, v)| {
171+
(
172+
InternedString::new(k),
173+
v.iter().map(InternedString::from).collect(),
174+
)
175+
})
176+
.collect()
177+
});
178+
let map: FeatureMap = features
179+
.iter()
180+
.map(|(feature, list)| {
181+
let fvs: Vec<_> = list
182+
.iter()
183+
.map(|feat_value| FeatureValue::new(*feat_value))
184+
.collect();
185+
(*feature, fvs)
186+
})
187+
.collect();
188+
189+
// Add implicit features for optional dependencies if they weren't
190+
// explicitly listed anywhere.
191+
let explicitly_listed: HashSet<_> = map
192+
.values()
193+
.flatten()
194+
.filter_map(|fv| match fv {
195+
Dep { dep_name } => Some(*dep_name),
196+
_ => None,
197+
})
198+
.collect();
199+
for dep in manifest.dependencies() {
200+
let dep_name_in_toml = dep.name_in_toml();
201+
if !dep.is_optional()
202+
|| features.contains_key(&dep_name_in_toml)
203+
|| explicitly_listed.contains(&dep_name_in_toml)
204+
{
205+
continue;
206+
}
207+
208+
if features.contains_key(&dep_name_in_toml) || explicitly_listed.contains(&dep_name_in_toml)
209+
{
210+
continue;
211+
}
212+
if lint_level == LintLevel::Forbid || lint_level == LintLevel::Deny {
213+
*error_count += 1;
214+
}
215+
let level = lint_level.to_diagnostic_level();
216+
let manifest_path = manifest_path(path, gctx);
217+
let message = level.title("unused optional dependency").snippet(
218+
Snippet::source(manifest.contents())
219+
.origin(&manifest_path)
220+
.annotation(
221+
level.span(
222+
get_span(
223+
manifest.document(),
224+
&["dependencies", &dep_name_in_toml],
225+
false,
226+
)
227+
.unwrap(),
228+
),
229+
)
230+
.fold(true),
231+
);
232+
let renderer = Renderer::styled().term_width(
233+
gctx.shell()
234+
.err_width()
235+
.diagnostic_terminal_width()
236+
.unwrap_or(annotate_snippets::renderer::DEFAULT_TERM_WIDTH),
237+
);
238+
writeln!(gctx.shell().err(), "{}", renderer.render(message))?;
239+
}
240+
Ok(())
241+
}

Diff for: src/cargo/util/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ pub mod into_url;
5151
mod into_url_with_base;
5252
mod io;
5353
pub mod job;
54+
pub mod lints;
5455
mod lockserver;
5556
pub mod machine_message;
5657
pub mod network;

Diff for: src/cargo/util/toml/mod.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1454,7 +1454,7 @@ fn verify_lints(lints: Option<&manifest::TomlLints>) -> CargoResult<()> {
14541454
};
14551455

14561456
for (tool, lints) in lints {
1457-
let supported = ["rust", "clippy", "rustdoc"];
1457+
let supported = ["rust", "clippy", "rustdoc", "cargo"];
14581458
if !supported.contains(&tool.as_str()) {
14591459
let supported = supported.join(", ");
14601460
anyhow::bail!("unsupported `{tool}` in `[lints]`, must be one of {supported}")
@@ -1482,6 +1482,7 @@ fn verify_lints(lints: Option<&manifest::TomlLints>) -> CargoResult<()> {
14821482
fn lints_to_rustflags(lints: &manifest::TomlLints) -> Vec<String> {
14831483
let mut rustflags = lints
14841484
.iter()
1485+
.filter(|(tool, _)| *tool != "cargo")
14851486
.flat_map(|(tool, lints)| {
14861487
lints.iter().map(move |(name, config)| {
14871488
let flag = match config.level() {

Diff for: tests/testsuite/features.rs

+36
Original file line numberDiff line numberDiff line change
@@ -2246,3 +2246,39 @@ fn default_features_conflicting_warning() {
22462246
)
22472247
.run();
22482248
}
2249+
2250+
#[cargo_test(nightly, reason = "edition2024 is not stable")]
2251+
fn implicit_features_2024() {
2252+
Package::new("bar", "0.1.0").publish();
2253+
let p = project()
2254+
.file(
2255+
"Cargo.toml",
2256+
r#"
2257+
cargo-features = ["edition2024"]
2258+
[package]
2259+
name = "foo"
2260+
version = "0.1.0"
2261+
edition = "2024"
2262+
2263+
[dependencies]
2264+
bar = { version = "0.1.0", optional = true }
2265+
"#,
2266+
)
2267+
.file("src/lib.rs", "")
2268+
.build();
2269+
2270+
p.cargo("check")
2271+
.masquerade_as_nightly_cargo(&["edition2024"])
2272+
.with_status(101)
2273+
.with_stderr(
2274+
"\
2275+
error: unused optional dependency
2276+
--> Cargo.toml:9:17
2277+
|
2278+
9 | bar = { version = \"0.1.0\", optional = true }
2279+
| ^^^
2280+
|
2281+
",
2282+
)
2283+
.run();
2284+
}

0 commit comments

Comments
 (0)