Skip to content

Commit 3a1de15

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

File tree

5 files changed

+357
-23
lines changed

5 files changed

+357
-23
lines changed

src/cargo/core/workspace.rs

Lines changed: 96 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ 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::{analyze_cargo_lints_table, check_im_a_teapot, global_mostly_unused};
2929
use crate::util::toml::{InheritableFields, read_manifest};
3030
use crate::util::{
3131
Filesystem, GlobalContext, IntoUrl, context::CargoResolverConfig, context::ConfigRelativePath,
@@ -409,10 +409,7 @@ impl<'gctx> Workspace<'gctx> {
409409
}
410410

411411
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-
}
412+
self.root_maybe().profiles()
416413
}
417414

418415
/// Returns the root path of this workspace.
@@ -907,10 +904,7 @@ impl<'gctx> Workspace<'gctx> {
907904

908905
/// Returns the unstable nightly-only features enabled via `cargo-features` in the manifest.
909906
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-
}
907+
self.root_maybe().unstable_features()
914908
}
915909

916910
pub fn resolve_behavior(&self) -> ResolveBehavior {
@@ -1206,10 +1200,20 @@ impl<'gctx> Workspace<'gctx> {
12061200

12071201
pub fn emit_warnings(&self) -> CargoResult<()> {
12081202
let mut first_emitted_error = None;
1203+
1204+
let cli_unstable = self.gctx.cli_unstable();
1205+
if cli_unstable.cargo_lints || cli_unstable.profile_hint_mostly_unused {
1206+
if let Err(e) = self.emit_ws_lints()
1207+
&& first_emitted_error.is_none()
1208+
{
1209+
first_emitted_error = Some(e);
1210+
}
1211+
}
1212+
12091213
for (path, maybe_pkg) in &self.packages.packages {
12101214
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)
1215+
if cli_unstable.cargo_lints {
1216+
if let Err(e) = self.emit_pkg_lints(pkg, &path)
12131217
&& first_emitted_error.is_none()
12141218
{
12151219
first_emitted_error = Some(e);
@@ -1248,7 +1252,7 @@ impl<'gctx> Workspace<'gctx> {
12481252
}
12491253
}
12501254

1251-
pub fn emit_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
1255+
pub fn emit_pkg_lints(&self, pkg: &Package, path: &Path) -> CargoResult<()> {
12521256
let mut error_count = 0;
12531257
let toml_lints = pkg
12541258
.manifest()
@@ -1262,15 +1266,9 @@ impl<'gctx> Workspace<'gctx> {
12621266
.cloned()
12631267
.unwrap_or(manifest::TomlToolLints::default());
12641268

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

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

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

18931969
impl WorkspaceRootConfig {

src/cargo/util/lints.rs

Lines changed: 111 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] = &[GLOBAL_HINT_MOSTLY_UNUSED, IM_A_TEAPOT, UNKNOWN_LINTS];
1212

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

476+
const GLOBAL_HINT_MOSTLY_UNUSED: Lint = Lint {
477+
name: "global_hint_mostly_unused",
478+
desc: "global_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 global_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) = GLOBAL_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 applied globally";
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 = group.element(
572+
Level::NOTE
573+
.message(GLOBAL_HINT_MOSTLY_UNUSED.emitted_source(lint_level, reason)),
574+
);
575+
}
576+
577+
gctx.shell().print_report(&[group], lint_level.force())?;
578+
}
579+
}
580+
581+
Ok(())
582+
}
583+
476584
const UNKNOWN_LINTS: Lint = Lint {
477585
name: "unknown_lints",
478586
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+
- [`global_hint_mostly_unused`](#global_hint_mostly_unused)
89
- [`unknown_lints`](#unknown_lints)
910

11+
## `global_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)