Skip to content

Commit da898bf

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of rust-lang#98261 - WaffleLapkin:attempt_to_remove_max_suggestion_highlight_lines, r=flip1995
Remove `MAX_SUGGESTION_HIGHLIGHT_LINES` After rust-lang#97798 the `MAX_SUGGESTION_HIGHLIGHT_LINES` constant doesn't really make sense since we always show full suggestions. This PR removes last usages of the constant and the constant itself. r? `@flip1995` (this mostly does changes in clippy)
2 parents 86eb0e5 + ffb593b commit da898bf

File tree

10 files changed

+68
-186
lines changed

10 files changed

+68
-186
lines changed

‎compiler/rustc_errors/src/emitter.rs

-5
Original file line numberDiff line numberDiff line change
@@ -656,11 +656,6 @@ impl Emitter for SilentEmitter {
656656
}
657657
}
658658

659-
/// Maximum number of lines we will print for a multiline suggestion; arbitrary.
660-
///
661-
/// This should be replaced with a more involved mechanism to output multiline suggestions that
662-
/// more closely mimics the regular diagnostic output, where irrelevant code lines are elided.
663-
pub const MAX_SUGGESTION_HIGHLIGHT_LINES: usize = 6;
664659
/// Maximum number of suggestions to be shown
665660
///
666661
/// Arbitrary, but taken from trait import suggestion limit

‎src/tools/clippy/clippy_lints/src/methods/map_flatten.rs

