Skip to content

Commit 1fa6f3c

Browse files
committed
feat: Add lint for global use of hint-mostly-unused
1 parent 706cae0 commit 1fa6f3c

File tree

5 files changed

+360
-23
lines changed

5 files changed

+360
-23
lines changed

src/cargo/core/workspace.rs

Lines changed: 98 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ use crate::util::context::FeatureUnification;
2525
use crate::util::edit_distance;
2626
use crate::util::errors::{CargoResult, ManifestError};
2727
use crate::util::interning::InternedString;
28-
use crate::util::lints::{analyze_cargo_lints_table, check_im_a_teapot};
28+
use crate::util::lints::{
29+
analyze_cargo_lints_table, blanket_hint_mostly_unused, check_im_a_teapot,
30+
};
2931
use crate::util::toml::{InheritableFields, read_manifest};
3032
use crate::util::{
3133
Filesystem, GlobalContext, IntoUrl, context::CargoResolverConfig, context::ConfigRelativePath,
@@ -409,10 +411,7 @@ impl<'gctx> Workspace<'gctx> {
409411
}
410412

411413
pub fn profiles(&self) -> Option<&TomlProfiles> {
412-
match self.root_maybe() {
413-
MaybePackage::Package(p) => p.manifest().profiles(),
414-
MaybePackage::Virtual(vm) => vm.profiles(),
415-
}
414+
self.root_maybe().profiles()
416415
}
417416

418417
/// Returns the root path of this workspace.
@@ -907,10 +906,7 @@ impl<'gctx> Workspace<'gctx> {
907906

908907
/// Returns the unstable nightly-only features enabled via `cargo-features` in the manifest.
909908
pub fn unstable_features(&self) -> &Features {
910-
match self.root_maybe() {
911-
MaybePackage::Package(p) => p.manifest().unstable_features(),
912-
MaybePackage::Virtual(vm) => vm.unstable_features(),
913-
}
909+
self.root_maybe().unstable_features()
914910
}
915911

916912
pub fn resolve_behavior(&self) -> ResolveBehavior {
@@ -1206,10 +1202,20 @@ impl<'gctx> Workspace<'gctx> {
12061202

12071203
pub fn emit_warnings(&self) -> CargoResult<()> {
12081204
let mut first_emitted_error = None;
1205+
1206+
let cli_unstable = self.gctx.cli_unstable();
1207+
if cli_unstable.cargo_lints || cli_unstable.profile_hint_mostly_unused {
1208+
if let Err(e) = self.emit_ws_lints()
1209+
&& first_emitted_error.is_none()
1210+
{
1211+
first_emitted_error = Some(e);
1212+
}
1213+
}
1214+
12091215
for (path, maybe_pkg) in &self.packages.packages {
12101216
if let MaybePackage::Package(pkg) = maybe_pkg {
1211-
if self.gctx.cli_unstable().cargo_lints {
1212-
if let Err(e) = self.emit_lints(pkg, &path)
1217+
if cli_unstable.cargo_lints {
1218+
if let Err(e) = self.emit_pkg_lints(pkg, &path)
12131219
&& first_emitted_error.is_none()
12141220
{
12151221
first_emitted_error = Some(e);
@@ -1248,7 +1254,7 @@ impl<'gctx> Workspace<'gctx> {
12481254
}
12491255
}
12501256

1251-
pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
1257+
pub fn emit_pkg_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
12521258
let mut error_count = 0;
12531259
let toml_lints = pkg
12541260
.manifest()
@@ -1262,15 +1268,9 @@ impl<'gctx> Workspace<'gctx> {
12621268
.cloned()
12631269
.unwrap_or(manifest::TomlToolLints::default());
12641270

1265-
let ws_contents = match self.root_maybe() {
1266-
MaybePackage::Package(pkg) => pkg.manifest().contents(),
1267-
MaybePackage::Virtual(v) => v.contents(),
1268-
};
1271+
let ws_contents = self.root_maybe().contents();
12691272

1270-
let ws_document = match self.root_maybe() {
1271-
MaybePackage::Package(pkg) => pkg.manifest().document(),
1272-
MaybePackage::Virtual(v) => v.document(),
1273-
};
1273+
let ws_document = self.root_maybe().document();
12741274

12751275
analyze_cargo_lints_table(
12761276
pkg,
@@ -1282,6 +1282,49 @@ impl<'gctx> Workspace<'gctx> {
12821282
self.gctx,
12831283
)?;
12841284
check_im_a_teapot(pkg, &path, &cargo_lints, &mut error_count, self.gctx)?;
1285+
1286+
if error_count > 0 {
1287+
Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
1288+
"encountered {error_count} errors(s) while running lints"
1289+
))
1290+
.into())
1291+
} else {
1292+
Ok(())
1293+
}
1294+
}
1295+
1296+
pub fn emit_ws_lints(&self) -> CargoResult<()> {
1297+
let mut error_count = 0;
1298+
1299+
let cargo_lints = match self.root_maybe() {
1300+
MaybePackage::Package(pkg) => {
1301+
let toml = pkg.manifest().normalized_toml();
1302+
if let Some(ws) = &toml.workspace {
1303+
ws.lints.as_ref()
1304+
} else {
1305+
toml.lints.as_ref().map(|l| &l.lints)
1306+
}
1307+
}
1308+
MaybePackage::Virtual(vm) => vm
1309+
.normalized_toml()
1310+
.workspace
1311+
.as_ref()
1312+
.unwrap()
1313+
.lints
1314+
.as_ref(),
1315+
}
1316+
.and_then(|t| t.get("cargo"))
1317+
.cloned()
1318+
.unwrap_or(manifest::TomlToolLints::default());
1319+
1320+
blanket_hint_mostly_unused(
1321+
self.root_maybe(),
1322+
self.root_manifest(),
1323+
&cargo_lints,
1324+
&mut error_count,
1325+
self.gctx,
1326+
)?;
1327+
12851328
if error_count > 0 {
12861329
Err(crate::util::errors::AlreadyPrintedError::new(anyhow!(
12871330
"encountered {error_count} errors(s) while running lints"
@@ -1888,6 +1931,41 @@ impl MaybePackage {
18881931
MaybePackage::Virtual(_) => false,
18891932
}
18901933
}
1934+
1935+
pub fn contents(&self) -> &str {
1936+
match self {
1937+
MaybePackage::Package(p) => p.manifest().contents(),
1938+
MaybePackage::Virtual(v) => v.contents(),
1939+
}
1940+
}
1941+
1942+
pub fn document(&self) -> &toml::Spanned<toml::de::DeTable<'static>> {
1943+
match self {
1944+
MaybePackage::Package(p) => p.manifest().document(),
1945+
MaybePackage::Virtual(v) => v.document(),
1946+
}
1947+
}
1948+
1949+
pub fn edition(&self) -> Edition {
1950+
match self {
1951+
MaybePackage::Package(p) => p.manifest().edition(),
1952+
MaybePackage::Virtual(_) => Edition::default(),
1953+
}
1954+
}
1955+
1956+
pub fn profiles(&self) -> Option<&TomlProfiles> {
1957+
match self {
1958+
MaybePackage::Package(p) => p.manifest().profiles(),
1959+
MaybePackage::Virtual(v) => v.profiles(),
1960+
}
1961+
}
1962+
1963+
pub fn unstable_features(&self) -> &Features {
1964+
match self {
1965+
MaybePackage::Package(p) => p.manifest().unstable_features(),
1966+
MaybePackage::Virtual(vm) => vm.unstable_features(),
1967+
}
1968+
}
18911969
}
18921970

18931971
impl WorkspaceRootConfig {

src/cargo/util/lints.rs

Lines changed: 112 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
1-
use crate::core::{Edition, Feature, Features, Manifest, Package};
1+
use crate::core::{Edition, Feature, Features, Manifest, MaybePackage, Package};
22
use crate::{CargoResult, GlobalContext};
33
use annotate_snippets::{AnnotationKind, Group, Level, Snippet};
4-
use cargo_util_schemas::manifest::{TomlLintLevel, TomlToolLints};
4+
use cargo_util_schemas::manifest::{ProfilePackageSpec, TomlLintLevel, TomlToolLints};
55
use pathdiff::diff_paths;
66
use std::fmt::Display;
77
use std::ops::Range;
88
use std::path::Path;
99

1010
const LINT_GROUPS: &[LintGroup] = &[TEST_DUMMY_UNSTABLE];
11-
pub const LINTS: &[Lint] = &[IM_A_TEAPOT, UNKNOWN_LINTS];
11+
pub const LINTS: &[Lint] = &[BLANKET_HINT_MOSTLY_UNUSED, IM_A_TEAPOT, UNKNOWN_LINTS];
1212

1313
pub fn analyze_cargo_lints_table(
1414
pkg: &Package,
@@ -473,6 +473,115 @@ pub fn check_im_a_teapot(
473473
Ok(())
474474
}
475475

476+
const BLANKET_HINT_MOSTLY_UNUSED: Lint = Lint {
477+
name: "blanket_hint_mostly_unused",
478+
desc: "blanket_hint_mostly_unused lint",
479+
groups: &[],
480+
default_level: LintLevel::Warn,
481+
edition_lint_opts: None,
482+
feature_gate: None,
483+
docs: Some(
484+
r#"
485+
### What it does
486+
Checks if `hint-mostly-unused` being applied to all dependencies.
487+
488+
### Why it is bad
489+
`hint-mostly-unused` indicates that most of a crate's API surface will go
490+
unused by anything depending on it; this hint can speed up the build by
491+
attempting to minimize compilation time for items that aren't used at all.
492+
Misapplication to crates that don't fit that criteria will slow down the build
493+
rather than speeding it up. It should be selectively applied to dependencies
494+
that meet these criteria. Applying it globally is always a misapplication and
495+
will likely slow down the build.
496+
497+
### Example
498+
```toml
499+
[profile.dev.package."*"]
500+
hint-mostly-unused = true
501+
```
502+
503+
Should instead be:
504+
```toml
505+
[profile.dev.package.huge-mostly-unused-dependency]
506+
hint-mostly-unused = true
507+
```
508+
"#,
509+
),
510+
};
511+
512+
pub fn blanket_hint_mostly_unused(
513+
maybe_pkg: &MaybePackage,
514+
path: &Path,
515+
pkg_lints: &TomlToolLints,
516+
error_count: &mut usize,
517+
gctx: &GlobalContext,
518+
) -> CargoResult<()> {
519+
let (lint_level, reason) = BLANKET_HINT_MOSTLY_UNUSED.level(
520+
pkg_lints,
521+
maybe_pkg.edition(),
522+
maybe_pkg.unstable_features(),
523+
);
524+
525+
if lint_level == LintLevel::Allow {
526+
return Ok(());
527+
}
528+
529+
let level = lint_level.to_diagnostic_level();
530+
let manifest_path = rel_cwd_manifest_path(path, gctx);
531+
let mut paths = Vec::new();
532+
533+
if let Some(profiles) = maybe_pkg.profiles() {
534+
for (profile_name, top_level_profile) in &profiles.0 {
535+
if let Some(true) = top_level_profile.hint_mostly_unused {
536+
paths.push(vec!["profile", profile_name.as_str(), "hint-mostly-unused"]);
537+
}
538+
539+
if let Some(packages) = &top_level_profile.package
540+
&& let Some(profile) = packages.get(&ProfilePackageSpec::All)
541+
&& let Some(true) = profile.hint_mostly_unused
542+
{
543+
paths.push(vec![
544+
"profile",
545+
profile_name.as_str(),
546+
"package",
547+
"*",
548+
"hint-mostly-unused",
549+
]);
550+
}
551+
}
552+
}
553+
554+
for (i, path) in paths.iter().enumerate() {
555+
if lint_level.is_error() {
556+
*error_count += 1;
557+
}
558+
let title = "`hint-mostly-unused` should not be blanket applied";
559+
if let (Some(span), Some(table_span)) = (
560+
get_key_value_span(maybe_pkg.document(), &path),
561+
get_key_value_span(maybe_pkg.document(), &path[..path.len() - 1]),
562+
) {
563+
let mut group = level.clone().primary_title(title).element(
564+
Snippet::source(maybe_pkg.contents())
565+
.path(&manifest_path)
566+
.annotation(AnnotationKind::Primary.span(span.key.start..span.value.end))
567+
.annotation(AnnotationKind::Visible.span(table_span.key)),
568+
);
569+
570+
if i == 0 {
571+
group =
572+
group
573+
.element(Level::NOTE.message(
574+
BLANKET_HINT_MOSTLY_UNUSED.emitted_source(lint_level, reason),
575+
));
576+
}
577+
578+
gctx.shell().print_report(&[group], lint_level.force())?;
579+
}
580+
}
581+
582+
Ok(())
583+
}
584+
476585
const UNKNOWN_LINTS: Lint = Lint {
477586
name: "unknown_lints",
478587
desc: "unknown lint",

src/doc/src/reference/lints.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,37 @@ Note: [Cargo's linting system is unstable](unstable.md#lintscargo) and can only
55
## Warn-by-default
66

77
These lints are all set to the 'warn' level by default.
8+
- [`blanket_hint_mostly_unused`](#blanket_hint_mostly_unused)
89
- [`unknown_lints`](#unknown_lints)
910

11+
## `blanket_hint_mostly_unused`
12+
Set to `warn` by default
13+
14+
### What it does
15+
Checks if `hint-mostly-unused` being applied to all dependencies.
16+
17+
### Why it is bad
18+
`hint-mostly-unused` indicates that most of a crate's API surface will go
19+
unused by anything depending on it; this hint can speed up the build by
20+
attempting to minimize compilation time for items that aren't used at all.
21+
Misapplication to crates that don't fit that criteria will slow down the build
22+
rather than speeding it up. It should be selectively applied to dependencies
23+
that meet these criteria. Applying it globally is always a misapplication and
24+
will likely slow down the build.
25+
26+
### Example
27+
```toml
28+
[profile.dev.package."*"]
29+
hint-mostly-unused = true
30+
```
31+
32+
Should instead be:
33+
```toml
34+
[profile.dev.package.huge-mostly-unused-dependency]
35+
hint-mostly-unused = true
36+
```
37+
38+
1039
## `unknown_lints`
1140
Set to `warn` by default
1241

0 commit comments

Comments
 (0)