Move range_zip_with_len into Methods lint pass

This commit is contained in:
Jason Newcomb 2022-06-05 20:39:57 -04:00
parent 226f135a03
commit fd5376194a
6 changed files with 77 additions and 70 deletions

View file

@ -196,6 +196,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(methods::OPTION_MAP_OR_NONE),
LintId::of(methods::OR_FUN_CALL),
LintId::of(methods::OR_THEN_UNWRAP),
LintId::of(methods::RANGE_ZIP_WITH_LEN),
LintId::of(methods::RESULT_MAP_OR_INTO_OPTION),
LintId::of(methods::SEARCH_IS_SOME),
LintId::of(methods::SHOULD_IMPLEMENT_TRAIT),
@ -276,7 +277,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(ptr_offset_with_cast::PTR_OFFSET_WITH_CAST),
LintId::of(question_mark::QUESTION_MARK),
LintId::of(ranges::MANUAL_RANGE_CONTAINS),
LintId::of(ranges::RANGE_ZIP_WITH_LEN),
LintId::of(ranges::REVERSED_EMPTY_RANGES),
LintId::of(rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT),
LintId::of(read_zero_byte_vec::READ_ZERO_BYTE_VEC),

View file

@ -51,6 +51,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(methods::OPTION_AS_REF_DEREF),
LintId::of(methods::OPTION_FILTER_MAP),
LintId::of(methods::OR_THEN_UNWRAP),
LintId::of(methods::RANGE_ZIP_WITH_LEN),
LintId::of(methods::SEARCH_IS_SOME),
LintId::of(methods::SKIP_WHILE_NEXT),
LintId::of(methods::UNNECESSARY_FILTER_MAP),
@ -76,7 +77,6 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL),
LintId::of(precedence::PRECEDENCE),
LintId::of(ptr_offset_with_cast::PTR_OFFSET_WITH_CAST),
LintId::of(ranges::RANGE_ZIP_WITH_LEN),
LintId::of(redundant_closure_call::REDUNDANT_CLOSURE_CALL),
LintId::of(redundant_slicing::REDUNDANT_SLICING),
LintId::of(reference::DEREF_ADDROF),

View file