+4-7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use clippy_utils::diagnostics::span_lint_and_sugg_for_edges;
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::is_trait_method;
33
use clippy_utils::source::snippet_with_applicability;
44
use clippy_utils::ty::is_type_diagnostic_item;
@@ -14,17 +14,14 @@ use super::MAP_FLATTEN;
1414
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, map_arg: &Expr<'_>, map_span: Span) {
1515
if let Some((caller_ty_name, method_to_use)) = try_get_caller_ty_name_and_method_name(cx, expr, recv, map_arg) {
1616
let mut applicability = Applicability::MachineApplicable;
17-
let help_msgs = [
18-
&format!("try replacing `map` with `{}`", method_to_use),
19-
"and remove the `.flatten()`",
20-
];
17+
2118
let closure_snippet = snippet_with_applicability(cx, map_arg.span, "..", &mut applicability);
22-
span_lint_and_sugg_for_edges(
19+
span_lint_and_sugg(
2320
cx,
2421
MAP_FLATTEN,
2522
expr.span.with_lo(map_span.lo()),
2623
&format!("called `map(..).flatten()` on `{}`", caller_ty_name),
27-
&help_msgs,
24+
&format!("try replacing `map` with `{}` and remove the `.flatten()`", method_to_use),
2825
format!("{}({})", method_to_use, closure_snippet),
2926
applicability,
3027
);

‎src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs

+3-10
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_mac
44
use clippy_utils::ty::{implements_trait, match_type};
55
use clippy_utils::{contains_return, is_trait_item, last_path_segment, paths};
66
use if_chain::if_chain;
7-
use rustc_errors::emitter::MAX_SUGGESTION_HIGHLIGHT_LINES;
87
use rustc_errors::Applicability;
98
use rustc_hir as hir;
109
use rustc_lint::LateContext;
@@ -33,7 +32,6 @@ pub(super) fn check<'tcx>(
3332
arg: &hir::Expr<'_>,
3433
or_has_args: bool,
3534
span: Span,
36-
method_span: Span,
3735
) -> bool {
3836
let is_default_default = || is_trait_item(cx, fun, sym::Default);
3937

@@ -56,19 +54,14 @@ pub(super) fn check<'tcx>(
5654
then {
5755
let mut applicability = Applicability::MachineApplicable;
5856
let hint = "unwrap_or_default()";
59-
let mut sugg_span = span;
57+
let sugg_span = span;
6058

61-
let mut sugg: String = format!(
59+
let sugg: String = format!(
6260
"{}.{}",
6361
snippet_with_applicability(cx, self_expr.span, "..", &mut applicability),
6462
hint
6563
);
6664

67-
if sugg.lines().count() > MAX_SUGGESTION_HIGHLIGHT_LINES {
68-
sugg_span = method_span.with_hi(span.hi());
69-
sugg = hint.to_string();
70-
}
71-
7265
span_lint_and_sugg(
7366
cx,
7467
OR_FUN_CALL,
@@ -178,7 +171,7 @@ pub(super) fn check<'tcx>(
178171
match inner_arg.kind {
179172
hir::ExprKind::Call(fun, or_args) => {
180173
let or_has_args = !or_args.is_empty();
181-
if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span, method_span) {
174+
if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span) {
182175
let fun_span = if or_has_args { None } else { Some(fun.span) };
183176
check_general_case(cx, name, method_span, self_arg, arg, expr.span, fun_span);
184177
}

‎src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs

-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ const LINT_EMISSION_FUNCTIONS: [&[&str]; 8] = [
112112
&["clippy_utils", "diagnostics", "span_lint_and_sugg"],
113113
&["clippy_utils", "diagnostics", "span_lint_and_then"],
114114
&["clippy_utils", "diagnostics", "span_lint_hir_and_then"],
115-
&["clippy_utils", "diagnostics", "span_lint_and_sugg_for_edges"],
116115
];
117116
const SUGGESTION_DIAGNOSTIC_BUILDER_METHODS: [(&str, bool); 9] = [
118117
("span_suggestion", false),

‎src/tools/clippy/clippy_utils/src/diagnostics.rs

+1-90
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
//! Thank you!
99
//! ~The `INTERNAL_METADATA_COLLECTOR` lint
1010
11-
use rustc_errors::{emitter::MAX_SUGGESTION_HIGHLIGHT_LINES, Applicability, Diagnostic, MultiSpan};
11+
use rustc_errors::{Applicability, Diagnostic, MultiSpan};
1212
use rustc_hir::HirId;
1313
use rustc_lint::{LateContext, Lint, LintContext};
1414
use rustc_span::source_map::Span;
@@ -219,95 +219,6 @@ pub fn span_lint_and_sugg<'a, T: LintContext>(
219219
});
220220
}
221221

222-
/// Like [`span_lint_and_sugg`] with a focus on the edges. The output will either
223-
/// emit single span or multispan suggestion depending on the number of its lines.
224-
///
225-
/// If the given suggestion string has more lines than the maximum display length defined by
226-
/// [`MAX_SUGGESTION_HIGHLIGHT_LINES`][`rustc_errors::emitter::MAX_SUGGESTION_HIGHLIGHT_LINES`],
227-
/// this function will split the suggestion and span to showcase the change for the top and
228-
/// bottom edge of the code. For normal suggestions, in one display window, the help message
229-
/// will be combined with a colon.
230-
///
231-
/// Multipart suggestions like the one being created here currently cannot be
232-
/// applied by rustfix (See [rustfix#141](https://github.com/rust-lang/rustfix/issues/141)).
233-
/// Testing rustfix with this lint emission function might require a file with
234-
/// suggestions that can be fixed and those that can't. See
235-
/// [clippy#8520](https://github.com/rust-lang/rust-clippy/pull/8520/files) for
236-
/// an example and of this.
237-
///
238-
/// # Example for a long suggestion
239-
///
240-
/// ```text
241-
/// error: called `map(..).flatten()` on `Option`
242-
/// --> $DIR/map_flatten.rs:8:10
243-
/// |
244-
/// LL | .map(|x| {
245-
/// | __________^
246-
/// LL | | if x <= 5 {
247-
/// LL | | Some(x)
248-
/// LL | | } else {
249-
/// ... |
250-
/// LL | | })
251-
/// LL | | .flatten();
252-
/// | |__________________^
253-
/// |
254-
/// = note: `-D clippy::map-flatten` implied by `-D warnings`
255-
/// help: try replacing `map` with `and_then`
256-
/// |
257-
/// LL ~ .and_then(|x| {
258-
/// LL + if x <= 5 {
259-
/// LL + Some(x)
260-
/// |
261-
/// help: and remove the `.flatten()`
262-
/// |
263-
/// LL + None
264-
/// LL + }
265-
/// LL ~ });
266-
/// |
267-
/// ```
268-
pub fn span_lint_and_sugg_for_edges(
269-
cx: &LateContext<'_>,
270-
lint: &'static Lint,
271-
sp: Span,
272-
msg: &str,
273-
helps: &[&str; 2],
274-
sugg: String,
275-
applicability: Applicability,
276-
) {
277-
span_lint_and_then(cx, lint, sp, msg, |diag| {
278-
let sugg_lines_count = sugg.lines().count();
279-
if sugg_lines_count > MAX_SUGGESTION_HIGHLIGHT_LINES {
280-
let sm = cx.sess().source_map();
281-
if let (Ok(line_upper), Ok(line_bottom)) =
282-
(sm.lookup_line(sp.lo()), sm.lookup_line(sp.hi()))
283-
{
284-
let split_idx = MAX_SUGGESTION_HIGHLIGHT_LINES / 2;
285-
let span_upper = sm.span_until_char(
286-
sp.with_hi(line_upper.sf.lines(|lines| lines[line_upper.line + split_idx])),
287-
'\n',
288-
);
289-
let span_bottom = sp.with_lo(line_bottom.sf.lines(|lines| lines[line_bottom.line - split_idx]));
290-
291-
let sugg_lines_vec = sugg.lines().collect::<Vec<&str>>();
292-
let sugg_upper = sugg_lines_vec[..split_idx].join("\n");
293-
let sugg_bottom = sugg_lines_vec[sugg_lines_count - split_idx..].join("\n");
294-
295-
diag.span_suggestion(span_upper, helps[0], sugg_upper, applicability);
296-
diag.span_suggestion(span_bottom, helps[1], sugg_bottom, applicability);
297-
298-
return;
299-
}
300-
}
301-
diag.span_suggestion_with_style(
302-
sp,
303-
&helps.join(", "),
304-
sugg,
305-
applicability,
306-
rustc_errors::SuggestionStyle::ShowAlways,
307-
);
308-
});
309-
}
310-
311222
/// Create a suggestion made from several `span → replacement`.
312223
///
313224
/// Note: in the JSON format (used by `compiletest_rs`), the help message will

‎src/tools/clippy/tests/ui/map_flatten.stderr

+9-16
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,12 @@ LL | | .flatten();
1212
| |__________________^
1313
|
1414
= note: `-D clippy::map-flatten` implied by `-D warnings`
15-
help: try replacing `map` with `and_then`
15+
help: try replacing `map` with `and_then` and remove the `.flatten()`
1616
|
1717
LL ~ .and_then(|x| {
1818
LL + if x <= 5 {
1919
LL + Some(x)
20-
|
21-
help: and remove the `.flatten()`
22-
|
20+
LL + } else {
2321
LL + None
2422
LL + }
2523
LL ~ });
@@ -38,14 +36,12 @@ LL | | })
3836
LL | | .flatten();
3937
| |__________________^
4038
|
41-
help: try replacing `map` with `and_then`
39+
help: try replacing `map` with `and_then` and remove the `.flatten()`
4240
|
4341
LL ~ .and_then(|x| {
4442
LL + if x == 1 {
4543
LL + Ok(x)
46-
|
47-
help: and remove the `.flatten()`
48-
|
44+
LL + } else {
4945
LL + Err(0)
5046
LL + }
5147
LL ~ });
@@ -64,14 +60,13 @@ LL | | })
6460
LL | | .flatten();
6561
| |__________________^
6662
|
67-
help: try replacing `map` with `and_then`
63+
help: try replacing `map` with `and_then` and remove the `.flatten()`
6864
|
6965
LL ~ .and_then(|res| {
7066
LL + if res > 0 {
7167
LL + do_something();
72-
|
73-
help: and remove the `.flatten()`
74-
|
68+
LL + Ok(res)
69+
LL + } else {
7570
LL + Err(0)
7671
LL + }
7772
LL ~ });
@@ -90,14 +85,12 @@ LL | | })
9085
LL | | .flatten()
9186
| |__________________^
9287
|
93-
help: try replacing `map` with `filter_map`
88+
help: try replacing `map` with `filter_map` and remove the `.flatten()`
9489
|
9590
LL ~ .filter_map(|some_value| {
9691
LL + if some_value > 3 {
9792
LL + Some(some_value)
98-
|
99-
help: and remove the `.flatten()`
100-
|
93+
LL + } else {
10194
LL + None
10295
LL + }
10396
LL + })

