X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Franges.rs;h=027ab70014fc0c2ca51ced2aa25f329cfcb87028;hb=02516c47ab089d7cf8f106c3fbc1921cce370462;hp=4b514bbd42ca7d0df4c0b5ebb5853974dec1f31a;hpb=39aca5ff9f5317fa1a66eb71772219575e05ba0f;p=rust.git diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index 4b514bbd42c..027ab70014f 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -1,32 +1,31 @@ -use crate::consts::{constant, Constant}; +use clippy_utils::consts::{constant, Constant}; +use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then}; +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, single_segment_path}; +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, PathSegment, QPath}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +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 rustc_span::symbol::Ident; use std::cmp::Ordering; -use crate::utils::sugg::Sugg; -use crate::utils::{ - get_parent_expr, is_integer_const, single_segment_path, snippet, snippet_opt, snippet_with_applicability, - span_lint, span_lint_and_sugg, span_lint_and_then, -}; -use crate::utils::{higher, SpanlessEq}; - declare_clippy_lint! { - /// **What it does:** Checks for zipping a collection with the range of + /// ### 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()`. - /// - /// **Known problems:** None. + /// ### Why is this bad? + /// The code is better expressed with `.enumerate()`. /// - /// **Example:** + /// ### Example /// ```rust /// # let x = vec![1]; /// x.iter().zip(0..x.len()); @@ -36,20 +35,24 @@ /// # let x = vec![1]; /// 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 + /// ### What it does + /// Checks for exclusive ranges where 1 is added to the /// upper bound, e.g., `x..(y+1)`. /// - /// **Why is this bad?** The code is more readable with an inclusive range + /// ### Why is this bad? + /// The code is more readable with an inclusive range /// like `x..=y`. /// - /// **Known problems:** Will add unnecessary pair of parentheses when the - /// expression is not wrapped in a pair but starts with a opening parenthesis + /// ### Known problems + /// Will add unnecessary pair of parentheses when the + /// expression is not wrapped in a pair but starts with an opening parenthesis /// and ends with a closing one. /// I.e., `let _ = (f()+1)..(f()+1)` results in `let _ = ((f()+1)..=f())`. /// @@ -62,7 +65,7 @@ /// `RangeBounds` trait /// ([#3307](https://github.com/rust-lang/rust-clippy/issues/3307)). /// - /// **Example:** + /// ### Example /// ```rust,ignore /// for x..(y+1) { .. } /// ``` @@ -70,24 +73,28 @@ /// ```rust,ignore /// for x..=y { .. } /// ``` + #[clippy::version = "pre 1.29.0"] pub RANGE_PLUS_ONE, pedantic, "`x..(y+1)` reads better as `x..=y`" } declare_clippy_lint! { - /// **What it does:** Checks for inclusive ranges where 1 is subtracted from + /// ### What it does + /// Checks for inclusive ranges where 1 is subtracted from /// the upper bound, e.g., `x..=(y-1)`. /// - /// **Why is this bad?** The code is more readable with an exclusive range + /// ### Why is this bad? + /// The code is more readable with an exclusive range /// like `x..y`. /// - /// **Known problems:** This will cause a warning that cannot be fixed if + /// ### Known problems + /// This will cause a warning that cannot be fixed if /// the consumer of the range only accepts a specific range type, instead of /// the generic `RangeBounds` trait /// ([#3307](https://github.com/rust-lang/rust-clippy/issues/3307)). /// - /// **Example:** + /// ### Example /// ```rust,ignore /// for x..=(y-1) { .. } /// ``` @@ -95,22 +102,22 @@ /// ```rust,ignore /// for x..y { .. } /// ``` + #[clippy::version = "pre 1.29.0"] pub RANGE_MINUS_ONE, pedantic, "`x..=(y-1)` reads better as `x..y`" } declare_clippy_lint! { - /// **What it does:** Checks for range expressions `x..y` where both `x` and `y` + /// ### What it does + /// Checks for range expressions `x..y` where both `x` and `y` /// are constant and `x` is greater or equal to `y`. /// - /// **Why is this bad?** Empty ranges yield no values so iterating them is a no-op. + /// ### Why is this bad? + /// Empty ranges yield no values so iterating them is a no-op. /// Moreover, trying to use a reversed range to index a slice will panic at run-time. /// - /// **Known problems:** None. - /// - /// **Example:** - /// + /// ### Example /// ```rust,no_run /// fn main() { /// (10..=0).for_each(|x| println!("{}", x)); @@ -128,22 +135,22 @@ /// let sub = &arr[1..3]; /// } /// ``` + #[clippy::version = "1.45.0"] pub REVERSED_EMPTY_RANGES, correctness, "reversing the limits of range expressions, resulting in empty ranges" } declare_clippy_lint! { - /// **What it does:** Checks for expressions like `x >= 3 && x < 8` that could + /// ### What it does + /// Checks for expressions like `x >= 3 && x < 8` that could /// be more readably expressed as `(3..8).contains(x)`. /// - /// **Why is this bad?** `contains` expresses the intent better and has less + /// ### Why is this bad? + /// `contains` expresses the intent better and has less /// failure modes (such as fencepost errors or using `||` instead of `&&`). /// - /// **Known problems:** None. - /// - /// **Example:** - /// + /// ### Example /// ```rust /// // given /// let x = 6; @@ -155,12 +162,24 @@ ///# let x = 6; /// assert!((3..8).contains(&x)); /// ``` + #[clippy::version = "1.49.0"] pub MANUAL_RANGE_CONTAINS, style, "manually reimplementing {`Range`, `RangeInclusive`}`::contains`" } -declare_lint_pass!(Ranges => [ +pub struct Ranges { + msrv: Option, +} + +impl Ranges { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} + +impl_lint_pass!(Ranges => [ RANGE_ZIP_WITH_LEN, RANGE_PLUS_ONE, RANGE_MINUS_ONE, @@ -171,11 +190,13 @@ impl<'tcx> LateLintPass<'tcx> for Ranges { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { match expr.kind { - ExprKind::MethodCall(ref path, _, ref args, _) => { + ExprKind::MethodCall(path, args, _) => { check_range_zip_with_len(cx, path, args, expr.span); }, - ExprKind::Binary(ref op, ref l, ref r) => { - check_possible_range_contains(cx, op.node, l, r, expr.span); + ExprKind::Binary(ref op, l, r) => { + if meets_msrv(self.msrv.as_ref(), &msrvs::RANGE_CONTAINS) { + check_possible_range_contains(cx, op.node, l, r, expr); + } }, _ => {}, } @@ -184,9 +205,15 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { check_inclusive_range_minus_one(cx, expr); check_reversed_empty_range(cx, expr); } + extract_msrv_attr!(LateContext); } -fn check_possible_range_contains(cx: &LateContext<'_>, op: BinOpKind, l: &Expr<'_>, r: &Expr<'_>, span: Span) { +fn check_possible_range_contains(cx: &LateContext<'_>, op: BinOpKind, l: &Expr<'_>, r: &Expr<'_>, expr: &Expr<'_>) { + if in_constant(cx, expr.hir_id) { + return; + } + + let span = expr.span; let combine_and = match op { BinOpKind::And | BinOpKind::BitAnd => true, BinOpKind::Or | BinOpKind::BitOr => false, @@ -267,7 +294,7 @@ fn check_possible_range_contains(cx: &LateContext<'_>, op: BinOpKind, l: &Expr<' } fn check_range_bounds(cx: &LateContext<'_>, ex: &Expr<'_>) -> Option<(Constant, Ident, Span, Span, Ordering, bool)> { - if let ExprKind::Binary(ref op, ref l, ref r) = ex.kind { + if let ExprKind::Binary(ref op, l, r) = ex.kind { let (inclusive, ordering) = match op.node { BinOpKind::Gt => (false, Ordering::Greater), BinOpKind::Ge => (true, Ordering::Greater), @@ -300,32 +327,29 @@ fn match_ident(e: &Expr<'_>) -> Option { } fn check_range_zip_with_len(cx: &LateContext<'_>, path: &PathSegment<'_>, args: &[Expr<'_>], span: Span) { - let name = path.ident.as_str(); - if name == "zip" && args.len() == 2 { - let iter = &args[0].kind; - let zip_arg = &args[1]; - if_chain! { - // `.iter()` call - if let ExprKind::MethodCall(ref iter_path, _, ref iter_args, _) = *iter; - 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(zip_arg); - if is_integer_const(cx, start, 0); - // `.len()` call - if let ExprKind::MethodCall(ref len_path, _, ref len_args, _) = end.kind; - if len_path.ident.name == sym!(len) && len_args.len() == 1; - // `.iter()` and `.len()` called on same `Path` - if let ExprKind::Path(QPath::Resolved(_, ref iter_path)) = iter_args[0].kind; - if let ExprKind::Path(QPath::Resolved(_, ref len_path)) = len_args[0].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_args[0].span, "_")) - ); - } + if_chain! { + if path.ident.as_str() == "zip"; + if let [iter, zip_arg] = args; + // `.iter()` call + if let ExprKind::MethodCall(iter_path, iter_args, _) = 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_args, _) = end.kind; + if len_path.ident.name == sym::len && len_args.len() == 1; + // `.iter()` and `.len()` called on same `Path` + if let ExprKind::Path(QPath::Resolved(_, iter_path)) = iter_args[0].kind; + if let ExprKind::Path(QPath::Resolved(_, len_path)) = len_args[0].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_args[0].span, "_")) + ); } } } @@ -337,7 +361,7 @@ fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) { start, end: Some(end), limits: RangeLimits::HalfOpen - }) = higher::range(expr); + }) = higher::Range::hir(expr); if let Some(y) = y_plus_one(cx, end); then { let span = if expr.span.from_expansion() { @@ -354,8 +378,8 @@ fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) { span, "an inclusive range would be more readable", |diag| { - let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string()); - let end = Sugg::hir(cx, y, "y"); + let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").maybe_par().to_string()); + let end = Sugg::hir(cx, y, "y").maybe_par(); if let Some(is_wrapped) = &snippet_opt(cx, span) { if is_wrapped.starts_with('(') && is_wrapped.ends_with(')') { diag.span_suggestion( @@ -382,7 +406,7 @@ fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) { // inclusive range minus one: `x..=(y-1)` fn check_inclusive_range_minus_one(cx: &LateContext<'_>, expr: &Expr<'_>) { if_chain! { - if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::range(expr); + if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::Range::hir(expr); if let Some(y) = y_minus_one(cx, end); then { span_lint_and_then( @@ -391,8 +415,8 @@ fn check_inclusive_range_minus_one(cx: &LateContext<'_>, expr: &Expr<'_>) { expr.span, "an exclusive range would be more readable", |diag| { - let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string()); - let end = Sugg::hir(cx, y, "y"); + let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").maybe_par().to_string()); + let end = Sugg::hir(cx, y, "y").maybe_par(); diag.span_suggestion( expr.span, "use", @@ -419,8 +443,8 @@ fn inside_indexing_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { fn is_for_loop_arg(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { let mut cur_expr = expr; while let Some(parent_expr) = get_parent_expr(cx, cur_expr) { - match higher::for_loop(parent_expr) { - Some((_, args, _)) if args.hir_id == expr.hir_id => return true, + match higher::ForLoop::hir(parent_expr) { + Some(higher::ForLoop { arg, .. }) if arg.hir_id == expr.hir_id => return true, _ => cur_expr = parent_expr, } } @@ -436,7 +460,7 @@ fn is_empty_range(limits: RangeLimits, ordering: Ordering) -> bool { } if_chain! { - if let Some(higher::Range { start: Some(start), end: Some(end), limits }) = higher::range(expr); + if let Some(higher::Range { start: Some(start), end: Some(end), limits }) = higher::Range::hir(expr); let ty = cx.typeck_results().expr_ty(start); if let ty::Int(_) | ty::Uint(_) = ty.kind(); if let Some((start_idx, _)) = constant(cx, cx.typeck_results(), start); @@ -491,8 +515,8 @@ fn y_plus_one<'t>(cx: &LateContext<'_>, expr: &'t Expr<'_>) -> Option<&'t Expr<' Spanned { node: BinOpKind::Add, .. }, - ref lhs, - ref rhs, + lhs, + rhs, ) => { if is_integer_const(cx, lhs, 1) { Some(rhs) @@ -512,8 +536,8 @@ fn y_minus_one<'t>(cx: &LateContext<'_>, expr: &'t Expr<'_>) -> Option<&'t Expr< Spanned { node: BinOpKind::Sub, .. }, - ref lhs, - ref rhs, + lhs, + rhs, ) if is_integer_const(cx, rhs, 1) => Some(lhs), _ => None, }