Skip to content

Commit a335d47

Browse files
weihangloweihanglo
authored
fix(diag): Remove sometimes-invalid removal suggestions (#17139)
### What does this PR try to resolve? Depending on where a removal shows up (like a basic key-value pair), it can be valid. However, it won't be if - the key is for a standard table - the key is used with dotted keys - there is a comma after the value For now, we'll remove this. I'll open an issue for adding these back in. One idea is we re-parse using `toml_parse`, detect our case, and directly edit the token stream to find the removal span. Fixes #16982 ### How to test and review this PR?
2 parents 92c4113 + 0f82885 commit a335d47

12 files changed

Lines changed: 52 additions & 267 deletions

‎src/cargo/diagnostics/rules/redundant_homepage.rs‎

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use cargo_util_terminal::report::AnnotationKind;
55
use cargo_util_terminal::report::Group;
66
use cargo_util_terminal::report::Level;
77
use cargo_util_terminal::report::Origin;
8-
use cargo_util_terminal::report::Patch;
98
use cargo_util_terminal::report::Snippet;
109
use tracing::instrument;
1110

@@ -132,26 +131,9 @@ fn lint_package_inner(
132131
}
133132
primary = primary.element(Level::NOTE.message(emitted_source));
134133
let mut report = vec![primary];
135-
if let Some(document) = document
136-
&& let Some(contents) = contents
137-
&& let Some(span) = get_key_value_span(document, &["package", "homepage"])
138-
{
139-
let span = if let Some(workspace_span) =
140-
get_key_value_span(document, &["package", "homepage", "workspace"])
141-
{
142-
span.key.start..workspace_span.value.end
143-
} else {
144-
span.key.start..span.value.end
145-
};
146-
let mut help =
147-
Group::with_title(Level::HELP.secondary_title("consider removing `package.homepage`"));
148-
help = help.element(
149-
Snippet::source(contents)
150-
.path(manifest_path)
151-
.patch(Patch::new(span, "")),
152-
);
153-
report.push(help);
154-
}
134+
let help =
135+
Group::with_title(Level::HELP.secondary_title("consider removing `package.homepage`"));
136+
report.push(help);
155137

156138
pkg_stats.record_lint(lint_level);
157139
gctx.shell().print_report(&report, lint_level.force())?;

‎src/cargo/diagnostics/rules/redundant_readme.rs‎

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use cargo_util_terminal::report::AnnotationKind;
66
use cargo_util_terminal::report::Group;
77
use cargo_util_terminal::report::Level;
88
use cargo_util_terminal::report::Origin;
9-
use cargo_util_terminal::report::Patch;
109
use cargo_util_terminal::report::Snippet;
1110
use tracing::instrument;
1211

@@ -135,26 +134,8 @@ fn lint_package_inner(
135134
}
136135
primary = primary.element(Level::NOTE.message(emitted_source));
137136
let mut report = vec![primary];
138-
if let Some(document) = document
139-
&& let Some(contents) = contents
140-
&& let Some(span) = get_key_value_span(document, &["package", "readme"])
141-
{
142-
let span = if let Some(workspace_span) =
143-
get_key_value_span(document, &["package", "readme", "workspace"])
144-
{
145-
span.key.start..workspace_span.value.end
146-
} else {
147-
span.key.start..span.value.end
148-
};
149-
let mut help =
150-
Group::with_title(Level::HELP.secondary_title("consider removing `package.readme`"));
151-
help = help.element(
152-
Snippet::source(contents)
153-
.path(manifest_path)
154-
.patch(Patch::new(span, "")),
155-
);
156-
report.push(help);
157-
}
137+
let help = Group::with_title(Level::HELP.secondary_title("consider removing `package.readme`"));
138+
report.push(help);
158139

159140
pkg_stats.record_lint(lint_level);
160141
gctx.shell().print_report(&report, lint_level.force())?;

‎src/cargo/diagnostics/rules/unused_dependencies.rs‎

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use cargo_util_terminal::report::AnnotationKind;
66
use cargo_util_terminal::report::Group;
77
use cargo_util_terminal::report::Level;
88
use cargo_util_terminal::report::Origin;
9-
use cargo_util_terminal::report::Patch;
109
use cargo_util_terminal::report::Snippet;
1110
use indexmap::IndexMap;
1211
use tracing::{debug, instrument, trace};
@@ -149,19 +148,10 @@ pub(crate) fn lint_package(
149148
primary = primary.element(Level::NOTE.message(emitted_source));
150149
}
151150
let mut report = vec![primary];
152-
if let Some(document) = document
153-
&& let Some(contents) = contents
154-
&& let Some(span) = get_key_value_span(document, &["build-dependencies", dep_name])
155-
{
156-
let span = span.key.start..span.value.end;
157-
let mut help = Group::with_title(Level::HELP.secondary_title("remove the dependency"));
158-
help = help.element(
159-
Snippet::source(contents)
160-
.path(&manifest_path)
161-
.patch(Patch::new(span, "")),
162-
);
163-
report.push(help);
164-
}
151+
let help = Group::with_title(
152+
Level::HELP.secondary_title("consider removing the unused dependency"),
153+
);
154+
report.push(help);
165155

166156
pkg_stats.record_lint(lint_level);
167157
gctx.shell().print_report(&report, lint_level.force())?;
@@ -335,20 +325,10 @@ fn lint_package_build_results(
335325
}
336326
lint_count += 1;
337327
let mut report = vec![primary];
338-
if let Some(document) = document
339-
&& let Some(contents) = contents
340-
&& let Some(span) = get_key_value_span(document, &toml_path)
341-
{
342-
let span = span.key.start..span.value.end;
343-
let mut help =
344-
Group::with_title(Level::HELP.secondary_title("remove the dependency"));
345-
help = help.element(
346-
Snippet::source(contents)
347-
.path(&manifest_path)
348-
.patch(Patch::new(span, "")),
349-
);
350-
report.push(help);
351-
}
328+
let help = Group::with_title(
329+
Level::HELP.secondary_title("consider removing the unused dependency"),
330+
);
331+
report.push(help);
352332
if used_in_dev {
353333
let help = Group::with_title(Level::HELP.secondary_title(
354334
"to still use for development builds, move to `dev-dependencies`",

‎src/cargo/diagnostics/rules/unused_workspace_dependencies.rs‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use cargo_util_terminal::report::AnnotationKind;
55
use cargo_util_terminal::report::Group;
66
use cargo_util_terminal::report::Level;
77
use cargo_util_terminal::report::Origin;
8-
use cargo_util_terminal::report::Patch;
98
use cargo_util_terminal::report::Snippet;
109
use indexmap::IndexSet;
1110
use tracing::instrument;
@@ -150,21 +149,10 @@ pub(crate) fn lint_workspace(
150149
primary = primary.element(Level::NOTE.message(emitted_source));
151150
}
152151
let mut report = vec![primary];
153-
if let Some(document) = document
154-
&& let Some(contents) = contents
155-
{
156-
let mut help = Group::with_title(
157-
Level::HELP.secondary_title("consider removing the unused dependency"),
158-
);
159-
let mut snippet = Snippet::source(contents).path(&manifest_path);
160-
if let Some(span) =
161-
get_key_value_span(document, &["workspace", "dependencies", unused.as_str()])
162-
{
163-
snippet = snippet.patch(Patch::new(span.key.start..span.value.end, ""));
164-
}
165-
help = help.element(snippet);
166-
report.push(help);
167-
}
152+
let help = Group::with_title(
153+
Level::HELP.secondary_title("consider removing the unused workspace dependency"),
154+
);
155+
report.push(help);
168156

169157
pkg_stats.record_lint(lint_level);
170158
gctx.shell().print_report(&report, lint_level.force())?;

‎src/cargo/diagnostics/rules/unused_workspace_package_fields.rs‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ use cargo_util_terminal::report::AnnotationKind;
44
use cargo_util_terminal::report::Group;
55
use cargo_util_terminal::report::Level;
66
use cargo_util_terminal::report::Origin;
7-
use cargo_util_terminal::report::Patch;
87
use cargo_util_terminal::report::Snippet;
98
use indexmap::IndexSet;
109
use tracing::instrument;
@@ -119,21 +118,9 @@ pub(crate) fn lint_workspace(
119118
primary = primary.element(Level::NOTE.message(emitted_source));
120119
}
121120
let mut report = vec![primary];
122-
if let Some(document) = document
123-
&& let Some(contents) = contents
124-
{
125-
let mut help = Group::with_title(
126-
Level::HELP.secondary_title("consider removing the unused field"),
127-
);
128-
let mut snippet = Snippet::source(contents).path(&manifest_path);
129-
if let Some(span) =
130-
get_key_value_span(document, &["workspace", "package", unused.as_ref()])
131-
{
132-
snippet = snippet.patch(Patch::new(span.key.start..span.value.end, ""));
133-
}
134-
help = help.element(snippet);
135-
report.push(help);
136-
}
121+
let help =
122+
Group::with_title(Level::HELP.secondary_title("consider removing the unused field"));
123+
report.push(help);
137124

138125
pkg_stats.record_lint(lint_level);
139126
gctx.shell().print_report(&report, lint_level.force())?;

‎tests/testsuite/lints/mod.rs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -484,9 +484,6 @@ fn explicit_lint_level_overrides_default() {
484484
|
485485
= [NOTE] `cargo::redundant_homepage` is set to `deny` in `[lints]`
486486
[HELP] consider removing `package.homepage`
487-
|
488-
8 - homepage = "https://github.com/rust-lang/cargo/"
489-
|
490487
[ERROR] could not parse `foo` (manifest) due to 1 previous error
491488
492489
"#]])

‎tests/testsuite/lints/redundant_homepage.rs‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,6 @@ redundant_homepage = "warn"
3737
|
3838
= [NOTE] `cargo::redundant_homepage` is set to `warn` in `[lints]`
3939
[HELP] consider removing `package.homepage`
40-
|
41-
7 - homepage = "https://github.com/rust-lang/cargo/"
42-
|
4340
[WARNING] `cargo` (manifest) generated 1 warning
4441
4542
"#]])
@@ -81,9 +78,6 @@ redundant_homepage = "warn"
8178
|
8279
= [NOTE] `cargo::redundant_homepage` is set to `warn` in `[lints]`
8380
[HELP] consider removing `package.homepage`
84-
|
85-
7 - homepage = "https://docs.rs/cargo/latest/cargo/"
86-
|
8781
[WARNING] `cargo` (manifest) generated 1 warning
8882
8983
"#]])
@@ -129,9 +123,6 @@ redundant_homepage = "warn"
129123
|
130124
= [NOTE] `cargo::redundant_homepage` is set to `warn` in `[lints]`
131125
[HELP] consider removing `package.homepage`
132-
|
133-
11 - homepage.workspace = true
134-
|
135126
[WARNING] `cargo` (manifest) generated 1 warning
136127
137128
"#]])

‎tests/testsuite/lints/redundant_readme.rs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,6 @@ redundant_readme = "warn"
3535
|
3636
= [NOTE] `cargo::redundant_readme` is set to `warn` in `[lints]`
3737
[HELP] consider removing `package.readme`
38-
|
39-
7 - readme = "README.md"
40-
|
4138
[WARNING] `foo` (manifest) generated 1 warning
4239
4340
"#]])

0 commit comments

Comments
 (0)