Skip to content

Commit

Permalink
Rollup merge of rust-lang#98261 - WaffleLapkin:attempt_to_remove_max_…
Browse files Browse the repository at this point in the history
…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)
  • Loading branch information
Dylan-DPC authored Jun 28, 2022
2 parents c703d11 + ffb593b commit 9b3dbb8
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 186 deletions.
5 changes: 0 additions & 5 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -656,11 +656,6 @@ impl Emitter for SilentEmitter {
}
}

/// Maximum number of lines we will print for a multiline suggestion; arbitrary.
///
/// This should be replaced with a more involved mechanism to output multiline suggestions that
/// more closely mimics the regular diagnostic output, where irrelevant code lines are elided.
pub const MAX_SUGGESTION_HIGHLIGHT_LINES: usize = 6;
/// Maximum number of suggestions to be shown
///
/// Arbitrary, but taken from trait import suggestion limit
Expand Down
11 changes: 4 additions & 7 deletions src/tools/clippy/clippy_lints/src/methods/map_flatten.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clippy_utils::diagnostics::span_lint_and_sugg_for_edges;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_trait_method;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::is_type_diagnostic_item;
Expand All @@ -14,17 +14,14 @@ use super::MAP_FLATTEN;
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, map_arg: &Expr<'_>, map_span: Span) {
if let Some((caller_ty_name, method_to_use)) = try_get_caller_ty_name_and_method_name(cx, expr, recv, map_arg) {
let mut applicability = Applicability::MachineApplicable;
let help_msgs = [
&format!("try replacing `map` with `{}`", method_to_use),
"and remove the `.flatten()`",
];

let closure_snippet = snippet_with_applicability(cx, map_arg.span, "..", &mut applicability);
span_lint_and_sugg_for_edges(
span_lint_and_sugg(
cx,
MAP_FLATTEN,
expr.span.with_lo(map_span.lo()),
&format!("called `map(..).flatten()` on `{}`", caller_ty_name),
&help_msgs,
&format!("try replacing `map` with `{}` and remove the `.flatten()`", method_to_use),
format!("{}({})", method_to_use, closure_snippet),
applicability,
);
Expand Down
13 changes: 3 additions & 10 deletions src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_mac
use clippy_utils::ty::{implements_trait, match_type};
use clippy_utils::{contains_return, is_trait_item, last_path_segment, paths};
use if_chain::if_chain;
use rustc_errors::emitter::MAX_SUGGESTION_HIGHLIGHT_LINES;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
Expand Down Expand Up @@ -33,7 +32,6 @@ pub(super) fn check<'tcx>(
arg: &hir::Expr<'_>,
or_has_args: bool,
span: Span,
method_span: Span,
) -> bool {
let is_default_default = || is_trait_item(cx, fun, sym::Default);

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

let mut sugg: String = format!(
let sugg: String = format!(
"{}.{}",
snippet_with_applicability(cx, self_expr.span, "..", &mut applicability),
hint
);

if sugg.lines().count() > MAX_SUGGESTION_HIGHLIGHT_LINES {
sugg_span = method_span.with_hi(span.hi());
sugg = hint.to_string();
}

span_lint_and_sugg(
cx,
OR_FUN_CALL,
Expand Down Expand Up @@ -178,7 +171,7 @@ pub(super) fn check<'tcx>(
match inner_arg.kind {
hir::ExprKind::Call(fun, or_args) => {
let or_has_args = !or_args.is_empty();
if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span, method_span) {
if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span) {
let fun_span = if or_has_args { None } else { Some(fun.span) };
check_general_case(cx, name, method_span, self_arg, arg, expr.span, fun_span);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ const LINT_EMISSION_FUNCTIONS: [&[&str]; 8] = [
&["clippy_utils", "diagnostics", "span_lint_and_sugg"],
&["clippy_utils", "diagnostics", "span_lint_and_then"],
&["clippy_utils", "diagnostics", "span_lint_hir_and_then"],
&["clippy_utils", "diagnostics", "span_lint_and_sugg_for_edges"],
];
const SUGGESTION_DIAGNOSTIC_BUILDER_METHODS: [(&str, bool); 9] = [
("span_suggestion", false),
Expand Down
91 changes: 1 addition & 90 deletions src/tools/clippy/clippy_utils/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
//! Thank you!
//! ~The `INTERNAL_METADATA_COLLECTOR` lint

use rustc_errors::{emitter::MAX_SUGGESTION_HIGHLIGHT_LINES, Applicability, Diagnostic, MultiSpan};
use rustc_errors::{Applicability, Diagnostic, MultiSpan};
use rustc_hir::HirId;
use rustc_lint::{LateContext, Lint, LintContext};
use rustc_span::source_map::Span;
Expand Down Expand Up @@ -219,95 +219,6 @@ pub fn span_lint_and_sugg<'a, T: LintContext>(
});
}

/// Like [`span_lint_and_sugg`] with a focus on the edges. The output will either
/// emit single span or multispan suggestion depending on the number of its lines.
///
/// If the given suggestion string has more lines than the maximum display length defined by
/// [`MAX_SUGGESTION_HIGHLIGHT_LINES`][`rustc_errors::emitter::MAX_SUGGESTION_HIGHLIGHT_LINES`],
/// this function will split the suggestion and span to showcase the change for the top and
/// bottom edge of the code. For normal suggestions, in one display window, the help message
/// will be combined with a colon.
///
/// Multipart suggestions like the one being created here currently cannot be
/// applied by rustfix (See [rustfix#141](https://github.com/rust-lang/rustfix/issues/141)).
/// Testing rustfix with this lint emission function might require a file with
/// suggestions that can be fixed and those that can't. See
/// [clippy#8520](https://github.com/rust-lang/rust-clippy/pull/8520/files) for
/// an example and of this.
///
/// # Example for a long suggestion
///
/// ```text
/// error: called `map(..).flatten()` on `Option`
/// --> $DIR/map_flatten.rs:8:10
/// |
/// LL | .map(|x| {
/// | __________^
/// LL | | if x <= 5 {
/// LL | | Some(x)
/// LL | | } else {
/// ... |
/// LL | | })
/// LL | | .flatten();
/// | |__________________^
/// |
/// = note: `-D clippy::map-flatten` implied by `-D warnings`
/// help: try replacing `map` with `and_then`
/// |
/// LL ~ .and_then(|x| {
/// LL + if x <= 5 {
/// LL + Some(x)
/// |
/// help: and remove the `.flatten()`
/// |
/// LL + None
/// LL + }
/// LL ~ });
/// |
/// ```
pub fn span_lint_and_sugg_for_edges(
cx: &LateContext<'_>,
lint: &'static Lint,
sp: Span,
msg: &str,
helps: &[&str; 2],
sugg: String,
applicability: Applicability,
) {
span_lint_and_then(cx, lint, sp, msg, |diag| {
let sugg_lines_count = sugg.lines().count();
if sugg_lines_count > MAX_SUGGESTION_HIGHLIGHT_LINES {
let sm = cx.sess().source_map();
if let (Ok(line_upper), Ok(line_bottom)) =
(sm.lookup_line(sp.lo()), sm.lookup_line(sp.hi()))
{
let split_idx = MAX_SUGGESTION_HIGHLIGHT_LINES / 2;
let span_upper = sm.span_until_char(
sp.with_hi(line_upper.sf.lines(|lines| lines[line_upper.line + split_idx])),
'\n',
);
let span_bottom = sp.with_lo(line_bottom.sf.lines(|lines| lines[line_bottom.line - split_idx]));

let sugg_lines_vec = sugg.lines().collect::<Vec<&str>>();
let sugg_upper = sugg_lines_vec[..split_idx].join("\n");
let sugg_bottom = sugg_lines_vec[sugg_lines_count - split_idx..].join("\n");

diag.span_suggestion(span_upper, helps[0], sugg_upper, applicability);
diag.span_suggestion(span_bottom, helps[1], sugg_bottom, applicability);

return;
}
}
diag.span_suggestion_with_style(
sp,
&helps.join(", "),
sugg,
applicability,
rustc_errors::SuggestionStyle::ShowAlways,
);
});
}

/// Create a suggestion made from several `span → replacement`.
///
/// Note: in the JSON format (used by `compiletest_rs`), the help message will
Expand Down
25 changes: 9 additions & 16 deletions src/tools/clippy/tests/ui/map_flatten.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,12 @@ LL | | .flatten();
| |__________________^
|
= note: `-D clippy::map-flatten` implied by `-D warnings`
help: try replacing `map` with `and_then`
help: try replacing `map` with `and_then` and remove the `.flatten()`
|
LL ~ .and_then(|x| {
LL + if x <= 5 {
LL + Some(x)
|
help: and remove the `.flatten()`
|
LL + } else {
LL + None
LL + }
LL ~ });
Expand All @@ -38,14 +36,12 @@ LL | | })
LL | | .flatten();
| |__________________^
|
help: try replacing `map` with `and_then`
help: try replacing `map` with `and_then` and remove the `.flatten()`
|
LL ~ .and_then(|x| {
LL + if x == 1 {
LL + Ok(x)
|
help: and remove the `.flatten()`
|
LL + } else {
LL + Err(0)
LL + }
LL ~ });
Expand All @@ -64,14 +60,13 @@ LL | | })
LL | | .flatten();
| |__________________^
|
help: try replacing `map` with `and_then`
help: try replacing `map` with `and_then` and remove the `.flatten()`
|
LL ~ .and_then(|res| {
LL + if res > 0 {
LL + do_something();
|
help: and remove the `.flatten()`
|
LL + Ok(res)
LL + } else {
LL + Err(0)
LL + }
LL ~ });
Expand All @@ -90,14 +85,12 @@ LL | | })
LL | | .flatten()
| |__________________^
|
help: try replacing `map` with `filter_map`
help: try replacing `map` with `filter_map` and remove the `.flatten()`
|
LL ~ .filter_map(|some_value| {
LL + if some_value > 3 {
LL + Some(some_value)
|
help: and remove the `.flatten()`
|
LL + } else {
LL + None
LL + }
LL + })
Expand Down
2 changes: 0 additions & 2 deletions src/tools/clippy/tests/ui/map_flatten_fixable.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ fn issue8878() {
.and_then(|_| {
// we need some newlines
// so that the span is big enough
// we need some newlines
// so that the span is big enough
// for a splitted output of the diagnostic
Some("")
// whitespace beforehand is important as well
Expand Down
54 changes: 9 additions & 45 deletions src/tools/clippy/tests/ui/map_flatten_fixable.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,79 +2,45 @@ error: called `map(..).flatten()` on `Iterator`
--> $DIR/map_flatten_fixable.rs:18:47
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id)`
|
= note: `-D clippy::map-flatten` implied by `-D warnings`
help: try replacing `map` with `filter_map`, and remove the `.flatten()`
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id).collect();
| ~~~~~~~~~~~~~~~~~~~~~

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

error: called `map(..).flatten()` on `Iterator`
--> $DIR/map_flatten_fixable.rs:20:47
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: try replacing `map` with `filter_map`, and remove the `.flatten()`
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(option_id_closure).collect();
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id_closure)`

error: called `map(..).flatten()` on `Iterator`
--> $DIR/map_flatten_fixable.rs:21:47
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: try replacing `map` with `filter_map`, and remove the `.flatten()`
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().filter_map(|x| x.checked_add(1)).collect();
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(|x| x.checked_add(1))`

error: called `map(..).flatten()` on `Iterator`
--> $DIR/map_flatten_fixable.rs:24:47
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect();
| ^^^^^^^^^^^^^^^^^^^^^^^
|
help: try replacing `map` with `flat_map`, and remove the `.flatten()`
|
LL | let _: Vec<_> = vec![5_i8; 6].into_iter().flat_map(|x| 0..x).collect();
| ~~~~~~~~~~~~~~~~~~
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `flat_map` and remove the `.flatten()`: `flat_map(|x| 0..x)`

error: called `map(..).flatten()` on `Option`
--> $DIR/map_flatten_fixable.rs:27:40
|
LL | let _: Option<_> = (Some(Some(1))).map(|x| x).flatten();
| ^^^^^^^^^^^^^^^^^^^^
|
help: try replacing `map` with `and_then`, and remove the `.flatten()`
|
LL | let _: Option<_> = (Some(Some(1))).and_then(|x| x);
| ~~~~~~~~~~~~~~~
| ^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `and_then` and remove the `.flatten()`: `and_then(|x| x)`

error: called `map(..).flatten()` on `Result`
--> $DIR/map_flatten_fixable.rs:30:42
|
LL | let _: Result<_, &str> = (Ok(Ok(1))).map(|x| x).flatten();
| ^^^^^^^^^^^^^^^^^^^^
|
help: try replacing `map` with `and_then`, and remove the `.flatten()`
|
LL | let _: Result<_, &str> = (Ok(Ok(1))).and_then(|x| x);
| ~~~~~~~~~~~~~~~
| ^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `and_then` and remove the `.flatten()`: `and_then(|x| x)`

error: called `map(..).flatten()` on `Option`
--> $DIR/map_flatten_fixable.rs:59:10
Expand All @@ -89,14 +55,12 @@ LL | | })
LL | | .flatten();
| |__________________^
|
help: try replacing `map` with `and_then`
help: try replacing `map` with `and_then` and remove the `.flatten()`
|
LL ~ .and_then(|_| {
LL + // we need some newlines
LL + // so that the span is big enough
|
help: and remove the `.flatten()`
|
LL + // for a splitted output of the diagnostic
LL + Some("")
LL + // whitespace beforehand is important as well
LL ~ });
Expand Down
Loading

0 comments on commit 9b3dbb8

Please sign in to comment.