@ -345,6 +345,7 @@ store.register_lints(&[
methods::OR_FUN_CALL,
methods::OR_THEN_UNWRAP,
methods::PATH_BUF_PUSH_OVERWRITE,
methods::RANGE_ZIP_WITH_LEN,
methods::RESULT_MAP_OR_INTO_OPTION,
methods::SEARCH_IS_SOME,
methods::SHOULD_IMPLEMENT_TRAIT,
@ -473,7 +474,6 @@ store.register_lints(&[
ranges::MANUAL_RANGE_CONTAINS,
ranges::RANGE_MINUS_ONE,
ranges::RANGE_PLUS_ONE,
ranges::RANGE_ZIP_WITH_LEN,
ranges::REVERSED_EMPTY_RANGES,
rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT,
read_zero_byte_vec::READ_ZERO_BYTE_VEC,

View file

@ -64,6 +64,7 @@ mod option_map_unwrap_or;
mod or_fun_call;
mod or_then_unwrap;
mod path_buf_push_overwrite;
mod range_zip_with_len;
mod search_is_some;
mod single_char_add_str;
mod single_char_insert_string;
@ -2734,6 +2735,31 @@ declare_clippy_lint! {
"calling `push` with file system root on `PathBuf` can overwrite it"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for zipping a collection with the range of
/// `0.._.len()`.
///
/// ### Why is this bad?
/// The code is better expressed with `.enumerate()`.
///
/// ### Example
/// ```rust
/// # let x = vec![1];
/// let _ = x.iter().zip(0..x.len());
/// ```
///
/// Use instead:
/// ```rust
/// # let x = vec![1];
/// let _ = x.iter().enumerate();
/// ```
#[clippy::version = "pre 1.29.0"]
pub RANGE_ZIP_WITH_LEN,
complexity,
"zipping iterator with a range when `enumerate()` would do"
}
pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Option<RustcVersion>,
@ -2848,6 +2874,7 @@ impl_lint_pass!(Methods => [
MUT_MUTEX_LOCK,
NONSENSICAL_OPEN_OPTIONS,
PATH_BUF_PUSH_OVERWRITE,
RANGE_ZIP_WITH_LEN,
]);
/// Extracts a method call name, args, and `Span` of the method name.
@ -3304,6 +3331,13 @@ impl Methods {
("replace" | "replacen", [arg1, arg2] | [arg1, arg2, _]) => {
no_effect_replace::check(cx, expr, arg1, arg2);
},
("zip", [arg]) => {
if let ExprKind::MethodCall(name, [iter_recv], _) = recv.kind
&& name.ident.name == sym::iter
{
range_zip_with_len::check(cx, expr, iter_recv, arg);
}
},
_ => {},
}
}

View file

@ -0,0 +1,34 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::source::snippet;
use clippy_utils::{higher, SpanlessEq};
use clippy_utils::{is_integer_const, is_trait_method};
use if_chain::if_chain;
use rustc_hir::{Expr, ExprKind, QPath};
use rustc_lint::LateContext;
use rustc_span::sym;
use super::RANGE_ZIP_WITH_LEN;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, recv: &'tcx Expr<'_>, zip_arg: &'tcx Expr<'_>) {
if_chain! {
if is_trait_method(cx, expr, sym::Iterator);
// range expression in `.zip()` call: `0..x.len()`
if let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::Range::hir(zip_arg);
if is_integer_const(cx, start, 0);
// `.len()` call
if let ExprKind::MethodCall(len_path, [len_recv], _) = end.kind;
if len_path.ident.name == sym::len;
// `.iter()` and `.len()` called on same `Path`
if let ExprKind::Path(QPath::Resolved(_, iter_path)) = recv.kind;
if let ExprKind::Path(QPath::Resolved(_, len_path)) = len_recv.kind;
if SpanlessEq::new(cx).eq_path_segments(iter_path.segments, len_path.segments);
then {
span_lint(cx,
RANGE_ZIP_WITH_LEN,
expr.span,
&format!("it is more idiomatic to use `{}.iter().enumerate()`",
snippet(cx, recv.span, "_"))
);
}
}
}

View file

@ -1,46 +1,20 @@
use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then};
use clippy_utils::higher;
use clippy_utils::source::{snippet, snippet_opt, snippet_with_applicability};
use clippy_utils::sugg::Sugg;
use clippy_utils::{get_parent_expr, in_constant, is_integer_const, meets_msrv, msrvs, path_to_local};
use clippy_utils::{higher, SpanlessEq};
use if_chain::if_chain;
use rustc_ast::ast::RangeLimits;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind, HirId, PathSegment, QPath};
use rustc_hir::{BinOpKind, Expr, ExprKind, HirId};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::{Span, Spanned};
use rustc_span::sym;
use std::cmp::Ordering;
declare_clippy_lint! {
/// ### What it does
/// Checks for zipping a collection with the range of
/// `0.._.len()`.
///
/// ### Why is this bad?
/// The code is better expressed with `.enumerate()`.
///
/// ### Example
/// ```rust
/// # let x = vec![1];
/// let _ = x.iter().zip(0..x.len());
/// ```
///
/// Use instead:
/// ```rust
/// # let x = vec![1];
/// let _ = x.iter().enumerate();
/// ```
#[clippy::version = "pre 1.29.0"]
pub RANGE_ZIP_WITH_LEN,
complexity,
"zipping iterator with a range when `enumerate()` would do"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for exclusive ranges where 1 is added to the
@ -198,7 +172,6 @@ impl Ranges {
}
impl_lint_pass!(Ranges => [
RANGE_ZIP_WITH_LEN,
RANGE_PLUS_ONE,
RANGE_MINUS_ONE,
REVERSED_EMPTY_RANGES,
@ -207,16 +180,10 @@ impl_lint_pass!(Ranges => [
impl<'tcx> LateLintPass<'tcx> for Ranges {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
match expr.kind {
ExprKind::MethodCall(path, args, _) => {
check_range_zip_with_len(cx, path, args, expr.span);
},
ExprKind::Binary(ref op, l, r) => {
if meets_msrv(self.msrv, msrvs::RANGE_CONTAINS) {
check_possible_range_contains(cx, op.node, l, r, expr, expr.span);
}
},
_ => {},
if let ExprKind::Binary(ref op, l, r) = expr.kind {
if meets_msrv(self.msrv, msrvs::RANGE_CONTAINS) {
check_possible_range_contains(cx, op.node, l, r, expr, expr.span);
}
}
check_exclusive_range_plus_one(cx, expr);
@ -380,34 +347,6 @@ fn check_range_bounds<'a>(cx: &'a LateContext<'_>, ex: &'a Expr<'_>) -> Option<R
None
}
fn check_range_zip_with_len(cx: &LateContext<'_>, path: &PathSegment<'_>, args: &[Expr<'_>], span: Span) {
if_chain! {
if path.ident.as_str() == "zip";
if let [iter, zip_arg] = args;
// `.iter()` call
if let ExprKind::MethodCall(iter_path, [iter_caller, ..], _) = iter.kind;
if iter_path.ident.name == sym::iter;
// range expression in `.zip()` call: `0..x.len()`
if let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::Range::hir(zip_arg);
if is_integer_const(cx, start, 0);
// `.len()` call
if let ExprKind::MethodCall(len_path, [len_caller], _) = end.kind;
if len_path.ident.name == sym::len;
// `.iter()` and `.len()` called on same `Path`
if let ExprKind::Path(QPath::Resolved(_, iter_path)) = iter_caller.kind;
if let ExprKind::Path(QPath::Resolved(_, len_path)) = len_caller.kind;
if SpanlessEq::new(cx).eq_path_segments(iter_path.segments, len_path.segments);
then {
span_lint(cx,
RANGE_ZIP_WITH_LEN,
span,
&format!("it is more idiomatic to use `{}.iter().enumerate()`",
snippet(cx, iter_caller.span, "_"))
);
}
}
}
// exclusive range plus one: `x..(y+1)`
fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
if_chain! {