Skip to content

Commit 983e027

Browse files
authored
refactor(lint): move lints to separate modules (#16364)
### What does this PR try to resolve? Instead of putting everything in a single module, this moves stuff to their own file for better logic isolation. ### How to test and review this PR?
2 parents 334b7b6 + 4e39141 commit 983e027

File tree

4 files changed

+370
-323
lines changed

4 files changed

+370
-323
lines changed
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
use std::path::Path;
2+
3+
use annotate_snippets::AnnotationKind;
4+
use annotate_snippets::Level;
5+
use annotate_snippets::Patch;
6+
use annotate_snippets::Snippet;
7+
use cargo_util_schemas::manifest::ProfilePackageSpec;
8+
use cargo_util_schemas::manifest::TomlToolLints;
9+
10+
use crate::CargoResult;
11+
use crate::GlobalContext;
12+
use crate::core::MaybePackage;
13+
use crate::util::lints::Lint;
14+
use crate::util::lints::LintLevel;
15+
use crate::util::lints::get_key_value_span;
16+
use crate::util::lints::rel_cwd_manifest_path;
17+
18+
pub const LINT: Lint = Lint {
19+
name: "blanket_hint_mostly_unused",
20+
desc: "blanket_hint_mostly_unused lint",
21+
groups: &[],
22+
default_level: LintLevel::Warn,
23+
edition_lint_opts: None,
24+
feature_gate: None,
25+
docs: Some(
26+
r#"
27+
### What it does
28+
Checks if `hint-mostly-unused` being applied to all dependencies.
29+
30+
### Why it is bad
31+
`hint-mostly-unused` indicates that most of a crate's API surface will go
32+
unused by anything depending on it; this hint can speed up the build by
33+
attempting to minimize compilation time for items that aren't used at all.
34+
Misapplication to crates that don't fit that criteria will slow down the build
35+
rather than speeding it up. It should be selectively applied to dependencies
36+
that meet these criteria. Applying it globally is always a misapplication and
37+
will likely slow down the build.
38+
39+
### Example
40+
```toml
41+
[profile.dev.package."*"]
42+
hint-mostly-unused = true
43+
```
44+
45+
Should instead be:
46+
```toml
47+
[profile.dev.package.huge-mostly-unused-dependency]
48+
hint-mostly-unused = true
49+
```
50+
"#,
51+
),
52+
};
53+
54+
pub fn blanket_hint_mostly_unused(
55+
maybe_pkg: &MaybePackage,
56+
path: &Path,
57+
pkg_lints: &TomlToolLints,
58+
error_count: &mut usize,
59+
gctx: &GlobalContext,
60+
) -> CargoResult<()> {
61+
let (lint_level, reason) = LINT.level(
62+
pkg_lints,
63+
maybe_pkg.edition(),
64+
maybe_pkg.unstable_features(),
65+
);
66+
67+
if lint_level == LintLevel::Allow {
68+
return Ok(());
69+
}
70+
71+
let level = lint_level.to_diagnostic_level();
72+
let manifest_path = rel_cwd_manifest_path(path, gctx);
73+
let mut paths = Vec::new();
74+
75+
if let Some(profiles) = maybe_pkg.profiles() {
76+
for (profile_name, top_level_profile) in &profiles.0 {
77+
if let Some(true) = top_level_profile.hint_mostly_unused {
78+
paths.push((
79+
vec!["profile", profile_name.as_str(), "hint-mostly-unused"],
80+
true,
81+
));
82+
}
83+
84+
if let Some(build_override) = &top_level_profile.build_override
85+
&& let Some(true) = build_override.hint_mostly_unused
86+
{
87+
paths.push((
88+
vec![
89+
"profile",
90+
profile_name.as_str(),
91+
"build-override",
92+
"hint-mostly-unused",
93+
],
94+
false,
95+
));
96+
}
97+
98+
if let Some(packages) = &top_level_profile.package
99+
&& let Some(profile) = packages.get(&ProfilePackageSpec::All)
100+
&& let Some(true) = profile.hint_mostly_unused
101+
{
102+
paths.push((
103+
vec![
104+
"profile",
105+
profile_name.as_str(),
106+
"package",
107+
"*",
108+
"hint-mostly-unused",
109+
],
110+
false,
111+
));
112+
}
113+
}
114+
}
115+
116+
for (i, (path, show_per_pkg_suggestion)) in paths.iter().enumerate() {
117+
if lint_level.is_error() {
118+
*error_count += 1;
119+
}
120+
let title = "`hint-mostly-unused` is being blanket applied to all dependencies";
121+
let help_txt =
122+
"scope `hint-mostly-unused` to specific packages with a lot of unused object code";
123+
if let (Some(span), Some(table_span)) = (
124+
get_key_value_span(maybe_pkg.document(), &path),
125+
get_key_value_span(maybe_pkg.document(), &path[..path.len() - 1]),
126+
) {
127+
let mut report = Vec::new();
128+
let mut primary_group = level.clone().primary_title(title).element(
129+
Snippet::source(maybe_pkg.contents())
130+
.path(&manifest_path)
131+
.annotation(
132+
AnnotationKind::Primary.span(table_span.key.start..table_span.key.end),
133+
)
134+
.annotation(AnnotationKind::Context.span(span.key.start..span.value.end)),
135+
);
136+
137+
if *show_per_pkg_suggestion {
138+
report.push(
139+
Level::HELP.secondary_title(help_txt).element(
140+
Snippet::source(maybe_pkg.contents())
141+
.path(&manifest_path)
142+
.patch(Patch::new(
143+
table_span.key.end..table_span.key.end,
144+
".package.<pkg_name>",
145+
)),
146+
),
147+
);
148+
} else {
149+
primary_group = primary_group.element(Level::HELP.message(help_txt));
150+
}
151+
152+
if i == 0 {
153+
primary_group = primary_group
154+
.element(Level::NOTE.message(LINT.emitted_source(lint_level, reason)));
155+
}
156+
157+
// The primary group should always be first
158+
report.insert(0, primary_group);
159+
160+
gctx.shell().print_report(&report, lint_level.force())?;
161+
}
162+
}
163+
164+
Ok(())
165+
}
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
use std::path::Path;
2+
3+
use annotate_snippets::AnnotationKind;
4+
use annotate_snippets::Group;
5+
use annotate_snippets::Level;
6+
use annotate_snippets::Snippet;
7+
use cargo_util_schemas::manifest::TomlToolLints;
8+
9+
use crate::CargoResult;
10+
use crate::GlobalContext;
11+
use crate::core::Feature;
12+
use crate::core::Package;
13+
use crate::util::lints::Lint;
14+
use crate::util::lints::LintLevel;
15+
use crate::util::lints::TEST_DUMMY_UNSTABLE;
16+
use crate::util::lints::get_key_value_span;
17+
use crate::util::lints::rel_cwd_manifest_path;
18+
19+
/// This lint is only to be used for testing purposes
20+
pub const LINT: Lint = Lint {
21+
name: "im_a_teapot",
22+
desc: "`im_a_teapot` is specified",
23+
groups: &[TEST_DUMMY_UNSTABLE],
24+
default_level: LintLevel::Allow,
25+
edition_lint_opts: None,
26+
feature_gate: Some(Feature::test_dummy_unstable()),
27+
docs: None,
28+
};
29+
30+
pub fn check_im_a_teapot(
31+
pkg: &Package,
32+
path: &Path,
33+
pkg_lints: &TomlToolLints,
34+
error_count: &mut usize,
35+
gctx: &GlobalContext,
36+
) -> CargoResult<()> {
37+
let manifest = pkg.manifest();
38+
let (lint_level, reason) =
39+
LINT.level(pkg_lints, manifest.edition(), manifest.unstable_features());
40+
41+
if lint_level == LintLevel::Allow {
42+
return Ok(());
43+
}
44+
45+
if manifest
46+
.normalized_toml()
47+
.package()
48+
.is_some_and(|p| p.im_a_teapot.is_some())
49+
{
50+
if lint_level.is_error() {
51+
*error_count += 1;
52+
}
53+
let level = lint_level.to_diagnostic_level();
54+
let manifest_path = rel_cwd_manifest_path(path, gctx);
55+
let emitted_reason = LINT.emitted_source(lint_level, reason);
56+
57+
let span = get_key_value_span(manifest.document(), &["package", "im-a-teapot"]).unwrap();
58+
59+
let report = &[Group::with_title(level.primary_title(LINT.desc))
60+
.element(
61+
Snippet::source(manifest.contents())
62+
.path(&manifest_path)
63+
.annotation(AnnotationKind::Primary.span(span.key.start..span.value.end)),
64+
)
65+
.element(Level::NOTE.message(&emitted_reason))];
66+
67+
gctx.shell().print_report(report, lint_level.force())?;
68+
}
69+
Ok(())
70+
}

0 commit comments

Comments
 (0)