Skip to content

Commit eb0a288

Browse files
borsbors
committed
Auto merge of #4149 - flip1995:rollup-6knpdqb, r=flip1995
Rollup of 2 pull requests Successful merges: - #4102 (Fix match_same_arms to fail late) - #4119 (Improve non ascii literal) Failed merges: r? @ghost
2 parents cf81a3b + dce670c commit eb0a288

8 files changed

+241
-82
lines changed

‎clippy_lints/src/copies.rs

+26-11
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ fn lint_same_cond(cx: &LateContext<'_, '_>, conds: &[&Expr]) {
152152
let eq: &dyn Fn(&&Expr, &&Expr) -> bool =
153153
&|&lhs, &rhs| -> bool { SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, rhs) };
154154

155-
if let Some((i, j)) = search_same(conds, hash, eq) {
155+
for (i, j) in search_same(conds, hash, eq) {
156156
span_note_and_lint(
157157
cx,
158158
IFS_SAME_COND,
@@ -185,7 +185,7 @@ fn lint_match_arms(cx: &LateContext<'_, '_>, expr: &Expr) {
185185
};
186186

187187
let indexed_arms: Vec<(usize, &Arm)> = arms.iter().enumerate().collect();
188-
if let Some((&(_, i), &(_, j))) = search_same(&indexed_arms, hash, eq) {
188+
for (&(_, i), &(_, j)) in search_same(&indexed_arms, hash, eq) {
189189
span_lint_and_then(
190190
cx,
191191
MATCH_SAME_ARMS,
@@ -217,7 +217,10 @@ fn lint_match_arms(cx: &LateContext<'_, '_>, expr: &Expr) {
217217
),
218218
);
219219
} else {
220-
db.span_note(i.body.span, &format!("consider refactoring into `{} | {}`", lhs, rhs));
220+
db.span_help(
221+
i.pats[0].span,
222+
&format!("consider refactoring into `{} | {}`", lhs, rhs),
223+
);
221224
}
222225
}
223226
},
@@ -323,21 +326,33 @@ where
323326
None
324327
}
325328

326-
fn search_same<T, Hash, Eq>(exprs: &[T], hash: Hash, eq: Eq) -> Option<(&T, &T)>
329+
fn search_common_cases<'a, T, Eq>(exprs: &'a [T], eq: &Eq) -> Option<(&'a T, &'a T)>
327330
where
328-
Hash: Fn(&T) -> u64,
329331
Eq: Fn(&T, &T) -> bool,
330332
{
331-
// common cases
332333
if exprs.len() < 2 {
333-
return None;
334+
None
334335
} else if exprs.len() == 2 {
335-
return if eq(&exprs[0], &exprs[1]) {
336+
if eq(&exprs[0], &exprs[1]) {
336337
Some((&exprs[0], &exprs[1]))
337338
} else {
338339
None
339-
};
340+
}
341+
} else {
342+
None
340343
}
344+
}
345+
346+
fn search_same<T, Hash, Eq>(exprs: &[T], hash: Hash, eq: Eq) -> Vec<(&T, &T)>
347+
where
348+
Hash: Fn(&T) -> u64,
349+
Eq: Fn(&T, &T) -> bool,
350+
{
351+
if let Some(expr) = search_common_cases(&exprs, &eq) {
352+
return vec![expr];
353+
}
354+
355+
let mut match_expr_list: Vec<(&T, &T)> = Vec::new();
341356

342357
let mut map: FxHashMap<_, Vec<&_>> =
343358
FxHashMap::with_capacity_and_hasher(exprs.len(), BuildHasherDefault::default());
@@ -347,7 +362,7 @@ where
347362
Entry::Occupied(mut o) => {
348363
for o in o.get() {
349364
if eq(o, expr) {
350-
return Some((o, expr));
365+
match_expr_list.push((o, expr));
351366
}
352367
}
353368
o.get_mut().push(expr);
@@ -358,5 +373,5 @@ where
358373
}
359374
}
360375

361-
None
376+
match_expr_list
362377
}

‎clippy_lints/src/unicode.rs