‎src/tools/clippy/tests/ui/map_flatten_fixable.fixed

-2
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,6 @@ fn issue8878() {
5959
.and_then(|_| {
6060
// we need some newlines
6161
// so that the span is big enough
62-
// we need some newlines
63-
// so that the span is big enough
6462
// for a splitted output of the diagnostic
6563
Some("")
6664
// whitespace beforehand is important as well

‎src/tools/clippy/tests/ui/map_flatten_fixable.stderr

+9-45
Original file line numberDiff line numberDiff line change
@@ -2,79 +2,45 @@ error: called `map(..).flatten()` on `Iterator`
22
--> $DIR/map_flatten_fixable.rs:18:47
33
|
44
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect();
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id)`
66
|
77
= note: `-D clippy::map-flatten` implied by `-D warnings`
8-
help: try replacing `map` with `filter_map`, and remove the `.flatten()`
9-
|
10-
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id).collect();
11-
| ~~~~~~~~~~~~~~~~~~~~~
128

139
error: called `map(..).flatten()` on `Iterator`
1410
--> $DIR/map_flatten_fixable.rs:19:47
1511
|
1612
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect();
17-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
18-
|
19-
help: try replacing `map` with `filter_map`, and remove the `.flatten()`
20-
|
21-
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id_ref).collect();
22-
| ~~~~~~~~~~~~~~~~~~~~~~~~~
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id_ref)`
2314

2415
error: called `map(..).flatten()` on `Iterator`
2516
--> $DIR/map_flatten_fixable.rs:20:47
2617
|
2718
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect();
28-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
29-
|
30-
help: try replacing `map` with `filter_map`, and remove the `.flatten()`
31-
|
32-
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id_closure).collect();
33-
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id_closure)`
3420

3521
error: called `map(..).flatten()` on `Iterator`
3622
--> $DIR/map_flatten_fixable.rs:21:47
3723
|
3824
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect();
39-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
40-
|
41-
help: try replacing `map` with `filter_map`, and remove the `.flatten()`
42-
|
43-
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(|x| x.checked_add(1)).collect();
44-
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
25+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(|x| x.checked_add(1))`
4526

