Skip to content

Commit 8e988e1

Browse files
committed
Merge remote-tracking branch 'upstream/master' into sync-from-rust
2 parents a1bb10e + de83f09 commit 8e988e1

File tree

84 files changed

+2808
-679
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

84 files changed

+2808
-679
lines changed

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1632,6 +1632,7 @@ Released 2018-09-13
16321632
[`assign_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_ops
16331633
[`async_yields_async`]: https://rust-lang.github.io/rust-clippy/master/index.html#async_yields_async
16341634
[`await_holding_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock
1635+
[`await_holding_refcell_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_refcell_ref
16351636
[`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
16361637
[`bind_instead_of_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map
16371638
[`blacklisted_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#blacklisted_name
@@ -1779,6 +1780,7 @@ Released 2018-09-13
17791780
[`large_digit_groups`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_digit_groups
17801781
[`large_enum_variant`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant
17811782
[`large_stack_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_arrays
1783+
[`large_types_passed_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_types_passed_by_value
17821784
[`len_without_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_without_is_empty
17831785
[`len_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_zero
17841786
[`let_and_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
@@ -1793,6 +1795,7 @@ Released 2018-09-13
17931795
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
17941796
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
17951797
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
1798+
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
17961799
[`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
17971800
[`manual_strip`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_strip
17981801
[`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap
@@ -1841,6 +1844,7 @@ Released 2018-09-13
18411844
[`must_use_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#must_use_unit
18421845
[`mut_from_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#mut_from_ref
18431846
[`mut_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#mut_mut
1847+
[`mut_mutex_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#mut_mutex_lock
18441848
[`mut_range_bound`]: https://rust-lang.github.io/rust-clippy/master/index.html#mut_range_bound
18451849
[`mutable_key_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutable_key_type
18461850
[`mutex_atomic`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutex_atomic
@@ -1936,6 +1940,7 @@ Released 2018-09-13
19361940
[`single_char_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern
19371941
[`single_char_push_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_push_str
19381942
[`single_component_path_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports
1943+
[`single_element_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_element_loop
19391944
[`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match
19401945
[`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else
19411946
[`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next
@@ -1979,6 +1984,7 @@ Released 2018-09-13
19791984
[`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err
19801985
[`type_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
19811986
[`type_repetition_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds
1987+
[`undropped_manually_drops`]: https://rust-lang.github.io/rust-clippy/master/index.html#undropped_manually_drops
19821988
[`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc
19831989
[`unimplemented`]: https://rust-lang.github.io/rust-clippy/master/index.html#unimplemented
19841990
[`uninit_assumed_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_assumed_init
@@ -2011,6 +2017,7 @@ Released 2018-09-13
20112017
[`unused_label`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_label
20122018
[`unused_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_self
20132019
[`unused_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit
2020+
[`unusual_byte_groupings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unusual_byte_groupings
20142021
[`unwrap_in_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_in_result
20152022
[`unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used
20162023
[`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug

clippy_lints/src/await_holding_lock.rs renamed to clippy_lints/src/await_holding_invalid.rs

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,52 @@ declare_clippy_lint! {
4545
/// }
4646
/// ```
4747
pub AWAIT_HOLDING_LOCK,
48-
pedantic,
48+
correctness,
4949
"Inside an async function, holding a MutexGuard while calling await"
5050
}
5151

52-
declare_lint_pass!(AwaitHoldingLock => [AWAIT_HOLDING_LOCK]);
52+
declare_clippy_lint! {
53+
/// **What it does:** Checks for calls to await while holding a
54+
/// `RefCell` `Ref` or `RefMut`.
55+
///
56+
/// **Why is this bad?** `RefCell` refs only check for exclusive mutable access
57+
/// at runtime. Holding onto a `RefCell` ref across an `await` suspension point
58+
/// risks panics from a mutable ref shared while other refs are outstanding.
59+
///
60+
/// **Known problems:** None.
61+
///
62+
/// **Example:**
63+
///
64+
/// ```rust,ignore
65+
/// use std::cell::RefCell;
66+
///
67+
/// async fn foo(x: &RefCell<u32>) {
68+
/// let b = x.borrow_mut()();
69+
/// *ref += 1;
70+
/// bar.await;
71+
/// }
72+
/// ```
73+
///
74+
/// Use instead:
75+
/// ```rust,ignore
76+
/// use std::cell::RefCell;
77+
///
78+
/// async fn foo(x: &RefCell<u32>) {
79+
/// {
80+
/// let b = x.borrow_mut();
81+
/// *ref += 1;
82+
/// }
83+
/// bar.await;
84+
/// }
85+
/// ```
86+
pub AWAIT_HOLDING_REFCELL_REF,
87+
correctness,
88+
"Inside an async function, holding a RefCell ref while calling await"
89+
}
5390

54-
impl LateLintPass<'_> for AwaitHoldingLock {
91+
declare_lint_pass!(AwaitHolding => [AWAIT_HOLDING_LOCK, AWAIT_HOLDING_REFCELL_REF]);
92+
93+
impl LateLintPass<'_> for AwaitHolding {
5594
fn check_body(&mut self, cx: &LateContext<'_>, body: &'_ Body<'_>) {
5695
use AsyncGeneratorKind::{Block, Closure, Fn};
5796
if let Some(GeneratorKind::Async(Block | Closure | Fn)) = body.generator_kind {
@@ -78,6 +117,16 @@ fn check_interior_types(cx: &LateContext<'_>, ty_causes: &[GeneratorInteriorType
78117
"these are all the await points this lock is held through",
79118
);
80119
}
120+
if is_refcell_ref(cx, adt.did) {
121+
span_lint_and_note(
122+
cx,
123+
AWAIT_HOLDING_REFCELL_REF,
124+
ty_cause.span,
125+
"this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await.",
126+
ty_cause.scope_span.or(Some(span)),
127+
"these are all the await points this ref is held through",
128+
);
129+
}
81130
}
82131
}
83132
}
@@ -90,3 +139,7 @@ fn is_mutex_guard(cx: &LateContext<'_>, def_id: DefId) -> bool {
90139
|| match_def_path(cx, def_id, &paths::PARKING_LOT_RWLOCK_READ_GUARD)
91140
|| match_def_path(cx, def_id, &paths::PARKING_LOT_RWLOCK_WRITE_GUARD)
92141
}
142+
143+
fn is_refcell_ref(cx: &LateContext<'_>, def_id: DefId) -> bool {
144+
match_def_path(cx, def_id, &paths::REFCELL_REF) || match_def_path(cx, def_id, &paths::REFCELL_REFMUT)
145+
}

clippy_lints/src/blocks_in_if_conditions.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ declare_clippy_lint! {
2828
///
2929
/// ```rust
3030
/// # fn somefunc() -> bool { true };
31-
///
3231
/// // Bad
3332
/// if { let x = somefunc(); x } { /* ... */ }
3433
///

clippy_lints/src/copies.rs

Lines changed: 4 additions & 211 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,8 @@
1-
use crate::utils::{eq_expr_value, in_macro, SpanlessEq, SpanlessHash};
2-
use crate::utils::{get_parent_expr, higher, if_sequence, snippet, span_lint_and_note, span_lint_and_then};
3-
use rustc_data_structures::fx::FxHashMap;
4-
use rustc_hir::{Arm, Block, Expr, ExprKind, MatchSource, Pat, PatKind};
1+
use crate::utils::{eq_expr_value, in_macro, search_same, SpanlessEq, SpanlessHash};
2+
use crate::utils::{get_parent_expr, higher, if_sequence, span_lint_and_note};
3+
use rustc_hir::{Block, Expr};
54
use rustc_lint::{LateContext, LateLintPass};
6-
use rustc_middle::ty::{Ty, TyS};
75
use rustc_session::{declare_lint_pass, declare_tool_lint};
8-
use rustc_span::symbol::Symbol;
9-
use std::collections::hash_map::Entry;
10-
use std::hash::BuildHasherDefault;
116

127
declare_clippy_lint! {
138
/// **What it does:** Checks for consecutive `if`s with the same condition.
@@ -108,48 +103,7 @@ declare_clippy_lint! {
108103
"`if` with the same `then` and `else` blocks"
109104
}
110105

111-
declare_clippy_lint! {
112-
/// **What it does:** Checks for `match` with identical arm bodies.
113-
///
114-
/// **Why is this bad?** This is probably a copy & paste error. If arm bodies
115-
/// are the same on purpose, you can factor them
116-
/// [using `|`](https://doc.rust-lang.org/book/patterns.html#multiple-patterns).
117-
///
118-
/// **Known problems:** False positive possible with order dependent `match`
119-
/// (see issue
120-
/// [#860](https://github.com/rust-lang/rust-clippy/issues/860)).
121-
///
122-
/// **Example:**
123-
/// ```rust,ignore
124-
/// match foo {
125-
/// Bar => bar(),
126-
/// Quz => quz(),
127-
/// Baz => bar(), // <= oops
128-
/// }
129-
/// ```
130-
///
131-
/// This should probably be
132-
/// ```rust,ignore
133-
/// match foo {
134-
/// Bar => bar(),
135-
/// Quz => quz(),
136-
/// Baz => baz(), // <= fixed
137-
/// }
138-
/// ```
139-
///
140-
/// or if the original code was not a typo:
141-
/// ```rust,ignore
142-
/// match foo {
143-
/// Bar | Baz => bar(), // <= shows the intent better
144-
/// Quz => quz(),
145-
/// }
146-
/// ```
147-
pub MATCH_SAME_ARMS,
148-
pedantic,
149-
"`match` with identical arm bodies"
150-
}
151-
152-
declare_lint_pass!(CopyAndPaste => [IFS_SAME_COND, SAME_FUNCTIONS_IN_IF_CONDITION, IF_SAME_THEN_ELSE, MATCH_SAME_ARMS]);
106+
declare_lint_pass!(CopyAndPaste => [IFS_SAME_COND, SAME_FUNCTIONS_IN_IF_CONDITION, IF_SAME_THEN_ELSE]);
153107

154108
impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
155109
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
@@ -167,7 +121,6 @@ impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
167121
lint_same_then_else(cx, &blocks);
168122
lint_same_cond(cx, &conds);
169123
lint_same_fns_in_if_cond(cx, &conds);
170-
lint_match_arms(cx, expr);
171124
}
172125
}
173126
}
@@ -243,122 +196,6 @@ fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) {
243196
}
244197
}
245198

246-
/// Implementation of `MATCH_SAME_ARMS`.
247-
fn lint_match_arms<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) {
248-
fn same_bindings<'tcx>(lhs: &FxHashMap<Symbol, Ty<'tcx>>, rhs: &FxHashMap<Symbol, Ty<'tcx>>) -> bool {
249-
lhs.len() == rhs.len()
250-
&& lhs
251-
.iter()
252-
.all(|(name, l_ty)| rhs.get(name).map_or(false, |r_ty| TyS::same_type(l_ty, r_ty)))
253-
}
254-
255-
if let ExprKind::Match(_, ref arms, MatchSource::Normal) = expr.kind {
256-
let hash = |&(_, arm): &(usize, &Arm<'_>)| -> u64 {
257-
let mut h = SpanlessHash::new(cx);
258-
h.hash_expr(&arm.body);
259-
h.finish()
260-
};
261-
262-
let eq = |&(lindex, lhs): &(usize, &Arm<'_>), &(rindex, rhs): &(usize, &Arm<'_>)| -> bool {
263-
let min_index = usize::min(lindex, rindex);
264-
let max_index = usize::max(lindex, rindex);
265-
266-
// Arms with a guard are ignored, those can’t always be merged together
267-
// This is also the case for arms in-between each there is an arm with a guard
268-
(min_index..=max_index).all(|index| arms[index].guard.is_none()) &&
269-
SpanlessEq::new(cx).eq_expr(&lhs.body, &rhs.body) &&
270-
// all patterns should have the same bindings
271-
same_bindings(&bindings(cx, &lhs.pat), &bindings(cx, &rhs.pat))
272-
};
273-
274-
let indexed_arms: Vec<(usize, &Arm<'_>)> = arms.iter().enumerate().collect();
275-
for (&(_, i), &(_, j)) in search_same(&indexed_arms, hash, eq) {
276-
span_lint_and_then(
277-
cx,
278-
MATCH_SAME_ARMS,
279-
j.body.span,
280-
"this `match` has identical arm bodies",
281-
|diag| {
282-
diag.span_note(i.body.span, "same as this");
283-
284-
// Note: this does not use `span_suggestion` on purpose:
285-
// there is no clean way
286-
// to remove the other arm. Building a span and suggest to replace it to ""
287-
// makes an even more confusing error message. Also in order not to make up a
288-
// span for the whole pattern, the suggestion is only shown when there is only
289-
// one pattern. The user should know about `|` if they are already using it…
290-
291-
let lhs = snippet(cx, i.pat.span, "<pat1>");
292-
let rhs = snippet(cx, j.pat.span, "<pat2>");
293-
294-
if let PatKind::Wild = j.pat.kind {
295-
// if the last arm is _, then i could be integrated into _
296-
// note that i.pat cannot be _, because that would mean that we're
297-
// hiding all the subsequent arms, and rust won't compile
298-
diag.span_note(
299-
i.body.span,
300-
&format!(
301-
"`{}` has the same arm body as the `_` wildcard, consider removing it",
302-
lhs
303-
),
304-
);
305-
} else {
306-
diag.span_help(i.pat.span, &format!("consider refactoring into `{} | {}`", lhs, rhs));
307-
}
308-
},
309-
);
310-
}
311-
}
312-
}
313-
314-
/// Returns the list of bindings in a pattern.
315-
fn bindings<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'_>) -> FxHashMap<Symbol, Ty<'tcx>> {
316-
fn bindings_impl<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'_>, map: &mut FxHashMap<Symbol, Ty<'tcx>>) {
317-
match pat.kind {
318-
PatKind::Box(ref pat) | PatKind::Ref(ref pat, _) => bindings_impl(cx, pat, map),
319-
PatKind::TupleStruct(_, pats, _) => {
320-
for pat in pats {
321-
bindings_impl(cx, pat, map);
322-
}
323-
},
324-
PatKind::Binding(.., ident, ref as_pat) => {
325-
if let Entry::Vacant(v) = map.entry(ident.name) {
326-
v.insert(cx.typeck_results().pat_ty(pat));
327-
}
328-
if let Some(ref as_pat) = *as_pat {
329-
bindings_impl(cx, as_pat, map);
330-
}
331-
},
332-
PatKind::Or(fields) | PatKind::Tuple(fields, _) => {
333-
for pat in fields {
334-
bindings_impl(cx, pat, map);
335-
}
336-
},
337-
PatKind::Struct(_, fields, _) => {
338-
for pat in fields {
339-
bindings_impl(cx, &pat.pat, map);
340-
}
341-
},
342-
PatKind::Slice(lhs, ref mid, rhs) => {
343-
for pat in lhs {
344-
bindings_impl(cx, pat, map);
345-
}
346-
if let Some(ref mid) = *mid {
347-
bindings_impl(cx, mid, map);
348-
}
349-
for pat in rhs {
350-
bindings_impl(cx, pat, map);
351-
}
352-
},
353-
PatKind::Lit(..) | PatKind::Range(..) | PatKind::Wild | PatKind::Path(..) => (),
354-
}
355-
}
356-
357-
let mut result = FxHashMap::default();
358-
bindings_impl(cx, pat, &mut result);
359-
result
360-
}
361-
362199
fn search_same_sequenced<T, Eq>(exprs: &[T], eq: Eq) -> Option<(&T, &T)>
363200
where
364201
Eq: Fn(&T, &T) -> bool,
@@ -370,47 +207,3 @@ where
370207
}
371208
None
372209
}
373-
374-
fn search_common_cases<'a, T, Eq>(exprs: &'a [T], eq: &Eq) -> Option<(&'a T, &'a T)>
375-
where
376-
Eq: Fn(&T, &T) -> bool,
377-
{
378-
if exprs.len() == 2 && eq(&exprs[0], &exprs[1]) {
379-
Some((&exprs[0], &exprs[1]))
380-
} else {
381-
None
382-
}
383-
}
384-
385-
fn search_same<T, Hash, Eq>(exprs: &[T], hash: Hash, eq: Eq) -> Vec<(&T, &T)>
386-
where
387-
Hash: Fn(&T) -> u64,
388-
Eq: Fn(&T, &T) -> bool,
389-
{
390-
if let Some(expr) = search_common_cases(&exprs, &eq) {
391-
return vec![expr];
392-
}
393-
394-
let mut match_expr_list: Vec<(&T, &T)> = Vec::new();
395-
396-
let mut map: FxHashMap<_, Vec<&_>> =
397-
FxHashMap::with_capacity_and_hasher(exprs.len(), BuildHasherDefault::default());
398-
399-
for expr in exprs {
400-
match map.entry(hash(expr)) {
401-
Entry::Occupied(mut o) => {
402-
for o in o.get() {
403-
if eq(o, expr) {
404-
match_expr_list.push((o, expr));
405-
}
406-
}
407-
o.get_mut().push(expr);
408-
},
409-
Entry::Vacant(v) => {
410-
v.insert(vec![expr]);
411-
},
412-
}
413-
}
414-
415-
match_expr_list
416-
}

0 commit comments

Comments
 (0)