Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue #8239: Printed hint for lint or_fun_call is cropped and does no… #8292

Merged
merged 3 commits into from
Jan 17, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions clippy_lints/src/methods/or_fun_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ 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 All @@ -23,6 +24,7 @@ pub(super) fn check<'tcx>(
args: &'tcx [hir::Expr<'_>],
) {
/// Checks for `unwrap_or(T::new())` or `unwrap_or(T::default())`.
#[allow(clippy::too_many_arguments)]
fn check_unwrap_or_default(
cx: &LateContext<'_>,
name: &str,
Expand All @@ -31,6 +33,7 @@ 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 @@ -52,16 +55,27 @@ pub(super) fn check<'tcx>(

then {
let mut applicability = Applicability::MachineApplicable;
let hint = "unwrap_or_default()";
let mut shrink = span;
xFrednet marked this conversation as resolved.
Show resolved Hide resolved

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

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

span_lint_and_sugg(
cx,
OR_FUN_CALL,
span,
shrink,
&format!("use of `{}` followed by a call to `{}`", name, path),
"try this",
format!(
"{}.unwrap_or_default()",
snippet_with_applicability(cx, self_expr.span, "..", &mut applicability)
),
sugg,
xFrednet marked this conversation as resolved.
Show resolved Hide resolved
applicability,
);

Expand Down Expand Up @@ -164,7 +178,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) {
if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span, method_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
48 changes: 48 additions & 0 deletions tests/ui/or_fun_call.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -176,4 +176,52 @@ mod issue6675 {
}
}

mod issue8239 {
unsafe fn more_than_max_suggestion_highest_lines_0() {
let frames = Vec::new();
frames
.iter()
.map(|f: &String| f.to_lowercase())
.reduce(|mut acc, f| {
acc.push_str(&f);
acc
})
.unwrap_or_default();
}

unsafe fn more_to_max_suggestion_highest_lines_1() {
let frames = Vec::new();
let iter = frames.iter();
iter.map(|f: &String| f.to_lowercase())
.reduce(|mut acc, f| {
let _ = "";
let _ = "";
acc.push_str(&f);
acc
})
.unwrap_or_default();
}

unsafe fn equal_to_max_suggestion_highest_lines() {
let frames = Vec::new();
let iter = frames.iter();
iter.map(|f: &String| f.to_lowercase())
.reduce(|mut acc, f| {
let _ = "";
acc.push_str(&f);
acc
}).unwrap_or_default();
}

unsafe fn less_than_max_suggestion_highest_lines() {
xFrednet marked this conversation as resolved.
Show resolved Hide resolved
let frames = Vec::new();
let iter = frames.iter();
let map = iter.map(|f: &String| f.to_lowercase());
map.reduce(|mut acc, f| {
acc.push_str(&f);
acc
}).unwrap_or_default();
}
}

fn main() {}
50 changes: 50 additions & 0 deletions tests/ui/or_fun_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,4 +176,54 @@ mod issue6675 {
}
}

mod issue8239 {
unsafe fn more_than_max_suggestion_highest_lines_0() {
let frames = Vec::new();
frames
.iter()
.map(|f: &String| f.to_lowercase())
.reduce(|mut acc, f| {
acc.push_str(&f);
acc
})
.unwrap_or(String::new());
}

unsafe fn more_to_max_suggestion_highest_lines_1() {
let frames = Vec::new();
let iter = frames.iter();
iter.map(|f: &String| f.to_lowercase())
.reduce(|mut acc, f| {
let _ = "";
let _ = "";
acc.push_str(&f);
acc
})
.unwrap_or(String::new());
}

unsafe fn equal_to_max_suggestion_highest_lines() {
let frames = Vec::new();
let iter = frames.iter();
iter.map(|f: &String| f.to_lowercase())
.reduce(|mut acc, f| {
let _ = "";
acc.push_str(&f);
acc
})
.unwrap_or(String::new());
}

unsafe fn less_than_max_suggestion_highest_lines() {
let frames = Vec::new();
let iter = frames.iter();
let map = iter.map(|f: &String| f.to_lowercase());
map.reduce(|mut acc, f| {
acc.push_str(&f);
acc
})
.unwrap_or(String::new());
}
}

fn main() {}
54 changes: 53 additions & 1 deletion tests/ui/or_fun_call.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -108,5 +108,57 @@ error: use of `unwrap_or` followed by a function call
LL | None.unwrap_or( unsafe { ptr_to_ref(s) } );
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { ptr_to_ref(s) })`

error: aborting due to 18 previous errors
error: use of `unwrap_or` followed by a call to `new`
--> $DIR/or_fun_call.rs:189:14
|
LL | .unwrap_or(String::new());
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()`

error: use of `unwrap_or` followed by a call to `new`
--> $DIR/or_fun_call.rs:202:14
|
LL | .unwrap_or(String::new());
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()`

error: use of `unwrap_or` followed by a call to `new`
--> $DIR/or_fun_call.rs:208:9
|
LL | / iter.map(|f: &String| f.to_lowercase())
LL | | .reduce(|mut acc, f| {
LL | | let _ = "";
LL | | acc.push_str(&f);
LL | | acc
LL | | })
LL | | .unwrap_or(String::new());
| |_____________________________________^
|
help: try this
|
LL ~ iter.map(|f: &String| f.to_lowercase())
LL + .reduce(|mut acc, f| {
LL + let _ = "";
LL + acc.push_str(&f);
LL + acc
LL ~ }).unwrap_or_default();
|

error: use of `unwrap_or` followed by a call to `new`
--> $DIR/or_fun_call.rs:221:9
|
LL | / map.reduce(|mut acc, f| {
LL | | acc.push_str(&f);
LL | | acc
LL | | })
LL | | .unwrap_or(String::new());
| |_________________________________^
|
help: try this
|
LL ~ map.reduce(|mut acc, f| {
LL + acc.push_str(&f);
LL + acc
LL ~ }).unwrap_or_default();
|

error: aborting due to 22 previous errors