Skip to content

Commit 61855e7

Browse files
committed
Auto merge of #13621 - Muscraft:linting-system, r=weihanglo
feat: Add a basic linting system This PR adds a basic linting system, the first step towards [User control over cargo warnings](#12235). To demonstrate the system, a lint for #12826 is added. It supports controlling cargo lints via the `[lints.cargo]` table. Lints can be controlled either by a group or individually. This is meant to lay down some fundamental infrastructure for the whole linting system and is not meant to be fully featured. Over time, features will be added that will help us reach a much more solid state.
2 parents 1728ac5 + 307c7f8 commit 61855e7

File tree

27 files changed

+970
-42
lines changed

27 files changed

+970
-42
lines changed

Cargo.lock

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

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"] }

src/cargo/core/features.rs

+2
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,7 @@ unstable_cli_options!(
750750
#[serde(deserialize_with = "deserialize_build_std")]
751751
build_std: Option<Vec<String>> = ("Enable Cargo to compile the standard library itself as part of a crate graph compilation"),
752752
build_std_features: Option<Vec<String>> = ("Configure features enabled for the standard library itself when building the standard library"),
753+
cargo_lints: bool = ("Enable the `[lints.cargo]` table"),
753754
check_cfg: bool = ("Enable compile-time checking of `cfg` names/values/features"),
754755
codegen_backend: bool = ("Enable the `codegen-backend` option in profiles in .cargo/config.toml file"),
755756
config_include: bool = ("Enable the `include` key in config files"),
@@ -1117,6 +1118,7 @@ impl CliUnstable {
11171118
self.build_std = Some(crate::core::compiler::standard_lib::parse_unstable_flag(v))
11181119
}
11191120
"build-std-features" => self.build_std_features = Some(parse_features(v)),
1121+
"cargo-lints" => self.cargo_lints = parse_empty(k, v)?,
11201122
"check-cfg" => {
11211123
self.check_cfg = parse_empty(k, v)?;
11221124
}

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
}

src/cargo/util/lints.rs

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

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;

0 commit comments

Comments
 (0)