Skip to content

Commit 8d14c94

Browse files
borsbors
committed
Auto merge of rust-lang#8292 - marekdownar:8239, r=xFrednet
issue rust-lang#8239: Printed hint for lint or_fun_call is cropped and does no… fixes rust-lang/rust-clippy#8239 changelog: [`or_fun_call`]: if suggestion contains more lines than MAX_SUGGESTION_HIGHLIGHT_LINES it is stripped to one line
2 parents 72c59fd + 69d78ce commit 8d14c94

File tree

4 files changed

+171
-7
lines changed

4 files changed

+171
-7
lines changed

‎clippy_lints/src/methods/or_fun_call.rs

+20-6
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ 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;
78
use rustc_errors::Applicability;
89
use rustc_hir as hir;
910
use rustc_lint::LateContext;
@@ -23,6 +24,7 @@ pub(super) fn check<'tcx>(
2324
args: &'tcx [hir::Expr<'_>],
2425
) {
2526
/// Checks for `unwrap_or(T::new())` or `unwrap_or(T::default())`.
27+
#[allow(clippy::too_many_arguments)]
2628
fn check_unwrap_or_default(
2729
cx: &LateContext<'_>,
2830
name: &str,
@@ -31,6 +33,7 @@ pub(super) fn check<'tcx>(
3133
arg: &hir::Expr<'_>,
3234
or_has_args: bool,
3335
span: Span,
36+
method_span: Span,
3437
) -> bool {
3538
let is_default_default = || is_trait_item(cx, fun, sym::Default);
3639

@@ -52,16 +55,27 @@ pub(super) fn check<'tcx>(
5255

5356
then {
5457
let mut applicability = Applicability::MachineApplicable;
58+
let hint = "unwrap_or_default()";
59+
let mut sugg_span = span;
60+
61+
let mut sugg: String = format!(
62+
"{}.{}",
63+
snippet_with_applicability(cx, self_expr.span, "..", &mut applicability),
64+
hint
65+
);
66+
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+
5572
span_lint_and_sugg(
5673
cx,
5774
OR_FUN_CALL,
58-
span,
75+
sugg_span,
5976
&format!("use of `{}` followed by a call to `{}`", name, path),
6077
"try this",
61-
format!(
62-
"{}.unwrap_or_default()",
63-
snippet_with_applicability(cx, self_expr.span, "..", &mut applicability)
64-
),
78+
sugg,
6579
applicability,
6680
);
6781

@@ -164,7 +178,7 @@ pub(super) fn check<'tcx>(
164178
match inner_arg.kind {
165179
hir::ExprKind::Call(fun, or_args) => {
166180
let or_has_args = !or_args.is_empty();
167-
if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span) {
181+
if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span, method_span) {
168182
let fun_span = if or_has_args { None } else { Some(fun.span) };
169183
check_general_case(cx, name, method_span, self_arg, arg, expr.span, fun_span);
170184
}

‎tests/ui/or_fun_call.fixed

+48
Original file line numberDiff line numberDiff line change
@@ -176,4 +176,52 @@ mod issue6675 {
176176
}
177177
}
178178

179+
mod issue8239 {
180+
fn more_than_max_suggestion_highest_lines_0() {
181+
let frames = Vec::new();
182+
frames
183+
.iter()
184+
.map(|f: &String| f.to_lowercase())
185+
.reduce(|mut acc, f| {
186+
acc.push_str(&f);
187+
acc
188+
})
189+
.unwrap_or_default();
190+
}
191+
192+
fn more_to_max_suggestion_highest_lines_1() {
193+
let frames = Vec::new();
194+
let iter = frames.iter();
195+
iter.map(|f: &String| f.to_lowercase())
196+
.reduce(|mut acc, f| {
197+
let _ = "";
198+
let _ = "";
199+
acc.push_str(&f);
200+
acc
201+
})
202+
.unwrap_or_default();
203+
}
204+
205+
fn equal_to_max_suggestion_highest_lines() {
206+
let frames = Vec::new();
207+
let iter = frames.iter();
208+
iter.map(|f: &String| f.to_lowercase())
209+
.reduce(|mut acc, f| {
210+
let _ = "";
211+
acc.push_str(&f);
212+
acc
213+
}).unwrap_or_default();
214+
}
215+
216+
fn less_than_max_suggestion_highest_lines() {
217+
let frames = Vec::new();
218+
let iter = frames.iter();
219+
let map = iter.map(|f: &String| f.to_lowercase());
220+
map.reduce(|mut acc, f| {
221+
acc.push_str(&f);
222+
acc
223+
}).unwrap_or_default();
224+
}
225+
}
226+
179227
fn main() {}

‎tests/ui/or_fun_call.rs

+50
Original file line numberDiff line numberDiff line change
@@ -176,4 +176,54 @@ mod issue6675 {
176176
}
177177
}
178178

179+
mod issue8239 {
180+
fn more_than_max_suggestion_highest_lines_0() {
181+
let frames = Vec::new();
182+
frames
183+
.iter()
184+
.map(|f: &String| f.to_lowercase())
185+
.reduce(|mut acc, f| {
186+
acc.push_str(&f);
187+
acc
188+
})
189+
.unwrap_or(String::new());
190+
}
191+
192+
fn more_to_max_suggestion_highest_lines_1() {
193+
let frames = Vec::new();
194+
let iter = frames.iter();
195+
iter.map(|f: &String| f.to_lowercase())
196+
.reduce(|mut acc, f| {
197+
let _ = "";
198+
let _ = "";
199+
acc.push_str(&f);
200+
acc
201+
})
202+
.unwrap_or(String::new());
203+
}
204+
205+
fn equal_to_max_suggestion_highest_lines() {
206+
let frames = Vec::new();
207+
let iter = frames.iter();
208+
iter.map(|f: &String| f.to_lowercase())
209+
.reduce(|mut acc, f| {
210+
let _ = "";
211+
acc.push_str(&f);
212+
acc
213+
})
214+
.unwrap_or(String::new());
215+
}
216+
217+
fn less_than_max_suggestion_highest_lines() {
218+
let frames = Vec::new();
219+
let iter = frames.iter();
220+
let map = iter.map(|f: &String| f.to_lowercase());
221+
map.reduce(|mut acc, f| {
222+
acc.push_str(&f);
223+
acc
224+
})
225+
.unwrap_or(String::new());
226+
}
227+
}
228+
179229
fn main() {}

‎tests/ui/or_fun_call.stderr

+53-1
Original file line numberDiff line numberDiff line change
@@ -108,5 +108,57 @@ error: use of `unwrap_or` followed by a function call
108108
LL | None.unwrap_or( unsafe { ptr_to_ref(s) } );
109109
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { ptr_to_ref(s) })`
110110

111-
error: aborting due to 18 previous errors
111+
error: use of `unwrap_or` followed by a call to `new`
112+
--> $DIR/or_fun_call.rs:189:14
113+
|
114+
LL | .unwrap_or(String::new());
115+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()`
116+
117+
error: use of `unwrap_or` followed by a call to `new`
118+
--> $DIR/or_fun_call.rs:202:14
119+
|
120+
LL | .unwrap_or(String::new());
121+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()`
122+
123+
error: use of `unwrap_or` followed by a call to `new`
124+
--> $DIR/or_fun_call.rs:208:9
125+
|
126+
LL | / iter.map(|f: &String| f.to_lowercase())
127+
LL | | .reduce(|mut acc, f| {
128+
LL | | let _ = "";
129+
LL | | acc.push_str(&f);
130+
LL | | acc
131+
LL | | })
132+
LL | | .unwrap_or(String::new());
133+
| |_____________________________________^
134+
|
135+
help: try this
136+
|
137+
LL ~ iter.map(|f: &String| f.to_lowercase())
138+
LL + .reduce(|mut acc, f| {
139+
LL + let _ = "";
140+
LL + acc.push_str(&f);
141+
LL + acc
142+
LL ~ }).unwrap_or_default();
143+
|
144+
145+
error: use of `unwrap_or` followed by a call to `new`
146+
--> $DIR/or_fun_call.rs:221:9
147+
|
148+
LL | / map.reduce(|mut acc, f| {
149+
LL | | acc.push_str(&f);
150+
LL | | acc
151+
LL | | })
152+
LL | | .unwrap_or(String::new());
153+
| |_________________________________^
154+
|
155+
help: try this
156+
|
157+
LL ~ map.reduce(|mut acc, f| {
158+
LL + acc.push_str(&f);
159+
LL + acc
160+
LL ~ }).unwrap_or_default();
161+
|
162+
163+
error: aborting due to 22 previous errors
112164

0 commit comments

Comments
 (0)