4627
error: called `map(..).flatten()` on `Iterator`
4728
--> $DIR/map_flatten_fixable.rs:24:47
4829
|
4930
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect();
50-
| ^^^^^^^^^^^^^^^^^^^^^^^
51-
|
52-
help: try replacing `map` with `flat_map`, and remove the `.flatten()`
53-
|
54-
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().flat_map(|x| 0..x).collect();
55-
| ~~~~~~~~~~~~~~~~~~
31+
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `flat_map` and remove the `.flatten()`: `flat_map(|x| 0..x)`
5632

5733
error: called `map(..).flatten()` on `Option`
5834
--> $DIR/map_flatten_fixable.rs:27:40
5935
|
6036
LL | let _: Option<_> = (Some(Some(1))).map(|x| x).flatten();
61-
| ^^^^^^^^^^^^^^^^^^^^
62-
|
63-
help: try replacing `map` with `and_then`, and remove the `.flatten()`
64-
|
65-
LL | let _: Option<_> = (Some(Some(1))).and_then(|x| x);
66-
| ~~~~~~~~~~~~~~~
37+
| ^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `and_then` and remove the `.flatten()`: `and_then(|x| x)`
6738

6839
error: called `map(..).flatten()` on `Result`
6940
--> $DIR/map_flatten_fixable.rs:30:42
7041
|
7142
LL | let _: Result<_, &str> = (Ok(Ok(1))).map(|x| x).flatten();
72-
| ^^^^^^^^^^^^^^^^^^^^
73-
|
74-
help: try replacing `map` with `and_then`, and remove the `.flatten()`
75-
|
76-
LL | let _: Result<_, &str> = (Ok(Ok(1))).and_then(|x| x);
77-
| ~~~~~~~~~~~~~~~
43+
| ^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `and_then` and remove the `.flatten()`: `and_then(|x| x)`
7844

7945
error: called `map(..).flatten()` on `Option`
8046
--> $DIR/map_flatten_fixable.rs:59:10
@@ -89,14 +55,12 @@ LL | | })
8955
LL | | .flatten();
9056
| |__________________^
9157
|
92-
help: try replacing `map` with `and_then`
58+
help: try replacing `map` with `and_then` and remove the `.flatten()`
9359
|
9460
LL ~ .and_then(|_| {
9561
LL + // we need some newlines
9662
LL + // so that the span is big enough
97-
|
98-
help: and remove the `.flatten()`
99-
|
63+
LL + // for a splitted output of the diagnostic
10064
LL + Some("")
10165
LL + // whitespace beforehand is important as well
10266
LL ~ });

0 commit comments

Comments
 (0)