Merge commit '43a1777b89cf6791f9e20878b4e5e3ae907867a5' into clippyup

This commit is contained in:
flip1995 2020-05-11 20:23:47 +02:00
parent 51158cc71f
commit d13d8987b0
34 changed files with 1573 additions and 256 deletions

View file

@ -1422,7 +1422,9 @@ Released 2018-09-13
[`lossy_float_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#lossy_float_literal [`lossy_float_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#lossy_float_literal
[`macro_use_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_use_imports [`macro_use_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_use_imports
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion [`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy [`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
[`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic [`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
[`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap [`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap
[`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names [`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names

View file

@ -31,7 +31,6 @@ path = "src/driver.rs"
# begin automatic update # begin automatic update
clippy_lints = { version = "0.0.212", path = "clippy_lints" } clippy_lints = { version = "0.0.212", path = "clippy_lints" }
# end automatic update # end automatic update
regex = "1"
semver = "0.9" semver = "0.9"
rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"} rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"}
tempfile = { version = "3.1.0", optional = true } tempfile = { version = "3.1.0", optional = true }

View file

@ -247,6 +247,8 @@ mod literal_representation;
mod loops; mod loops;
mod macro_use; mod macro_use;
mod main_recursion; mod main_recursion;
mod manual_async_fn;
mod manual_non_exhaustive;
mod map_clone; mod map_clone;
mod map_unit_fn; mod map_unit_fn;
mod match_on_vec_items; mod match_on_vec_items;
@ -628,6 +630,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&loops::WHILE_LET_ON_ITERATOR, &loops::WHILE_LET_ON_ITERATOR,
&macro_use::MACRO_USE_IMPORTS, &macro_use::MACRO_USE_IMPORTS,
&main_recursion::MAIN_RECURSION, &main_recursion::MAIN_RECURSION,
&manual_async_fn::MANUAL_ASYNC_FN,
&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
&map_clone::MAP_CLONE, &map_clone::MAP_CLONE,
&map_unit_fn::OPTION_MAP_UNIT_FN, &map_unit_fn::OPTION_MAP_UNIT_FN,
&map_unit_fn::RESULT_MAP_UNIT_FN, &map_unit_fn::RESULT_MAP_UNIT_FN,
@ -1054,7 +1058,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
let max_struct_bools = conf.max_struct_bools; let max_struct_bools = conf.max_struct_bools;
store.register_early_pass(move || box excessive_bools::ExcessiveBools::new(max_struct_bools, max_fn_params_bools)); store.register_early_pass(move || box excessive_bools::ExcessiveBools::new(max_struct_bools, max_fn_params_bools));
store.register_early_pass(|| box option_env_unwrap::OptionEnvUnwrap); store.register_early_pass(|| box option_env_unwrap::OptionEnvUnwrap);
store.register_late_pass(|| box wildcard_imports::WildcardImports); let warn_on_all_wildcard_imports = conf.warn_on_all_wildcard_imports;
store.register_late_pass(move || box wildcard_imports::WildcardImports::new(warn_on_all_wildcard_imports));
store.register_early_pass(|| box macro_use::MacroUseImports); store.register_early_pass(|| box macro_use::MacroUseImports);
store.register_late_pass(|| box verbose_file_reads::VerboseFileReads); store.register_late_pass(|| box verbose_file_reads::VerboseFileReads);
store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default()); store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default());
@ -1064,6 +1069,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls); store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls);
store.register_late_pass(|| box if_let_mutex::IfLetMutex); store.register_late_pass(|| box if_let_mutex::IfLetMutex);
store.register_late_pass(|| box match_on_vec_items::MatchOnVecItems); store.register_late_pass(|| box match_on_vec_items::MatchOnVecItems);
store.register_early_pass(|| box manual_non_exhaustive::ManualNonExhaustive);
store.register_late_pass(|| box manual_async_fn::ManualAsyncFn);
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC), LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@ -1138,6 +1145,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&loops::EXPLICIT_INTO_ITER_LOOP), LintId::of(&loops::EXPLICIT_INTO_ITER_LOOP),
LintId::of(&loops::EXPLICIT_ITER_LOOP), LintId::of(&loops::EXPLICIT_ITER_LOOP),
LintId::of(&macro_use::MACRO_USE_IMPORTS), LintId::of(&macro_use::MACRO_USE_IMPORTS),
LintId::of(&match_on_vec_items::MATCH_ON_VEC_ITEMS),
LintId::of(&matches::MATCH_BOOL), LintId::of(&matches::MATCH_BOOL),
LintId::of(&matches::SINGLE_MATCH_ELSE), LintId::of(&matches::SINGLE_MATCH_ELSE),
LintId::of(&methods::FILTER_MAP), LintId::of(&methods::FILTER_MAP),
@ -1280,10 +1288,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&loops::WHILE_LET_LOOP), LintId::of(&loops::WHILE_LET_LOOP),
LintId::of(&loops::WHILE_LET_ON_ITERATOR), LintId::of(&loops::WHILE_LET_ON_ITERATOR),
LintId::of(&main_recursion::MAIN_RECURSION), LintId::of(&main_recursion::MAIN_RECURSION),
LintId::of(&manual_async_fn::MANUAL_ASYNC_FN),
LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
LintId::of(&map_clone::MAP_CLONE), LintId::of(&map_clone::MAP_CLONE),
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN), LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
LintId::of(&match_on_vec_items::MATCH_ON_VEC_ITEMS),
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
LintId::of(&matches::MATCH_AS_REF), LintId::of(&matches::MATCH_AS_REF),
LintId::of(&matches::MATCH_OVERLAPPING_ARM), LintId::of(&matches::MATCH_OVERLAPPING_ARM),
@ -1474,6 +1483,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&loops::NEEDLESS_RANGE_LOOP), LintId::of(&loops::NEEDLESS_RANGE_LOOP),
LintId::of(&loops::WHILE_LET_ON_ITERATOR), LintId::of(&loops::WHILE_LET_ON_ITERATOR),
LintId::of(&main_recursion::MAIN_RECURSION), LintId::of(&main_recursion::MAIN_RECURSION),
LintId::of(&manual_async_fn::MANUAL_ASYNC_FN),
LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
LintId::of(&map_clone::MAP_CLONE), LintId::of(&map_clone::MAP_CLONE),
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
LintId::of(&matches::MATCH_OVERLAPPING_ARM), LintId::of(&matches::MATCH_OVERLAPPING_ARM),
@ -1647,7 +1658,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&loops::NEVER_LOOP), LintId::of(&loops::NEVER_LOOP),
LintId::of(&loops::REVERSE_RANGE_LOOP), LintId::of(&loops::REVERSE_RANGE_LOOP),
LintId::of(&loops::WHILE_IMMUTABLE_CONDITION), LintId::of(&loops::WHILE_IMMUTABLE_CONDITION),
LintId::of(&match_on_vec_items::MATCH_ON_VEC_ITEMS),
LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM), LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM),
LintId::of(&mem_replace::MEM_REPLACE_WITH_UNINIT), LintId::of(&mem_replace::MEM_REPLACE_WITH_UNINIT),
LintId::of(&methods::CLONE_DOUBLE_REF), LintId::of(&methods::CLONE_DOUBLE_REF),

View file

