Use span_lint_and_sugg + move infaillible lint
- moving infaillible lint to prevent collisions
This commit is contained in:
parent
3445d41f07
commit
6afd7ea147
|
@ -1217,6 +1217,7 @@ Released 2018-09-13
|
|||
[`match_overlapping_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_overlapping_arm
|
||||
[`match_ref_pats`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_ref_pats
|
||||
[`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms
|
||||
[`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
|
||||
[`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm
|
||||
[`maybe_infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_infinite_iter
|
||||
[`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum
|
||||
|
|
|
@ -1,77 +0,0 @@
|
|||
use super::utils::{get_arg_name, match_var, remove_blocks, snippet_with_applicability, span_lint_and_sugg};
|
||||
use if_chain::if_chain;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::*;
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for matches being used to destructure a single-variant enum
|
||||
/// or tuple struct where a `let` will suffice.
|
||||
///
|
||||
/// **Why is this bad?** Just readability – `let` doesn't nest, whereas a `match` does.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// enum Wrapper {
|
||||
/// Data(i32),
|
||||
/// }
|
||||
///
|
||||
/// let wrapper = Wrapper::Data(42);
|
||||
///
|
||||
/// let data = match wrapper {
|
||||
/// Wrapper::Data(i) => i,
|
||||
/// };
|
||||
/// ```
|
||||
///
|
||||
/// The correct use would be:
|
||||
/// ```rust
|
||||
/// enum Wrapper {
|
||||
/// Data(i32),
|
||||
/// }
|
||||
///
|
||||
/// let wrapper = Wrapper::Data(42);
|
||||
/// let Wrapper::Data(data) = wrapper;
|
||||
/// ```
|
||||
pub INFALLIBLE_DESTRUCTURING_MATCH,
|
||||
style,
|
||||
"a `match` statement with a single infallible arm instead of a `let`"
|
||||
}
|
||||
|
||||
declare_lint_pass!(InfallibleDestructingMatch => [INFALLIBLE_DESTRUCTURING_MATCH]);
|
||||
|
||||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InfallibleDestructingMatch {
|
||||
fn check_local(&mut self, cx: &LateContext<'a, 'tcx>, local: &'tcx Local<'_>) {
|
||||
if_chain! {
|
||||
if let Some(ref expr) = local.init;
|
||||
if let ExprKind::Match(ref target, ref arms, MatchSource::Normal) = expr.kind;
|
||||
if arms.len() == 1 && arms[0].guard.is_none();
|
||||
if let PatKind::TupleStruct(QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pat.kind;
|
||||
if args.len() == 1;
|
||||
if let Some(arg) = get_arg_name(&args[0]);
|
||||
let body = remove_blocks(&arms[0].body);
|
||||
if match_var(body, arg);
|
||||
|
||||
then {
|
||||
let mut applicability = Applicability::MachineApplicable;
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
INFALLIBLE_DESTRUCTURING_MATCH,
|
||||
local.span,
|
||||
"you seem to be trying to use `match` to destructure a single infallible pattern. \
|
||||
Consider using `let`",
|
||||
"try this",
|
||||
format!(
|
||||
"let {}({}) = {};",
|
||||
snippet_with_applicability(cx, variant_name.span, "..", &mut applicability),
|
||||
snippet_with_applicability(cx, local.pat.span, "..", &mut applicability),
|
||||
snippet_with_applicability(cx, target.span, "..", &mut applicability),
|
||||
),
|
||||
applicability,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
|
@ -218,7 +218,6 @@ pub mod if_let_some_result;
|
|||
pub mod if_not_else;
|
||||
pub mod implicit_return;
|
||||
pub mod indexing_slicing;
|
||||
pub mod infallible_destructuring_match;
|
||||
pub mod infinite_iter;
|
||||
pub mod inherent_impl;
|
||||
pub mod inherent_to_string;
|
||||
|
@ -555,7 +554,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
&implicit_return::IMPLICIT_RETURN,
|
||||
&indexing_slicing::INDEXING_SLICING,
|
||||
&indexing_slicing::OUT_OF_BOUNDS_INDEXING,
|
||||
&infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH,
|
||||
&infinite_iter::INFINITE_ITER,
|
||||
&infinite_iter::MAYBE_INFINITE_ITER,
|
||||
&inherent_impl::MULTIPLE_INHERENT_IMPL,
|
||||
|
@ -600,12 +598,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
&map_clone::MAP_CLONE,
|
||||
&map_unit_fn::OPTION_MAP_UNIT_FN,
|
||||
&map_unit_fn::RESULT_MAP_UNIT_FN,
|
||||
&matches::INFALLIBLE_DESTRUCTURING_MATCH,
|
||||
&matches::MATCH_AS_REF,
|
||||
&matches::MATCH_BOOL,
|
||||
&matches::MATCH_OVERLAPPING_ARM,
|
||||
&matches::MATCH_REF_PATS,
|
||||
&matches::MATCH_WILD_ERR_ARM,
|
||||
&matches::MATCH_SINGLE_BINDING,
|
||||
&matches::MATCH_WILD_ERR_ARM,
|
||||
&matches::SINGLE_MATCH,
|
||||
&matches::SINGLE_MATCH_ELSE,
|
||||
&matches::WILDCARD_ENUM_MATCH_ARM,
|
||||
|
@ -865,7 +864,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
store.register_late_pass(|| box types::Casts);
|
||||
let type_complexity_threshold = conf.type_complexity_threshold;
|
||||
store.register_late_pass(move || box types::TypeComplexity::new(type_complexity_threshold));
|
||||
store.register_late_pass(|| box matches::Matches);
|
||||
store.register_late_pass(|| box matches::Matches::default());
|
||||
store.register_late_pass(|| box minmax::MinMaxPass);
|
||||
store.register_late_pass(|| box open_options::OpenOptions);
|
||||
store.register_late_pass(|| box zero_div_zero::ZeroDiv);
|
||||
|
@ -942,7 +941,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
store.register_late_pass(|| box question_mark::QuestionMark);
|
||||
store.register_late_pass(|| box suspicious_trait_impl::SuspiciousImpl);
|
||||
store.register_late_pass(|| box map_unit_fn::MapUnit);
|
||||
store.register_late_pass(|| box infallible_destructuring_match::InfallibleDestructingMatch);
|
||||
store.register_late_pass(|| box inherent_impl::MultipleInherentImpl::default());
|
||||
store.register_late_pass(|| box neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd);
|
||||
store.register_late_pass(|| box unwrap::Unwrap);
|
||||
|
@ -1167,7 +1165,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(&identity_op::IDENTITY_OP),
|
||||
LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
|
||||
LintId::of(&indexing_slicing::OUT_OF_BOUNDS_INDEXING),
|
||||
LintId::of(&infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH),
|
||||
LintId::of(&infinite_iter::INFINITE_ITER),
|
||||
LintId::of(&inherent_to_string::INHERENT_TO_STRING),
|
||||
LintId::of(&inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY),
|
||||
|
@ -1202,12 +1199,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(&map_clone::MAP_CLONE),
|
||||
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
|
||||
LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
|
||||
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
|
||||
LintId::of(&matches::MATCH_AS_REF),
|
||||
LintId::of(&matches::MATCH_BOOL),
|
||||
LintId::of(&matches::MATCH_OVERLAPPING_ARM),
|
||||
LintId::of(&matches::MATCH_REF_PATS),
|
||||
LintId::of(&matches::MATCH_WILD_ERR_ARM),
|
||||
LintId::of(&matches::MATCH_SINGLE_BINDING),
|
||||
LintId::of(&matches::MATCH_WILD_ERR_ARM),
|
||||
LintId::of(&matches::SINGLE_MATCH),
|
||||
LintId::of(&matches::WILDCARD_IN_OR_PATTERNS),
|
||||
LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM),
|
||||
|
@ -1384,7 +1382,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(&functions::DOUBLE_MUST_USE),
|
||||
LintId::of(&functions::MUST_USE_UNIT),
|
||||
LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
|
||||
LintId::of(&infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH),
|
||||
LintId::of(&inherent_to_string::INHERENT_TO_STRING),
|
||||
LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
|
||||
LintId::of(&len_zero::LEN_ZERO),
|
||||
|
@ -1397,6 +1394,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(&loops::WHILE_LET_ON_ITERATOR),
|
||||
LintId::of(&main_recursion::MAIN_RECURSION),
|
||||
LintId::of(&map_clone::MAP_CLONE),
|
||||
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
|
||||
LintId::of(&matches::MATCH_BOOL),
|
||||
LintId::of(&matches::MATCH_OVERLAPPING_ARM),
|
||||
LintId::of(&matches::MATCH_REF_PATS),
|
||||
|
|
|
@ -5,7 +5,7 @@ use crate::utils::usage::is_unused;
|
|||
use crate::utils::{
|
||||
span_lint_and_help, span_lint_and_note,
|
||||
expr_block, in_macro, is_allowed, is_expn_of, is_wild, match_qpath, match_type, multispan_sugg, remove_blocks,
|
||||
snippet, snippet_with_applicability, span_lint_and_sugg, span_lint_and_then,
|
||||
snippet, snippet_block, snippet_with_applicability, span_lint_and_sugg, span_lint_and_then,
|
||||
};
|
||||
use if_chain::if_chain;
|
||||
use rustc::lint::in_external_macro;
|
||||
|
@ -14,7 +14,7 @@ use rustc_errors::Applicability;
|
|||
use rustc_hir::def::CtorKind;
|
||||
use rustc_hir::*;
|
||||
use rustc_lint::{LateContext, LateLintPass, LintContext};
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||
use rustc_span::source_map::Span;
|
||||
use std::cmp::Ordering;
|
||||
use std::collections::Bound;
|
||||
|
@ -245,12 +245,47 @@ declare_clippy_lint! {
|
|||
"a wildcard pattern used with others patterns in same match arm"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for matches being used to destructure a single-variant enum
|
||||
/// or tuple struct where a `let` will suffice.
|
||||
///
|
||||
/// **Why is this bad?** Just readability – `let` doesn't nest, whereas a `match` does.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// enum Wrapper {
|
||||
/// Data(i32),
|
||||
/// }
|
||||
///
|
||||
/// let wrapper = Wrapper::Data(42);
|
||||
///
|
||||
/// let data = match wrapper {
|
||||
/// Wrapper::Data(i) => i,
|
||||
/// };
|
||||
/// ```
|
||||
///
|
||||
/// The correct use would be:
|
||||
/// ```rust
|
||||
/// enum Wrapper {
|
||||
/// Data(i32),
|
||||
/// }
|
||||
///
|
||||
/// let wrapper = Wrapper::Data(42);
|
||||
/// let Wrapper::Data(data) = wrapper;
|
||||
/// ```
|
||||
pub INFALLIBLE_DESTRUCTURING_MATCH,
|
||||
style,
|
||||
"a `match` statement with a single infallible arm instead of a `let`"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for useless match that binds to only one value.
|
||||
///
|
||||
/// **Why is this bad?** Readability and needless complexity.
|
||||
///
|
||||
/// **Known problems:** This situation frequently happen in macros, so can't lint there.
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
|
@ -272,7 +307,12 @@ declare_clippy_lint! {
|
|||
"a match with a single binding instead of using `let` statement"
|
||||
}
|
||||
|
||||
declare_lint_pass!(Matches => [
|
||||
#[derive(Default)]
|
||||
pub struct Matches {
|
||||
infallible_destructuring_match_linted: bool,
|
||||
}
|
||||
|
||||
impl_lint_pass!(Matches => [
|
||||
SINGLE_MATCH,
|
||||
MATCH_REF_PATS,
|
||||
MATCH_BOOL,
|
||||
|
@ -283,6 +323,7 @@ declare_lint_pass!(Matches => [
|
|||
WILDCARD_ENUM_MATCH_ARM,
|
||||
WILDCARD_IN_OR_PATTERNS,
|
||||
MATCH_SINGLE_BINDING,
|
||||
INFALLIBLE_DESTRUCTURING_MATCH
|
||||
]);
|
||||
|
||||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches {
|
||||
|
@ -298,12 +339,51 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches {
|
|||
check_wild_enum_match(cx, ex, arms);
|
||||
check_match_as_ref(cx, ex, arms, expr);
|
||||
check_wild_in_or_pats(cx, arms);
|
||||
check_match_single_binding(cx, ex, arms, expr);
|
||||
|
||||
if self.infallible_destructuring_match_linted {
|
||||
self.infallible_destructuring_match_linted = false;
|
||||
} else {
|
||||
check_match_single_binding(cx, ex, arms, expr);
|
||||
}
|
||||
}
|
||||
if let ExprKind::Match(ref ex, ref arms, _) = expr.kind {
|
||||
check_match_ref_pats(cx, ex, arms, expr);
|
||||
}
|
||||
}
|
||||
|
||||
fn check_local(&mut self, cx: &LateContext<'a, 'tcx>, local: &'tcx Local<'_>) {
|
||||
if_chain! {
|
||||
if let Some(ref expr) = local.init;
|
||||
if let ExprKind::Match(ref target, ref arms, MatchSource::Normal) = expr.kind;
|
||||
if arms.len() == 1 && arms[0].guard.is_none();
|
||||
if let PatKind::TupleStruct(
|
||||
QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pat.kind;
|
||||
if args.len() == 1;
|
||||
if let Some(arg) = get_arg_name(&args[0]);
|
||||
let body = remove_blocks(&arms[0].body);
|
||||
if match_var(body, arg);
|
||||
|
||||
then {
|
||||
let mut applicability = Applicability::MachineApplicable;
|
||||
self.infallible_destructuring_match_linted = true;
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
INFALLIBLE_DESTRUCTURING_MATCH,
|
||||
local.span,
|
||||
"you seem to be trying to use `match` to destructure a single infallible pattern. \
|
||||
Consider using `let`",
|
||||
"try this",
|
||||
format!(
|
||||
"let {}({}) = {};",
|
||||
snippet_with_applicability(cx, variant_name.span, "..", &mut applicability),
|
||||
snippet_with_applicability(cx, local.pat.span, "..", &mut applicability),
|
||||
snippet_with_applicability(cx, target.span, "..", &mut applicability),
|
||||
),
|
||||
applicability,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[rustfmt::skip]
|
||||
|
@ -746,21 +826,31 @@ fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[A
|
|||
return;
|
||||
}
|
||||
if arms.len() == 1 {
|
||||
let bind_names = arms[0].pat.span;
|
||||
let matched_vars = ex.span;
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
MATCH_SINGLE_BINDING,
|
||||
expr.span,
|
||||
"this match could be written as a `let` statement",
|
||||
"try this",
|
||||
format!(
|
||||
"let {} = {};",
|
||||
snippet(cx, bind_names, ".."),
|
||||
snippet(cx, matched_vars, "..")
|
||||
),
|
||||
Applicability::HasPlaceholders,
|
||||
);
|
||||
if is_refutable(cx, arms[0].pat) {
|
||||
return;
|
||||
}
|
||||
match arms[0].pat.kind {
|
||||
PatKind::Binding(..) | PatKind::Tuple(_, _) => {
|
||||
let bind_names = arms[0].pat.span;
|
||||
let matched_vars = ex.span;
|
||||
let match_body = remove_blocks(&arms[0].body);
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
MATCH_SINGLE_BINDING,
|
||||
expr.span,
|
||||
"this match could be written as a `let` statement",
|
||||
"consider using `let` statement",
|
||||
format!(
|
||||
"let {} = {};\n{}",
|
||||
snippet(cx, bind_names, ".."),
|
||||
snippet(cx, matched_vars, ".."),
|
||||
snippet_block(cx, match_body.span, "..")
|
||||
),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
},
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -775,7 +775,7 @@ pub const ALL_LINTS: [Lint; 351] = [
|
|||
group: "style",
|
||||
desc: "a `match` statement with a single infallible arm instead of a `let`",
|
||||
deprecation: None,
|
||||
module: "infallible_destructuring_match",
|
||||
module: "matches",
|
||||
},
|
||||
Lint {
|
||||
name: "infinite_iter",
|
||||
|
@ -1092,6 +1092,13 @@ pub const ALL_LINTS: [Lint; 351] = [
|
|||
deprecation: None,
|
||||
module: "copies",
|
||||
},
|
||||
Lint {
|
||||
name: "match_single_binding",
|
||||
group: "complexity",
|
||||
desc: "a match with a single binding instead of using `let` statement",
|
||||
deprecation: None,
|
||||
module: "matches",
|
||||
},
|
||||
Lint {
|
||||
name: "match_wild_err_arm",
|
||||
group: "style",
|
||||
|
|
|
@ -3,7 +3,7 @@
|
|||
clippy::borrowed_box,
|
||||
clippy::needless_pass_by_value,
|
||||
clippy::unused_unit,
|
||||
clippy::redundant_clone
|
||||
clippy::redundant_clone,
|
||||
)]
|
||||
#![warn(clippy::boxed_local)]
|
||||
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
error: local variable doesn't need to be boxed here
|
||||
--> $DIR/escape_analysis.rs:39:13
|
||||
--> $DIR/escape_analysis.rs:40:13
|
||||
|
|
||||
LL | fn warn_arg(x: Box<A>) {
|
||||
| ^
|
||||
|
@ -7,7 +7,7 @@ LL | fn warn_arg(x: Box<A>) {
|
|||
= note: `-D clippy::boxed-local` implied by `-D warnings`
|
||||
|
||||
error: local variable doesn't need to be boxed here
|
||||
--> $DIR/escape_analysis.rs:130:12
|
||||
--> $DIR/escape_analysis.rs:131:12
|
||||
|
|
||||
LL | pub fn new(_needs_name: Box<PeekableSeekable<&()>>) -> () {}
|
||||
| ^^^^^^^^^^^
|
||||
|
|
|
@ -0,0 +1,26 @@
|
|||
// run-rustfix
|
||||
|
||||
#![warn(clippy::match_single_binding)]
|
||||
#[allow(clippy::many_single_char_names)]
|
||||
|
||||
fn main() {
|
||||
let a = 1;
|
||||
let b = 2;
|
||||
let c = 3;
|
||||
// Lint
|
||||
let (x, y, z) = (a, b, c);
|
||||
{
|
||||
println!("{} {} {}", x, y, z);
|
||||
}
|
||||
// Ok
|
||||
match a {
|
||||
2 => println!("2"),
|
||||
_ => println!("Not 2"),
|
||||
}
|
||||
// Ok
|
||||
let d = Some(5);
|
||||
match d {
|
||||
Some(d) => println!("{}", d),
|
||||
_ => println!("None"),
|
||||
}
|
||||
}
|
|
@ -1,3 +1,5 @@
|
|||
// run-rustfix
|
||||
|
||||
#![warn(clippy::match_single_binding)]
|
||||
#[allow(clippy::many_single_char_names)]
|
||||
|
||||
|
@ -19,7 +21,7 @@ fn main() {
|
|||
// Ok
|
||||
let d = Some(5);
|
||||
match d {
|
||||
Some(d) => println!("5"),
|
||||
Some(d) => println!("{}", d),
|
||||
_ => println!("None"),
|
||||
}
|
||||
}
|
||||
|
|
|
@ -1,14 +1,21 @@
|
|||
error: this match could be written as a `let` statement
|
||||
--> $DIR/match_single_binding.rs:9:5
|
||||
--> $DIR/match_single_binding.rs:11:5
|
||||
|
|
||||
LL | / match (a, b, c) {
|
||||
LL | | (x, y, z) => {
|
||||
LL | | println!("{} {} {}", x, y, z);
|
||||
LL | | },
|
||||
LL | | }
|
||||
| |_____^ help: try this: `let (x, y, z) = (a, b, c);`
|
||||
| |_____^
|
||||
|
|
||||
= note: `-D clippy::match-single-binding` implied by `-D warnings`
|
||||
help: consider using `let` statement
|
||||
|
|
||||
LL | let (x, y, z) = (a, b, c);
|
||||
LL | {
|
||||
LL | println!("{} {} {}", x, y, z);
|
||||
LL | }
|
||||
|
|
||||
|
||||
error: aborting due to previous error
|
||||
|
||||
|
|
Loading…
Reference in New Issue