Skip to content

Commit 0f82885

Browse files
epageepage
committed
fix(diag): Remove sometimes-invalid removal suggestions
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
1 parent 6b68893 commit 0f82885

12 files changed

Lines changed: 6 additions & 223 deletions

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

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

156138
pkg_stats.record_lint(lint_level);

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

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

159140
pkg_stats.record_lint(lint_level);

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

Lines changed: 2 additions & 25 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,20 +148,9 @@ pub(crate) fn lint_package(
149148
primary = primary.element(Level::NOTE.message(emitted_source));
150149
}
151150
let mut report = vec![primary];
152-
let mut help = Group::with_title(
151+
let help = Group::with_title(
153152
Level::HELP.secondary_title("consider removing the unused dependency"),
154153
);
155-
if let Some(document) = document
156-
&& let Some(contents) = contents
157-
&& let Some(span) = get_key_value_span(document, &["build-dependencies", dep_name])
158-
{
159-
let span = span.key.start..span.value.end;
160-
help = help.element(
161-
Snippet::source(contents)
162-
.path(&manifest_path)
163-
.patch(Patch::new(span, "")),
164-
);
165-
}
166154
report.push(help);
167155

168156
pkg_stats.record_lint(lint_level);
@@ -337,20 +325,9 @@ fn lint_package_build_results(
337325
}
338326
lint_count += 1;
339327
let mut report = vec![primary];
340-
let mut help = Group::with_title(
328+
let help = Group::with_title(
341329
Level::HELP.secondary_title("consider removing the unused dependency"),
342330
);
343-
if let Some(document) = document
344-
&& let Some(contents) = contents
345-
&& let Some(span) = get_key_value_span(document, &toml_path)
346-
{
347-
let span = span.key.start..span.value.end;
348-
help = help.element(
349-
Snippet::source(contents)
350-
.path(&manifest_path)
351-
.patch(Patch::new(span, "")),
352-
);
353-
}
354331
report.push(help);
355332
if used_in_dev {
356333
let help = Group::with_title(Level::HELP.secondary_title(

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

Lines changed: 1 addition & 13 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,20 +149,9 @@ pub(crate) fn lint_workspace(
150149
primary = primary.element(Level::NOTE.message(emitted_source));
151150
}
152151
let mut report = vec![primary];
153-
let mut help = Group::with_title(
152+
let help = Group::with_title(
154153
Level::HELP.secondary_title("consider removing the unused workspace dependency"),
155154
);
156-
if let Some(document) = document
157-
&& let Some(contents) = contents
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-
}
167155
report.push(help);
168156

169157
pkg_stats.record_lint(lint_level);

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

Lines changed: 1 addition & 13 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,19 +118,8 @@ pub(crate) fn lint_workspace(
119118
primary = primary.element(Level::NOTE.message(emitted_source));
120119
}
121120
let mut report = vec![primary];
122-
let mut help =
121+
let help =
123122
Group::with_title(Level::HELP.secondary_title("consider removing the unused field"));
124-
if let Some(document) = document
125-
&& let Some(contents) = contents
126-
{
127-
let mut snippet = Snippet::source(contents).path(&manifest_path);
128-
if let Some(span) =
129-
get_key_value_span(document, &["workspace", "package", unused.as_ref()])
130-
{
131-
snippet = snippet.patch(Patch::new(span.key.start..span.value.end, ""));
132-
}
133-
help = help.element(snippet);
134-
}
135123
report.push(help);
136124

137125
pkg_stats.record_lint(lint_level);

‎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)