@ -10,7 +10,6 @@ use crate::utils::{
}; };
use crate::utils::{is_type_diagnostic_item, qpath_res, same_tys, sext, sugg}; use crate::utils::{is_type_diagnostic_item, qpath_res, same_tys, sext, sugg};
use if_chain::if_chain; use if_chain::if_chain;
use itertools::Itertools;
use rustc_ast::ast; use rustc_ast::ast;
use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_errors::Applicability; use rustc_errors::Applicability;
@ -772,40 +771,48 @@ fn check_for_loop<'a, 'tcx>(
fn same_var<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &Expr<'_>, var: HirId) -> bool { fn same_var<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &Expr<'_>, var: HirId) -> bool {
if_chain! { if_chain! {
if let ExprKind::Path(ref qpath) = expr.kind; if let ExprKind::Path(qpath) = &expr.kind;
if let QPath::Resolved(None, ref path) = *qpath; if let QPath::Resolved(None, path) = qpath;
if path.segments.len() == 1; if path.segments.len() == 1;
if let Res::Local(local_id) = qpath_res(cx, qpath, expr.hir_id); if let Res::Local(local_id) = qpath_res(cx, qpath, expr.hir_id);
// our variable!
if local_id == var;
then { then {
return true; // our variable!
local_id == var
} else {
false
} }
} }
}
false #[derive(Clone, Copy)]
enum OffsetSign {
Positive,
Negative,
} }
struct Offset { struct Offset {
value: String, value: String,
negate: bool, sign: OffsetSign,
} }
impl Offset { impl Offset {
fn negative(s: String) -> Self { fn negative(value: String) -> Self {
Self { value: s, negate: true } Self {
value,
sign: OffsetSign::Negative,
}
} }
fn positive(s: String) -> Self { fn positive(value: String) -> Self {
Self { Self {
value: s, value,
negate: false, sign: OffsetSign::Positive,
} }
} }
} }
struct FixedOffsetVar { struct FixedOffsetVar<'hir> {
var_name: String, var: &'hir Expr<'hir>,
offset: Offset, offset: Offset,
} }
@ -819,10 +826,20 @@ fn is_slice_like<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'_>) -> bool {
is_slice || is_type_diagnostic_item(cx, ty, sym!(vec_type)) || is_type_diagnostic_item(cx, ty, sym!(vecdeque_type)) is_slice || is_type_diagnostic_item(cx, ty, sym!(vec_type)) || is_type_diagnostic_item(cx, ty, sym!(vecdeque_type))
} }
fn get_fixed_offset_var<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &Expr<'_>, var: HirId) -> Option<FixedOffsetVar> { fn fetch_cloned_expr<'tcx>(expr: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> {
if_chain! {
if let ExprKind::MethodCall(method, _, args) = expr.kind;
if method.ident.name == sym!(clone);
if args.len() == 1;
if let Some(arg) = args.get(0);
then { arg } else { expr }
}
}
fn get_offset<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, idx: &Expr<'_>, var: HirId) -> Option<Offset> {
fn extract_offset<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, e: &Expr<'_>, var: HirId) -> Option<String> { fn extract_offset<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, e: &Expr<'_>, var: HirId) -> Option<String> {
match e.kind { match &e.kind {
ExprKind::Lit(ref l) => match l.node { ExprKind::Lit(l) => match l.node {
ast::LitKind::Int(x, _ty) => Some(x.to_string()), ast::LitKind::Int(x, _ty) => Some(x.to_string()),
_ => None, _ => None,
}, },
@ -831,115 +848,139 @@ fn get_fixed_offset_var<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &Expr<'_>, v
} }
} }
if let ExprKind::Index(ref seqexpr, ref idx) = expr.kind { match idx.kind {
let ty = cx.tables.expr_ty(seqexpr); ExprKind::Binary(op, lhs, rhs) => match op.node {
if !is_slice_like(cx, ty) { BinOpKind::Add => {
return None; let offset_opt = if same_var(cx, lhs, var) {
} extract_offset(cx, rhs, var)
} else if same_var(cx, rhs, var) {
let offset = match idx.kind { extract_offset(cx, lhs, var)
ExprKind::Binary(op, ref lhs, ref rhs) => match op.node {
BinOpKind::Add => {
let offset_opt = if same_var(cx, lhs, var) {
extract_offset(cx, rhs, var)
} else if same_var(cx, rhs, var) {
extract_offset(cx, lhs, var)
} else {
None
};
offset_opt.map(Offset::positive)
},
BinOpKind::Sub if same_var(cx, lhs, var) => extract_offset(cx, rhs, var).map(Offset::negative),
_ => None,
},
ExprKind::Path(..) => {
if same_var(cx, idx, var) {
Some(Offset::positive("0".into()))
} else { } else {
None None
} };
offset_opt.map(Offset::positive)
}, },
BinOpKind::Sub if same_var(cx, lhs, var) => extract_offset(cx, rhs, var).map(Offset::negative),
_ => None, _ => None,
}; },
ExprKind::Path(..) if same_var(cx, idx, var) => Some(Offset::positive("0".into())),
offset.map(|o| FixedOffsetVar { _ => None,
var_name: snippet_opt(cx, seqexpr.span).unwrap_or_else(|| "???".into()),
offset: o,
})
} else {
None
} }
} }
fn fetch_cloned_fixed_offset_var<'a, 'tcx>( fn get_assignments<'tcx>(body: &'tcx Expr<'tcx>) -> impl Iterator<Item = Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>> {
cx: &LateContext<'a, 'tcx>, fn get_assignment<'tcx>(e: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> {
expr: &Expr<'_>, if let ExprKind::Assign(lhs, rhs, _) = e.kind {
var: HirId, Some((lhs, rhs))
) -> Option<FixedOffsetVar> {
if_chain! {
if let ExprKind::MethodCall(ref method, _, ref args) = expr.kind;
if method.ident.name == sym!(clone);
if args.len() == 1;
if let Some(arg) = args.get(0);
then {
return get_fixed_offset_var(cx, arg, var);
}
}
get_fixed_offset_var(cx, expr, var)
}
fn get_indexed_assignments<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
body: &Expr<'_>,
var: HirId,
) -> Vec<(FixedOffsetVar, FixedOffsetVar)> {
fn get_assignment<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
e: &Expr<'_>,
var: HirId,
) -> Option<(FixedOffsetVar, FixedOffsetVar)> {
if let ExprKind::Assign(ref lhs, ref rhs, _) = e.kind {
match (
get_fixed_offset_var(cx, lhs, var),
fetch_cloned_fixed_offset_var(cx, rhs, var),
) {
(Some(offset_left), Some(offset_right)) => {
// Source and destination must be different
if offset_left.var_name == offset_right.var_name {
None
} else {
Some((offset_left, offset_right))
}
},
_ => None,
}
} else { } else {
None None
} }
} }
if let ExprKind::Block(ref b, _) = body.kind { // This is one of few ways to return different iterators
let Block { // derived from: https://stackoverflow.com/questions/29760668/conditionally-iterate-over-one-of-several-possible-iterators/52064434#52064434
ref stmts, ref expr, .. let mut iter_a = None;
} = **b; let mut iter_b = None;
stmts if let ExprKind::Block(b, _) = body.kind {
let Block { stmts, expr, .. } = *b;
iter_a = stmts
.iter() .iter()
.map(|stmt| match stmt.kind { .filter_map(|stmt| match stmt.kind {
StmtKind::Local(..) | StmtKind::Item(..) => None, StmtKind::Local(..) | StmtKind::Item(..) => None,
StmtKind::Expr(ref e) | StmtKind::Semi(ref e) => Some(get_assignment(cx, e, var)), StmtKind::Expr(e) | StmtKind::Semi(e) => Some(e),
}) })
.chain(expr.as_ref().into_iter().map(|e| Some(get_assignment(cx, &*e, var)))) .chain(expr.into_iter())
.filter_map(|op| op) .map(get_assignment)
.collect::<Option<Vec<_>>>() .into()
.unwrap_or_default()
} else { } else {
get_assignment(cx, body, var).into_iter().collect() iter_b = Some(get_assignment(body))
} }
iter_a.into_iter().flatten().chain(iter_b.into_iter())
} }
fn build_manual_memcpy_suggestion<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
start: &Expr<'_>,
end: &Expr<'_>,
limits: ast::RangeLimits,
dst_var: FixedOffsetVar<'_>,
src_var: FixedOffsetVar<'_>,
) -> String {
fn print_sum(arg1: &str, arg2: &Offset) -> String {
match (arg1, &arg2.value[..], arg2.sign) {
("0", "0", _) => "0".into(),
("0", x, OffsetSign::Positive) | (x, "0", _) => x.into(),
("0", x, OffsetSign::Negative) => format!("-{}", x),
(x, y, OffsetSign::Positive) => format!("({} + {})", x, y),
(x, y, OffsetSign::Negative) => {
if x == y {
"0".into()
} else {
format!("({} - {})", x, y)
}
},
}
}
fn print_offset(start_str: &str, inline_offset: &Offset) -> String {
let offset = print_sum(start_str, inline_offset);
if offset.as_str() == "0" {
"".into()
} else {
offset
}
}
let print_limit = |end: &Expr<'_>, offset: Offset, var: &Expr<'_>| {
if_chain! {
if let ExprKind::MethodCall(method, _, len_args) = end.kind;
if method.ident.name == sym!(len);
if len_args.len() == 1;
if let Some(arg) = len_args.get(0);
if var_def_id(cx, arg) == var_def_id(cx, var);
then {
match offset.sign {
OffsetSign::Negative => format!("({} - {})", snippet(cx, end.span, "<src>.len()"), offset.value),
OffsetSign::Positive => "".into(),
}
} else {
let end_str = match limits {
ast::RangeLimits::Closed => {
let end = sugg::Sugg::hir(cx, end, "<count>");
format!("{}", end + sugg::ONE)
},
ast::RangeLimits::HalfOpen => format!("{}", snippet(cx, end.span, "..")),
};
print_sum(&end_str, &offset)
}
}
};
let start_str = snippet(cx, start.span, "").to_string();
let dst_offset = print_offset(&start_str, &dst_var.offset);
let dst_limit = print_limit(end, dst_var.offset, dst_var.var);
let src_offset = print_offset(&start_str, &src_var.offset);
let src_limit = print_limit(end, src_var.offset, src_var.var);
let dst_var_name = snippet_opt(cx, dst_var.var.span).unwrap_or_else(|| "???".into());
let src_var_name = snippet_opt(cx, src_var.var.span).unwrap_or_else(|| "???".into());
let dst = if dst_offset == "" && dst_limit == "" {
dst_var_name
} else {
format!("{}[{}..{}]", dst_var_name, dst_offset, dst_limit)
};
format!(
"{}.clone_from_slice(&{}[{}..{}])",
dst, src_var_name, src_offset, src_limit
)
}
/// Checks for for loops that sequentially copy items from one slice-like /// Checks for for loops that sequentially copy items from one slice-like
/// object to another. /// object to another.
fn detect_manual_memcpy<'a, 'tcx>( fn detect_manual_memcpy<'a, 'tcx>(
@ -951,93 +992,43 @@ fn detect_manual_memcpy<'a, 'tcx>(
) { ) {
if let Some(higher::Range { if let Some(higher::Range {
start: Some(start), start: Some(start),
ref end, end: Some(end),
limits, limits,
}) = higher::range(cx, arg) }) = higher::range(cx, arg)
{ {
// the var must be a single name // the var must be a single name
if let PatKind::Binding(_, canonical_id, _, _) = pat.kind { if let PatKind::Binding(_, canonical_id, _, _) = pat.kind {
let print_sum = |arg1: &Offset, arg2: &Offset| -> String {
match (&arg1.value[..], arg1.negate, &arg2.value[..], arg2.negate) {
("0", _, "0", _) => "".into(),
("0", _, x, false) | (x, false, "0", false) => x.into(),
("0", _, x, true) | (x, false, "0", true) => format!("-{}", x),
(x, false, y, false) => format!("({} + {})", x, y),
(x, false, y, true) => {
if x == y {
"0".into()
} else {
format!("({} - {})", x, y)
}
},
(x, true, y, false) => {
if x == y {
"0".into()
} else {
format!("({} - {})", y, x)
}
},
(x, true, y, true) => format!("-({} + {})", x, y),
}
};
let print_limit = |end: &Option<&Expr<'_>>, offset: Offset, var_name: &str| {
if let Some(end) = *end {
if_chain! {
if let ExprKind::MethodCall(ref method, _, ref len_args) = end.kind;
if method.ident.name == sym!(len);
if len_args.len() == 1;
if let Some(arg) = len_args.get(0);
if snippet(cx, arg.span, "??") == var_name;
then {
return if offset.negate {
format!("({} - {})", snippet(cx, end.span, "<src>.len()"), offset.value)
} else {
String::new()
};
}
}
let end_str = match limits {
ast::RangeLimits::Closed => {
let end = sugg::Sugg::hir(cx, end, "<count>");
format!("{}", end + sugg::ONE)
},
ast::RangeLimits::HalfOpen => format!("{}", snippet(cx, end.span, "..")),
};
print_sum(&Offset::positive(end_str), &offset)
} else {
"..".into()
}
};
// The only statements in the for loops can be indexed assignments from // The only statements in the for loops can be indexed assignments from
// indexed retrievals. // indexed retrievals.
let manual_copies = get_indexed_assignments(cx, body, canonical_id); let big_sugg = get_assignments(body)
.map(|o| {
o.and_then(|(lhs, rhs)| {
let rhs = fetch_cloned_expr(rhs);
if_chain! {
if let ExprKind::Index(seqexpr_left, idx_left) = lhs.kind;
if let ExprKind::Index(seqexpr_right, idx_right) = rhs.kind;
if is_slice_like(cx, cx.tables.expr_ty(seqexpr_left))
&& is_slice_like(cx, cx.tables.expr_ty(seqexpr_right));
if let Some(offset_left) = get_offset(cx, &idx_left, canonical_id);
if let Some(offset_right) = get_offset(cx, &idx_right, canonical_id);
let big_sugg = manual_copies // Source and destination must be different
.into_iter() if var_def_id(cx, seqexpr_left) != var_def_id(cx, seqexpr_right);
.map(|(dst_var, src_var)| { then {
let start_str = Offset::positive(snippet(cx, start.span, "").to_string()); Some((FixedOffsetVar { var: seqexpr_left, offset: offset_left },
let dst_offset = print_sum(&start_str, &dst_var.offset); FixedOffsetVar { var: seqexpr_right, offset: offset_right }))
let dst_limit = print_limit(end, dst_var.offset, &dst_var.var_name); } else {
let src_offset = print_sum(&start_str, &src_var.offset); None
let src_limit = print_limit(end, src_var.offset, &src_var.var_name); }
let dst = if dst_offset == "" && dst_limit == "" { }
dst_var.var_name })
} else {
format!("{}[{}..{}]", dst_var.var_name, dst_offset, dst_limit)
};
format!(
"{}.clone_from_slice(&{}[{}..{}])",
dst, src_var.var_name, src_offset, src_limit
)
}) })
.join("\n "); .map(|o| o.map(|(dst, src)| build_manual_memcpy_suggestion(cx, start, end, limits, dst, src)))
.collect::<Option<Vec<_>>>()
.filter(|v| !v.is_empty())
.map(|v| v.join("\n "));
if !big_sugg.is_empty() { if let Some(big_sugg) = big_sugg {
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
MANUAL_MEMCPY, MANUAL_MEMCPY,

View file

@ -0,0 +1,159 @@
use crate::utils::paths::FUTURE_FROM_GENERATOR;
use crate::utils::{match_function_call, snippet_block, snippet_opt, span_lint_and_then};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::FnKind;
use rustc_hir::{
AsyncGeneratorKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, GeneratorKind, GenericBound, HirId, IsAsync,
ItemKind, TraitRef, Ty, TyKind, TypeBindingKind,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::Span;
declare_clippy_lint! {
/// **What it does:** It checks for manual implementations of `async` functions.
///
/// **Why is this bad?** It's more idiomatic to use the dedicated syntax.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// use std::future::Future;
///
/// fn foo() -> impl Future<Output = i32> { async { 42 } }
/// ```
/// Use instead:
/// ```rust
/// use std::future::Future;
///
/// async fn foo() -> i32 { 42 }
/// ```
pub MANUAL_ASYNC_FN,
style,
"manual implementations of `async` functions can be simplified using the dedicated syntax"
}
declare_lint_pass!(ManualAsyncFn => [MANUAL_ASYNC_FN]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ManualAsyncFn {
fn check_fn(
&mut self,
cx: &LateContext<'a, 'tcx>,
kind: FnKind<'tcx>,
decl: &'tcx FnDecl<'_>,
body: &'tcx Body<'_>,
span: Span,
_: HirId,
) {
if_chain! {
if let Some(header) = kind.header();
if let IsAsync::NotAsync = header.asyncness;
// Check that this function returns `impl Future`
if let FnRetTy::Return(ret_ty) = decl.output;
if let Some(trait_ref) = future_trait_ref(cx, ret_ty);
if let Some(output) = future_output_ty(trait_ref);
// Check that the body of the function consists of one async block
if let ExprKind::Block(block, _) = body.value.kind;
if block.stmts.is_empty();
if let Some(closure_body) = desugared_async_block(cx, block);
then {
let header_span = span.with_hi(ret_ty.span.hi());
span_lint_and_then(
cx,
MANUAL_ASYNC_FN,
header_span,
"this function can be simplified using the `async fn` syntax",
|diag| {
if_chain! {
if let Some(header_snip) = snippet_opt(cx, header_span);
if let Some(ret_pos) = header_snip.rfind("->");
if let Some((ret_sugg, ret_snip)) = suggested_ret(cx, output);
then {
let help = format!("make the function `async` and {}", ret_sugg);
diag.span_suggestion(
header_span,
&help,
format!("async {}{}", &header_snip[..ret_pos], ret_snip),
Applicability::MachineApplicable
);
let body_snip = snippet_block(cx, closure_body.value.span, "..", Some(block.span));
diag.span_suggestion(
block.span,
"move the body of the async block to the enclosing function",
body_snip.to_string(),
Applicability::MachineApplicable
);
}
}
},
);
}
}
}
}
fn future_trait_ref<'tcx>(cx: &LateContext<'_, 'tcx>, ty: &'tcx Ty<'tcx>) -> Option<&'tcx TraitRef<'tcx>> {
if_chain! {
if let TyKind::Def(item_id, _) = ty.kind;
let item = cx.tcx.hir().item(item_id.id);
if let ItemKind::OpaqueTy(opaque) = &item.kind;
if opaque.bounds.len() == 1;
if let GenericBound::Trait(poly, _) = &opaque.bounds[0];
if poly.trait_ref.trait_def_id() == cx.tcx.lang_items().future_trait();
then {
return Some(&poly.trait_ref);
}
}
None
}
fn future_output_ty<'tcx>(trait_ref: &'tcx TraitRef<'tcx>) -> Option<&'tcx Ty<'tcx>> {
if_chain! {
if let Some(segment) = trait_ref.path.segments.last();
if let Some(args) = segment.args;
if args.bindings.len() == 1;
let binding = &args.bindings[0];
if binding.ident.as_str() == "Output";
if let TypeBindingKind::Equality{ty: output} = binding.kind;
then {
return Some(output)
}
}
None
}
fn desugared_async_block<'tcx>(cx: &LateContext<'_, 'tcx>, block: &'tcx Block<'tcx>) -> Option<&'tcx Body<'tcx>> {
if_chain! {
if let Some(block_expr) = block.expr;
if let Some(args) = match_function_call(cx, block_expr, &FUTURE_FROM_GENERATOR);
if args.len() == 1;
if let Expr{kind: ExprKind::Closure(_, _, body_id, ..), ..} = args[0];
let closure_body = cx.tcx.hir().body(body_id);
if let Some(GeneratorKind::Async(AsyncGeneratorKind::Block)) = closure_body.generator_kind;
then {
return Some(closure_body);
}
}
None
}
fn suggested_ret(cx: &LateContext<'_, '_>, output: &Ty<'_>) -> Option<(&'static str, String)> {
match output.kind {
TyKind::Tup(tys) if tys.is_empty() => {
let sugg = "remove the return type";
Some((sugg, "".into()))
},
_ => {
let sugg = "return the output of the future directly";
snippet_opt(cx, output.span).map(|snip| (sugg, format!("-> {}", snip)))
},
}
}

View file

@ -0,0 +1,173 @@
use crate::utils::{snippet_opt, span_lint_and_then};
use if_chain::if_chain;
use rustc_ast::ast::{Attribute, Item, ItemKind, StructField, Variant, VariantData, VisibilityKind};
use rustc_attr as attr;
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::Span;
declare_clippy_lint! {
/// **What it does:** Checks for manual implementations of the non-exhaustive pattern.
///
/// **Why is this bad?** Using the #[non_exhaustive] attribute expresses better the intent
/// and allows possible optimizations when applied to enums.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// struct S {
/// pub a: i32,
/// pub b: i32,
/// _c: (),
/// }
///
/// enum E {
/// A,
/// B,
/// #[doc(hidden)]
/// _C,
/// }
///
/// struct T(pub i32, pub i32, ());
/// ```
/// Use instead:
/// ```rust
/// #[non_exhaustive]
/// struct S {
/// pub a: i32,
/// pub b: i32,
/// }
///
/// #[non_exhaustive]
/// enum E {
/// A,
/// B,
/// }
///
/// #[non_exhaustive]
/// struct T(pub i32, pub i32);
/// ```
pub MANUAL_NON_EXHAUSTIVE,
style,
"manual implementations of the non-exhaustive pattern can be simplified using #[non_exhaustive]"
}
declare_lint_pass!(ManualNonExhaustive => [MANUAL_NON_EXHAUSTIVE]);
impl EarlyLintPass for ManualNonExhaustive {
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
match &item.kind {
ItemKind::Enum(def, _) => {
check_manual_non_exhaustive_enum(cx, item, &def.variants);
},
ItemKind::Struct(variant_data, _) => {
if let VariantData::Unit(..) = variant_data {
return;
}
check_manual_non_exhaustive_struct(cx, item, variant_data);
},
_ => {},
}
}
}
fn check_manual_non_exhaustive_enum(cx: &EarlyContext<'_>, item: &Item, variants: &[Variant]) {
fn is_non_exhaustive_marker(variant: &Variant) -> bool {
matches!(variant.data, VariantData::Unit(_))
&& variant.ident.as_str().starts_with('_')
&& variant.attrs.iter().any(|a| is_doc_hidden(a))
}
fn is_doc_hidden(attr: &Attribute) -> bool {
attr.check_name(sym!(doc))
&& match attr.meta_item_list() {
Some(l) => attr::list_contains_name(&l, sym!(hidden)),
None => false,
}
}
if_chain! {
let mut markers = variants.iter().filter(|v| is_non_exhaustive_marker(v));
if let Some(marker) = markers.next();
if markers.count() == 0 && variants.len() > 1;
then {
span_lint_and_then(
cx,
MANUAL_NON_EXHAUSTIVE,
item.span,
"this seems like a manual implementation of the non-exhaustive pattern",
|diag| {
if_chain! {
if !attr::contains_name(&item.attrs, sym!(non_exhaustive));
let header_span = cx.sess.source_map().span_until_char(item.span, '{');
if let Some(snippet) = snippet_opt(cx, header_span);
then {
diag.span_suggestion(
header_span,
"add the attribute",
format!("#[non_exhaustive] {}", snippet),
Applicability::Unspecified,
);
}
}
diag.span_help(marker.span, "remove this variant");
});
}
}
}
fn check_manual_non_exhaustive_struct(cx: &EarlyContext<'_>, item: &Item, data: &VariantData) {
fn is_private(field: &StructField) -> bool {
matches!(field.vis.node, VisibilityKind::Inherited)
}
fn is_non_exhaustive_marker(field: &StructField) -> bool {
is_private(field) && field.ty.kind.is_unit() && field.ident.map_or(true, |n| n.as_str().starts_with('_'))
}
fn find_header_span(cx: &EarlyContext<'_>, item: &Item, data: &VariantData) -> Span {
let delimiter = match data {
VariantData::Struct(..) => '{',
VariantData::Tuple(..) => '(',
VariantData::Unit(_) => unreachable!("`VariantData::Unit` is already handled above"),
};
cx.sess.source_map().span_until_char(item.span, delimiter)
}
let fields = data.fields();
let private_fields = fields.iter().filter(|f| is_private(f)).count();
let public_fields = fields.iter().filter(|f| f.vis.node.is_pub()).count();
if_chain! {
if private_fields == 1 && public_fields >= 1 && public_fields == fields.len() - 1;
if let Some(marker) = fields.iter().find(|f| is_non_exhaustive_marker(f));
then {
span_lint_and_then(
cx,
MANUAL_NON_EXHAUSTIVE,
item.span,
"this seems like a manual implementation of the non-exhaustive pattern",
|diag| {
if_chain! {
if !attr::contains_name(&item.attrs, sym!(non_exhaustive));
let header_span = find_header_span(cx, item, data);
if let Some(snippet) = snippet_opt(cx, header_span);
then {
diag.span_suggestion(
header_span,
"add the attribute",
format!("#[non_exhaustive] {}", snippet),
Applicability::Unspecified,
);
}
}
diag.span_help(marker.span, "remove this field");
});
}
}
}

View file

@ -1,4 +1,4 @@
use crate::utils::{is_type_diagnostic_item, snippet, span_lint_and_sugg, walk_ptrs_ty}; use crate::utils::{self, is_type_diagnostic_item, match_type, snippet, span_lint_and_sugg, walk_ptrs_ty};
use if_chain::if_chain; use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, MatchSource}; use rustc_hir::{Expr, ExprKind, MatchSource};
@ -38,7 +38,7 @@ declare_clippy_lint! {
/// } /// }
/// ``` /// ```
pub MATCH_ON_VEC_ITEMS, pub MATCH_ON_VEC_ITEMS,
correctness, pedantic,
"matching on vector elements can panic" "matching on vector elements can panic"
} }
@ -75,10 +75,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MatchOnVecItems {
fn is_vec_indexing<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> { fn is_vec_indexing<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
if_chain! { if_chain! {
if let ExprKind::Index(ref array, _) = expr.kind; if let ExprKind::Index(ref array, ref index) = expr.kind;
let ty = cx.tables.expr_ty(array); if is_vector(cx, array);
let ty = walk_ptrs_ty(ty); if !is_full_range(cx, index);
if is_type_diagnostic_item(cx, ty, sym!(vec_type));
then { then {
return Some(expr); return Some(expr);
@ -87,3 +86,15 @@ fn is_vec_indexing<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>)
None None
} }
fn is_vector(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
let ty = cx.tables.expr_ty(expr);
let ty = walk_ptrs_ty(ty);
is_type_diagnostic_item(cx, ty, sym!(vec_type))
}
fn is_full_range(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
let ty = cx.tables.expr_ty(expr);
let ty = walk_ptrs_ty(ty);
match_type(cx, ty, &utils::paths::RANGE_FULL)
}

View file

@ -24,8 +24,25 @@ declare_clippy_lint! {
/// **Known problems:** None. /// **Known problems:** None.
/// ///
/// **Example:** /// **Example:**
/// ```ignore /// ```rust
/// let { a: _, b: ref b, c: _ } = .. /// # struct Foo {
/// # a: i32,
/// # b: i32,
/// # c: i32,
/// # }
/// let f = Foo { a: 0, b: 0, c: 0 };
///
/// // Bad
/// match f {
/// Foo { a: _, b: 0, .. } => {},
/// Foo { a: _, b: _, c: _ } => {},
/// }
///
/// // Good
/// match f {
/// Foo { b: 0, .. } => {},
/// Foo { .. } => {},
/// }
/// ``` /// ```
pub UNNEEDED_FIELD_PATTERN, pub UNNEEDED_FIELD_PATTERN,
restriction, restriction,

View file

@ -1,4 +1,7 @@
use crate::utils::{higher::if_block, is_type_diagnostic_item, span_lint_and_then, usage::is_potentially_mutated}; use crate::utils::{
differing_macro_contexts, higher::if_block, is_type_diagnostic_item, span_lint_and_then,
usage::is_potentially_mutated,
};
use if_chain::if_chain; use if_chain::if_chain;
use rustc_hir::intravisit::{walk_expr, walk_fn, FnKind, NestedVisitorMap, Visitor}; use rustc_hir::intravisit::{walk_expr, walk_fn, FnKind, NestedVisitorMap, Visitor};
use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, HirId, Path, QPath, UnOp}; use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, HirId, Path, QPath, UnOp};
@ -73,6 +76,8 @@ struct UnwrapInfo<'tcx> {
ident: &'tcx Path<'tcx>, ident: &'tcx Path<'tcx>,
/// The check, like `x.is_ok()` /// The check, like `x.is_ok()`
check: &'tcx Expr<'tcx>, check: &'tcx Expr<'tcx>,
/// The branch where the check takes place, like `if x.is_ok() { .. }`
branch: &'tcx Expr<'tcx>,
/// Whether `is_some()` or `is_ok()` was called (as opposed to `is_err()` or `is_none()`). /// Whether `is_some()` or `is_ok()` was called (as opposed to `is_err()` or `is_none()`).
safe_to_unwrap: bool, safe_to_unwrap: bool,
} }
@ -82,19 +87,20 @@ struct UnwrapInfo<'tcx> {
fn collect_unwrap_info<'a, 'tcx>( fn collect_unwrap_info<'a, 'tcx>(
cx: &'a LateContext<'a, 'tcx>, cx: &'a LateContext<'a, 'tcx>,
expr: &'tcx Expr<'_>, expr: &'tcx Expr<'_>,
branch: &'tcx Expr<'_>,
invert: bool, invert: bool,
) -> Vec<UnwrapInfo<'tcx>> { ) -> Vec<UnwrapInfo<'tcx>> {
if let ExprKind::Binary(op, left, right) = &expr.kind { if let ExprKind::Binary(op, left, right) = &expr.kind {
match (invert, op.node) { match (invert, op.node) {
(false, BinOpKind::And) | (false, BinOpKind::BitAnd) | (true, BinOpKind::Or) | (true, BinOpKind::BitOr) => { (false, BinOpKind::And) | (false, BinOpKind::BitAnd) | (true, BinOpKind::Or) | (true, BinOpKind::BitOr) => {
let mut unwrap_info = collect_unwrap_info(cx, left, invert); let mut unwrap_info = collect_unwrap_info(cx, left, branch, invert);
unwrap_info.append(&mut collect_unwrap_info(cx, right, invert)); unwrap_info.append(&mut collect_unwrap_info(cx, right, branch, invert));
return unwrap_info; return unwrap_info;
}, },
_ => (), _ => (),
} }
} else if let ExprKind::Unary(UnOp::UnNot, expr) = &expr.kind { } else if let ExprKind::Unary(UnOp::UnNot, expr) = &expr.kind {
return collect_unwrap_info(cx, expr, !invert); return collect_unwrap_info(cx, expr, branch, !invert);
} else { } else {
if_chain! { if_chain! {
if let ExprKind::MethodCall(method_name, _, args) = &expr.kind; if let ExprKind::MethodCall(method_name, _, args) = &expr.kind;
@ -111,7 +117,7 @@ fn collect_unwrap_info<'a, 'tcx>(
_ => unreachable!(), _ => unreachable!(),
}; };
let safe_to_unwrap = unwrappable != invert; let safe_to_unwrap = unwrappable != invert;
return vec![UnwrapInfo { ident: path, check: expr, safe_to_unwrap }]; return vec![UnwrapInfo { ident: path, check: expr, branch, safe_to_unwrap }];
} }
} }
} }
@ -121,7 +127,7 @@ fn collect_unwrap_info<'a, 'tcx>(
impl<'a, 'tcx> UnwrappableVariablesVisitor<'a, 'tcx> { impl<'a, 'tcx> UnwrappableVariablesVisitor<'a, 'tcx> {
fn visit_branch(&mut self, cond: &'tcx Expr<'_>, branch: &'tcx Expr<'_>, else_branch: bool) { fn visit_branch(&mut self, cond: &'tcx Expr<'_>, branch: &'tcx Expr<'_>, else_branch: bool) {
let prev_len = self.unwrappables.len(); let prev_len = self.unwrappables.len();
for unwrap_info in collect_unwrap_info(self.cx, cond, else_branch) { for unwrap_info in collect_unwrap_info(self.cx, cond, branch, else_branch) {
if is_potentially_mutated(unwrap_info.ident, cond, self.cx) if is_potentially_mutated(unwrap_info.ident, cond, self.cx)
|| is_potentially_mutated(unwrap_info.ident, branch, self.cx) || is_potentially_mutated(unwrap_info.ident, branch, self.cx)
{ {
@ -158,6 +164,9 @@ impl<'a, 'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> {
let call_to_unwrap = method_name.ident.name == sym!(unwrap); let call_to_unwrap = method_name.ident.name == sym!(unwrap);
if let Some(unwrappable) = self.unwrappables.iter() if let Some(unwrappable) = self.unwrappables.iter()
.find(|u| u.ident.res == path.res); .find(|u| u.ident.res == path.res);
// Span contexts should not differ with the conditional branch
if !differing_macro_contexts(unwrappable.branch.span, expr.span);
if !differing_macro_contexts(unwrappable.branch.span, unwrappable.check.span);
then { then {
if call_to_unwrap == unwrappable.safe_to_unwrap { if call_to_unwrap == unwrappable.safe_to_unwrap {
span_lint_and_then( span_lint_and_then(

View file

@ -158,6 +158,8 @@ define_Conf! {
(max_struct_bools, "max_struct_bools": u64, 3), (max_struct_bools, "max_struct_bools": u64, 3),
/// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bools function parameters can have /// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bools function parameters can have
(max_fn_params_bools, "max_fn_params_bools": u64, 3), (max_fn_params_bools, "max_fn_params_bools": u64, 3),
/// Lint: WILDCARD_IMPORTS. Whether to allow certain wildcard imports (prelude, super in tests).
(warn_on_all_wildcard_imports, "warn_on_all_wildcard_imports": bool, false),
} }
impl Default for Conf { impl Default for Conf {

View file

@ -933,6 +933,7 @@ pub fn is_ctor_or_promotable_const_function(cx: &LateContext<'_, '_>, expr: &Exp
} }
/// Returns `true` if a pattern is refutable. /// Returns `true` if a pattern is refutable.
// TODO: should be implemented using rustc/mir_build/hair machinery
pub fn is_refutable(cx: &LateContext<'_, '_>, pat: &Pat<'_>) -> bool { pub fn is_refutable(cx: &LateContext<'_, '_>, pat: &Pat<'_>) -> bool {
fn is_enum_variant(cx: &LateContext<'_, '_>, qpath: &QPath<'_>, id: HirId) -> bool { fn is_enum_variant(cx: &LateContext<'_, '_>, qpath: &QPath<'_>, id: HirId) -> bool {
matches!( matches!(
@ -946,27 +947,34 @@ pub fn is_refutable(cx: &LateContext<'_, '_>, pat: &Pat<'_>) -> bool {
} }
match pat.kind { match pat.kind {
PatKind::Binding(..) | PatKind::Wild => false, PatKind::Wild => false,
PatKind::Binding(_, _, _, pat) => pat.map_or(false, |pat| is_refutable(cx, pat)),
PatKind::Box(ref pat) | PatKind::Ref(ref pat, _) => is_refutable(cx, pat), PatKind::Box(ref pat) | PatKind::Ref(ref pat, _) => is_refutable(cx, pat),
PatKind::Lit(..) | PatKind::Range(..) => true, PatKind::Lit(..) | PatKind::Range(..) => true,
PatKind::Path(ref qpath) => is_enum_variant(cx, qpath, pat.hir_id), PatKind::Path(ref qpath) => is_enum_variant(cx, qpath, pat.hir_id),
PatKind::Or(ref pats) | PatKind::Tuple(ref pats, _) => are_refutable(cx, pats.iter().map(|pat| &**pat)), PatKind::Or(ref pats) => {
// TODO: should be the honest check, that pats is exhaustive set
are_refutable(cx, pats.iter().map(|pat| &**pat))
},
PatKind::Tuple(ref pats, _) => are_refutable(cx, pats.iter().map(|pat| &**pat)),
PatKind::Struct(ref qpath, ref fields, _) => { PatKind::Struct(ref qpath, ref fields, _) => {
if is_enum_variant(cx, qpath, pat.hir_id) { is_enum_variant(cx, qpath, pat.hir_id) || are_refutable(cx, fields.iter().map(|field| &*field.pat))
true
} else {
are_refutable(cx, fields.iter().map(|field| &*field.pat))
}
}, },
PatKind::TupleStruct(ref qpath, ref pats, _) => { PatKind::TupleStruct(ref qpath, ref pats, _) => {
if is_enum_variant(cx, qpath, pat.hir_id) { is_enum_variant(cx, qpath, pat.hir_id) || are_refutable(cx, pats.iter().map(|pat| &**pat))
true
} else {
are_refutable(cx, pats.iter().map(|pat| &**pat))
}
}, },
PatKind::Slice(ref head, ref middle, ref tail) => { PatKind::Slice(ref head, ref middle, ref tail) => {
are_refutable(cx, head.iter().chain(middle).chain(tail.iter()).map(|pat| &**pat)) match &cx.tables.node_type(pat.hir_id).kind {
ty::Slice(..) => {
// [..] is the only irrefutable slice pattern.
!head.is_empty() || middle.is_none() || !tail.is_empty()
},
ty::Array(..) => are_refutable(cx, head.iter().chain(middle).chain(tail.iter()).map(|pat| &**pat)),
_ => {
// unreachable!()
true
},
}
}, },
} }
} }

View file

@ -42,6 +42,7 @@ pub const FMT_ARGUMENTS_NEW_V1_FORMATTED: [&str; 4] = ["core", "fmt", "Arguments
pub const FMT_ARGUMENTV1_NEW: [&str; 4] = ["core", "fmt", "ArgumentV1", "new"]; pub const FMT_ARGUMENTV1_NEW: [&str; 4] = ["core", "fmt", "ArgumentV1", "new"];
pub const FROM_FROM: [&str; 4] = ["core", "convert", "From", "from"]; pub const FROM_FROM: [&str; 4] = ["core", "convert", "From", "from"];
pub const FROM_TRAIT: [&str; 3] = ["core", "convert", "From"]; pub const FROM_TRAIT: [&str; 3] = ["core", "convert", "From"];
pub const FUTURE_FROM_GENERATOR: [&str; 3] = ["core", "future", "from_generator"];
pub const HASH: [&str; 2] = ["hash", "Hash"]; pub const HASH: [&str; 2] = ["hash", "Hash"];
pub const HASHMAP: [&str; 5] = ["std", "collections", "hash", "map", "HashMap"]; pub const HASHMAP: [&str; 5] = ["std", "collections", "hash", "map", "HashMap"];
pub const HASHMAP_ENTRY: [&str; 5] = ["std", "collections", "hash", "map", "Entry"]; pub const HASHMAP_ENTRY: [&str; 5] = ["std", "collections", "hash", "map", "Entry"];
@ -85,7 +86,7 @@ pub const RANGE: [&str; 3] = ["core", "ops", "Range"];
pub const RANGE_ARGUMENT_TRAIT: [&str; 3] = ["core", "ops", "RangeBounds"]; pub const RANGE_ARGUMENT_TRAIT: [&str; 3] = ["core", "ops", "RangeBounds"];
pub const RANGE_FROM: [&str; 3] = ["core", "ops", "RangeFrom"]; pub const RANGE_FROM: [&str; 3] = ["core", "ops", "RangeFrom"];
pub const RANGE_FROM_STD: [&str; 3] = ["std", "ops", "RangeFrom"]; pub const RANGE_FROM_STD: [&str; 3] = ["std", "ops", "RangeFrom"];
pub const RANGE_FULL: [&str; 3] = ["core", "ops", "RangeFull"]; pub const RANGE_FULL: [&str; 4] = ["core", "ops", "range", "RangeFull"];
pub const RANGE_FULL_STD: [&str; 3] = ["std", "ops", "RangeFull"]; pub const RANGE_FULL_STD: [&str; 3] = ["std", "ops", "RangeFull"];
pub const RANGE_INCLUSIVE_NEW: [&str; 4] = ["core", "ops", "RangeInclusive", "new"]; pub const RANGE_INCLUSIVE_NEW: [&str; 4] = ["core", "ops", "RangeInclusive", "new"];
pub const RANGE_INCLUSIVE_STD_NEW: [&str; 4] = ["std", "ops", "RangeInclusive", "new"]; pub const RANGE_INCLUSIVE_STD_NEW: [&str; 4] = ["std", "ops", "RangeInclusive", "new"];

View file

@ -3,10 +3,10 @@ use if_chain::if_chain;
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_hir::{ use rustc_hir::{
def::{DefKind, Res}, def::{DefKind, Res},
Item, ItemKind, UseKind, Item, ItemKind, PathSegment, UseKind,
}; };
use rustc_lint::{LateContext, LateLintPass}; use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::BytePos; use rustc_span::BytePos;
declare_clippy_lint! { declare_clippy_lint! {
@ -43,9 +43,14 @@ declare_clippy_lint! {
/// ///
/// This can lead to confusing error messages at best and to unexpected behavior at worst. /// This can lead to confusing error messages at best and to unexpected behavior at worst.
/// ///
/// Note that this will not warn about wildcard imports from modules named `prelude`; many /// **Exceptions:**
/// crates (including the standard library) provide modules named "prelude" specifically ///
/// designed for wildcard import. /// Wildcard imports are allowed from modules named `prelude`. Many crates (including the standard library)
/// provide modules named "prelude" specifically designed for wildcard import.
///
/// `use super::*` is allowed in test modules. This is defined as any module with "test" in the name.
///
/// These exceptions can be disabled using the `warn-on-all-wildcard-imports` configuration flag.
/// ///
/// **Known problems:** If macros are imported through the wildcard, this macro is not included /// **Known problems:** If macros are imported through the wildcard, this macro is not included
/// by the suggestion and has to be added by hand. /// by the suggestion and has to be added by hand.
@ -73,18 +78,34 @@ declare_clippy_lint! {
"lint `use _::*` statements" "lint `use _::*` statements"
} }
declare_lint_pass!(WildcardImports => [ENUM_GLOB_USE, WILDCARD_IMPORTS]); #[derive(Default)]
pub struct WildcardImports {
warn_on_all: bool,
test_modules_deep: u32,
}
impl WildcardImports {
pub fn new(warn_on_all: bool) -> Self {
Self {
warn_on_all,
test_modules_deep: 0,
}
}
}
impl_lint_pass!(WildcardImports => [ENUM_GLOB_USE, WILDCARD_IMPORTS]);
impl LateLintPass<'_, '_> for WildcardImports { impl LateLintPass<'_, '_> for WildcardImports {
fn check_item(&mut self, cx: &LateContext<'_, '_>, item: &Item<'_>) { fn check_item(&mut self, cx: &LateContext<'_, '_>, item: &Item<'_>) {
if is_test_module_or_function(item) {
self.test_modules_deep = self.test_modules_deep.saturating_add(1);
}
if item.vis.node.is_pub() || item.vis.node.is_pub_restricted() { if item.vis.node.is_pub() || item.vis.node.is_pub_restricted() {
return; return;
} }
if_chain! { if_chain! {
if !in_macro(item.span);
if let ItemKind::Use(use_path, UseKind::Glob) = &item.kind; if let ItemKind::Use(use_path, UseKind::Glob) = &item.kind;
// don't lint prelude glob imports if self.warn_on_all || !self.check_exceptions(item, use_path.segments);
if !use_path.segments.iter().last().map_or(false, |ps| ps.ident.as_str() == "prelude");
let used_imports = cx.tcx.names_imported_by_glob_use(item.hir_id.owner); let used_imports = cx.tcx.names_imported_by_glob_use(item.hir_id.owner);
if !used_imports.is_empty(); // Already handled by `unused_imports` if !used_imports.is_empty(); // Already handled by `unused_imports`
then { then {
@ -109,8 +130,7 @@ impl LateLintPass<'_, '_> for WildcardImports {
span = use_path.span.with_hi(item.span.hi() - BytePos(1)); span = use_path.span.with_hi(item.span.hi() - BytePos(1));
} }
( (
span, span, false,
false,
) )
}; };
@ -153,4 +173,36 @@ impl LateLintPass<'_, '_> for WildcardImports {
} }
} }
} }
fn check_item_post(&mut self, _: &LateContext<'_, '_>, item: &Item<'_>) {
if is_test_module_or_function(item) {
self.test_modules_deep = self.test_modules_deep.saturating_sub(1);
}
}
}
impl WildcardImports {
fn check_exceptions(&self, item: &Item<'_>, segments: &[PathSegment<'_>]) -> bool {
in_macro(item.span)
|| is_prelude_import(segments)
|| (is_super_only_import(segments) && self.test_modules_deep > 0)
}
}
// Allow "...prelude::*" imports.
// Many crates have a prelude, and it is imported as a glob by design.
fn is_prelude_import(segments: &[PathSegment<'_>]) -> bool {
segments
.iter()
.last()
.map_or(false, |ps| ps.ident.as_str() == "prelude")
}
// Allow "super::*" imports in tests.
fn is_super_only_import(segments: &[PathSegment<'_>]) -> bool {
segments.len() == 1 && segments[0].ident.as_str() == "super"
}
fn is_test_module_or_function(item: &Item<'_>) -> bool {
matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().contains("test")
} }

View file

@ -1081,6 +1081,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None, deprecation: None,
module: "main_recursion", module: "main_recursion",
}, },
Lint {
name: "manual_async_fn",
group: "style",
desc: "manual implementations of `async` functions can be simplified using the dedicated syntax",
deprecation: None,
module: "manual_async_fn",
},
Lint { Lint {
name: "manual_memcpy", name: "manual_memcpy",
group: "perf", group: "perf",
@ -1088,6 +1095,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None, deprecation: None,
module: "loops", module: "loops",
}, },
Lint {
name: "manual_non_exhaustive",
group: "style",
desc: "manual implementations of the non-exhaustive pattern can be simplified using #[non_exhaustive]",
deprecation: None,
module: "manual_non_exhaustive",
},
Lint { Lint {
name: "manual_saturating_arithmetic", name: "manual_saturating_arithmetic",
group: "style", group: "style",
@ -1146,7 +1160,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
}, },
Lint { Lint {
name: "match_on_vec_items", name: "match_on_vec_items",
group: "correctness", group: "pedantic",
desc: "matching on vector elements can panic", desc: "matching on vector elements can panic",
deprecation: None, deprecation: None,
module: "match_on_vec_items", module: "match_on_vec_items",

View file

@ -1,4 +1,4 @@
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-struct-bools`, `max-fn-params-bools`, `third-party` at line 5 column 1 error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `third-party` at line 5 column 1
error: aborting due to previous error error: aborting due to previous error

View file

@ -18,8 +18,6 @@ LL | 42
| |
= note: expected type parameter `u32` = note: expected type parameter `u32`
found type `{integer}` found type `{integer}`
= help: type parameters must be constrained to match other types
= note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters
error: aborting due to 2 previous errors error: aborting due to 2 previous errors

View file

@ -9,6 +9,30 @@ macro_rules! m {
}; };
} }
macro_rules! checks_in_param {
($a:expr, $b:expr) => {
if $a {
$b;
}
};
}
macro_rules! checks_unwrap {
($a:expr, $b:expr) => {
if $a.is_some() {
$b;
}
};
}
macro_rules! checks_some {
($a:expr, $b:expr) => {
if $a {
$b.unwrap();
}
};
}
fn main() { fn main() {
let x = Some(()); let x = Some(());
if x.is_some() { if x.is_some() {
@ -22,6 +46,9 @@ fn main() {
x.unwrap(); // unnecessary x.unwrap(); // unnecessary
} }
m!(x); m!(x);
checks_in_param!(x.is_some(), x.unwrap()); // ok
checks_unwrap!(x, x.unwrap()); // ok
checks_some!(x.is_some(), x); // ok
let mut x: Result<(), ()> = Ok(()); let mut x: Result<(), ()> = Ok(());
if x.is_ok() { if x.is_ok() {
x.unwrap(); // unnecessary x.unwrap(); // unnecessary

View file

@ -1,5 +1,5 @@
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`. error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/simple_conditionals.rs:15:9 --> $DIR/simple_conditionals.rs:39:9
| |
LL | if x.is_some() { LL | if x.is_some() {
| ----------- the check is happening here | ----------- the check is happening here
@ -13,7 +13,7 @@ LL | #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^
error: This call to `unwrap()` will always panic. error: This call to `unwrap()` will always panic.
--> $DIR/simple_conditionals.rs:17:9 --> $DIR/simple_conditionals.rs:41:9
| |
LL | if x.is_some() { LL | if x.is_some() {
| ----------- because of this check | ----------- because of this check
@ -28,7 +28,7 @@ LL | #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
| ^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^
error: This call to `unwrap()` will always panic. error: This call to `unwrap()` will always panic.
--> $DIR/simple_conditionals.rs:20:9 --> $DIR/simple_conditionals.rs:44:9
| |
LL | if x.is_none() { LL | if x.is_none() {
| ----------- because of this check | ----------- because of this check
@ -36,7 +36,7 @@ LL | x.unwrap(); // will panic
| ^^^^^^^^^^ | ^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`. error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/simple_conditionals.rs:22:9 --> $DIR/simple_conditionals.rs:46:9
| |
LL | if x.is_none() { LL | if x.is_none() {
| ----------- the check is happening here | ----------- the check is happening here
@ -58,7 +58,7 @@ LL | m!(x);
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`. error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/simple_conditionals.rs:27:9 --> $DIR/simple_conditionals.rs:54:9
| |
LL | if x.is_ok() { LL | if x.is_ok() {
| --------- the check is happening here | --------- the check is happening here
@ -66,7 +66,7 @@ LL | x.unwrap(); // unnecessary
| ^^^^^^^^^^ | ^^^^^^^^^^
error: This call to `unwrap_err()` will always panic. error: This call to `unwrap_err()` will always panic.
--> $DIR/simple_conditionals.rs:28:9 --> $DIR/simple_conditionals.rs:55:9
| |
LL | if x.is_ok() { LL | if x.is_ok() {
| --------- because of this check | --------- because of this check
@ -75,7 +75,7 @@ LL | x.unwrap_err(); // will panic
| ^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^
error: This call to `unwrap()` will always panic. error: This call to `unwrap()` will always panic.
--> $DIR/simple_conditionals.rs:30:9 --> $DIR/simple_conditionals.rs:57:9
| |
LL | if x.is_ok() { LL | if x.is_ok() {
| --------- because of this check | --------- because of this check
@ -84,7 +84,7 @@ LL | x.unwrap(); // will panic
| ^^^^^^^^^^ | ^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`. error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/simple_conditionals.rs:31:9 --> $DIR/simple_conditionals.rs:58:9
| |
LL | if x.is_ok() { LL | if x.is_ok() {
| --------- the check is happening here | --------- the check is happening here
@ -93,7 +93,7 @@ LL | x.unwrap_err(); // unnecessary
| ^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^
error: This call to `unwrap()` will always panic. error: This call to `unwrap()` will always panic.
--> $DIR/simple_conditionals.rs:34:9 --> $DIR/simple_conditionals.rs:61:9
| |
LL | if x.is_err() { LL | if x.is_err() {
| ---------- because of this check | ---------- because of this check
@ -101,7 +101,7 @@ LL | x.unwrap(); // will panic
| ^^^^^^^^^^ | ^^^^^^^^^^
error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`. error: You checked before that `unwrap_err()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/simple_conditionals.rs:35:9 --> $DIR/simple_conditionals.rs:62:9
| |
LL | if x.is_err() { LL | if x.is_err() {
| ---------- the check is happening here | ---------- the check is happening here
@ -110,7 +110,7 @@ LL | x.unwrap_err(); // unnecessary
| ^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^
error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`. error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
--> $DIR/simple_conditionals.rs:37:9 --> $DIR/simple_conditionals.rs:64:9
| |
LL | if x.is_err() { LL | if x.is_err() {
| ---------- the check is happening here | ---------- the check is happening here
@ -119,7 +119,7 @@ LL | x.unwrap(); // unnecessary
| ^^^^^^^^^^ | ^^^^^^^^^^
error: This call to `unwrap_err()` will always panic. error: This call to `unwrap_err()` will always panic.
--> $DIR/simple_conditionals.rs:38:9 --> $DIR/simple_conditionals.rs:65:9
| |
LL | if x.is_err() { LL | if x.is_err() {
| ---------- because of this check | ---------- because of this check

View file

@ -41,6 +41,7 @@ impl Dummy {
self.private_future().await; self.private_future().await;
} }
#[allow(clippy::manual_async_fn)]
pub fn public_send(&self) -> impl std::future::Future<Output = bool> { pub fn public_send(&self) -> impl std::future::Future<Output = bool> {
async { false } async { false }
} }

View file

@ -96,13 +96,13 @@ LL | }
= note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Sync` = note: `std::rc::Rc<[u8]>` doesn't implement `std::marker::Sync`
error: future cannot be sent between threads safely error: future cannot be sent between threads safely
--> $DIR/future_not_send.rs:49:37 --> $DIR/future_not_send.rs:50:37
| |
LL | async fn generic_future<T>(t: T) -> T LL | async fn generic_future<T>(t: T) -> T
| ^ future returned by `generic_future` is not `Send` | ^ future returned by `generic_future` is not `Send`
| |
note: future is not `Send` as this value is used across an await note: future is not `Send` as this value is used across an await
--> $DIR/future_not_send.rs:54:5 --> $DIR/future_not_send.rs:55:5
| |
LL | let rt = &t; LL | let rt = &t;
| -- has type `&T` which is not `Send` | -- has type `&T` which is not `Send`
@ -114,7 +114,7 @@ LL | }
= note: `T` doesn't implement `std::marker::Sync` = note: `T` doesn't implement `std::marker::Sync`
error: future cannot be sent between threads safely error: future cannot be sent between threads safely
--> $DIR/future_not_send.rs:65:34 --> $DIR/future_not_send.rs:66:34
| |
LL | async fn unclear_future<T>(t: T) {} LL | async fn unclear_future<T>(t: T) {}
| ^ | ^

View file

@ -0,0 +1,67 @@
// run-rustfix
// edition:2018
#![warn(clippy::manual_async_fn)]
#![allow(unused)]
use std::future::Future;
async fn fut() -> i32 { 42 }
async fn empty_fut() {}
async fn core_fut() -> i32 { 42 }
// should be ignored
fn has_other_stmts() -> impl core::future::Future<Output = i32> {
let _ = 42;
async move { 42 }
}
// should be ignored
fn not_fut() -> i32 {
42
}
// should be ignored
async fn already_async() -> impl Future<Output = i32> {
async { 42 }
}
struct S {}
impl S {
async fn inh_fut() -> i32 {
// NOTE: this code is here just to check that the identation is correct in the suggested fix
let a = 42;
let b = 21;
if a < b {
let c = 21;
let d = 42;
if c < d {
let _ = 42;
}
}
42
}
async fn meth_fut(&self) -> i32 { 42 }
async fn empty_fut(&self) {}
// should be ignored
fn not_fut(&self) -> i32 {
42
}
// should be ignored
fn has_other_stmts() -> impl core::future::Future<Output = i32> {
let _ = 42;
async move { 42 }
}
// should be ignored
async fn already_async(&self) -> impl Future<Output = i32> {
async { 42 }
}
}
fn main() {}

View file

@ -0,0 +1,79 @@
// run-rustfix
// edition:2018
#![warn(clippy::manual_async_fn)]
#![allow(unused)]
use std::future::Future;
fn fut() -> impl Future<Output = i32> {
async { 42 }
}
fn empty_fut() -> impl Future<Output = ()> {
async {}
}
fn core_fut() -> impl core::future::Future<Output = i32> {
async move { 42 }
}
// should be ignored
fn has_other_stmts() -> impl core::future::Future<Output = i32> {
let _ = 42;
async move { 42 }
}
// should be ignored
fn not_fut() -> i32 {
42
}
// should be ignored
async fn already_async() -> impl Future<Output = i32> {
async { 42 }
}
struct S {}
impl S {
fn inh_fut() -> impl Future<Output = i32> {
async {
// NOTE: this code is here just to check that the identation is correct in the suggested fix
let a = 42;
let b = 21;
if a < b {
let c = 21;
let d = 42;
if c < d {
let _ = 42;
}
}
42
}
}
fn meth_fut(&self) -> impl Future<Output = i32> {
async { 42 }
}
fn empty_fut(&self) -> impl Future<Output = ()> {
async {}
}
// should be ignored
fn not_fut(&self) -> i32 {
42
}
// should be ignored
fn has_other_stmts() -> impl core::future::Future<Output = i32> {
let _ = 42;
async move { 42 }
}
// should be ignored
async fn already_async(&self) -> impl Future<Output = i32> {
async { 42 }
}
}
fn main() {}

View file

@ -0,0 +1,98 @@
error: this function can be simplified using the `async fn` syntax
--> $DIR/manual_async_fn.rs:8:1
|
LL | fn fut() -> impl Future<Output = i32> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::manual-async-fn` implied by `-D warnings`
help: make the function `async` and return the output of the future directly
|
LL | async fn fut() -> i32 {
| ^^^^^^^^^^^^^^^^^^^^^
help: move the body of the async block to the enclosing function
|
LL | fn fut() -> impl Future<Output = i32> { 42 }
| ^^^^^^
error: this function can be simplified using the `async fn` syntax
--> $DIR/manual_async_fn.rs:12:1
|
LL | fn empty_fut() -> impl Future<Output = ()> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: make the function `async` and remove the return type
|
LL | async fn empty_fut() {
| ^^^^^^^^^^^^^^^^^^^^
help: move the body of the async block to the enclosing function
|
LL | fn empty_fut() -> impl Future<Output = ()> {}
| ^^
error: this function can be simplified using the `async fn` syntax
--> $DIR/manual_async_fn.rs:16:1
|
LL | fn core_fut() -> impl core::future::Future<Output = i32> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: make the function `async` and return the output of the future directly
|
LL | async fn core_fut() -> i32 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
help: move the body of the async block to the enclosing function
|
LL | fn core_fut() -> impl core::future::Future<Output = i32> { 42 }
| ^^^^^^
error: this function can be simplified using the `async fn` syntax
--> $DIR/manual_async_fn.rs:38:5
|
LL | fn inh_fut() -> impl Future<Output = i32> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: make the function `async` and return the output of the future directly
|
LL | async fn inh_fut() -> i32 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
help: move the body of the async block to the enclosing function
|
LL | fn inh_fut() -> impl Future<Output = i32> {
LL | // NOTE: this code is here just to check that the identation is correct in the suggested fix
LL | let a = 42;
LL | let b = 21;
LL | if a < b {
LL | let c = 21;
...
error: this function can be simplified using the `async fn` syntax
--> $DIR/manual_async_fn.rs:54:5
|
LL | fn meth_fut(&self) -> impl Future<Output = i32> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: make the function `async` and return the output of the future directly
|
LL | async fn meth_fut(&self) -> i32 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: move the body of the async block to the enclosing function
|
LL | fn meth_fut(&self) -> impl Future<Output = i32> { 42 }
| ^^^^^^
error: this function can be simplified using the `async fn` syntax
--> $DIR/manual_async_fn.rs:58:5
|
LL | fn empty_fut(&self) -> impl Future<Output = ()> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: make the function `async` and remove the return type
|
LL | async fn empty_fut(&self) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^
help: move the body of the async block to the enclosing function
|
LL | fn empty_fut(&self) -> impl Future<Output = ()> {}
| ^^
error: aborting due to 6 previous errors

View file

@ -98,6 +98,21 @@ pub fn manual_copy(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) {
for i in from..from + 3 { for i in from..from + 3 {
dst[i] = src[i - from]; dst[i] = src[i - from];
} }
#[allow(clippy::identity_op)]
for i in 0..5 {
dst[i - 0] = src[i];
}
#[allow(clippy::reverse_range_loop)]
for i in 0..0 {
dst[i] = src[i];
}
// `RangeTo` `for` loop - don't trigger lint
for i in 0.. {
dst[i] = src[i];
}
} }
#[warn(clippy::needless_range_loop, clippy::manual_memcpy)] #[warn(clippy::needless_range_loop, clippy::manual_memcpy)]

View file

@ -58,19 +58,31 @@ error: it looks like you're manually copying between slices
--> $DIR/manual_memcpy.rs:94:14 --> $DIR/manual_memcpy.rs:94:14
| |
LL | for i in from..from + src.len() { LL | for i in from..from + src.len() {
| ^^^^^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + src.len()].clone_from_slice(&src[0..(from + src.len() - from)])` | ^^^^^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + src.len()].clone_from_slice(&src[..(from + src.len() - from)])`
error: it looks like you're manually copying between slices error: it looks like you're manually copying between slices
--> $DIR/manual_memcpy.rs:98:14 --> $DIR/manual_memcpy.rs:98:14
| |
LL | for i in from..from + 3 { LL | for i in from..from + 3 {
| ^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + 3].clone_from_slice(&src[0..(from + 3 - from)])` | ^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + 3].clone_from_slice(&src[..(from + 3 - from)])`
error: it looks like you're manually copying between slices error: it looks like you're manually copying between slices
--> $DIR/manual_memcpy.rs:105:14 --> $DIR/manual_memcpy.rs:103:14
|
LL | for i in 0..5 {
| ^^^^ help: try replacing the loop by: `dst[..5].clone_from_slice(&src[..5])`
error: it looks like you're manually copying between slices
--> $DIR/manual_memcpy.rs:108:14
|
LL | for i in 0..0 {
| ^^^^ help: try replacing the loop by: `dst[..0].clone_from_slice(&src[..0])`
error: it looks like you're manually copying between slices
--> $DIR/manual_memcpy.rs:120:14
| |
LL | for i in 0..src.len() { LL | for i in 0..src.len() {
| ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])` | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])`
error: aborting due to 11 previous errors error: aborting due to 13 previous errors

View file

@ -0,0 +1,137 @@
#![warn(clippy::manual_non_exhaustive)]
#![allow(unused)]
mod enums {
enum E {
A,
B,
#[doc(hidden)]
_C,
}
// user forgot to remove the marker
#[non_exhaustive]
enum Ep {
A,
B,
#[doc(hidden)]
_C,
}
// marker variant does not have doc hidden attribute, should be ignored
enum NoDocHidden {
A,
B,
_C,
}
// name of variant with doc hidden does not start with underscore, should be ignored
enum NoUnderscore {
A,
B,
#[doc(hidden)]
C,
}
// variant with doc hidden is not unit, should be ignored
enum NotUnit {
A,
B,
#[doc(hidden)]
_C(bool),
}
// variant with doc hidden is the only one, should be ignored
enum OnlyMarker {
#[doc(hidden)]
_A,
}
// variant with multiple markers, should be ignored
enum MultipleMarkers {
A,
#[doc(hidden)]
_B,
#[doc(hidden)]
_C,
}
// already non_exhaustive and no markers, should be ignored
#[non_exhaustive]
enum NonExhaustive {
A,
B,
}
}
mod structs {
struct S {
pub a: i32,
pub b: i32,
_c: (),
}
// user forgot to remove the private field
#[non_exhaustive]
struct Sp {
pub a: i32,
pub b: i32,
_c: (),
}
// some other fields are private, should be ignored
struct PrivateFields {
a: i32,
pub b: i32,
_c: (),
}
// private field name does not start with underscore, should be ignored
struct NoUnderscore {
pub a: i32,
pub b: i32,
c: (),
}
// private field is not unit type, should be ignored
struct NotUnit {
pub a: i32,
pub b: i32,
_c: i32,
}
// private field is the only field, should be ignored
struct OnlyMarker {
_a: (),
}
// already non exhaustive and no private fields, should be ignored
#[non_exhaustive]
struct NonExhaustive {
pub a: i32,
pub b: i32,
}
}
mod tuple_structs {
struct T(pub i32, pub i32, ());
// user forgot to remove the private field
#[non_exhaustive]
struct Tp(pub i32, pub i32, ());
// some other fields are private, should be ignored
struct PrivateFields(pub i32, i32, ());
// private field is not unit type, should be ignored
struct NotUnit(pub i32, pub i32, i32);
// private field is the only field, should be ignored
struct OnlyMarker(());
// already non exhaustive and no private fields, should be ignored
#[non_exhaustive]
struct NonExhaustive(pub i32, pub i32);
}
fn main() {}

View file

@ -0,0 +1,103 @@
error: this seems like a manual implementation of the non-exhaustive pattern
--> $DIR/manual_non_exhaustive.rs:5:5
|
LL | enum E {
| ^-----
| |
| _____help: add the attribute: `#[non_exhaustive] enum E`
| |
LL | | A,
LL | | B,
LL | | #[doc(hidden)]
LL | | _C,
LL | | }
| |_____^
|
= note: `-D clippy::manual-non-exhaustive` implied by `-D warnings`
help: remove this variant
--> $DIR/manual_non_exhaustive.rs:9:9
|
LL | _C,
| ^^
error: this seems like a manual implementation of the non-exhaustive pattern
--> $DIR/manual_non_exhaustive.rs:14:5
|
LL | / enum Ep {
LL | | A,
LL | | B,
LL | | #[doc(hidden)]
LL | | _C,
LL | | }
| |_____^
|
help: remove this variant
--> $DIR/manual_non_exhaustive.rs:18:9
|
LL | _C,
| ^^
error: this seems like a manual implementation of the non-exhaustive pattern
--> $DIR/manual_non_exhaustive.rs:68:5
|
LL | struct S {
| ^-------
| |
| _____help: add the attribute: `#[non_exhaustive] struct S`
| |
LL | | pub a: i32,
LL | | pub b: i32,
LL | | _c: (),
LL | | }
| |_____^
|
help: remove this field
--> $DIR/manual_non_exhaustive.rs:71:9
|
LL | _c: (),
| ^^^^^^
error: this seems like a manual implementation of the non-exhaustive pattern
--> $DIR/manual_non_exhaustive.rs:76:5
|
LL | / struct Sp {
LL | | pub a: i32,
LL | | pub b: i32,
LL | | _c: (),
LL | | }
| |_____^
|
help: remove this field
--> $DIR/manual_non_exhaustive.rs:79:9
|
LL | _c: (),
| ^^^^^^
error: this seems like a manual implementation of the non-exhaustive pattern
--> $DIR/manual_non_exhaustive.rs:117:5
|
LL | struct T(pub i32, pub i32, ());
| --------^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: add the attribute: `#[non_exhaustive] struct T`
|
help: remove this field
--> $DIR/manual_non_exhaustive.rs:117:32
|
LL | struct T(pub i32, pub i32, ());
| ^^
error: this seems like a manual implementation of the non-exhaustive pattern
--> $DIR/manual_non_exhaustive.rs:121:5
|
LL | struct Tp(pub i32, pub i32, ());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: remove this field
--> $DIR/manual_non_exhaustive.rs:121:33
|
LL | struct Tp(pub i32, pub i32, ());
| ^^
error: aborting due to 6 previous errors

View file

@ -120,6 +120,27 @@ fn match_with_array() {
} }
} }
fn match_with_endless_range() {
let arr = vec![0, 1, 2, 3];
let range = ..;
// Ok
match arr[range] {
[0, 1] => println!("0 1"),
[1, 2] => println!("1 2"),
[0, 1, 2, 3] => println!("0, 1, 2, 3"),
_ => {},
}
// Ok
match arr[..] {
[0, 1] => println!("0 1"),
[1, 2] => println!("1 2"),
[0, 1, 2, 3] => println!("0, 1, 2, 3"),
_ => {},
}
}
fn main() { fn main() {
match_with_wildcard(); match_with_wildcard();
match_without_wildcard(); match_without_wildcard();
@ -127,4 +148,5 @@ fn main() {
match_vec_ref(); match_vec_ref();
match_with_get(); match_with_get();
match_with_array(); match_with_array();
match_with_endless_range();
} }

View file

@ -2,6 +2,7 @@
#![warn(clippy::while_let_on_iterator)] #![warn(clippy::while_let_on_iterator)]
#![allow(clippy::never_loop, unreachable_code, unused_mut)] #![allow(clippy::never_loop, unreachable_code, unused_mut)]
#![feature(or_patterns)]
fn base() { fn base() {
let mut iter = 1..20; let mut iter = 1..20;
@ -77,6 +78,62 @@ fn refutable() {
// */ // */
} }
fn refutable2() {
// Issue 3780
{
let v = vec![1, 2, 3];
let mut it = v.windows(2);
while let Some([x, y]) = it.next() {
println!("x: {}", x);
println!("y: {}", y);
}
let mut it = v.windows(2);
while let Some([x, ..]) = it.next() {
println!("x: {}", x);
}
let mut it = v.windows(2);
while let Some([.., y]) = it.next() {
println!("y: {}", y);
}
let mut it = v.windows(2);
for [..] in it {}
let v = vec![[1], [2], [3]];
let mut it = v.iter();
while let Some([1]) = it.next() {}
let mut it = v.iter();
for [_x] in it {}
}
// binding
{
let v = vec![1, 2, 3];
let mut it = v.iter();
while let Some(x @ 1) = it.next() {
println!("{}", x);
}
let v = vec![[1], [2], [3]];
let mut it = v.iter();
for x @ [_] in it {
println!("{:?}", x);
}
}
// false negative
{
let v = vec![1, 2, 3];
let mut it = v.iter().map(Some);
while let Some(Some(_) | None) = it.next() {
println!("1");
}
}
}
fn nested_loops() { fn nested_loops() {
let a = [42, 1337]; let a = [42, 1337];
let mut y = a.iter(); let mut y = a.iter();
@ -152,6 +209,7 @@ fn issue1654() {
fn main() { fn main() {
base(); base();
refutable(); refutable();
refutable2();
nested_loops(); nested_loops();
issue1121(); issue1121();
issue2965(); issue2965();

View file

@ -2,6 +2,7 @@
#![warn(clippy::while_let_on_iterator)] #![warn(clippy::while_let_on_iterator)]
#![allow(clippy::never_loop, unreachable_code, unused_mut)] #![allow(clippy::never_loop, unreachable_code, unused_mut)]
#![feature(or_patterns)]
fn base() { fn base() {
let mut iter = 1..20; let mut iter = 1..20;
@ -77,6 +78,62 @@ fn refutable() {
// */ // */
} }
fn refutable2() {
// Issue 3780
{
let v = vec![1, 2, 3];
let mut it = v.windows(2);
while let Some([x, y]) = it.next() {
println!("x: {}", x);
println!("y: {}", y);
}
let mut it = v.windows(2);
while let Some([x, ..]) = it.next() {
println!("x: {}", x);
}
let mut it = v.windows(2);
while let Some([.., y]) = it.next() {
println!("y: {}", y);
}
let mut it = v.windows(2);
while let Some([..]) = it.next() {}
let v = vec![[1], [2], [3]];
let mut it = v.iter();
while let Some([1]) = it.next() {}
let mut it = v.iter();
while let Some([_x]) = it.next() {}
}
// binding
{
let v = vec![1, 2, 3];
let mut it = v.iter();
while let Some(x @ 1) = it.next() {
println!("{}", x);
}
let v = vec![[1], [2], [3]];
let mut it = v.iter();
while let Some(x @ [_]) = it.next() {
println!("{:?}", x);
}
}
// false negative
{
let v = vec![1, 2, 3];
let mut it = v.iter().map(Some);
while let Some(Some(_) | None) = it.next() {
println!("1");
}
}
}
fn nested_loops() { fn nested_loops() {
let a = [42, 1337]; let a = [42, 1337];
let mut y = a.iter(); let mut y = a.iter();
@ -152,6 +209,7 @@ fn issue1654() {
fn main() { fn main() {
base(); base();
refutable(); refutable();
refutable2();
nested_loops(); nested_loops();
issue1121(); issue1121();
issue2965(); issue2965();

View file

@ -1,5 +1,5 @@
error: this loop could be written as a `for` loop error: this loop could be written as a `for` loop
--> $DIR/while_let_on_iterator.rs:8:5 --> $DIR/while_let_on_iterator.rs:9:5
| |
LL | while let Option::Some(x) = iter.next() { LL | while let Option::Some(x) = iter.next() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in iter` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in iter`
@ -7,22 +7,40 @@ LL | while let Option::Some(x) = iter.next() {
= note: `-D clippy::while-let-on-iterator` implied by `-D warnings` = note: `-D clippy::while-let-on-iterator` implied by `-D warnings`
error: this loop could be written as a `for` loop error: this loop could be written as a `for` loop
--> $DIR/while_let_on_iterator.rs:13:5 --> $DIR/while_let_on_iterator.rs:14:5
| |
LL | while let Some(x) = iter.next() { LL | while let Some(x) = iter.next() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in iter` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in iter`
error: this loop could be written as a `for` loop error: this loop could be written as a `for` loop
--> $DIR/while_let_on_iterator.rs:18:5 --> $DIR/while_let_on_iterator.rs:19:5
| |
LL | while let Some(_) = iter.next() {} LL | while let Some(_) = iter.next() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in iter` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in iter`
error: this loop could be written as a `for` loop error: this loop could be written as a `for` loop
--> $DIR/while_let_on_iterator.rs:97:9 --> $DIR/while_let_on_iterator.rs:102:9
|
LL | while let Some([..]) = it.next() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for [..] in it`
error: this loop could be written as a `for` loop
--> $DIR/while_let_on_iterator.rs:109:9
|
LL | while let Some([_x]) = it.next() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for [_x] in it`
error: this loop could be written as a `for` loop
--> $DIR/while_let_on_iterator.rs:122:9
|
LL | while let Some(x @ [_]) = it.next() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x @ [_] in it`
error: this loop could be written as a `for` loop
--> $DIR/while_let_on_iterator.rs:154:9
| |
LL | while let Some(_) = y.next() { LL | while let Some(_) = y.next() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in y` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in y`
error: aborting due to 4 previous errors error: aborting due to 7 previous errors

View file

@ -155,3 +155,76 @@ fn test_weird_formatting() {
exported(); exported();
foo(); foo();
} }
mod super_imports {
fn foofoo() {}
mod should_be_replaced {
use super::foofoo;
fn with_super() {
let _ = foofoo();
}
}
mod test_should_pass {
use super::*;
fn with_super() {
let _ = foofoo();
}
}
mod test_should_pass_inside_function {
fn with_super_inside_function() {
use super::*;
let _ = foofoo();
}
}
mod test_should_pass_further_inside {
fn insidefoo() {}
mod inner {
use super::*;
fn with_super() {
let _ = insidefoo();
}
}
}
mod should_be_replaced_futher_inside {
fn insidefoo() {}
mod inner {
use super::insidefoo;
fn with_super() {
let _ = insidefoo();
}
}
}
mod use_explicit_should_be_replaced {
use super_imports::foofoo;
fn with_explicit() {
let _ = foofoo();
}
}
mod use_double_super_should_be_replaced {
mod inner {
use super::super::foofoo;
fn with_double_super() {
let _ = foofoo();
}
}
}
mod use_super_explicit_should_be_replaced {
use super::super::super_imports::foofoo;
fn with_super_explicit() {
let _ = foofoo();
}
}
}

View file

@ -156,3 +156,76 @@ fn test_weird_formatting() {
exported(); exported();
foo(); foo();
} }
mod super_imports {
fn foofoo() {}
mod should_be_replaced {
use super::*;
fn with_super() {
let _ = foofoo();
}
}
mod test_should_pass {
use super::*;
fn with_super() {
let _ = foofoo();
}
}
mod test_should_pass_inside_function {
fn with_super_inside_function() {
use super::*;
let _ = foofoo();
}
}
mod test_should_pass_further_inside {
fn insidefoo() {}
mod inner {
use super::*;
fn with_super() {
let _ = insidefoo();
}
}
}
mod should_be_replaced_futher_inside {
fn insidefoo() {}
mod inner {
use super::*;
fn with_super() {
let _ = insidefoo();
}
}
}
mod use_explicit_should_be_replaced {
use super_imports::*;
fn with_explicit() {
let _ = foofoo();
}
}
mod use_double_super_should_be_replaced {
mod inner {
use super::super::*;
fn with_double_super() {
let _ = foofoo();
}
}
}
mod use_super_explicit_should_be_replaced {
use super::super::super_imports::*;
fn with_super_explicit() {
let _ = foofoo();
}
}
}

View file

@ -92,5 +92,35 @@ LL | use crate:: fn_mod::
LL | | *; LL | | *;
| |_________^ help: try: `crate:: fn_mod::foo` | |_________^ help: try: `crate:: fn_mod::foo`
error: aborting due to 15 previous errors error: usage of wildcard import
--> $DIR/wildcard_imports.rs:164:13
|
LL | use super::*;
| ^^^^^^^^ help: try: `super::foofoo`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:199:17
|
LL | use super::*;
| ^^^^^^^^ help: try: `super::insidefoo`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:207:13
|
LL | use super_imports::*;
| ^^^^^^^^^^^^^^^^ help: try: `super_imports::foofoo`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:216:17
|
LL | use super::super::*;
| ^^^^^^^^^^^^^^^ help: try: `super::super::foofoo`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:225:13
|
LL | use super::super::super_imports::*;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::super_imports::foofoo`
error: aborting due to 20 previous errors