+23-21
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
use crate::utils::{is_allowed, snippet, span_help_and_lint};
1+
use crate::utils::{is_allowed, snippet, span_lint_and_sugg};
22
use rustc::hir::*;
33
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
44
use rustc::{declare_lint_pass, declare_tool_lint};
5+
use rustc_errors::Applicability;
56
use syntax::ast::LitKind;
67
use syntax::source_map::Span;
78
use unicode_normalization::UnicodeNormalization;
@@ -34,7 +35,11 @@ declare_clippy_lint! {
3435
///
3536
/// **Example:**
3637
/// ```rust
37-
/// let x = "Hä?"
38+
/// let x = String::from("€");
39+
/// ```
40+
/// Could be written as:
41+
/// ```rust
42+
/// let x = String::from("\u{20ac}");
3843
/// ```
3944
pub NON_ASCII_LITERAL,
4045
pedantic,
@@ -87,43 +92,40 @@ fn escape<T: Iterator<Item = char>>(s: T) -> String {
8792
fn check_str(cx: &LateContext<'_, '_>, span: Span, id: HirId) {
8893
let string = snippet(cx, span, "");
8994
if string.contains('\u{200B}') {
90-
span_help_and_lint(
95+
span_lint_and_sugg(
9196
cx,
9297
ZERO_WIDTH_SPACE,
9398
span,
9499
"zero-width space detected",
95-
&format!(
96-
"Consider replacing the string with:\n\"{}\"",
97-
string.replace("\u{200B}", "\\u{200B}")
98-
),
100+
"consider replacing the string with",
101+
string.replace("\u{200B}", "\\u{200B}"),
102+
Applicability::MachineApplicable,
99103
);
100104
}
101105
if string.chars().any(|c| c as u32 > 0x7F) {
102-
span_help_and_lint(
106+
span_lint_and_sugg(
103107
cx,
104108
NON_ASCII_LITERAL,
105109
span,
106110
"literal non-ASCII character detected",
107-
&format!(
108-
"Consider replacing the string with:\n\"{}\"",
109-
if is_allowed(cx, UNICODE_NOT_NFC, id) {
110-
escape(string.chars())
111-
} else {
112-
escape(string.nfc())
113-
}
114-
),
111+
"consider replacing the string with",
112+
if is_allowed(cx, UNICODE_NOT_NFC, id) {
113+
escape(string.chars())
114+
} else {
115+
escape(string.nfc())
116+
},
117+
Applicability::MachineApplicable,
115118
);
116119
}
117120
if is_allowed(cx, NON_ASCII_LITERAL, id) && string.chars().zip(string.nfc()).any(|(a, b)| a != b) {
118-
span_help_and_lint(
121+
span_lint_and_sugg(
119122
cx,
120123
UNICODE_NOT_NFC,
121124
span,
122125
"non-nfc unicode sequence detected",
123-
&format!(
124-
"Consider replacing the string with:\n\"{}\"",
125-
string.nfc().collect::<String>()
126-
),
126+
"consider replacing the string with",
127+
string.nfc().collect::<String>(),
128+
Applicability::MachineApplicable,
127129
);
128130
}
129131
}

‎tests/ui/if_same_then_else.rs

+14
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,20 @@ fn if_same_then_else() -> Result<&'static str, ()> {
232232
return Ok(&foo[0..]);
233233
}
234234

235+
if true {
236+
let foo = "";
237+
return Ok(&foo[0..]);
238+
} else if false {
239+
let foo = "bar";
240+
return Ok(&foo[0..]);
241+
} else if true {
242+
let foo = "";
243+
return Ok(&foo[0..]);
244+
} else {
245+
let foo = "";
246+
return Ok(&foo[0..]);
247+
}
248+
235249
// False positive `if_same_then_else`: `let (x, y)` vs. `let (y, x)`; see issue #3559.
236250
if true {
237251
let foo = "";

‎tests/ui/if_same_then_else.stderr

+34-1
Original file line numberDiff line numberDiff line change
@@ -210,5 +210,38 @@ LL | | try!(Ok("foo"));
210210
LL | | } else {
211211
| |_____^
212212

213-
error: aborting due to 10 previous errors
213+
error: this `if` has identical blocks
214+
--> $DIR/if_same_then_else.rs:244:12
215+
|
216+
LL | } else {
217+
| ____________^
218+
LL | | let foo = "";
219+
LL | | return Ok(&foo[0..]);
220+
LL | | }
221+
| |_____^
222+
|
223+
note: same as this
224+
--> $DIR/if_same_then_else.rs:241:20
225+
|
226+
LL | } else if true {
227+
| ____________________^
228+
LL | | let foo = "";
229+
LL | | return Ok(&foo[0..]);
230+
LL | | } else {
231+
| |_____^
232+
233+
error: this `if` has the same condition as a previous if
234+
--> $DIR/if_same_then_else.rs:241:15
235+
|
236+
LL | } else if true {
237+
| ^^^^
238+
|
239+
= note: #[deny(clippy::ifs_same_cond)] on by default
240+
note: same as this
241+
--> $DIR/if_same_then_else.rs:235:8
242+
|
243+
LL | if true {
244+
| ^^^^
245+
246+
error: aborting due to 12 previous errors
214247

‎tests/ui/match_same_arms.rs

+16
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,22 @@ fn match_same_arms() {
108108
(None, Some(a)) => bar(a), // bindings have different types
109109
_ => (),
110110
}
111+
112+
let _ = match 42 {
113+
42 => 1,
114+
51 => 1, //~ ERROR match arms have same body
115+
41 => 2,
116+
52 => 2, //~ ERROR match arms have same body
117+
_ => 0,
118+
};
119+
120+
let _ = match 42 {
121+
1 => 2,
122+
2 => 2, //~ ERROR 2nd matched arms have same body
123+
3 => 2, //~ ERROR 3rd matched arms have same body
124+
4 => 3,
125+
_ => 0,
126+
};
111127
}
112128

113129
fn main() {}

0 commit comments

Comments
 (0)