diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index b99213011..a9d42159c 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy.yml @@ -39,7 +39,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3.0.2 + uses: actions/checkout@v3 - name: Install toolchain run: rustup show active-toolchain diff --git a/.github/workflows/clippy_bors.yml b/.github/workflows/clippy_bors.yml index 93198aabd..30a156c92 100644 --- a/.github/workflows/clippy_bors.yml +++ b/.github/workflows/clippy_bors.yml @@ -27,7 +27,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3.0.2 + uses: actions/checkout@v3 with: ref: ${{ github.ref }} @@ -83,7 +83,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3.0.2 + uses: actions/checkout@v3 - name: Install toolchain run: rustup show active-toolchain @@ -149,7 +149,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3.0.2 + uses: actions/checkout@v3 - name: Install toolchain run: rustup show active-toolchain @@ -173,7 +173,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3.0.2 + uses: actions/checkout@v3 - name: Install toolchain run: rustup show active-toolchain @@ -233,7 +233,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3.0.2 + uses: actions/checkout@v3 - name: Install toolchain run: rustup show active-toolchain diff --git a/.github/workflows/clippy_dev.yml b/.github/workflows/clippy_dev.yml index 14f20212a..514706d64 100644 --- a/.github/workflows/clippy_dev.yml +++ b/.github/workflows/clippy_dev.yml @@ -25,7 +25,7 @@ jobs: steps: # Setup - name: Checkout - uses: actions/checkout@v3.0.2 + uses: actions/checkout@v3 # Run - name: Build diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 71d71d103..f42928c2c 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -21,10 +21,10 @@ jobs: steps: # Setup - name: Checkout - uses: actions/checkout@v3.0.2 + uses: actions/checkout@v3 - name: Checkout - uses: actions/checkout@v3.0.2 + uses: actions/checkout@v3 with: ref: ${{ env.TARGET_BRANCH }} path: 'out' diff --git a/.github/workflows/remark.yml b/.github/workflows/remark.yml index 0bc2f49f5..7d25b6a2b 100644 --- a/.github/workflows/remark.yml +++ b/.github/workflows/remark.yml @@ -16,10 +16,10 @@ jobs: steps: # Setup - name: Checkout - uses: actions/checkout@v3.0.2 + uses: actions/checkout@v3 - name: Setup Node.js - uses: actions/setup-node@v1.4.4 + uses: actions/setup-node@v3 with: node-version: '14.x' diff --git a/CHANGELOG.md b/CHANGELOG.md index ebf5b58a5..79f2a4711 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4620,6 +4620,7 @@ Released 2018-09-13 [`else_if_without_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#else_if_without_else [`empty_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_drop [`empty_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum +[`empty_line_after_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_doc_comments [`empty_line_after_outer_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_outer_attr [`empty_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_loop [`empty_structs_with_brackets`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_structs_with_brackets @@ -4785,6 +4786,7 @@ Released 2018-09-13 [`manual_main_separator_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_main_separator_str [`manual_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_map [`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy +[`manual_next_back`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_next_back [`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive [`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or [`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains @@ -4897,6 +4899,7 @@ Released 2018-09-13 [`no_effect_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect_underscore_binding [`no_mangle_with_rust_abi`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_mangle_with_rust_abi [`non_ascii_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal +[`non_minimal_cfg`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_minimal_cfg [`non_octal_unix_permissions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_octal_unix_permissions [`non_send_fields_in_send_ty`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_send_fields_in_send_ty [`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool @@ -4978,6 +4981,7 @@ Released 2018-09-13 [`ref_binding_to_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_binding_to_reference [`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref [`ref_option_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_option_ref +[`ref_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_patterns [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro [`repeat_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_once [`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts diff --git a/README.md b/README.md index 6745e15c0..d712d3e67 100644 --- a/README.md +++ b/README.md @@ -278,7 +278,7 @@ If you want to contribute to Clippy, you can find more information in [CONTRIBUT -Copyright 2014-2022 The Rust Project Developers +Copyright 2014-2023 The Rust Project Developers Licensed under the Apache License, Version 2.0 or the MIT license diff --git a/book/src/development/type_checking.md b/book/src/development/type_checking.md index 225de8495..d7c2775b8 100644 --- a/book/src/development/type_checking.md +++ b/book/src/development/type_checking.md @@ -133,7 +133,7 @@ in this chapter: - [Type checking](https://rustc-dev-guide.rust-lang.org/type-checking.html) - [Ty module](https://rustc-dev-guide.rust-lang.org/ty.html) -[Adt]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/enum.TyKind.html#variant.Adt +[Adt]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/sty/enum.TyKind.html#variant.Adt [AdtDef]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/adt/struct.AdtDef.html [expr_ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TypeckResults.html#method.expr_ty [node_type]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TypeckResults.html#method.node_type @@ -142,9 +142,9 @@ in this chapter: [kind]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.Ty.html#method.kind [LateContext]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/struct.LateContext.html [LateLintPass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html -[pat_ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/context/struct.TypeckResults.html#method.pat_ty +[pat_ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/typeck_results/struct.TypeckResults.html#method.pat_ty [Ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.Ty.html -[TyKind]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/enum.TyKind.html +[TyKind]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/sty/enum.TyKind.html [TypeckResults]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TypeckResults.html [middle_ty]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_middle/ty/struct.Ty.html [hir_ty]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_hir/struct.Ty.html diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index 37e1e6a74..98e69c7fd 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -17,7 +17,7 @@ if_chain = "1.0" itertools = "0.10.1" pulldown-cmark = { version = "0.9", default-features = false } quine-mc_cluskey = "0.2" -regex-syntax = "0.6" +regex-syntax = "0.7" serde = { version = "1.0", features = ["derive"] } serde_json = { version = "1.0", optional = true } tempfile = { version = "3.2", optional = true } diff --git a/clippy_lints/src/assertions_on_constants.rs b/clippy_lints/src/assertions_on_constants.rs index a36df55d0..a8dc0cb3b 100644 --- a/clippy_lints/src/assertions_on_constants.rs +++ b/clippy_lints/src/assertions_on_constants.rs @@ -38,7 +38,7 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnConstants { _ => return, }; let Some((condition, panic_expn)) = find_assert_args(cx, e, macro_call.expn) else { return }; - let Some((Constant::Bool(val), _)) = constant(cx, cx.typeck_results(), condition) else { return }; + let Some(Constant::Bool(val)) = constant(cx, cx.typeck_results(), condition) else { return }; if val { span_lint_and_help( cx, diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 751c26267..897495ba1 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -176,6 +176,52 @@ declare_clippy_lint! { "empty line after outer attribute" } +declare_clippy_lint! { + /// ### What it does + /// Checks for empty lines after documenation comments. + /// + /// ### Why is this bad? + /// The documentation comment was most likely meant to be an inner attribute or regular comment. + /// If it was intended to be a documentation comment, then the empty line should be removed to + /// be more idiomatic. + /// + /// ### Known problems + /// Only detects empty lines immediately following the documentation. If the doc comment is followed + /// by an attribute and then an empty line, this lint will not trigger. Use `empty_line_after_outer_attr` + /// in combination with this lint to detect both cases. + /// + /// Does not detect empty lines after doc attributes (e.g. `#[doc = ""]`). + /// + /// ### Example + /// ```rust + /// /// Some doc comment with a blank line after it. + /// + /// fn not_quite_good_code() { } + /// ``` + /// + /// Use instead: + /// ```rust + /// /// Good (no blank line) + /// fn this_is_fine() { } + /// ``` + /// + /// ```rust + /// // Good (convert to a regular comment) + /// + /// fn this_is_fine_too() { } + /// ``` + /// + /// ```rust + /// //! Good (convert to a comment on an inner attribute) + /// + /// fn this_is_fine_as_well() { } + /// ``` + #[clippy::version = "1.70.0"] + pub EMPTY_LINE_AFTER_DOC_COMMENTS, + nursery, + "empty line after documentation comments" +} + declare_clippy_lint! { /// ### What it does /// Checks for `warn`/`deny`/`forbid` attributes targeting the whole clippy::restriction category. @@ -292,6 +338,30 @@ declare_clippy_lint! { "ensures that all `allow` and `expect` attributes have a reason" } +declare_clippy_lint! { + /// ### What it does + /// Checks for `any` and `all` combinators in `cfg` with only one condition. + /// + /// ### Why is this bad? + /// If there is only one condition, no need to wrap it into `any` or `all` combinators. + /// + /// ### Example + /// ```rust + /// #[cfg(any(unix))] + /// pub struct Bar; + /// ``` + /// + /// Use instead: + /// ```rust + /// #[cfg(unix)] + /// pub struct Bar; + /// ``` + #[clippy::version = "1.71.0"] + pub NON_MINIMAL_CFG, + style, + "ensure that all `cfg(any())` and `cfg(all())` have more than one condition" +} + declare_lint_pass!(Attributes => [ ALLOW_ATTRIBUTES_WITHOUT_REASON, INLINE_ALWAYS, @@ -604,6 +674,8 @@ impl_lint_pass!(EarlyAttributes => [ DEPRECATED_CFG_ATTR, MISMATCHED_TARGET_OS, EMPTY_LINE_AFTER_OUTER_ATTR, + EMPTY_LINE_AFTER_DOC_COMMENTS, + NON_MINIMAL_CFG, ]); impl EarlyLintPass for EarlyAttributes { @@ -614,15 +686,22 @@ impl EarlyLintPass for EarlyAttributes { fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) { check_deprecated_cfg_attr(cx, attr, &self.msrv); check_mismatched_target_os(cx, attr); + check_minimal_cfg_condition(cx, attr); } extract_msrv_attr!(EarlyContext); } +/// Check for empty lines after outer attributes. +/// +/// Attributes and documenation comments are both considered outer attributes +/// by the AST. However, the average user likely considers them to be different. +/// Checking for empty lines after each of these attributes is split into two different +/// lints but can share the same logic. fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::Item) { let mut iter = item.attrs.iter().peekable(); while let Some(attr) = iter.next() { - if matches!(attr.kind, AttrKind::Normal(..)) + if (matches!(attr.kind, AttrKind::Normal(..)) || matches!(attr.kind, AttrKind::DocComment(..))) && attr.style == AttrStyle::Outer && is_present_in_source(cx, attr.span) { @@ -639,13 +718,20 @@ fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::It let lines = without_block_comments(lines); if lines.iter().filter(|l| l.trim().is_empty()).count() > 2 { - span_lint( - cx, - EMPTY_LINE_AFTER_OUTER_ATTR, - begin_of_attr_to_item, - "found an empty line after an outer attribute. \ - Perhaps you forgot to add a `!` to make it an inner attribute?", - ); + let (lint_msg, lint_type) = match attr.kind { + AttrKind::DocComment(..) => ( + "found an empty line after a doc comment. \ + Perhaps you need to use `//!` to make a comment on a module, remove the empty line, or make a regular comment with `//`?", + EMPTY_LINE_AFTER_DOC_COMMENTS, + ), + AttrKind::Normal(..) => ( + "found an empty line after an outer attribute. \ + Perhaps you forgot to add a `!` to make it an inner attribute?", + EMPTY_LINE_AFTER_OUTER_ATTR, + ), + }; + + span_lint(cx, lint_type, begin_of_attr_to_item, lint_msg); } } } @@ -690,6 +776,48 @@ fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msr } } +fn check_nested_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) { + for item in items.iter() { + if let NestedMetaItem::MetaItem(meta) = item { + if !meta.has_name(sym::any) && !meta.has_name(sym::all) { + continue; + } + if let MetaItemKind::List(list) = &meta.kind { + check_nested_cfg(cx, list); + if list.len() == 1 { + span_lint_and_then( + cx, + NON_MINIMAL_CFG, + meta.span, + "unneeded sub `cfg` when there is only one condition", + |diag| { + if let Some(snippet) = snippet_opt(cx, list[0].span()) { + diag.span_suggestion(meta.span, "try", snippet, Applicability::MaybeIncorrect); + } + }, + ); + } else if list.is_empty() && meta.has_name(sym::all) { + span_lint_and_then( + cx, + NON_MINIMAL_CFG, + meta.span, + "unneeded sub `cfg` when there is no condition", + |_| {}, + ); + } + } + } + } +} + +fn check_minimal_cfg_condition(cx: &EarlyContext<'_>, attr: &Attribute) { + if attr.has_name(sym::cfg) && + let Some(items) = attr.meta_item_list() + { + check_nested_cfg(cx, &items); + } +} + fn check_mismatched_target_os(cx: &EarlyContext<'_>, attr: &Attribute) { fn find_os(name: &str) -> Option<&'static str> { UNIX_SYSTEMS diff --git a/clippy_lints/src/borrow_deref_ref.rs b/clippy_lints/src/borrow_deref_ref.rs index c4520d003..814108ed8 100644 --- a/clippy_lints/src/borrow_deref_ref.rs +++ b/clippy_lints/src/borrow_deref_ref.rs @@ -1,5 +1,6 @@ use crate::reference::DEREF_ADDROF; use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::is_from_proc_macro; use clippy_utils::source::snippet_opt; use clippy_utils::ty::implements_trait; use clippy_utils::{get_parent_expr, is_lint_allowed}; @@ -47,8 +48,8 @@ declare_clippy_lint! { declare_lint_pass!(BorrowDerefRef => [BORROW_DEREF_REF]); -impl LateLintPass<'_> for BorrowDerefRef { - fn check_expr(&mut self, cx: &LateContext<'_>, e: &rustc_hir::Expr<'_>) { +impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef { + fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &rustc_hir::Expr<'tcx>) { if_chain! { if !e.span.from_expansion(); if let ExprKind::AddrOf(_, Mutability::Not, addrof_target) = e.kind; @@ -58,6 +59,7 @@ impl LateLintPass<'_> for BorrowDerefRef { if !matches!(deref_target.kind, ExprKind::Unary(UnOp::Deref, ..) ); let ref_ty = cx.typeck_results().expr_ty(deref_target); if let ty::Ref(_, inner_ty, Mutability::Not) = ref_ty.kind(); + if !is_from_proc_macro(cx, e); then{ if let Some(parent_expr) = get_parent_expr(cx, e){ diff --git a/clippy_lints/src/box_default.rs b/clippy_lints/src/box_default.rs index dfa949d1a..e42c3fe24 100644 --- a/clippy_lints/src/box_default.rs +++ b/clippy_lints/src/box_default.rs @@ -8,7 +8,9 @@ use rustc_hir::{ Block, Expr, ExprKind, Local, Node, QPath, TyKind, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; -use rustc_middle::{lint::in_external_macro, ty::print::with_forced_trimmed_paths}; +use rustc_middle::lint::in_external_macro; +use rustc_middle::ty::print::with_forced_trimmed_paths; +use rustc_middle::ty::IsSuggestable; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -49,7 +51,6 @@ impl LateLintPass<'_> for BoxDefault { && path_def_id(cx, ty).map_or(false, |id| Some(id) == cx.tcx.lang_items().owned_box()) && is_default_equivalent(cx, arg) { - let arg_ty = cx.typeck_results().expr_ty(arg); span_lint_and_sugg( cx, BOX_DEFAULT, @@ -58,8 +59,10 @@ impl LateLintPass<'_> for BoxDefault { "try", if is_plain_default(arg_path) || given_type(cx, expr) { "Box::default()".into() - } else { + } else if let Some(arg_ty) = cx.typeck_results().expr_ty(arg).make_suggestable(cx.tcx, true) { with_forced_trimmed_paths!(format!("Box::<{arg_ty}>::default()")) + } else { + return }, Applicability::MachineApplicable ); diff --git a/clippy_lints/src/casts/cast_nan_to_int.rs b/clippy_lints/src/casts/cast_nan_to_int.rs index 322dc41b3..da756129d 100644 --- a/clippy_lints/src/casts/cast_nan_to_int.rs +++ b/clippy_lints/src/casts/cast_nan_to_int.rs @@ -21,8 +21,8 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, fn is_known_nan(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { match constant(cx, cx.typeck_results(), e) { - Some((Constant::F64(n), _)) => n.is_nan(), - Some((Constant::F32(n), _)) => n.is_nan(), + Some(Constant::F64(n)) => n.is_nan(), + Some(Constant::F32(n)) => n.is_nan(), _ => false, } } diff --git a/clippy_lints/src/casts/cast_possible_truncation.rs b/clippy_lints/src/casts/cast_possible_truncation.rs index 95c2ecbf7..84b99ad5c 100644 --- a/clippy_lints/src/casts/cast_possible_truncation.rs +++ b/clippy_lints/src/casts/cast_possible_truncation.rs @@ -15,7 +15,7 @@ use rustc_target::abi::IntegerType; use super::{utils, CAST_ENUM_TRUNCATION, CAST_POSSIBLE_TRUNCATION}; fn constant_int(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { - if let Some((Constant::Int(c), _)) = constant(cx, cx.typeck_results(), expr) { + if let Some(Constant::Int(c)) = constant(cx, cx.typeck_results(), expr) { Some(c) } else { None diff --git a/clippy_lints/src/casts/cast_sign_loss.rs b/clippy_lints/src/casts/cast_sign_loss.rs index a20a97d4e..a83dfd94d 100644 --- a/clippy_lints/src/casts/cast_sign_loss.rs +++ b/clippy_lints/src/casts/cast_sign_loss.rs @@ -29,7 +29,7 @@ fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast // Don't lint for positive constants. let const_val = constant(cx, cx.typeck_results(), cast_op); if_chain! { - if let Some((Constant::Int(n), _)) = const_val; + if let Some(Constant::Int(n)) = const_val; if let ty::Int(ity) = *cast_from.kind(); if sext(cx.tcx, n, ity) >= 0; then { diff --git a/clippy_lints/src/dbg_macro.rs b/clippy_lints/src/dbg_macro.rs index 799e71e84..ea17e7a60 100644 --- a/clippy_lints/src/dbg_macro.rs +++ b/clippy_lints/src/dbg_macro.rs @@ -3,10 +3,10 @@ use clippy_utils::macros::root_macro_call_first_node; use clippy_utils::source::snippet_with_applicability; use clippy_utils::{is_in_cfg_test, is_in_test_function}; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_hir::{Expr, ExprKind, Node}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::sym; +use rustc_span::{sym, BytePos, Pos, Span}; declare_clippy_lint! { /// ### What it does @@ -31,6 +31,31 @@ declare_clippy_lint! { "`dbg!` macro is intended as a debugging tool" } +/// Gets the span of the statement up to the next semicolon, if and only if the next +/// non-whitespace character actually is a semicolon. +/// E.g. +/// ```rust,ignore +/// +/// dbg!(); +/// ^^^^^^^ this span is returned +/// +/// foo!(dbg!()); +/// no span is returned +/// ``` +fn span_including_semi(cx: &LateContext<'_>, span: Span) -> Option { + let sm = cx.sess().source_map(); + let sf = sm.lookup_source_file(span.hi()); + let src = sf.src.as_ref()?.get(span.hi().to_usize()..)?; + let first_non_whitespace = src.find(|c: char| !c.is_whitespace())?; + + if src.as_bytes()[first_non_whitespace] == b';' { + let hi = span.hi() + BytePos::from_usize(first_non_whitespace + 1); + Some(span.with_hi(hi)) + } else { + None + } +} + #[derive(Copy, Clone)] pub struct DbgMacro { allow_dbg_in_tests: bool, @@ -55,13 +80,25 @@ impl LateLintPass<'_> for DbgMacro { return; } let mut applicability = Applicability::MachineApplicable; - let suggestion = match expr.peel_drop_temps().kind { + + let (sugg_span, suggestion) = match expr.peel_drop_temps().kind { // dbg!() - ExprKind::Block(_, _) => String::new(), - // dbg!(1) - ExprKind::Match(val, ..) => { - snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability).to_string() + ExprKind::Block(..) => { + // If the `dbg!` macro is a "free" statement and not contained within other expressions, + // remove the whole statement. + if let Some(Node::Stmt(stmt)) = cx.tcx.hir().find_parent(expr.hir_id) + && let Some(span) = span_including_semi(cx, stmt.span.source_callsite()) + { + (span, String::new()) + } else { + (macro_call.span, String::from("()")) + } }, + // dbg!(1) + ExprKind::Match(val, ..) => ( + macro_call.span, + snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability).to_string(), + ), // dbg!(2, 3) ExprKind::Tup( [ @@ -82,7 +119,7 @@ impl LateLintPass<'_> for DbgMacro { "..", &mut applicability, ); - format!("({snippet})") + (macro_call.span, format!("({snippet})")) }, _ => return, }; @@ -90,7 +127,7 @@ impl LateLintPass<'_> for DbgMacro { span_lint_and_sugg( cx, DBG_MACRO, - macro_call.span, + sugg_span, "the `dbg!` macro is intended as a debugging tool", "remove the invocation before committing it to a version control system", suggestion, diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 04993e492..423eee477 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -48,9 +48,11 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::attrs::BLANKET_CLIPPY_RESTRICTION_LINTS_INFO, crate::attrs::DEPRECATED_CFG_ATTR_INFO, crate::attrs::DEPRECATED_SEMVER_INFO, + crate::attrs::EMPTY_LINE_AFTER_DOC_COMMENTS_INFO, crate::attrs::EMPTY_LINE_AFTER_OUTER_ATTR_INFO, crate::attrs::INLINE_ALWAYS_INFO, crate::attrs::MISMATCHED_TARGET_OS_INFO, + crate::attrs::NON_MINIMAL_CFG_INFO, crate::attrs::USELESS_ATTRIBUTE_INFO, crate::await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE_INFO, crate::await_holding_invalid::AWAIT_HOLDING_LOCK_INFO, @@ -347,6 +349,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::ITER_WITH_DRAIN_INFO, crate::methods::MANUAL_FILTER_MAP_INFO, crate::methods::MANUAL_FIND_MAP_INFO, + crate::methods::MANUAL_NEXT_BACK_INFO, crate::methods::MANUAL_OK_OR_INFO, crate::methods::MANUAL_SATURATING_ARITHMETIC_INFO, crate::methods::MANUAL_SPLIT_ONCE_INFO, @@ -485,7 +488,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::operators::FLOAT_EQUALITY_WITHOUT_ABS_INFO, crate::operators::IDENTITY_OP_INFO, crate::operators::INEFFECTIVE_BIT_MASK_INFO, - crate::operators::INTEGER_ARITHMETIC_INFO, crate::operators::INTEGER_DIVISION_INFO, crate::operators::MISREFACTORED_ASSIGN_OP_INFO, crate::operators::MODULO_ARITHMETIC_INFO, @@ -535,6 +537,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::redundant_slicing::REDUNDANT_SLICING_INFO, crate::redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES_INFO, crate::ref_option_ref::REF_OPTION_REF_INFO, + crate::ref_patterns::REF_PATTERNS_INFO, crate::reference::DEREF_ADDROF_INFO, crate::regex::INVALID_REGEX_INFO, crate::regex::TRIVIAL_REGEX_INFO, diff --git a/clippy_lints/src/default_constructed_unit_structs.rs b/clippy_lints/src/default_constructed_unit_structs.rs index e529d81a7..9bd7a0dc0 100644 --- a/clippy_lints/src/default_constructed_unit_structs.rs +++ b/clippy_lints/src/default_constructed_unit_structs.rs @@ -1,4 +1,4 @@ -use clippy_utils::{diagnostics::span_lint_and_sugg, is_from_proc_macro, match_def_path, paths}; +use clippy_utils::{diagnostics::span_lint_and_sugg, match_def_path, paths}; use hir::{def::Res, ExprKind}; use rustc_errors::Applicability; use rustc_hir as hir; @@ -55,7 +55,8 @@ impl LateLintPass<'_> for DefaultConstructedUnitStructs { if let ty::Adt(def, ..) = cx.typeck_results().expr_ty(expr).kind(); if def.is_struct(); if let var @ ty::VariantDef { ctor: Some((hir::def::CtorKind::Const, _)), .. } = def.non_enum_variant(); - if !var.is_field_list_non_exhaustive() && !is_from_proc_macro(cx, expr); + if !var.is_field_list_non_exhaustive(); + if !expr.span.from_expansion() && !qpath.span().from_expansion(); then { span_lint_and_sugg( cx, diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index a1a2c398a..3c55a563a 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -114,7 +114,7 @@ declare_lint_pass!(FloatingPointArithmetic => [ // Returns the specialized log method for a given base if base is constant // and is one of 2, 10 and e fn get_specialized_log_method(cx: &LateContext<'_>, base: &Expr<'_>) -> Option<&'static str> { - if let Some((value, _)) = constant(cx, cx.typeck_results(), base) { + if let Some(value) = constant(cx, cx.typeck_results(), base) { if F32(2.0) == value || F64(2.0) == value { return Some("log2"); } else if F32(10.0) == value || F64(10.0) == value { @@ -193,8 +193,8 @@ fn check_ln1p(cx: &LateContext<'_>, expr: &Expr<'_>, receiver: &Expr<'_>) { constant(cx, cx.typeck_results(), lhs), constant(cx, cx.typeck_results(), rhs), ) { - (Some((value, _)), _) if F32(1.0) == value || F64(1.0) == value => rhs, - (_, Some((value, _))) if F32(1.0) == value || F64(1.0) == value => lhs, + (Some(value), _) if F32(1.0) == value || F64(1.0) == value => rhs, + (_, Some(value)) if F32(1.0) == value || F64(1.0) == value => lhs, _ => return, }; @@ -237,7 +237,7 @@ fn get_integer_from_float_constant(value: &Constant) -> Option { fn check_powf(cx: &LateContext<'_>, expr: &Expr<'_>, receiver: &Expr<'_>, args: &[Expr<'_>]) { // Check receiver - if let Some((value, _)) = constant(cx, cx.typeck_results(), receiver) { + if let Some(value) = constant(cx, cx.typeck_results(), receiver) { if let Some(method) = if F32(f32_consts::E) == value || F64(f64_consts::E) == value { Some("exp") } else if F32(2.0) == value || F64(2.0) == value { @@ -258,7 +258,7 @@ fn check_powf(cx: &LateContext<'_>, expr: &Expr<'_>, receiver: &Expr<'_>, args: } // Check argument - if let Some((value, _)) = constant(cx, cx.typeck_results(), &args[0]) { + if let Some(value) = constant(cx, cx.typeck_results(), &args[0]) { let (lint, help, suggestion) = if F32(1.0 / 2.0) == value || F64(1.0 / 2.0) == value { ( SUBOPTIMAL_FLOPS, @@ -298,7 +298,7 @@ fn check_powf(cx: &LateContext<'_>, expr: &Expr<'_>, receiver: &Expr<'_>, args: } fn check_powi(cx: &LateContext<'_>, expr: &Expr<'_>, receiver: &Expr<'_>, args: &[Expr<'_>]) { - if let Some((value, _)) = constant(cx, cx.typeck_results(), &args[0]) { + if let Some(value) = constant(cx, cx.typeck_results(), &args[0]) { if value == Int(2) { if let Some(parent) = get_parent_expr(cx, expr) { if let Some(grandparent) = get_parent_expr(cx, parent) { @@ -384,8 +384,8 @@ fn detect_hypot(cx: &LateContext<'_>, receiver: &Expr<'_>) -> Option { _ ) = &add_rhs.kind; if lmethod_name.as_str() == "powi" && rmethod_name.as_str() == "powi"; - if let Some((lvalue, _)) = constant(cx, cx.typeck_results(), largs_1); - if let Some((rvalue, _)) = constant(cx, cx.typeck_results(), rargs_1); + if let Some(lvalue) = constant(cx, cx.typeck_results(), largs_1); + if let Some(rvalue) = constant(cx, cx.typeck_results(), rargs_1); if Int(2) == lvalue && Int(2) == rvalue; then { return Some(format!("{}.hypot({})", Sugg::hir(cx, largs_0, "..").maybe_par(), Sugg::hir(cx, rargs_0, ".."))); @@ -416,7 +416,7 @@ fn check_expm1(cx: &LateContext<'_>, expr: &Expr<'_>) { if_chain! { if let ExprKind::Binary(Spanned { node: BinOpKind::Sub, .. }, lhs, rhs) = expr.kind; if cx.typeck_results().expr_ty(lhs).is_floating_point(); - if let Some((value, _)) = constant(cx, cx.typeck_results(), rhs); + if let Some(value) = constant(cx, cx.typeck_results(), rhs); if F32(1.0) == value || F64(1.0) == value; if let ExprKind::MethodCall(path, self_arg, ..) = &lhs.kind; if cx.typeck_results().expr_ty(self_arg).is_floating_point(); @@ -669,8 +669,8 @@ fn check_radians(cx: &LateContext<'_>, expr: &Expr<'_>) { mul_lhs, mul_rhs, ) = &div_lhs.kind; - if let Some((rvalue, _)) = constant(cx, cx.typeck_results(), div_rhs); - if let Some((lvalue, _)) = constant(cx, cx.typeck_results(), mul_rhs); + if let Some(rvalue) = constant(cx, cx.typeck_results(), div_rhs); + if let Some(lvalue) = constant(cx, cx.typeck_results(), mul_rhs); then { // TODO: also check for constant values near PI/180 or 180/PI if (F32(f32_consts::PI) == rvalue || F64(f64_consts::PI) == rvalue) && diff --git a/clippy_lints/src/fn_null_check.rs b/clippy_lints/src/fn_null_check.rs index d8f4a5fe2..521045a9f 100644 --- a/clippy_lints/src/fn_null_check.rs +++ b/clippy_lints/src/fn_null_check.rs @@ -89,11 +89,7 @@ impl<'tcx> LateLintPass<'tcx> for FnNullCheck { // Catching: // (fn_ptr as * ) == - _ if matches!( - constant(cx, cx.typeck_results(), to_check), - Some((Constant::RawPtr(0), _)) - ) => - { + _ if matches!(constant(cx, cx.typeck_results(), to_check), Some(Constant::RawPtr(0))) => { lint_expr(cx, expr); }, diff --git a/clippy_lints/src/implicit_saturating_add.rs b/clippy_lints/src/implicit_saturating_add.rs index 012aa5a1d..ee7973b82 100644 --- a/clippy_lints/src/implicit_saturating_add.rs +++ b/clippy_lints/src/implicit_saturating_add.rs @@ -101,10 +101,10 @@ fn get_int_max(ty: Ty<'_>) -> Option { fn get_const<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<(u128, BinOpKind, &'tcx Expr<'tcx>)> { if let ExprKind::Binary(op, l, r) = expr.kind { let tr = cx.typeck_results(); - if let Some((Constant::Int(c), _)) = constant(cx, tr, r) { + if let Some(Constant::Int(c)) = constant(cx, tr, r) { return Some((c, op.node, l)); }; - if let Some((Constant::Int(c), _)) = constant(cx, tr, l) { + if let Some(Constant::Int(c)) = constant(cx, tr, l) { return Some((c, invert_op(op.node)?, r)); } } diff --git a/clippy_lints/src/index_refutable_slice.rs b/clippy_lints/src/index_refutable_slice.rs index bdeddf44d..7a269e98f 100644 --- a/clippy_lints/src/index_refutable_slice.rs +++ b/clippy_lints/src/index_refutable_slice.rs @@ -254,7 +254,7 @@ impl<'a, 'tcx> Visitor<'tcx> for SliceIndexLintingVisitor<'a, 'tcx> { let parent_id = map.parent_id(expr.hir_id); if let Some(hir::Node::Expr(parent_expr)) = map.find(parent_id); if let hir::ExprKind::Index(_, index_expr) = parent_expr.kind; - if let Some((Constant::Int(index_value), _)) = constant(cx, cx.typeck_results(), index_expr); + if let Some(Constant::Int(index_value)) = constant(cx, cx.typeck_results(), index_expr); if let Ok(index_value) = index_value.try_into(); if index_value < max_suggested_slice; diff --git a/clippy_lints/src/indexing_slicing.rs b/clippy_lints/src/indexing_slicing.rs index 924a361c0..22c14d9b0 100644 --- a/clippy_lints/src/indexing_slicing.rs +++ b/clippy_lints/src/indexing_slicing.rs @@ -191,18 +191,14 @@ impl<'tcx> LateLintPass<'tcx> for IndexingSlicing { /// Returns a tuple of options with the start and end (exclusive) values of /// the range. If the start or end is not constant, None is returned. fn to_const_range(cx: &LateContext<'_>, range: higher::Range<'_>, array_size: u128) -> (Option, Option) { - let s = range - .start - .map(|expr| constant(cx, cx.typeck_results(), expr).map(|(c, _)| c)); + let s = range.start.map(|expr| constant(cx, cx.typeck_results(), expr)); let start = match s { Some(Some(Constant::Int(x))) => Some(x), Some(_) => None, None => Some(0), }; - let e = range - .end - .map(|expr| constant(cx, cx.typeck_results(), expr).map(|(c, _)| c)); + let e = range.end.map(|expr| constant(cx, cx.typeck_results(), expr)); let end = match e { Some(Some(Constant::Int(x))) => { if range.limits == RangeLimits::Closed { diff --git a/clippy_lints/src/let_underscore.rs b/clippy_lints/src/let_underscore.rs index 16772a9d5..e66141809 100644 --- a/clippy_lints/src/let_underscore.rs +++ b/clippy_lints/src/let_underscore.rs @@ -1,10 +1,12 @@ use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::is_from_proc_macro; use clippy_utils::ty::{implements_trait, is_must_use_ty, match_type}; use clippy_utils::{is_must_use_func_call, paths}; -use rustc_hir::{ExprKind, Local, PatKind}; +use rustc_hir::{Local, PatKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::subst::GenericArgKind; +use rustc_middle::ty::IsSuggestable; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{BytePos, Span}; @@ -138,7 +140,7 @@ const SYNC_GUARD_PATHS: [&[&str]; 3] = [ ]; impl<'tcx> LateLintPass<'tcx> for LetUnderscore { - fn check_local(&mut self, cx: &LateContext<'_>, local: &Local<'_>) { + fn check_local(&mut self, cx: &LateContext<'tcx>, local: &Local<'tcx>) { if !in_external_macro(cx.tcx.sess, local.span) && let PatKind::Wild = local.pat.kind && let Some(init) = local.init @@ -191,15 +193,17 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore { if local.pat.default_binding_modes && local.ty.is_none() { // When `default_binding_modes` is true, the `let` keyword is present. - // Ignore function calls that return impl traits... - if let Some(init) = local.init && - matches!(init.kind, ExprKind::Call(_, _) | ExprKind::MethodCall(_, _, _, _)) { - let expr_ty = cx.typeck_results().expr_ty(init); - if expr_ty.is_impl_trait() { - return; - } - } + // Ignore unnameable types + if let Some(init) = local.init + && !cx.typeck_results().expr_ty(init).is_suggestable(cx.tcx, true) + { + return; + } + // Ignore if it is from a procedural macro... + if is_from_proc_macro(cx, init) { + return; + } span_lint_and_help( cx, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 3517842a0..b442a4ac5 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -266,6 +266,7 @@ mod redundant_pub_crate; mod redundant_slicing; mod redundant_static_lifetimes; mod ref_option_ref; +mod ref_patterns; mod reference; mod regex; mod return_self_not_must_use; @@ -331,8 +332,11 @@ mod zero_div_zero; mod zero_sized_map_values; // end lints modules, do not remove this comment, it’s used in `update_lints` -use crate::utils::conf::{format_error, TryConf}; pub use crate::utils::conf::{lookup_conf_file, Conf}; +use crate::utils::{ + conf::{format_error, metadata::get_configuration_metadata, TryConf}, + FindAll, +}; /// Register all pre expansion lints /// @@ -471,7 +475,22 @@ pub(crate) struct LintInfo { pub fn explain(name: &str) { let target = format!("clippy::{}", name.to_ascii_uppercase()); match declared_lints::LINTS.iter().find(|info| info.lint.name == target) { - Some(info) => print!("{}", info.explanation), + Some(info) => { + println!("{}", info.explanation); + // Check if the lint has configuration + let mdconf = get_configuration_metadata(); + if let Some(config_vec_positions) = mdconf + .iter() + .find_all(|cconf| cconf.lints.contains(&info.lint.name_lower()[8..].to_owned())) + { + // If it has, print it + println!("### Configuration for {}:\n", info.lint.name_lower()); + for position in config_vec_positions { + let conf = &mdconf[position]; + println!(" - {}: {} (default: {})", conf.name, conf.doc, conf.default); + } + } + }, None => println!("unknown lint: {name}"), } } @@ -971,6 +990,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(manual_slice_size_calculation::ManualSliceSizeCalculation)); store.register_early_pass(|| Box::new(suspicious_doc_comments::SuspiciousDocComments)); store.register_late_pass(|_| Box::new(items_after_test_module::ItemsAfterTestModule)); + store.register_early_pass(|| Box::new(ref_patterns::RefPatterns)); store.register_late_pass(|_| Box::new(default_constructed_unit_structs::DefaultConstructedUnitStructs)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/lines_filter_map_ok.rs b/clippy_lints/src/lines_filter_map_ok.rs index bba9bb445..09b2032e2 100644 --- a/clippy_lints/src/lines_filter_map_ok.rs +++ b/clippy_lints/src/lines_filter_map_ok.rs @@ -25,7 +25,7 @@ declare_clippy_lint! { /// /// ### Known problems /// This lint suggests replacing `filter_map()` or `flat_map()` applied to a `Lines` - /// instance in all cases. There two cases where the suggestion might not be + /// instance in all cases. There are two cases where the suggestion might not be /// appropriate or necessary: /// /// - If the `Lines` instance can never produce any error, or if an error is produced diff --git a/clippy_lints/src/manual_let_else.rs b/clippy_lints/src/manual_let_else.rs index 1247370b7..3f8b42ffe 100644 --- a/clippy_lints/src/manual_let_else.rs +++ b/clippy_lints/src/manual_let_else.rs @@ -38,7 +38,6 @@ declare_clippy_lint! { /// Could be written: /// /// ```rust - /// # #![feature(let_else)] /// # fn main () { /// # let w = Some(0); /// let Some(v) = w else { return }; @@ -69,29 +68,23 @@ impl_lint_pass!(ManualLetElse => [MANUAL_LET_ELSE]); impl<'tcx> LateLintPass<'tcx> for ManualLetElse { fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &'tcx Stmt<'tcx>) { - let if_let_or_match = if_chain! { - if self.msrv.meets(msrvs::LET_ELSE); - if !in_external_macro(cx.sess(), stmt.span); - if let StmtKind::Local(local) = stmt.kind; - if let Some(init) = local.init; - if local.els.is_none(); - if local.ty.is_none(); - if init.span.ctxt() == stmt.span.ctxt(); - if let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init); - then { - if_let_or_match - } else { - return; - } - }; + if !self.msrv.meets(msrvs::LET_ELSE) || in_external_macro(cx.sess(), stmt.span) { + return; + } + if let StmtKind::Local(local) = stmt.kind && + let Some(init) = local.init && + local.els.is_none() && + local.ty.is_none() && + init.span.ctxt() == stmt.span.ctxt() && + let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init) { match if_let_or_match { IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => if_chain! { if expr_is_simple_identity(let_pat, if_then); if let Some(if_else) = if_else; if expr_diverges(cx, if_else); then { - emit_manual_let_else(cx, stmt.span, if_let_expr, let_pat, if_else); + emit_manual_let_else(cx, stmt.span, if_let_expr, local.pat, let_pat, if_else); } }, IfLetOrMatch::Match(match_expr, arms, source) => { @@ -128,15 +121,23 @@ impl<'tcx> LateLintPass<'tcx> for ManualLetElse { return; } - emit_manual_let_else(cx, stmt.span, match_expr, pat_arm.pat, diverging_arm.body); + emit_manual_let_else(cx, stmt.span, match_expr, local.pat, pat_arm.pat, diverging_arm.body); }, } + }; } extract_msrv_attr!(LateContext); } -fn emit_manual_let_else(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, pat: &Pat<'_>, else_body: &Expr<'_>) { +fn emit_manual_let_else( + cx: &LateContext<'_>, + span: Span, + expr: &Expr<'_>, + local: &Pat<'_>, + pat: &Pat<'_>, + else_body: &Expr<'_>, +) { span_lint_and_then( cx, MANUAL_LET_ELSE, @@ -145,12 +146,11 @@ fn emit_manual_let_else(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, pat: |diag| { // This is far from perfect, for example there needs to be: // * mut additions for the bindings - // * renamings of the bindings + // * renamings of the bindings for `PatKind::Or` // * unused binding collision detection with existing ones // * putting patterns with at the top level | inside () // for this to be machine applicable. let mut app = Applicability::HasPlaceholders; - let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", &mut app); let (sn_expr, _) = snippet_with_context(cx, expr.span, span.ctxt(), "", &mut app); let (sn_else, _) = snippet_with_context(cx, else_body.span, span.ctxt(), "", &mut app); @@ -159,10 +159,21 @@ fn emit_manual_let_else(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, pat: } else { format!("{{ {sn_else} }}") }; - let sn_bl = if matches!(pat.kind, PatKind::Or(..)) { - format!("({sn_pat})") - } else { - sn_pat.into_owned() + let sn_bl = match pat.kind { + PatKind::Or(..) => { + let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", &mut app); + format!("({sn_pat})") + }, + // Replace the variable name iff `TupleStruct` has one argument like `Variant(v)`. + PatKind::TupleStruct(ref w, args, ..) if args.len() == 1 => { + let sn_wrapper = cx.sess().source_map().span_to_snippet(w.span()).unwrap_or_default(); + let (sn_inner, _) = snippet_with_context(cx, local.span, span.ctxt(), "", &mut app); + format!("{sn_wrapper}({sn_inner})") + }, + _ => { + let (sn_pat, _) = snippet_with_context(cx, pat.span, span.ctxt(), "", &mut app); + sn_pat.into_owned() + }, }; let sugg = format!("let {sn_bl} = {sn_expr} else {else_bl};"); diag.span_suggestion(span, "consider writing", sugg, app); diff --git a/clippy_lints/src/manual_strip.rs b/clippy_lints/src/manual_strip.rs index 7d28c1116..93d977a5c 100644 --- a/clippy_lints/src/manual_strip.rs +++ b/clippy_lints/src/manual_strip.rs @@ -144,7 +144,7 @@ fn len_arg<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx E // Returns the length of the `expr` if it's a constant string or char. fn constant_length(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { - let (value, _) = constant(cx, cx.typeck_results(), expr)?; + let value = constant(cx, cx.typeck_results(), expr)?; match value { Constant::Str(value) => Some(value.len() as u128), Constant::Char(value) => Some(value.len_utf8() as u128), diff --git a/clippy_lints/src/matches/match_like_matches.rs b/clippy_lints/src/matches/match_like_matches.rs index 33bc20dad..0064619ef 100644 --- a/clippy_lints/src/matches/match_like_matches.rs +++ b/clippy_lints/src/matches/match_like_matches.rs @@ -1,10 +1,12 @@ +use super::REDUNDANT_PATTERN_MATCHING; use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::is_lint_allowed; use clippy_utils::is_wild; use clippy_utils::source::snippet_with_applicability; use clippy_utils::span_contains_comment; use rustc_ast::{Attribute, LitKind}; use rustc_errors::Applicability; -use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Guard, Pat}; +use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Guard, Pat, PatKind, QPath}; use rustc_lint::{LateContext, LintContext}; use rustc_middle::ty; use rustc_span::source_map::Spanned; @@ -99,6 +101,14 @@ where } } + for arm in iter_without_last.clone() { + if let Some(pat) = arm.1 { + if !is_lint_allowed(cx, REDUNDANT_PATTERN_MATCHING, pat.hir_id) && is_some(pat.kind) { + return false; + } + } + } + // The suggestion may be incorrect, because some arms can have `cfg` attributes // evaluated into `false` and so such arms will be stripped before. let mut applicability = Applicability::MaybeIncorrect; @@ -170,3 +180,13 @@ fn find_bool_lit(ex: &ExprKind<'_>) -> Option { _ => None, } } + +fn is_some(path_kind: PatKind<'_>) -> bool { + match path_kind { + PatKind::TupleStruct(QPath::Resolved(_, path), [first, ..], _) if is_wild(first) => { + let name = path.segments[0].ident; + name.name == rustc_span::sym::Some + }, + _ => false, + } +} diff --git a/clippy_lints/src/matches/match_same_arms.rs b/clippy_lints/src/matches/match_same_arms.rs index a48f4c77f..ae8262ace 100644 --- a/clippy_lints/src/matches/match_same_arms.rs +++ b/clippy_lints/src/matches/match_same_arms.rs @@ -282,9 +282,8 @@ impl<'a> NormalizedPat<'a> { // TODO: Handle negative integers. They're currently treated as a wild match. ExprKind::Lit(lit) => match lit.node { LitKind::Str(sym, _) => Self::LitStr(sym), - LitKind::ByteStr(ref bytes, _) => Self::LitBytes(bytes), + LitKind::ByteStr(ref bytes, _) | LitKind::CStr(ref bytes, _) => Self::LitBytes(bytes), LitKind::Byte(val) => Self::LitInt(val.into()), - LitKind::CStr(ref bytes, _) => Self::LitBytes(bytes), LitKind::Char(val) => Self::LitInt(val.into()), LitKind::Int(val, _) => Self::LitInt(val), LitKind::Bool(val) => Self::LitBool(val), diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index 87b63eead..55ec9d447 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -25,9 +25,9 @@ mod wild_in_or_pats; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::source::{snippet_opt, walk_span_to_context}; -use clippy_utils::{higher, in_constant, is_span_match}; +use clippy_utils::{higher, in_constant, is_span_match, tokenize_with_text}; use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat}; -use rustc_lexer::{tokenize, TokenKind}; +use rustc_lexer::TokenKind; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -1147,12 +1147,7 @@ fn span_contains_cfg(cx: &LateContext<'_>, s: Span) -> bool { // Assume true. This would require either an invalid span, or one which crosses file boundaries. return true; }; - let mut pos = 0usize; - let mut iter = tokenize(&snip).map(|t| { - let start = pos; - pos += t.len as usize; - (t.kind, start..pos) - }); + let mut iter = tokenize_with_text(&snip); // Search for the token sequence [`#`, `[`, `cfg`] while iter.any(|(t, _)| matches!(t, TokenKind::Pound)) { @@ -1163,7 +1158,7 @@ fn span_contains_cfg(cx: &LateContext<'_>, s: Span) -> bool { ) }); if matches!(iter.next(), Some((TokenKind::OpenBracket, _))) - && matches!(iter.next(), Some((TokenKind::Ident, range)) if &snip[range.clone()] == "cfg") + && matches!(iter.next(), Some((TokenKind::Ident, "cfg"))) { return true; } diff --git a/clippy_lints/src/matches/overlapping_arms.rs b/clippy_lints/src/matches/overlapping_arms.rs index ae69ca8a3..abf2525a6 100644 --- a/clippy_lints/src/matches/overlapping_arms.rs +++ b/clippy_lints/src/matches/overlapping_arms.rs @@ -34,7 +34,7 @@ fn all_ranges<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], ty: Ty<'tcx>) if let Arm { pat, guard: None, .. } = *arm { if let PatKind::Range(ref lhs, ref rhs, range_end) = pat.kind { let lhs_const = match lhs { - Some(lhs) => constant(cx, cx.typeck_results(), lhs)?.0, + Some(lhs) => constant(cx, cx.typeck_results(), lhs)?, None => { let min_val_const = ty.numeric_min_val(cx.tcx)?; let min_constant = mir::ConstantKind::from_value( @@ -45,7 +45,7 @@ fn all_ranges<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], ty: Ty<'tcx>) }, }; let rhs_const = match rhs { - Some(rhs) => constant(cx, cx.typeck_results(), rhs)?.0, + Some(rhs) => constant(cx, cx.typeck_results(), rhs)?, None => { let max_val_const = ty.numeric_max_val(cx.tcx)?; let max_constant = mir::ConstantKind::from_value( diff --git a/clippy_lints/src/matches/redundant_pattern_match.rs b/clippy_lints/src/matches/redundant_pattern_match.rs index 0809837d1..e81e09da4 100644 --- a/clippy_lints/src/matches/redundant_pattern_match.rs +++ b/clippy_lints/src/matches/redundant_pattern_match.rs @@ -189,73 +189,7 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op if arms.len() == 2 { let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind); - let found_good_method = match node_pair { - ( - PatKind::TupleStruct(ref path_left, patterns_left, _), - PatKind::TupleStruct(ref path_right, patterns_right, _), - ) if patterns_left.len() == 1 && patterns_right.len() == 1 => { - if let (PatKind::Wild, PatKind::Wild) = (&patterns_left[0].kind, &patterns_right[0].kind) { - find_good_method_for_match( - cx, - arms, - path_left, - path_right, - Item::Lang(ResultOk), - Item::Lang(ResultErr), - "is_ok()", - "is_err()", - ) - .or_else(|| { - find_good_method_for_match( - cx, - arms, - path_left, - path_right, - Item::Diag(sym::IpAddr, sym!(V4)), - Item::Diag(sym::IpAddr, sym!(V6)), - "is_ipv4()", - "is_ipv6()", - ) - }) - } else { - None - } - }, - (PatKind::TupleStruct(ref path_left, patterns, _), PatKind::Path(ref path_right)) - | (PatKind::Path(ref path_left), PatKind::TupleStruct(ref path_right, patterns, _)) - if patterns.len() == 1 => - { - if let PatKind::Wild = patterns[0].kind { - find_good_method_for_match( - cx, - arms, - path_left, - path_right, - Item::Lang(OptionSome), - Item::Lang(OptionNone), - "is_some()", - "is_none()", - ) - .or_else(|| { - find_good_method_for_match( - cx, - arms, - path_left, - path_right, - Item::Lang(PollReady), - Item::Lang(PollPending), - "is_ready()", - "is_pending()", - ) - }) - } else { - None - } - }, - _ => None, - }; - - if let Some(good_method) = found_good_method { + if let Some(good_method) = found_good_method(cx, arms, node_pair) { let span = expr.span.to(op.span); let result_expr = match &op.kind { ExprKind::AddrOf(_, _, borrowed) => borrowed, @@ -279,6 +213,127 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op } } +fn found_good_method<'a>( + cx: &LateContext<'_>, + arms: &[Arm<'_>], + node: (&PatKind<'_>, &PatKind<'_>), +) -> Option<&'a str> { + match node { + ( + PatKind::TupleStruct(ref path_left, patterns_left, _), + PatKind::TupleStruct(ref path_right, patterns_right, _), + ) if patterns_left.len() == 1 && patterns_right.len() == 1 => { + if let (PatKind::Wild, PatKind::Wild) = (&patterns_left[0].kind, &patterns_right[0].kind) { + find_good_method_for_match( + cx, + arms, + path_left, + path_right, + Item::Lang(ResultOk), + Item::Lang(ResultErr), + "is_ok()", + "is_err()", + ) + .or_else(|| { + find_good_method_for_match( + cx, + arms, + path_left, + path_right, + Item::Diag(sym::IpAddr, sym!(V4)), + Item::Diag(sym::IpAddr, sym!(V6)), + "is_ipv4()", + "is_ipv6()", + ) + }) + } else { + None + } + }, + (PatKind::TupleStruct(ref path_left, patterns, _), PatKind::Path(ref path_right)) + | (PatKind::Path(ref path_left), PatKind::TupleStruct(ref path_right, patterns, _)) + if patterns.len() == 1 => + { + if let PatKind::Wild = patterns[0].kind { + find_good_method_for_match( + cx, + arms, + path_left, + path_right, + Item::Lang(OptionSome), + Item::Lang(OptionNone), + "is_some()", + "is_none()", + ) + .or_else(|| { + find_good_method_for_match( + cx, + arms, + path_left, + path_right, + Item::Lang(PollReady), + Item::Lang(PollPending), + "is_ready()", + "is_pending()", + ) + }) + } else { + None + } + }, + (PatKind::TupleStruct(ref path_left, patterns, _), PatKind::Wild) if patterns.len() == 1 => { + if let PatKind::Wild = patterns[0].kind { + get_good_method(cx, arms, path_left) + } else { + None + } + }, + (PatKind::Path(ref path_left), PatKind::Wild) => get_good_method(cx, arms, path_left), + _ => None, + } +} + +fn get_ident(path: &QPath<'_>) -> Option { + match path { + QPath::Resolved(_, path) => { + let name = path.segments[0].ident; + Some(name) + }, + _ => None, + } +} + +fn get_good_method<'a>(cx: &LateContext<'_>, arms: &[Arm<'_>], path_left: &QPath<'_>) -> Option<&'a str> { + if let Some(name) = get_ident(path_left) { + return match name.as_str() { + "Ok" => { + find_good_method_for_matches_macro(cx, arms, path_left, Item::Lang(ResultOk), "is_ok()", "is_err()") + }, + "Err" => { + find_good_method_for_matches_macro(cx, arms, path_left, Item::Lang(ResultErr), "is_err()", "is_ok()") + }, + "Some" => find_good_method_for_matches_macro( + cx, + arms, + path_left, + Item::Lang(OptionSome), + "is_some()", + "is_none()", + ), + "None" => find_good_method_for_matches_macro( + cx, + arms, + path_left, + Item::Lang(OptionNone), + "is_none()", + "is_some()", + ), + _ => None, + }; + } + None +} + #[derive(Clone, Copy)] enum Item { Lang(LangItem), @@ -346,3 +401,29 @@ fn find_good_method_for_match<'a>( _ => None, } } + +fn find_good_method_for_matches_macro<'a>( + cx: &LateContext<'_>, + arms: &[Arm<'_>], + path_left: &QPath<'_>, + expected_item_left: Item, + should_be_left: &'a str, + should_be_right: &'a str, +) -> Option<&'a str> { + let first_pat = arms[0].pat; + + let body_node_pair = if is_pat_variant(cx, first_pat, path_left, expected_item_left) { + (&arms[0].body.kind, &arms[1].body.kind) + } else { + return None; + }; + + match body_node_pair { + (ExprKind::Lit(lit_left), ExprKind::Lit(lit_right)) => match (&lit_left.node, &lit_right.node) { + (LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left), + (LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right), + _ => None, + }, + _ => None, + } +} diff --git a/clippy_lints/src/methods/iter_nth_zero.rs b/clippy_lints/src/methods/iter_nth_zero.rs index c830958d5..d1609eebf 100644 --- a/clippy_lints/src/methods/iter_nth_zero.rs +++ b/clippy_lints/src/methods/iter_nth_zero.rs @@ -13,7 +13,7 @@ use super::ITER_NTH_ZERO; pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, arg: &hir::Expr<'_>) { if_chain! { if is_trait_method(cx, expr, sym::Iterator); - if let Some((Constant::Int(0), _)) = constant(cx, cx.typeck_results(), arg); + if let Some(Constant::Int(0)) = constant(cx, cx.typeck_results(), arg); then { let mut applicability = Applicability::MachineApplicable; span_lint_and_sugg( diff --git a/clippy_lints/src/methods/iterator_step_by_zero.rs b/clippy_lints/src/methods/iterator_step_by_zero.rs index 64c09214a..b631cd00c 100644 --- a/clippy_lints/src/methods/iterator_step_by_zero.rs +++ b/clippy_lints/src/methods/iterator_step_by_zero.rs @@ -9,7 +9,7 @@ use super::ITERATOR_STEP_BY_ZERO; pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, arg: &'tcx hir::Expr<'_>) { if is_trait_method(cx, expr, sym::Iterator) { - if let Some((Constant::Int(0), _)) = constant(cx, cx.typeck_results(), arg) { + if let Some(Constant::Int(0)) = constant(cx, cx.typeck_results(), arg) { span_lint( cx, ITERATOR_STEP_BY_ZERO, diff --git a/clippy_lints/src/methods/manual_next_back.rs b/clippy_lints/src/methods/manual_next_back.rs new file mode 100644 index 000000000..5f3fec538 --- /dev/null +++ b/clippy_lints/src/methods/manual_next_back.rs @@ -0,0 +1,38 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::is_trait_method; +use clippy_utils::ty::implements_trait; +use rustc_errors::Applicability; +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_span::symbol::sym; + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + rev_call: &'tcx Expr<'_>, + rev_recv: &'tcx Expr<'_>, +) { + let rev_recv_ty = cx.typeck_results().expr_ty(rev_recv); + + // check that the receiver of `rev` implements `DoubleEndedIterator` and + // that `rev` and `next` come from `Iterator` + if cx + .tcx + .get_diagnostic_item(sym::DoubleEndedIterator) + .map_or(false, |double_ended_iterator| { + implements_trait(cx, rev_recv_ty, double_ended_iterator, &[]) + }) + && is_trait_method(cx, rev_call, sym::Iterator) + && is_trait_method(cx, expr, sym::Iterator) + { + span_lint_and_sugg( + cx, + super::MANUAL_NEXT_BACK, + expr.span.with_lo(rev_recv.span.hi()), + "manual backwards iteration", + "use", + String::from(".next_back()"), + Applicability::MachineApplicable, + ); + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 06b88e34d..9a594d964 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -45,6 +45,7 @@ mod iter_overeager_cloned; mod iter_skip_next; mod iter_with_drain; mod iterator_step_by_zero; +mod manual_next_back; mod manual_ok_or; mod manual_saturating_arithmetic; mod manual_str_repeat; @@ -3132,8 +3133,11 @@ declare_clippy_lint! { /// ### Example /// ```rust /// # let iterator = vec![1].into_iter(); - /// let len = iterator.clone().collect::>().len(); - /// // should be + /// let len = iterator.collect::>().len(); + /// ``` + /// Use instead: + /// ```rust + /// # let iterator = vec![1].into_iter(); /// let len = iterator.count(); /// ``` #[clippy::version = "1.30.0"] @@ -3193,6 +3197,29 @@ declare_clippy_lint! { "calling `drain` in order to `clear` a container" } +declare_clippy_lint! { + /// ### What it does + /// Checks for `.rev().next()` on a `DoubleEndedIterator` + /// + /// ### Why is this bad? + /// `.next_back()` is cleaner. + /// + /// ### Example + /// ```rust + /// # let foo = [0; 10]; + /// foo.iter().rev().next(); + /// ``` + /// Use instead: + /// ```rust + /// # let foo = [0; 10]; + /// foo.iter().next_back(); + /// ``` + #[clippy::version = "1.71.0"] + pub MANUAL_NEXT_BACK, + style, + "manual reverse iteration of `DoubleEndedIterator`" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Msrv, @@ -3321,6 +3348,7 @@ impl_lint_pass!(Methods => [ NEEDLESS_COLLECT, SUSPICIOUS_COMMAND_ARG_SPACE, CLEAR_WITH_DRAIN, + MANUAL_NEXT_BACK, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -3677,6 +3705,7 @@ impl Methods { ("iter", []) => iter_next_slice::check(cx, expr, recv2), ("skip", [arg]) => iter_skip_next::check(cx, expr, recv2, arg), ("skip_while", [_]) => skip_while_next::check(cx, expr), + ("rev", [])=> manual_next_back::check(cx, expr, recv, recv2), _ => {}, } } @@ -3741,13 +3770,13 @@ impl Methods { unnecessary_sort_by::check(cx, expr, recv, arg, true); }, ("splitn" | "rsplitn", [count_arg, pat_arg]) => { - if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) { + if let Some(Constant::Int(count)) = constant(cx, cx.typeck_results(), count_arg) { suspicious_splitn::check(cx, name, expr, recv, count); str_splitn::check(cx, name, expr, recv, pat_arg, count, &self.msrv); } }, ("splitn_mut" | "rsplitn_mut", [count_arg, _]) => { - if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) { + if let Some(Constant::Int(count)) = constant(cx, cx.typeck_results(), count_arg) { suspicious_splitn::check(cx, name, expr, recv, count); } }, diff --git a/clippy_lints/src/methods/needless_collect.rs b/clippy_lints/src/methods/needless_collect.rs index 0b0c6adc5..6841aaf62 100644 --- a/clippy_lints/src/methods/needless_collect.rs +++ b/clippy_lints/src/methods/needless_collect.rs @@ -1,6 +1,5 @@ use super::NEEDLESS_COLLECT; use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then}; -use clippy_utils::higher; use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::sugg::Sugg; use clippy_utils::ty::{is_type_diagnostic_item, make_normalized_projection, make_projection}; @@ -8,6 +7,7 @@ use clippy_utils::{ can_move_expr_to_closure, get_enclosing_block, get_parent_node, is_trait_method, path_to_local, path_to_local_id, CaptureKind, }; +use clippy_utils::{fn_def_id, higher}; use rustc_data_structures::fx::FxHashMap; use rustc_errors::{Applicability, MultiSpan}; use rustc_hir::intravisit::{walk_block, walk_expr, Visitor}; @@ -16,7 +16,7 @@ use rustc_hir::{ }; use rustc_lint::LateContext; use rustc_middle::hir::nested_filter; -use rustc_middle::ty::{self, AssocKind, EarlyBinder, GenericArg, GenericArgKind, Ty}; +use rustc_middle::ty::{self, AssocKind, Clause, EarlyBinder, GenericArg, GenericArgKind, PredicateKind, Ty}; use rustc_span::symbol::Ident; use rustc_span::{sym, Span, Symbol}; @@ -32,6 +32,8 @@ pub(super) fn check<'tcx>( if let Some(parent) = get_parent_node(cx.tcx, collect_expr.hir_id) { match parent { Node::Expr(parent) => { + check_collect_into_intoiterator(cx, parent, collect_expr, call_span, iter_expr); + if let ExprKind::MethodCall(name, _, args @ ([] | [_]), _) = parent.kind { let mut app = Applicability::MachineApplicable; let name = name.ident.as_str(); @@ -134,6 +136,68 @@ pub(super) fn check<'tcx>( } } +/// checks for for collecting into a (generic) method or function argument +/// taking an `IntoIterator` +fn check_collect_into_intoiterator<'tcx>( + cx: &LateContext<'tcx>, + parent: &'tcx Expr<'tcx>, + collect_expr: &'tcx Expr<'tcx>, + call_span: Span, + iter_expr: &'tcx Expr<'tcx>, +) { + if let Some(id) = fn_def_id(cx, parent) { + let args = match parent.kind { + ExprKind::Call(_, args) | ExprKind::MethodCall(_, _, args, _) => args, + _ => &[], + }; + // find the argument index of the `collect_expr` in the + // function / method call + if let Some(arg_idx) = args.iter().position(|e| e.hir_id == collect_expr.hir_id).map(|i| { + if matches!(parent.kind, ExprKind::MethodCall(_, _, _, _)) { + i + 1 + } else { + i + } + }) { + // extract the input types of the function/method call + // that contains `collect_expr` + let inputs = cx + .tcx + .liberate_late_bound_regions(id, cx.tcx.fn_sig(id).subst_identity()) + .inputs(); + + // map IntoIterator generic bounds to their signature + // types and check whether the argument type is an + // `IntoIterator` + if cx + .tcx + .param_env(id) + .caller_bounds() + .into_iter() + .filter_map(|p| { + if let PredicateKind::Clause(Clause::Trait(t)) = p.kind().skip_binder() + && cx.tcx.is_diagnostic_item(sym::IntoIterator,t.trait_ref.def_id) { + Some(t.self_ty()) + } else { + None + } + }) + .any(|ty| ty == inputs[arg_idx]) + { + span_lint_and_sugg( + cx, + NEEDLESS_COLLECT, + call_span.with_lo(iter_expr.span.hi()), + NEEDLESS_COLLECT_MSG, + "remove this call", + String::new(), + Applicability::MachineApplicable, + ); + } + } + } +} + /// Checks if the given method call matches the expected signature of `([&[mut]] self) -> bool` fn is_is_empty_sig(cx: &LateContext<'_>, call_id: HirId) -> bool { cx.typeck_results().type_dependent_def_id(call_id).map_or(false, |id| { diff --git a/clippy_lints/src/methods/repeat_once.rs b/clippy_lints/src/methods/repeat_once.rs index a345ec813..bb4cdd2a6 100644 --- a/clippy_lints/src/methods/repeat_once.rs +++ b/clippy_lints/src/methods/repeat_once.rs @@ -1,4 +1,4 @@ -use clippy_utils::consts::{constant_context, Constant}; +use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet; use clippy_utils::ty::is_type_lang_item; @@ -14,7 +14,7 @@ pub(super) fn check<'tcx>( recv: &'tcx Expr<'_>, repeat_arg: &'tcx Expr<'_>, ) { - if constant_context(cx, cx.typeck_results()).expr(repeat_arg) == Some(Constant::Int(1)) { + if constant(cx, cx.typeck_results(), repeat_arg) == Some(Constant::Int(1)) { let ty = cx.typeck_results().expr_ty(recv).peel_refs(); if ty.is_str() { span_lint_and_sugg( diff --git a/clippy_lints/src/methods/str_splitn.rs b/clippy_lints/src/methods/str_splitn.rs index 91f7ce1db..5ea12c441 100644 --- a/clippy_lints/src/methods/str_splitn.rs +++ b/clippy_lints/src/methods/str_splitn.rs @@ -316,7 +316,7 @@ fn parse_iter_usage<'tcx>( }; }, ("nth" | "skip", [idx_expr]) if cx.tcx.trait_of_item(did) == Some(iter_id) => { - if let Some((Constant::Int(idx), _)) = constant(cx, cx.typeck_results(), idx_expr) { + if let Some(Constant::Int(idx)) = constant(cx, cx.typeck_results(), idx_expr) { let span = if name.ident.as_str() == "nth" { e.span } else { diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 3752b9a94..303f01256 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -16,9 +16,12 @@ use rustc_span::source_map::{ExpnKind, Span}; use clippy_utils::sugg::Sugg; use clippy_utils::{ - get_parent_expr, in_constant, is_integer_literal, is_no_std_crate, iter_input_pats, last_path_segment, SpanlessEq, + get_parent_expr, in_constant, is_integer_literal, is_lint_allowed, is_no_std_crate, iter_input_pats, + last_path_segment, SpanlessEq, }; +use crate::ref_patterns::REF_PATTERNS; + declare_clippy_lint! { /// ### What it does /// Checks for function arguments and let bindings denoted as @@ -162,6 +165,10 @@ impl<'tcx> LateLintPass<'tcx> for LintPass { return; } for arg in iter_input_pats(decl, body) { + // Do not emit if clippy::ref_patterns is not allowed to avoid having two lints for the same issue. + if !is_lint_allowed(cx, REF_PATTERNS, arg.pat.hir_id) { + return; + } if let PatKind::Binding(BindingAnnotation(ByRef::Yes, _), ..) = arg.pat.kind { span_lint( cx, @@ -180,6 +187,8 @@ impl<'tcx> LateLintPass<'tcx> for LintPass { if let StmtKind::Local(local) = stmt.kind; if let PatKind::Binding(BindingAnnotation(ByRef::Yes, mutabl), .., name, None) = local.pat.kind; if let Some(init) = local.init; + // Do not emit if clippy::ref_patterns is not allowed to avoid having two lints for the same issue. + if is_lint_allowed(cx, REF_PATTERNS, local.pat.hir_id); then { let ctxt = local.span.ctxt(); let mut app = Applicability::MachineApplicable; diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs index 71281a0b4..62af42a39 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -146,7 +146,7 @@ fn is_parent_stmt(cx: &LateContext<'_>, id: HirId) -> bool { impl<'tcx> LateLintPass<'tcx> for NeedlessBool { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { use self::Expression::{Bool, RetBool}; - if e.span.from_expansion() { + if e.span.from_expansion() || !span_extract_comment(cx.tcx.sess.source_map(), e.span).is_empty() { return; } if let Some(higher::If { @@ -209,8 +209,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBool { } if let Some((lhs_a, a)) = fetch_assign(then) && let Some((lhs_b, b)) = fetch_assign(r#else) && - SpanlessEq::new(cx).eq_expr(lhs_a, lhs_b) && - span_extract_comment(cx.tcx.sess.source_map(), e.span).is_empty() + SpanlessEq::new(cx).eq_expr(lhs_a, lhs_b) { let mut applicability = Applicability::MachineApplicable; let cond = Sugg::hir_with_applicability(cx, cond, "..", &mut applicability); diff --git a/clippy_lints/src/operators/absurd_extreme_comparisons.rs b/clippy_lints/src/operators/absurd_extreme_comparisons.rs index d29ca37ea..f4863600c 100644 --- a/clippy_lints/src/operators/absurd_extreme_comparisons.rs +++ b/clippy_lints/src/operators/absurd_extreme_comparisons.rs @@ -121,7 +121,7 @@ fn detect_absurd_comparison<'tcx>( fn detect_extreme_expr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option> { let ty = cx.typeck_results().expr_ty(expr); - let cv = constant(cx, cx.typeck_results(), expr)?.0; + let cv = constant(cx, cx.typeck_results(), expr)?; let which = match (ty.kind(), cv) { (&ty::Bool, Constant::Bool(false)) | (&ty::Uint(_), Constant::Int(0)) => ExtremeType::Minimum, diff --git a/clippy_lints/src/operators/arithmetic_side_effects.rs b/clippy_lints/src/operators/arithmetic_side_effects.rs index f72595987..5c240276b 100644 --- a/clippy_lints/src/operators/arithmetic_side_effects.rs +++ b/clippy_lints/src/operators/arithmetic_side_effects.rs @@ -21,7 +21,7 @@ const HARD_CODED_ALLOWED_BINARY: &[[&str; 2]] = &[ ["f64", "f64"], ["std::num::Saturating", "std::num::Saturating"], ["std::num::Wrapping", "std::num::Wrapping"], - ["std::string::String", "&str"], + ["std::string::String", "str"], ]; const HARD_CODED_ALLOWED_UNARY: &[&str] = &["f32", "f64", "std::num::Saturating", "std::num::Wrapping"]; const INTEGER_METHODS: &[&str] = &["saturating_div", "wrapping_div", "wrapping_rem", "wrapping_rem_euclid"]; @@ -113,7 +113,7 @@ impl ArithmeticSideEffects { if let hir::ExprKind::Lit(lit) = actual.kind && let ast::LitKind::Int(n, _) = lit.node { return Some(n) } - if let Some((Constant::Int(n), _)) = constant(cx, cx.typeck_results(), expr) { + if let Some(Constant::Int(n)) = constant(cx, cx.typeck_results(), expr) { return Some(n); } None @@ -144,8 +144,10 @@ impl ArithmeticSideEffects { ) { return; }; - let lhs_ty = cx.typeck_results().expr_ty(lhs); - let rhs_ty = cx.typeck_results().expr_ty(rhs); + let (actual_lhs, lhs_ref_counter) = peel_hir_expr_refs(lhs); + let (actual_rhs, rhs_ref_counter) = peel_hir_expr_refs(rhs); + let lhs_ty = cx.typeck_results().expr_ty(actual_lhs).peel_refs(); + let rhs_ty = cx.typeck_results().expr_ty(actual_rhs).peel_refs(); if self.has_allowed_binary(lhs_ty, rhs_ty) { return; } @@ -154,8 +156,6 @@ impl ArithmeticSideEffects { // At least for integers, shifts are already handled by the CTFE return; } - let (actual_lhs, lhs_ref_counter) = peel_hir_expr_refs(lhs); - let (actual_rhs, rhs_ref_counter) = peel_hir_expr_refs(rhs); match ( Self::literal_integer(cx, actual_lhs), Self::literal_integer(cx, actual_rhs), diff --git a/clippy_lints/src/operators/bit_mask.rs b/clippy_lints/src/operators/bit_mask.rs index 1369b3e74..1fddf0f50 100644 --- a/clippy_lints/src/operators/bit_mask.rs +++ b/clippy_lints/src/operators/bit_mask.rs @@ -166,7 +166,7 @@ fn check_ineffective_gt(cx: &LateContext<'_>, span: Span, m: u128, c: u128, op: } fn fetch_int_literal(cx: &LateContext<'_>, lit: &Expr<'_>) -> Option { - match constant(cx, cx.typeck_results(), lit)?.0 { + match constant(cx, cx.typeck_results(), lit)? { Constant::Int(n) => Some(n), _ => None, } diff --git a/clippy_lints/src/operators/cmp_nan.rs b/clippy_lints/src/operators/cmp_nan.rs index 786ae1552..e18064b70 100644 --- a/clippy_lints/src/operators/cmp_nan.rs +++ b/clippy_lints/src/operators/cmp_nan.rs @@ -18,7 +18,7 @@ pub(super) fn check(cx: &LateContext<'_>, e: &Expr<'_>, op: BinOpKind, lhs: &Exp } fn is_nan(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { - if let Some((value, _)) = constant(cx, cx.typeck_results(), e) { + if let Some(value) = constant(cx, cx.typeck_results(), e) { match value { Constant::F32(num) => num.is_nan(), Constant::F64(num) => num.is_nan(), diff --git a/clippy_lints/src/operators/duration_subsec.rs b/clippy_lints/src/operators/duration_subsec.rs index 49e662cac..f120be138 100644 --- a/clippy_lints/src/operators/duration_subsec.rs +++ b/clippy_lints/src/operators/duration_subsec.rs @@ -19,7 +19,7 @@ pub(crate) fn check<'tcx>( if op == BinOpKind::Div && let ExprKind::MethodCall(method_path, self_arg, [], _) = left.kind && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_arg).peel_refs(), sym::Duration) - && let Some((Constant::Int(divisor), _)) = constant(cx, cx.typeck_results(), right) + && let Some(Constant::Int(divisor)) = constant(cx, cx.typeck_results(), right) { let suggested_fn = match (method_path.ident.as_str(), divisor) { ("subsec_micros", 1_000) | ("subsec_nanos", 1_000_000) => "subsec_millis", diff --git a/clippy_lints/src/operators/float_cmp.rs b/clippy_lints/src/operators/float_cmp.rs index 97ddcdb24..15dff126b 100644 --- a/clippy_lints/src/operators/float_cmp.rs +++ b/clippy_lints/src/operators/float_cmp.rs @@ -1,4 +1,4 @@ -use clippy_utils::consts::{constant, Constant}; +use clippy_utils::consts::{constant_with_source, Constant}; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::get_item_name; use clippy_utils::sugg::Sugg; @@ -18,9 +18,16 @@ pub(crate) fn check<'tcx>( right: &'tcx Expr<'_>, ) { if (op == BinOpKind::Eq || op == BinOpKind::Ne) && (is_float(cx, left) || is_float(cx, right)) { - if is_allowed(cx, left) || is_allowed(cx, right) { - return; - } + let left_is_local = match constant_with_source(cx, cx.typeck_results(), left) { + Some((c, s)) if !is_allowed(&c) => s.is_local(), + Some(_) => return, + None => true, + }; + let right_is_local = match constant_with_source(cx, cx.typeck_results(), right) { + Some((c, s)) if !is_allowed(&c) => s.is_local(), + Some(_) => return, + None => true, + }; // Allow comparing the results of signum() if is_signum(cx, left) && is_signum(cx, right) { @@ -34,10 +41,7 @@ pub(crate) fn check<'tcx>( } } let is_comparing_arrays = is_array(cx, left) || is_array(cx, right); - let (lint, msg) = get_lint_and_message( - is_named_constant(cx, left) || is_named_constant(cx, right), - is_comparing_arrays, - ); + let (lint, msg) = get_lint_and_message(left_is_local && right_is_local, is_comparing_arrays); span_lint_and_then(cx, lint, expr.span, msg, |diag| { let lhs = Sugg::hir(cx, left, ".."); let rhs = Sugg::hir(cx, right, ".."); @@ -59,20 +63,8 @@ pub(crate) fn check<'tcx>( } } -fn get_lint_and_message( - is_comparing_constants: bool, - is_comparing_arrays: bool, -) -> (&'static rustc_lint::Lint, &'static str) { - if is_comparing_constants { - ( - FLOAT_CMP_CONST, - if is_comparing_arrays { - "strict comparison of `f32` or `f64` constant arrays" - } else { - "strict comparison of `f32` or `f64` constant" - }, - ) - } else { +fn get_lint_and_message(is_local: bool, is_comparing_arrays: bool) -> (&'static rustc_lint::Lint, &'static str) { + if is_local { ( FLOAT_CMP, if is_comparing_arrays { @@ -81,22 +73,23 @@ fn get_lint_and_message( "strict comparison of `f32` or `f64`" }, ) - } -} - -fn is_named_constant<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { - if let Some((_, res)) = constant(cx, cx.typeck_results(), expr) { - res } else { - false + ( + FLOAT_CMP_CONST, + if is_comparing_arrays { + "strict comparison of `f32` or `f64` constant arrays" + } else { + "strict comparison of `f32` or `f64` constant" + }, + ) } } -fn is_allowed<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { - match constant(cx, cx.typeck_results(), expr) { - Some((Constant::F32(f), _)) => f == 0.0 || f.is_infinite(), - Some((Constant::F64(f), _)) => f == 0.0 || f.is_infinite(), - Some((Constant::Vec(vec), _)) => vec.iter().all(|f| match f { +fn is_allowed(val: &Constant) -> bool { + match val { + &Constant::F32(f) => f == 0.0 || f.is_infinite(), + &Constant::F64(f) => f == 0.0 || f.is_infinite(), + Constant::Vec(vec) => vec.iter().all(|f| match f { Constant::F32(f) => *f == 0.0 || (*f).is_infinite(), Constant::F64(f) => *f == 0.0 || (*f).is_infinite(), _ => false, diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index 19599731b..d63a836e7 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/clippy_lints/src/operators/mod.rs @@ -96,32 +96,6 @@ declare_clippy_lint! { "any arithmetic expression that can cause side effects like overflows or panics" } -declare_clippy_lint! { - /// ### What it does - /// Checks for integer arithmetic operations which could overflow or panic. - /// - /// Specifically, checks for any operators (`+`, `-`, `*`, `<<`, etc) which are capable - /// of overflowing according to the [Rust - /// Reference](https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow), - /// or which can panic (`/`, `%`). No bounds analysis or sophisticated reasoning is - /// attempted. - /// - /// ### Why is this bad? - /// Integer overflow will trigger a panic in debug builds or will wrap in - /// release mode. Division by zero will cause a panic in either mode. In some applications one - /// wants explicitly checked, wrapping or saturating arithmetic. - /// - /// ### Example - /// ```rust - /// # let a = 0; - /// a + 1; - /// ``` - #[clippy::version = "pre 1.29.0"] - pub INTEGER_ARITHMETIC, - restriction, - "any integer arithmetic expression which could overflow or panic" -} - declare_clippy_lint! { /// ### What it does /// Checks for float arithmetic. @@ -787,7 +761,6 @@ pub struct Operators { impl_lint_pass!(Operators => [ ABSURD_EXTREME_COMPARISONS, ARITHMETIC_SIDE_EFFECTS, - INTEGER_ARITHMETIC, FLOAT_ARITHMETIC, ASSIGN_OP_PATTERN, MISREFACTORED_ASSIGN_OP, diff --git a/clippy_lints/src/operators/modulo_arithmetic.rs b/clippy_lints/src/operators/modulo_arithmetic.rs index af4e74947..a2c3a4d8b 100644 --- a/clippy_lints/src/operators/modulo_arithmetic.rs +++ b/clippy_lints/src/operators/modulo_arithmetic.rs @@ -40,7 +40,7 @@ struct OperandInfo { fn analyze_operand(operand: &Expr<'_>, cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { match constant(cx, cx.typeck_results(), operand) { - Some((Constant::Int(v), _)) => match *cx.typeck_results().expr_ty(expr).kind() { + Some(Constant::Int(v)) => match *cx.typeck_results().expr_ty(expr).kind() { ty::Int(ity) => { let value = sext(cx.tcx, v, ity); return Some(OperandInfo { @@ -58,10 +58,10 @@ fn analyze_operand(operand: &Expr<'_>, cx: &LateContext<'_>, expr: &Expr<'_>) -> }, _ => {}, }, - Some((Constant::F32(f), _)) => { + Some(Constant::F32(f)) => { return Some(floating_point_operand_info(&f)); }, - Some((Constant::F64(f), _)) => { + Some(Constant::F64(f)) => { return Some(floating_point_operand_info(&f)); }, _ => {}, diff --git a/clippy_lints/src/operators/numeric_arithmetic.rs b/clippy_lints/src/operators/numeric_arithmetic.rs index 77fd45b19..102845cee 100644 --- a/clippy_lints/src/operators/numeric_arithmetic.rs +++ b/clippy_lints/src/operators/numeric_arithmetic.rs @@ -1,8 +1,6 @@ -use super::{FLOAT_ARITHMETIC, INTEGER_ARITHMETIC}; +use super::FLOAT_ARITHMETIC; use clippy_utils::consts::constant_simple; use clippy_utils::diagnostics::span_lint; -use clippy_utils::is_from_proc_macro; -use clippy_utils::is_integer_literal; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_span::source_map::Span; @@ -45,31 +43,8 @@ impl Context { _ => (), } - let (l_ty, r_ty) = (cx.typeck_results().expr_ty(l), cx.typeck_results().expr_ty(r)); - if l_ty.peel_refs().is_integral() && r_ty.peel_refs().is_integral() { - if is_from_proc_macro(cx, expr) { - return; - } - match op { - hir::BinOpKind::Div | hir::BinOpKind::Rem => match &r.kind { - hir::ExprKind::Lit(_lit) => (), - hir::ExprKind::Unary(hir::UnOp::Neg, expr) => { - if is_integer_literal(expr, 1) { - span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected"); - self.expr_id = Some(expr.hir_id); - } - }, - _ => { - span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected"); - self.expr_id = Some(expr.hir_id); - }, - }, - _ => { - span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected"); - self.expr_id = Some(expr.hir_id); - }, - } - } else if r_ty.peel_refs().is_floating_point() && r_ty.peel_refs().is_floating_point() { + let (_, r_ty) = (cx.typeck_results().expr_ty(l), cx.typeck_results().expr_ty(r)); + if r_ty.peel_refs().is_floating_point() && r_ty.peel_refs().is_floating_point() { span_lint(cx, FLOAT_ARITHMETIC, expr.span, "floating-point arithmetic detected"); self.expr_id = Some(expr.hir_id); } @@ -80,17 +55,9 @@ impl Context { return; } let ty = cx.typeck_results().expr_ty(arg); - if constant_simple(cx, cx.typeck_results(), expr).is_none() { - if ty.is_integral() { - if is_from_proc_macro(cx, expr) { - return; - } - span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected"); - self.expr_id = Some(expr.hir_id); - } else if ty.is_floating_point() { - span_lint(cx, FLOAT_ARITHMETIC, expr.span, "floating-point arithmetic detected"); - self.expr_id = Some(expr.hir_id); - } + if constant_simple(cx, cx.typeck_results(), expr).is_none() && ty.is_floating_point() { + span_lint(cx, FLOAT_ARITHMETIC, expr.span, "floating-point arithmetic detected"); + self.expr_id = Some(expr.hir_id); } } diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs index bbbcda069..aa6d40042 100644 --- a/clippy_lints/src/option_if_let_else.rs +++ b/clippy_lints/src/option_if_let_else.rs @@ -122,7 +122,7 @@ fn try_get_option_occurrence<'tcx>( ExprKind::Unary(UnOp::Deref, inner_expr) | ExprKind::AddrOf(_, _, inner_expr) => inner_expr, _ => expr, }; - let inner_pat = try_get_inner_pat(cx, pat)?; + let (inner_pat, is_result) = try_get_inner_pat_and_is_result(cx, pat)?; if_chain! { if let PatKind::Binding(bind_annotation, _, id, None) = inner_pat.kind; if let Some(some_captures) = can_move_expr_to_closure(cx, if_then); @@ -176,7 +176,7 @@ fn try_get_option_occurrence<'tcx>( ), none_expr: format!( "{}{}", - if method_sugg == "map_or" { "" } else { "|| " }, + if method_sugg == "map_or" { "" } else if is_result { "|_| " } else { "|| "}, Sugg::hir_with_context(cx, none_body, ctxt, "..", &mut app), ), }); @@ -186,11 +186,13 @@ fn try_get_option_occurrence<'tcx>( None } -fn try_get_inner_pat<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<&'tcx Pat<'tcx>> { +fn try_get_inner_pat_and_is_result<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<(&'tcx Pat<'tcx>, bool)> { if let PatKind::TupleStruct(ref qpath, [inner_pat], ..) = pat.kind { let res = cx.qpath_res(qpath, pat.hir_id); - if is_res_lang_ctor(cx, res, OptionSome) || is_res_lang_ctor(cx, res, ResultOk) { - return Some(inner_pat); + if is_res_lang_ctor(cx, res, OptionSome) { + return Some((inner_pat, false)); + } else if is_res_lang_ctor(cx, res, ResultOk) { + return Some((inner_pat, true)); } } None diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index fc655fe2d..dd7ded491 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -319,7 +319,7 @@ fn check_range_bounds<'a>(cx: &'a LateContext<'_>, ex: &'a Expr<'_>) -> Option return None, }; if let Some(id) = path_to_local(l) { - if let Some((c, _)) = constant(cx, cx.typeck_results(), r) { + if let Some(c) = constant(cx, cx.typeck_results(), r) { return Some(RangeBounds { val: c, expr: r, @@ -331,7 +331,7 @@ fn check_range_bounds<'a>(cx: &'a LateContext<'_>, ex: &'a Expr<'_>) -> Option, expr: &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); - if let Some((end_idx, _)) = constant(cx, cx.typeck_results(), end); + if let Some(start_idx) = constant(cx, cx.typeck_results(), start); + if let Some(end_idx) = constant(cx, cx.typeck_results(), end); if let Some(ordering) = Constant::partial_cmp(cx.tcx, ty, &start_idx, &end_idx); if is_empty_range(limits, ordering); then { diff --git a/clippy_lints/src/ref_patterns.rs b/clippy_lints/src/ref_patterns.rs new file mode 100644 index 000000000..b1530eed1 --- /dev/null +++ b/clippy_lints/src/ref_patterns.rs @@ -0,0 +1,44 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use rustc_ast::ast::{BindingAnnotation, Pat, PatKind}; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// Checks for usages of the `ref` keyword. + /// ### Why is this bad? + /// The `ref` keyword can be confusing for people unfamiliar with it, and often + /// it is more concise to use `&` instead. + /// ### Example + /// ```rust + /// let opt = Some(5); + /// if let Some(ref foo) = opt {} + /// ``` + /// Use instead: + /// ```rust + /// let opt = Some(5); + /// if let Some(foo) = &opt {} + /// ``` + #[clippy::version = "1.71.0"] + pub REF_PATTERNS, + restriction, + "use of a ref pattern, e.g. Some(ref value)" +} +declare_lint_pass!(RefPatterns => [REF_PATTERNS]); + +impl EarlyLintPass for RefPatterns { + fn check_pat(&mut self, cx: &EarlyContext<'_>, pat: &Pat) { + if let PatKind::Ident(BindingAnnotation::REF, _, _) = pat.kind + && !pat.span.from_expansion() + { + span_lint_and_help( + cx, + REF_PATTERNS, + pat.span, + "usage of ref pattern", + None, + "consider using `&` for clarity instead", + ); + } + } +} diff --git a/clippy_lints/src/regex.rs b/clippy_lints/src/regex.rs index b8b32df6c..ef19c6f46 100644 --- a/clippy_lints/src/regex.rs +++ b/clippy_lints/src/regex.rs @@ -122,37 +122,39 @@ fn lint_syntax_error(cx: &LateContext<'_>, error: ®ex_syntax::Error, unescape } fn const_str<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> Option { - constant(cx, cx.typeck_results(), e).and_then(|(c, _)| match c { + constant(cx, cx.typeck_results(), e).and_then(|c| match c { Constant::Str(s) => Some(s), _ => None, }) } fn is_trivial_regex(s: ®ex_syntax::hir::Hir) -> Option<&'static str> { - use regex_syntax::hir::Anchor::{EndText, StartText}; - use regex_syntax::hir::HirKind::{Alternation, Anchor, Concat, Empty, Literal}; + use regex_syntax::hir::HirKind::{Alternation, Concat, Empty, Literal, Look}; + use regex_syntax::hir::Look as HirLook; let is_literal = |e: &[regex_syntax::hir::Hir]| e.iter().all(|e| matches!(*e.kind(), Literal(_))); match *s.kind() { - Empty | Anchor(_) => Some("the regex is unlikely to be useful as it is"), + Empty | Look(_) => Some("the regex is unlikely to be useful as it is"), Literal(_) => Some("consider using `str::contains`"), Alternation(ref exprs) => { - if exprs.iter().all(|e| e.kind().is_empty()) { + if exprs.iter().all(|e| matches!(e.kind(), Empty)) { Some("the regex is unlikely to be useful as it is") } else { None } }, Concat(ref exprs) => match (exprs[0].kind(), exprs[exprs.len() - 1].kind()) { - (&Anchor(StartText), &Anchor(EndText)) if exprs[1..(exprs.len() - 1)].is_empty() => { + (&Look(HirLook::Start), &Look(HirLook::End)) if exprs[1..(exprs.len() - 1)].is_empty() => { Some("consider using `str::is_empty`") }, - (&Anchor(StartText), &Anchor(EndText)) if is_literal(&exprs[1..(exprs.len() - 1)]) => { + (&Look(HirLook::Start), &Look(HirLook::End)) if is_literal(&exprs[1..(exprs.len() - 1)]) => { Some("consider using `==` on `str`s") }, - (&Anchor(StartText), &Literal(_)) if is_literal(&exprs[1..]) => Some("consider using `str::starts_with`"), - (&Literal(_), &Anchor(EndText)) if is_literal(&exprs[1..(exprs.len() - 1)]) => { + (&Look(HirLook::Start), &Literal(_)) if is_literal(&exprs[1..]) => { + Some("consider using `str::starts_with`") + }, + (&Literal(_), &Look(HirLook::End)) if is_literal(&exprs[1..(exprs.len() - 1)]) => { Some("consider using `str::ends_with`") }, _ if is_literal(exprs) => Some("consider using `str::contains`"), @@ -175,10 +177,7 @@ fn check_set<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, utf8: bool) { } fn check_regex<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, utf8: bool) { - let mut parser = regex_syntax::ParserBuilder::new() - .unicode(true) - .allow_invalid_utf8(!utf8) - .build(); + let mut parser = regex_syntax::ParserBuilder::new().unicode(true).utf8(!utf8).build(); if let ExprKind::Lit(lit) = expr.kind { if let LitKind::Str(ref r, style) = lit.node { diff --git a/clippy_lints/src/renamed_lints.rs b/clippy_lints/src/renamed_lints.rs index 52e22c0c6..a2c3465cd 100644 --- a/clippy_lints/src/renamed_lints.rs +++ b/clippy_lints/src/renamed_lints.rs @@ -15,6 +15,7 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[ ("clippy::eval_order_dependence", "clippy::mixed_read_write_in_expression"), ("clippy::identity_conversion", "clippy::useless_conversion"), ("clippy::if_let_some_result", "clippy::match_result_ok"), + ("clippy::integer_arithmetic", "clippy::arithmetic_side_effects"), ("clippy::logic_bug", "clippy::overly_complex_bool_expr"), ("clippy::new_without_default_derive", "clippy::new_without_default"), ("clippy::option_and_then_some", "clippy::bind_instead_of_map"), diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index 5b588e914..483f860a8 100644 --- a/clippy_lints/src/strings.rs +++ b/clippy_lints/src/strings.rs @@ -132,7 +132,7 @@ declare_clippy_lint! { /// Probably lots of false positives. If an index comes from a known valid position (e.g. /// obtained via `char_indices` over the same string), it is totally OK. /// - /// # Example + /// ### Example /// ```rust,should_panic /// &"Ölkanne"[1..]; /// ``` diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index b5f11b4ac..4ccda1506 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -37,12 +37,12 @@ declare_clippy_lint! { #[clippy::version = "1.38.0"] pub TYPE_REPETITION_IN_BOUNDS, nursery, - "types are repeated unnecessary in trait bounds use `+` instead of using `T: _, T: _`" + "types are repeated unnecessarily in trait bounds, use `+` instead of using `T: _, T: _`" } declare_clippy_lint! { /// ### What it does - /// Checks for cases where generics are being used and multiple + /// Checks for cases where generics or trait objects are being used and multiple /// syntax specifications for trait bounds are used simultaneously. /// /// ### Why is this bad? @@ -167,6 +167,61 @@ impl<'tcx> LateLintPass<'tcx> for TraitBounds { } } } + + fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>) { + if_chain! { + if let TyKind::Ref(.., mut_ty) = &ty.kind; + if let TyKind::TraitObject(bounds, ..) = mut_ty.ty.kind; + if bounds.len() > 2; + then { + + // Build up a hash of every trait we've seen + // When we see a trait for the first time, add it to unique_traits + // so we can later use it to build a string of all traits exactly once, without duplicates + + let mut seen_def_ids = FxHashSet::default(); + let mut unique_traits = Vec::new(); + + // Iterate the bounds and add them to our seen hash + // If we haven't yet seen it, add it to the fixed traits + for bound in bounds.iter() { + let Some(def_id) = bound.trait_ref.trait_def_id() else { continue; }; + + let new_trait = seen_def_ids.insert(def_id); + + if new_trait { + unique_traits.push(bound); + } + } + + // If the number of unique traits isn't the same as the number of traits in the bounds, + // there must be 1 or more duplicates + if bounds.len() != unique_traits.len() { + let mut bounds_span = bounds[0].span; + + for bound in bounds.iter().skip(1) { + bounds_span = bounds_span.to(bound.span); + } + + let fixed_trait_snippet = unique_traits + .iter() + .filter_map(|b| snippet_opt(cx, b.span)) + .collect::>() + .join(" + "); + + span_lint_and_sugg( + cx, + TRAIT_DUPLICATION_IN_BOUNDS, + bounds_span, + "this trait bound is already specified in trait declaration", + "try", + fixed_trait_snippet, + Applicability::MaybeIncorrect, + ); + } + } + } + } } impl TraitBounds { diff --git a/clippy_lints/src/transmute/transmute_null_to_fn.rs b/clippy_lints/src/transmute/transmute_null_to_fn.rs index e75d7f6bf..4944381da 100644 --- a/clippy_lints/src/transmute/transmute_null_to_fn.rs +++ b/clippy_lints/src/transmute/transmute_null_to_fn.rs @@ -31,9 +31,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arg: &'t match arg.kind { // Catching: // transmute over constants that resolve to `null`. - ExprKind::Path(ref _qpath) - if matches!(constant(cx, cx.typeck_results(), arg), Some((Constant::RawPtr(0), _))) => - { + ExprKind::Path(ref _qpath) if matches!(constant(cx, cx.typeck_results(), arg), Some(Constant::RawPtr(0))) => { lint_expr(cx, expr); true }, diff --git a/clippy_lints/src/transmute/transmuting_null.rs b/clippy_lints/src/transmute/transmuting_null.rs index 1e407fc41..770914e99 100644 --- a/clippy_lints/src/transmute/transmuting_null.rs +++ b/clippy_lints/src/transmute/transmuting_null.rs @@ -1,4 +1,4 @@ -use clippy_utils::consts::{constant_context, Constant}; +use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::span_lint; use clippy_utils::{is_integer_literal, is_path_diagnostic_item}; use rustc_hir::{Expr, ExprKind}; @@ -16,9 +16,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arg: &'t } // Catching transmute over constants that resolve to `null`. - let mut const_eval_context = constant_context(cx, cx.typeck_results()); if let ExprKind::Path(ref _qpath) = arg.kind && - let Some(Constant::RawPtr(0)) = const_eval_context.expr(arg) + let Some(Constant::RawPtr(0)) = constant(cx, cx.typeck_results(), arg) { span_lint(cx, TRANSMUTING_NULL, expr.span, LINT_MSG); return true; diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs index ddbe6b2c7..28c3fc859 100644 --- a/clippy_lints/src/useless_conversion.rs +++ b/clippy_lints/src/useless_conversion.rs @@ -1,4 +1,5 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; +use clippy_utils::is_ty_alias; use clippy_utils::source::{snippet, snippet_with_context}; use clippy_utils::sugg::Sugg; use clippy_utils::ty::{is_copy, is_type_diagnostic_item, same_type_and_consts}; @@ -138,6 +139,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { if_chain! { if let ExprKind::Path(ref qpath) = path.kind; if let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id(); + if !is_ty_alias(qpath); then { let a = cx.typeck_results().expr_ty(e); let b = cx.typeck_results().expr_ty(arg); diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index 935ea90d2..3c2bf5aba 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -308,7 +308,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> { bind!(self, vec); kind!("CStr(ref {vec})"); chain!(self, "let [{:?}] = **{vec}", vec.value); - } + }, LitKind::Str(s, _) => { bind!(self, s); kind!("Str({s}, _)"); diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 5f05d971f..f6de66bb5 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -174,16 +174,15 @@ macro_rules! define_Conf { } } - #[cfg(feature = "internal")] pub mod metadata { - use crate::utils::internal_lints::metadata_collector::ClippyConfiguration; + use crate::utils::ClippyConfiguration; macro_rules! wrap_option { () => (None); ($x:literal) => (Some($x)); } - pub(crate) fn get_configuration_metadata() -> Vec { + pub fn get_configuration_metadata() -> Vec { vec![ $( { diff --git a/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/clippy_lints/src/utils/internal_lints/metadata_collector.rs index 3d0d4a525..7a1cd3eff 100644 --- a/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -8,7 +8,11 @@ //! a simple mistake) use crate::renamed_lints::RENAMED_LINTS; -use crate::utils::internal_lints::lint_without_lint_pass::{extract_clippy_version_value, is_lint_ref_type}; +use crate::utils::{ + collect_configs, + internal_lints::lint_without_lint_pass::{extract_clippy_version_value, is_lint_ref_type}, + ClippyConfiguration, +}; use clippy_utils::diagnostics::span_lint; use clippy_utils::ty::{match_type, walk_ptrs_ty_depth}; @@ -520,111 +524,6 @@ impl Serialize for ApplicabilityInfo { } } -// ================================================================== -// Configuration -// ================================================================== -#[derive(Debug, Clone, Default)] -pub struct ClippyConfiguration { - name: String, - config_type: &'static str, - default: String, - lints: Vec, - doc: String, - #[allow(dead_code)] - deprecation_reason: Option<&'static str>, -} - -impl ClippyConfiguration { - pub fn new( - name: &'static str, - config_type: &'static str, - default: String, - doc_comment: &'static str, - deprecation_reason: Option<&'static str>, - ) -> Self { - let (lints, doc) = parse_config_field_doc(doc_comment) - .unwrap_or_else(|| (vec![], "[ERROR] MALFORMED DOC COMMENT".to_string())); - - Self { - name: to_kebab(name), - lints, - doc, - config_type, - default, - deprecation_reason, - } - } - - fn to_markdown_paragraph(&self) -> String { - format!( - "### {}\n{}\n\n**Default Value:** `{}` (`{}`)\n\n{}\n\n", - self.name, - self.doc - .lines() - .map(|line| line.strip_prefix(" ").unwrap_or(line)) - .join("\n"), - self.default, - self.config_type, - self.lints - .iter() - .map(|name| name.to_string().split_whitespace().next().unwrap().to_string()) - .map(|name| format!("* [{name}](https://rust-lang.github.io/rust-clippy/master/index.html#{name})")) - .join("\n"), - ) - } - - fn to_markdown_table_entry(&self) -> String { - format!("| [{}](#{}) | `{}` |", self.name, self.name, self.default) - } -} - -fn collect_configs() -> Vec { - crate::utils::conf::metadata::get_configuration_metadata() -} - -/// This parses the field documentation of the config struct. -/// -/// ```rust, ignore -/// parse_config_field_doc(cx, "Lint: LINT_NAME_1, LINT_NAME_2. Papa penguin, papa penguin") -/// ``` -/// -/// Would yield: -/// ```rust, ignore -/// Some(["lint_name_1", "lint_name_2"], "Papa penguin, papa penguin") -/// ``` -fn parse_config_field_doc(doc_comment: &str) -> Option<(Vec, String)> { - const DOC_START: &str = " Lint: "; - if_chain! { - if doc_comment.starts_with(DOC_START); - if let Some(split_pos) = doc_comment.find('.'); - then { - let mut doc_comment = doc_comment.to_string(); - let mut documentation = doc_comment.split_off(split_pos); - - // Extract lints - doc_comment.make_ascii_lowercase(); - let lints: Vec = doc_comment - .split_off(DOC_START.len()) - .split(", ") - .map(str::to_string) - .collect(); - - // Format documentation correctly - // split off leading `.` from lint name list and indent for correct formatting - documentation = documentation.trim_start_matches('.').trim().replace("\n ", "\n "); - - Some((lints, documentation)) - } else { - None - } - } -} - -/// Transforms a given `snake_case_string` to a tasty `kebab-case-string` -fn to_kebab(config_name: &str) -> String { - config_name.replace('_', "-") -} - impl fmt::Display for ClippyConfiguration { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> fmt::Result { writeln!( diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index dc647af26..d3ea7cafa 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -4,3 +4,143 @@ pub mod dump_hir; pub mod format_args_collector; #[cfg(feature = "internal")] pub mod internal_lints; +#[cfg(feature = "internal")] +use itertools::Itertools; + +/// Transforms a given `snake_case_string` to a tasty `kebab-case-string` +fn to_kebab(config_name: &str) -> String { + config_name.replace('_', "-") +} + +// ================================================================== +// Configuration +// ================================================================== +#[derive(Debug, Clone, Default)] //~ ERROR no such field +pub struct ClippyConfiguration { + pub name: String, + #[allow(dead_code)] + config_type: &'static str, + pub default: String, + pub lints: Vec, + pub doc: String, + #[allow(dead_code)] + deprecation_reason: Option<&'static str>, +} + +impl ClippyConfiguration { + pub fn new( + name: &'static str, + config_type: &'static str, + default: String, + doc_comment: &'static str, + deprecation_reason: Option<&'static str>, + ) -> Self { + let (lints, doc) = parse_config_field_doc(doc_comment) + .unwrap_or_else(|| (vec![], "[ERROR] MALFORMED DOC COMMENT".to_string())); + + Self { + name: to_kebab(name), + lints, + doc, + config_type, + default, + deprecation_reason, + } + } + + #[cfg(feature = "internal")] + fn to_markdown_paragraph(&self) -> String { + format!( + "### {}\n{}\n\n**Default Value:** `{}` (`{}`)\n\n{}\n\n", + self.name, + self.doc + .lines() + .map(|line| line.strip_prefix(" ").unwrap_or(line)) + .join("\n"), + self.default, + self.config_type, + self.lints + .iter() + .map(|name| name.to_string().split_whitespace().next().unwrap().to_string()) + .map(|name| format!("* [{name}](https://rust-lang.github.io/rust-clippy/master/index.html#{name})")) + .join("\n"), + ) + } + + #[cfg(feature = "internal")] + fn to_markdown_table_entry(&self) -> String { + format!("| [{}](#{}) | `{}` |", self.name, self.name, self.default) + } +} + +#[cfg(feature = "internal")] +fn collect_configs() -> Vec { + crate::utils::conf::metadata::get_configuration_metadata() +} + +/// This parses the field documentation of the config struct. +/// +/// ```rust, ignore +/// parse_config_field_doc(cx, "Lint: LINT_NAME_1, LINT_NAME_2. Papa penguin, papa penguin") +/// ``` +/// +/// Would yield: +/// ```rust, ignore +/// Some(["lint_name_1", "lint_name_2"], "Papa penguin, papa penguin") +/// ``` +fn parse_config_field_doc(doc_comment: &str) -> Option<(Vec, String)> { + const DOC_START: &str = " Lint: "; + if_chain! { + if doc_comment.starts_with(DOC_START); + if let Some(split_pos) = doc_comment.find('.'); + then { + let mut doc_comment = doc_comment.to_string(); + let mut documentation = doc_comment.split_off(split_pos); + + // Extract lints + doc_comment.make_ascii_lowercase(); + let lints: Vec = doc_comment + .split_off(DOC_START.len()) + .split(", ") + .map(str::to_string) + .collect(); + + // Format documentation correctly + // split off leading `.` from lint name list and indent for correct formatting + documentation = documentation.trim_start_matches('.').trim().replace("\n ", "\n "); + + Some((lints, documentation)) + } else { + None + } + } +} + +// Shamelessly stolen from find_all (https://github.com/nectariner/find_all) +pub trait FindAll: Iterator + Sized { + fn find_all

(&mut self, predicate: P) -> Option> + where + P: FnMut(&Self::Item) -> bool; +} + +impl FindAll for I +where + I: Iterator, +{ + fn find_all

(&mut self, mut predicate: P) -> Option> + where + P: FnMut(&Self::Item) -> bool, + { + let mut occurences = Vec::::default(); + for (index, element) in self.enumerate() { + if predicate(&element) { + occurences.push(index); + } + } + + match occurences.len() { + 0 => None, + _ => Some(occurences), + } + } +} diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index 297a80e57..7329e5081 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -84,7 +84,7 @@ impl UselessVec { let mut applicability = Applicability::MachineApplicable; let snippet = match *vec_args { higher::VecArgs::Repeat(elem, len) => { - if let Some((Constant::Int(len_constant), _)) = constant(cx, cx.typeck_results(), len) { + if let Some(Constant::Int(len_constant)) = constant(cx, cx.typeck_results(), len) { #[expect(clippy::cast_possible_truncation)] if len_constant as u64 * size_of(cx, elem) > self.too_large_for_stack { return; diff --git a/clippy_lints/src/wildcard_imports.rs b/clippy_lints/src/wildcard_imports.rs index 36f910c98..a9089fba3 100644 --- a/clippy_lints/src/wildcard_imports.rs +++ b/clippy_lints/src/wildcard_imports.rs @@ -7,7 +7,7 @@ use rustc_hir::{ def::{DefKind, Res}, Item, ItemKind, PathSegment, UseKind, }; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::ty; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::symbol::kw; @@ -117,6 +117,10 @@ impl_lint_pass!(WildcardImports => [ENUM_GLOB_USE, WILDCARD_IMPORTS]); impl LateLintPass<'_> for WildcardImports { fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + if cx.sess().is_test_crate() { + return; + } + if is_test_module_or_function(cx.tcx, item) { self.test_modules_deep = self.test_modules_deep.saturating_add(1); } diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 8075881e3..fb772644c 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -1,18 +1,21 @@ #![allow(clippy::float_cmp)] +use crate::source::{get_source_text, walk_span_to_context}; use crate::{clip, is_direct_expn_of, sext, unsext}; use if_chain::if_chain; use rustc_ast::ast::{self, LitFloatType, LitKind}; use rustc_data_structures::sync::Lrc; use rustc_hir::def::{DefKind, Res}; use rustc_hir::{BinOp, BinOpKind, Block, Expr, ExprKind, HirId, Item, ItemKind, Node, QPath, UnOp}; +use rustc_lexer::tokenize; use rustc_lint::LateContext; use rustc_middle::mir; use rustc_middle::mir::interpret::Scalar; -use rustc_middle::ty::SubstsRef; use rustc_middle::ty::{self, EarlyBinder, FloatTy, ScalarInt, Ty, TyCtxt}; +use rustc_middle::ty::{List, SubstsRef}; use rustc_middle::{bug, span_bug}; use rustc_span::symbol::Symbol; +use rustc_span::SyntaxContext; use std::cmp::Ordering::{self, Equal}; use std::hash::{Hash, Hasher}; use std::iter; @@ -210,8 +213,7 @@ pub fn lit_to_mir_constant(lit: &LitKind, ty: Option>) -> Constant { match *lit { LitKind::Str(ref is, _) => Constant::Str(is.to_string()), LitKind::Byte(b) => Constant::Int(u128::from(b)), - LitKind::ByteStr(ref s, _) => Constant::Binary(Lrc::clone(s)), - LitKind::CStr(ref s, _) => Constant::Binary(Lrc::clone(s)), + LitKind::ByteStr(ref s, _) | LitKind::CStr(ref s, _) => Constant::Binary(Lrc::clone(s)), LitKind::Char(c) => Constant::Char(c), LitKind::Int(n, _) => Constant::Int(n), LitKind::Float(ref is, LitFloatType::Suffixed(fty)) => match fty { @@ -228,27 +230,46 @@ pub fn lit_to_mir_constant(lit: &LitKind, ty: Option>) -> Constant { } } +/// The source of a constant value. +pub enum ConstantSource { + /// The value is determined solely from the expression. + Local, + /// The value is dependent on a defined constant. + Constant, +} +impl ConstantSource { + pub fn is_local(&self) -> bool { + matches!(self, Self::Local) + } +} + +/// Attempts to evaluate the expression as a constant. pub fn constant<'tcx>( lcx: &LateContext<'tcx>, typeck_results: &ty::TypeckResults<'tcx>, e: &Expr<'_>, -) -> Option<(Constant, bool)> { - let mut cx = ConstEvalLateContext { - lcx, - typeck_results, - param_env: lcx.param_env, - needed_resolution: false, - substs: ty::List::empty(), - }; - cx.expr(e).map(|cst| (cst, cx.needed_resolution)) +) -> Option { + ConstEvalLateContext::new(lcx, typeck_results).expr(e) } +/// Attempts to evaluate the expression as a constant. +pub fn constant_with_source<'tcx>( + lcx: &LateContext<'tcx>, + typeck_results: &ty::TypeckResults<'tcx>, + e: &Expr<'_>, +) -> Option<(Constant, ConstantSource)> { + let mut ctxt = ConstEvalLateContext::new(lcx, typeck_results); + let res = ctxt.expr(e); + res.map(|x| (x, ctxt.source)) +} + +/// Attempts to evaluate an expression only if it's value is not dependent on other items. pub fn constant_simple<'tcx>( lcx: &LateContext<'tcx>, typeck_results: &ty::TypeckResults<'tcx>, e: &Expr<'_>, ) -> Option { - constant(lcx, typeck_results, e).and_then(|(cst, res)| if res { None } else { Some(cst) }) + constant_with_source(lcx, typeck_results, e).and_then(|(c, s)| s.is_local().then_some(c)) } pub fn constant_full_int<'tcx>( @@ -297,29 +318,25 @@ impl Ord for FullInt { } } -/// Creates a `ConstEvalLateContext` from the given `LateContext` and `TypeckResults`. -pub fn constant_context<'a, 'tcx>( - lcx: &'a LateContext<'tcx>, - typeck_results: &'a ty::TypeckResults<'tcx>, -) -> ConstEvalLateContext<'a, 'tcx> { - ConstEvalLateContext { - lcx, - typeck_results, - param_env: lcx.param_env, - needed_resolution: false, - substs: ty::List::empty(), - } -} - pub struct ConstEvalLateContext<'a, 'tcx> { lcx: &'a LateContext<'tcx>, typeck_results: &'a ty::TypeckResults<'tcx>, param_env: ty::ParamEnv<'tcx>, - needed_resolution: bool, + source: ConstantSource, substs: SubstsRef<'tcx>, } impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { + fn new(lcx: &'a LateContext<'tcx>, typeck_results: &'a ty::TypeckResults<'tcx>) -> Self { + Self { + lcx, + typeck_results, + param_env: lcx.param_env, + source: ConstantSource::Local, + substs: List::empty(), + } + } + /// Simple constant folding: Insert an expression, get a constant or none. pub fn expr(&mut self, e: &Expr<'_>) -> Option { match e.kind { @@ -454,11 +471,9 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { .const_eval_resolve(self.param_env, mir::UnevaluatedConst::new(def_id, substs), None) .ok() .map(|val| rustc_middle::mir::ConstantKind::from_value(val, ty))?; - let result = miri_to_const(self.lcx.tcx, result); - if result.is_some() { - self.needed_resolution = true; - } - result + let result = miri_to_const(self.lcx.tcx, result)?; + self.source = ConstantSource::Constant; + Some(result) }, // FIXME: cover all usable cases. _ => None, @@ -492,8 +507,33 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { /// A block can only yield a constant if it only has one constant expression. fn block(&mut self, block: &Block<'_>) -> Option { - if block.stmts.is_empty() { - block.expr.as_ref().and_then(|b| self.expr(b)) + if block.stmts.is_empty() + && let Some(expr) = block.expr + { + // Try to detect any `cfg`ed statements or empty macro expansions. + let span = block.span.data(); + if span.ctxt == SyntaxContext::root() { + if let Some(expr_span) = walk_span_to_context(expr.span, span.ctxt) + && let expr_lo = expr_span.lo() + && expr_lo >= span.lo + && let Some(src) = get_source_text(self.lcx, span.lo..expr_lo) + && let Some(src) = src.as_str() + { + use rustc_lexer::TokenKind::{Whitespace, LineComment, BlockComment, Semi, OpenBrace}; + if !tokenize(src) + .map(|t| t.kind) + .filter(|t| !matches!(t, Whitespace | LineComment { .. } | BlockComment { .. } | Semi)) + .eq([OpenBrace]) + { + self.source = ConstantSource::Constant; + } + } else { + // Unable to access the source. Assume a non-local dependency. + self.source = ConstantSource::Constant; + } + } + + self.expr(expr) } else { None } diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 9b7408d51..a49246a78 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -1,6 +1,7 @@ use crate::consts::constant_simple; use crate::macros::macro_backtrace; -use crate::source::snippet_opt; +use crate::source::{get_source_text, snippet_opt, walk_span_to_context, SpanRange}; +use crate::tokenize_with_text; use rustc_ast::ast::InlineAsmTemplatePiece; use rustc_data_structures::fx::FxHasher; use rustc_hir::def::Res; @@ -13,8 +14,9 @@ use rustc_hir::{ use rustc_lexer::{tokenize, TokenKind}; use rustc_lint::LateContext; use rustc_middle::ty::TypeckResults; -use rustc_span::{sym, Symbol}; +use rustc_span::{sym, BytePos, ExpnKind, MacroKind, Symbol, SyntaxContext}; use std::hash::{Hash, Hasher}; +use std::ops::Range; /// Callback that is called when two expressions are not equal in the sense of `SpanlessEq`, but /// other conditions would make them equal. @@ -65,6 +67,8 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { pub fn inter_expr(&mut self) -> HirEqInterExpr<'_, 'a, 'tcx> { HirEqInterExpr { inner: self, + left_ctxt: SyntaxContext::root(), + right_ctxt: SyntaxContext::root(), locals: HirIdMap::default(), } } @@ -92,6 +96,8 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { pub struct HirEqInterExpr<'a, 'b, 'tcx> { inner: &'a mut SpanlessEq<'b, 'tcx>, + left_ctxt: SyntaxContext, + right_ctxt: SyntaxContext, // When binding are declared, the binding ID in the left expression is mapped to the one on the // right. For example, when comparing `{ let x = 1; x + 2 }` and `{ let y = 1; y + 2 }`, @@ -126,52 +132,88 @@ impl HirEqInterExpr<'_, '_, '_> { } /// Checks whether two blocks are the same. + #[expect(clippy::similar_names)] fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool { - match (left.stmts, left.expr, right.stmts, right.expr) { - ([], None, [], None) => { - // For empty blocks, check to see if the tokens are equal. This will catch the case where a macro - // expanded to nothing, or the cfg attribute was used. - let (Some(left), Some(right)) = ( - snippet_opt(self.inner.cx, left.span), - snippet_opt(self.inner.cx, right.span), - ) else { return true }; - let mut left_pos = 0; - let left = tokenize(&left) - .map(|t| { - let end = left_pos + t.len as usize; - let s = &left[left_pos..end]; - left_pos = end; - (t, s) - }) - .filter(|(t, _)| { - !matches!( - t.kind, - TokenKind::LineComment { .. } | TokenKind::BlockComment { .. } | TokenKind::Whitespace - ) - }) - .map(|(_, s)| s); - let mut right_pos = 0; - let right = tokenize(&right) - .map(|t| { - let end = right_pos + t.len as usize; - let s = &right[right_pos..end]; - right_pos = end; - (t, s) - }) - .filter(|(t, _)| { - !matches!( - t.kind, - TokenKind::LineComment { .. } | TokenKind::BlockComment { .. } | TokenKind::Whitespace - ) - }) - .map(|(_, s)| s); - left.eq(right) - }, - _ => { - over(left.stmts, right.stmts, |l, r| self.eq_stmt(l, r)) - && both(&left.expr, &right.expr, |l, r| self.eq_expr(l, r)) - }, + use TokenKind::{BlockComment, LineComment, Semi, Whitespace}; + if left.stmts.len() != right.stmts.len() { + return false; } + let lspan = left.span.data(); + let rspan = right.span.data(); + if lspan.ctxt != SyntaxContext::root() && rspan.ctxt != SyntaxContext::root() { + // Don't try to check in between statements inside macros. + return over(left.stmts, right.stmts, |left, right| self.eq_stmt(left, right)) + && both(&left.expr, &right.expr, |left, right| self.eq_expr(left, right)); + } + if lspan.ctxt != rspan.ctxt { + return false; + } + + let mut lstart = lspan.lo; + let mut rstart = rspan.lo; + + for (left, right) in left.stmts.iter().zip(right.stmts) { + if !self.eq_stmt(left, right) { + return false; + } + + // Try to detect any `cfg`ed statements or empty macro expansions. + let Some(lstmt_span) = walk_span_to_context(left.span, lspan.ctxt) else { + return false; + }; + let Some(rstmt_span) = walk_span_to_context(right.span, rspan.ctxt) else { + return false; + }; + let lstmt_span = lstmt_span.data(); + let rstmt_span = rstmt_span.data(); + + if lstmt_span.lo < lstart && rstmt_span.lo < rstart { + // Can happen when macros expand to multiple statements, or rearrange statements. + // Nothing in between the statements to check in this case. + continue; + } + if lstmt_span.lo < lstart || rstmt_span.lo < rstart { + // Only one of the blocks had a weird macro. + return false; + } + if !eq_span_tokens(self.inner.cx, lstart..lstmt_span.lo, rstart..rstmt_span.lo, |t| { + !matches!(t, Whitespace | LineComment { .. } | BlockComment { .. } | Semi) + }) { + return false; + } + + lstart = lstmt_span.hi; + rstart = rstmt_span.hi; + } + + let (lend, rend) = match (left.expr, right.expr) { + (Some(left), Some(right)) => { + if !self.eq_expr(left, right) { + return false; + } + let Some(lexpr_span) = walk_span_to_context(left.span, lspan.ctxt) else { + return false; + }; + let Some(rexpr_span) = walk_span_to_context(right.span, rspan.ctxt) else { + return false; + }; + (lexpr_span.lo(), rexpr_span.lo()) + }, + (None, None) => (lspan.hi, rspan.hi), + (Some(_), None) | (None, Some(_)) => return false, + }; + + if lend < lstart && rend < rstart { + // Can happen when macros rearrange the input. + // Nothing in between the statements to check in this case. + return true; + } else if lend < lstart || rend < rstart { + // Only one of the blocks had a weird macro + return false; + } + eq_span_tokens(self.inner.cx, lstart..lend, rstart..rend, |t| { + !matches!(t, Whitespace | LineComment { .. } | BlockComment { .. } | Semi) + }) } fn should_ignore(&mut self, expr: &Expr<'_>) -> bool { @@ -207,7 +249,7 @@ impl HirEqInterExpr<'_, '_, '_> { #[expect(clippy::similar_names)] pub fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool { - if !self.inner.allow_side_effects && left.span.ctxt() != right.span.ctxt() { + if !self.check_ctxt(left.span.ctxt(), right.span.ctxt()) { return false; } @@ -440,6 +482,45 @@ impl HirEqInterExpr<'_, '_, '_> { fn eq_type_binding(&mut self, left: &TypeBinding<'_>, right: &TypeBinding<'_>) -> bool { left.ident.name == right.ident.name && self.eq_ty(left.ty(), right.ty()) } + + fn check_ctxt(&mut self, left: SyntaxContext, right: SyntaxContext) -> bool { + if self.left_ctxt == left && self.right_ctxt == right { + return true; + } else if self.left_ctxt == left || self.right_ctxt == right { + // Only one context has changed. This can only happen if the two nodes are written differently. + return false; + } else if left != SyntaxContext::root() { + let mut left_data = left.outer_expn_data(); + let mut right_data = right.outer_expn_data(); + loop { + use TokenKind::{BlockComment, LineComment, Whitespace}; + if left_data.macro_def_id != right_data.macro_def_id + || (matches!(left_data.kind, ExpnKind::Macro(MacroKind::Bang, name) if name == sym::cfg) + && !eq_span_tokens(self.inner.cx, left_data.call_site, right_data.call_site, |t| { + !matches!(t, Whitespace | LineComment { .. } | BlockComment { .. }) + })) + { + // Either a different chain of macro calls, or different arguments to the `cfg` macro. + return false; + } + let left_ctxt = left_data.call_site.ctxt(); + let right_ctxt = right_data.call_site.ctxt(); + if left_ctxt == SyntaxContext::root() && right_ctxt == SyntaxContext::root() { + break; + } + if left_ctxt == SyntaxContext::root() || right_ctxt == SyntaxContext::root() { + // Different lengths for the expansion stack. This can only happen if nodes are written differently, + // or shouldn't be compared to start with. + return false; + } + left_data = left_ctxt.outer_expn_data(); + right_data = right_ctxt.outer_expn_data(); + } + } + self.left_ctxt = left; + self.right_ctxt = right; + true + } } /// Some simple reductions like `{ return }` => `return` @@ -1038,3 +1119,34 @@ pub fn hash_expr(cx: &LateContext<'_>, e: &Expr<'_>) -> u64 { h.hash_expr(e); h.finish() } + +#[expect(clippy::similar_names)] +fn eq_span_tokens( + cx: &LateContext<'_>, + left: impl SpanRange, + right: impl SpanRange, + pred: impl Fn(TokenKind) -> bool, +) -> bool { + fn f(cx: &LateContext<'_>, left: Range, right: Range, pred: impl Fn(TokenKind) -> bool) -> bool { + if let Some(lsrc) = get_source_text(cx, left) + && let Some(lsrc) = lsrc.as_str() + && let Some(rsrc) = get_source_text(cx, right) + && let Some(rsrc) = rsrc.as_str() + { + let pred = |t: &(_, _)| pred(t.0); + let map = |(_, x)| x; + + let ltok = tokenize_with_text(lsrc) + .filter(pred) + .map(map); + let rtok = tokenize_with_text(rsrc) + .filter(pred) + .map(map); + ltok.eq(rtok) + } else { + // Unable to access the source. Conservatively assume the blocks aren't equal. + false + } + } + f(cx, left.into_range(), right.into_range(), pred) +} diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 964104fc3..575c29a6b 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1,5 +1,6 @@ #![feature(array_chunks)] #![feature(box_patterns)] +#![feature(if_let_guard)] #![feature(let_chains)] #![feature(lint_reasons)] #![feature(never_type)] @@ -76,6 +77,7 @@ use std::sync::OnceLock; use std::sync::{Mutex, MutexGuard}; use if_chain::if_chain; +use itertools::Itertools; use rustc_ast::ast::{self, LitKind, RangeLimits}; use rustc_ast::Attribute; use rustc_data_structures::fx::FxHashMap; @@ -282,6 +284,15 @@ pub fn is_wild(pat: &Pat<'_>) -> bool { matches!(pat.kind, PatKind::Wild) } +/// Checks if the given `QPath` belongs to a type alias. +pub fn is_ty_alias(qpath: &QPath<'_>) -> bool { + match *qpath { + QPath::Resolved(_, path) => matches!(path.res, Res::Def(DefKind::TyAlias, ..)), + QPath::TypeRelative(ty, _) if let TyKind::Path(qpath) = ty.kind => { is_ty_alias(&qpath) }, + _ => false, + } +} + /// Checks if the method call given in `expr` belongs to the given trait. /// This is a deprecated function, consider using [`is_trait_method`]. pub fn match_trait_method(cx: &LateContext<'_>, expr: &Expr<'_>, path: &[&str]) -> bool { @@ -1488,7 +1499,7 @@ pub fn is_range_full(cx: &LateContext<'_>, expr: &Expr<'_>, container_path: Opti && let const_val = cx.tcx.valtree_to_const_val((bnd_ty, min_val.to_valtree())) && let min_const_kind = ConstantKind::from_value(const_val, bnd_ty) && let Some(min_const) = miri_to_const(cx.tcx, min_const_kind) - && let Some((start_const, _)) = constant(cx, cx.typeck_results(), start) + && let Some(start_const) = constant(cx, cx.typeck_results(), start) { start_const == min_const } else { @@ -1504,7 +1515,7 @@ pub fn is_range_full(cx: &LateContext<'_>, expr: &Expr<'_>, container_path: Opti && let const_val = cx.tcx.valtree_to_const_val((bnd_ty, max_val.to_valtree())) && let max_const_kind = ConstantKind::from_value(const_val, bnd_ty) && let Some(max_const) = miri_to_const(cx.tcx, max_const_kind) - && let Some((end_const, _)) = constant(cx, cx.typeck_results(), end) + && let Some(end_const) = constant(cx, cx.typeck_results(), end) { end_const == max_const } else { @@ -1536,7 +1547,7 @@ pub fn is_integer_const(cx: &LateContext<'_>, e: &Expr<'_>, value: u128) -> bool return true; } let enclosing_body = cx.tcx.hir().enclosing_body_owner(e.hir_id); - if let Some((Constant::Int(v), _)) = constant(cx, cx.tcx.typeck(enclosing_body), e) { + if let Some(Constant::Int(v)) = constant(cx, cx.tcx.typeck(enclosing_body), e) { return value == v; } false @@ -2480,6 +2491,17 @@ pub fn walk_to_expr_usage<'tcx, T>( None } +/// Tokenizes the input while keeping the text associated with each token. +pub fn tokenize_with_text(s: &str) -> impl Iterator { + let mut pos = 0; + tokenize(s).map(move |t| { + let end = pos + t.len; + let range = pos as usize..end as usize; + pos = end; + (t.kind, s.get(range).unwrap_or_default()) + }) +} + /// Checks whether a given span has any comment token /// This checks for all types of comment: line "//", block "/**", doc "///" "//!" pub fn span_contains_comment(sm: &SourceMap, span: Span) -> bool { @@ -2496,23 +2518,11 @@ pub fn span_contains_comment(sm: &SourceMap, span: Span) -> bool { /// Comments are returned wrapped with their relevant delimiters pub fn span_extract_comment(sm: &SourceMap, span: Span) -> String { let snippet = sm.span_to_snippet(span).unwrap_or_default(); - let mut comments_buf: Vec = Vec::new(); - let mut index: usize = 0; - - for token in tokenize(&snippet) { - let token_range = index..(index + token.len as usize); - index += token.len as usize; - match token.kind { - TokenKind::BlockComment { .. } | TokenKind::LineComment { .. } => { - if let Some(comment) = snippet.get(token_range) { - comments_buf.push(comment.to_string()); - } - }, - _ => (), - } - } - - comments_buf.join("\n") + let res = tokenize_with_text(&snippet) + .filter(|(t, _)| matches!(t, TokenKind::BlockComment { .. } | TokenKind::LineComment { .. })) + .map(|(_, s)| s) + .join("\n"); + res } pub fn span_find_starting_semi(sm: &SourceMap, span: Span) -> Span { diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs index 62fa37660..0f6029064 100644 --- a/clippy_utils/src/source.rs +++ b/clippy_utils/src/source.rs @@ -2,14 +2,64 @@ #![allow(clippy::module_name_repetitions)] +use rustc_data_structures::sync::Lrc; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LintContext}; use rustc_session::Session; -use rustc_span::hygiene; use rustc_span::source_map::{original_sp, SourceMap}; +use rustc_span::{hygiene, SourceFile}; use rustc_span::{BytePos, Pos, Span, SpanData, SyntaxContext, DUMMY_SP}; use std::borrow::Cow; +use std::ops::Range; + +/// A type which can be converted to the range portion of a `Span`. +pub trait SpanRange { + fn into_range(self) -> Range; +} +impl SpanRange for Span { + fn into_range(self) -> Range { + let data = self.data(); + data.lo..data.hi + } +} +impl SpanRange for SpanData { + fn into_range(self) -> Range { + self.lo..self.hi + } +} +impl SpanRange for Range { + fn into_range(self) -> Range { + self + } +} + +pub struct SourceFileRange { + pub sf: Lrc, + pub range: Range, +} +impl SourceFileRange { + /// Attempts to get the text from the source file. This can fail if the source text isn't + /// loaded. + pub fn as_str(&self) -> Option<&str> { + self.sf.src.as_ref().and_then(|x| x.get(self.range.clone())) + } +} + +/// Gets the source file, and range in the file, of the given span. Returns `None` if the span +/// extends through multiple files, or is malformed. +pub fn get_source_text(cx: &impl LintContext, sp: impl SpanRange) -> Option { + fn f(sm: &SourceMap, sp: Range) -> Option { + let start = sm.lookup_byte_offset(sp.start); + let end = sm.lookup_byte_offset(sp.end); + if !Lrc::ptr_eq(&start.sf, &end.sf) || start.pos > end.pos { + return None; + } + let range = start.pos.to_usize()..end.pos.to_usize(); + Some(SourceFileRange { sf: start.sf, range }) + } + f(cx.sess().source_map(), sp.into_range()) +} /// Like `snippet_block`, but add braces if the expr is not an `ExprKind::Block`. pub fn expr_block( diff --git a/rust-toolchain b/rust-toolchain index 60b8a5ac0..bc7fb711e 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2023-05-05" +channel = "nightly-2023-05-20" components = ["cargo", "llvm-tools", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"] diff --git a/tests/ui-internal/custom_ice_message.rs b/tests/ui-internal/custom_ice_message.rs index acb98d7ba..99ce70283 100644 --- a/tests/ui-internal/custom_ice_message.rs +++ b/tests/ui-internal/custom_ice_message.rs @@ -3,6 +3,7 @@ //@normalize-stderr-test: "produce_ice.rs:\d*:\d*" -> "produce_ice.rs" //@normalize-stderr-test: "', .*clippy_lints" -> "', clippy_lints" //@normalize-stderr-test: "'rustc'" -> "''" +//@normalize-stderr-test: "running on .*" -> "running on " //@normalize-stderr-test: "(?ms)query stack during panic:\n.*end of query stack\n" -> "" #![deny(clippy::internal)] diff --git a/tests/ui-internal/custom_ice_message.stderr b/tests/ui-internal/custom_ice_message.stderr index b4619e980..0fc385cd6 100644 --- a/tests/ui-internal/custom_ice_message.stderr +++ b/tests/ui-internal/custom_ice_message.stderr @@ -1,12 +1,14 @@ thread '' panicked at 'Would you like some help with that?', clippy_lints/src/utils/internal_lints/produce_ice.rs note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace -error: internal compiler error: unexpected panic - -note: the compiler unexpectedly panicked. this is a bug. +error: the compiler unexpectedly panicked. this is a bug. note: we would appreciate a bug report: https://github.com/rust-lang/rust-clippy/issues/new +note: rustc 1.71.0-nightly (521f4dae1 2023-05-19) running on + +note: compiler flags: -C prefer-dynamic -Z ui-testing + note: Clippy version: foo thread panicked while panicking. aborting. diff --git a/tests/ui/arithmetic_side_effects.rs b/tests/ui/arithmetic_side_effects.rs index ab408bdf2..f95af1017 100644 --- a/tests/ui/arithmetic_side_effects.rs +++ b/tests/ui/arithmetic_side_effects.rs @@ -458,4 +458,12 @@ pub fn issue_10583(a: u16) -> u16 { 10 / a } +pub fn issue_10767() { + let n = &1.0; + n + n; + 3.1_f32 + &1.2_f32; + &3.4_f32 + 1.5_f32; + &3.5_f32 + &1.3_f32; +} + fn main() {} diff --git a/tests/ui/borrow_deref_ref.fixed b/tests/ui/borrow_deref_ref.fixed index 165e4bc82..755264617 100644 --- a/tests/ui/borrow_deref_ref.fixed +++ b/tests/ui/borrow_deref_ref.fixed @@ -1,7 +1,11 @@ //@run-rustfix +//@aux-build: proc_macros.rs #![allow(dead_code, unused_variables)] +extern crate proc_macros; +use proc_macros::with_span; + fn main() {} mod should_lint { @@ -47,6 +51,17 @@ mod should_not_lint2 { } } +with_span!( + span + + fn just_returning(x: &u32) -> &u32 { + x + } + + fn dont_lint_proc_macro() { + let a = &mut &*just_returning(&12); + } +); // this mod explains why we should not lint `& &* (&T)` mod false_negative { fn foo() { diff --git a/tests/ui/borrow_deref_ref.rs b/tests/ui/borrow_deref_ref.rs index 66c8d69be..e319d365f 100644 --- a/tests/ui/borrow_deref_ref.rs +++ b/tests/ui/borrow_deref_ref.rs @@ -1,7 +1,11 @@ //@run-rustfix +//@aux-build: proc_macros.rs #![allow(dead_code, unused_variables)] +extern crate proc_macros; +use proc_macros::with_span; + fn main() {} mod should_lint { @@ -47,6 +51,17 @@ mod should_not_lint2 { } } +with_span!( + span + + fn just_returning(x: &u32) -> &u32 { + x + } + + fn dont_lint_proc_macro() { + let a = &mut &*just_returning(&12); + } +); // this mod explains why we should not lint `& &* (&T)` mod false_negative { fn foo() { diff --git a/tests/ui/borrow_deref_ref.stderr b/tests/ui/borrow_deref_ref.stderr index d72de37c6..1e47cda67 100644 --- a/tests/ui/borrow_deref_ref.stderr +++ b/tests/ui/borrow_deref_ref.stderr @@ -1,5 +1,5 @@ error: deref on an immutable reference - --> $DIR/borrow_deref_ref.rs:10:17 + --> $DIR/borrow_deref_ref.rs:14:17 | LL | let b = &*a; | ^^^ help: if you would like to reborrow, try removing `&*`: `a` @@ -7,13 +7,13 @@ LL | let b = &*a; = note: `-D clippy::borrow-deref-ref` implied by `-D warnings` error: deref on an immutable reference - --> $DIR/borrow_deref_ref.rs:12:22 + --> $DIR/borrow_deref_ref.rs:16:22 | LL | let b = &mut &*bar(&12); | ^^^^^^^^^^ help: if you would like to reborrow, try removing `&*`: `bar(&12)` error: deref on an immutable reference - --> $DIR/borrow_deref_ref.rs:55:23 + --> $DIR/borrow_deref_ref.rs:70:23 | LL | let addr_y = &&*x as *const _ as usize; // assert ok | ^^^ help: if you would like to reborrow, try removing `&*`: `x` diff --git a/tests/ui/box_default.fixed b/tests/ui/box_default.fixed index e63312904..840902b53 100644 --- a/tests/ui/box_default.fixed +++ b/tests/ui/box_default.fixed @@ -35,6 +35,13 @@ fn main() { let _more = ret_ty_fn(); call_ty_fn(Box::default()); issue_10381(); + + // `Box::>::default()` would be valid here, but not `Box::default()` or + // `Box::::default()` + // + // Would have a suggestion after https://github.com/rust-lang/rust/blob/fdd030127cc68afec44a8d3f6341525dd34e50ae/compiler/rustc_middle/src/ty/diagnostics.rs#L554-L563 + let mut unnameable = Box::new(Option::default()); + let _ = unnameable.insert(|| {}); } fn ret_ty_fn() -> Box { diff --git a/tests/ui/box_default.rs b/tests/ui/box_default.rs index 34a05a29c..3618486a4 100644 --- a/tests/ui/box_default.rs +++ b/tests/ui/box_default.rs @@ -35,6 +35,13 @@ fn main() { let _more = ret_ty_fn(); call_ty_fn(Box::new(u8::default())); issue_10381(); + + // `Box::>::default()` would be valid here, but not `Box::default()` or + // `Box::::default()` + // + // Would have a suggestion after https://github.com/rust-lang/rust/blob/fdd030127cc68afec44a8d3f6341525dd34e50ae/compiler/rustc_middle/src/ty/diagnostics.rs#L554-L563 + let mut unnameable = Box::new(Option::default()); + let _ = unnameable.insert(|| {}); } fn ret_ty_fn() -> Box { diff --git a/tests/ui/box_default.stderr b/tests/ui/box_default.stderr index c98348636..13dfc5ae4 100644 --- a/tests/ui/box_default.stderr +++ b/tests/ui/box_default.stderr @@ -73,25 +73,25 @@ LL | call_ty_fn(Box::new(u8::default())); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:41:5 + --> $DIR/box_default.rs:48:5 | LL | Box::new(bool::default()) | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:58:28 + --> $DIR/box_default.rs:65:28 | LL | let _: Box = Box::new(ImplementsDefault::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:67:17 + --> $DIR/box_default.rs:74:17 | LL | let _ = Box::new(WeirdPathed::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::::default()` error: `Box::new(_)` of default value - --> $DIR/box_default.rs:79:18 + --> $DIR/box_default.rs:86:18 | LL | Some(Box::new(Foo::default())) | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::::default()` diff --git a/tests/ui/collapsible_if.fixed b/tests/ui/collapsible_if.fixed index d2aba2ac5..c6514a559 100644 --- a/tests/ui/collapsible_if.fixed +++ b/tests/ui/collapsible_if.fixed @@ -1,5 +1,10 @@ //@run-rustfix -#![allow(clippy::assertions_on_constants, clippy::equatable_if_let)] +#![allow( + clippy::assertions_on_constants, + clippy::equatable_if_let, + clippy::nonminimal_bool, + clippy::eq_op +)] #[rustfmt::skip] #[warn(clippy::collapsible_if)] diff --git a/tests/ui/collapsible_if.rs b/tests/ui/collapsible_if.rs index e0bef7f9c..2c85b68df 100644 --- a/tests/ui/collapsible_if.rs +++ b/tests/ui/collapsible_if.rs @@ -1,5 +1,10 @@ //@run-rustfix -#![allow(clippy::assertions_on_constants, clippy::equatable_if_let)] +#![allow( + clippy::assertions_on_constants, + clippy::equatable_if_let, + clippy::nonminimal_bool, + clippy::eq_op +)] #[rustfmt::skip] #[warn(clippy::collapsible_if)] diff --git a/tests/ui/collapsible_if.stderr b/tests/ui/collapsible_if.stderr index 6327444df..c687bae1a 100644 --- a/tests/ui/collapsible_if.stderr +++ b/tests/ui/collapsible_if.stderr @@ -1,5 +1,5 @@ error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:9:5 + --> $DIR/collapsible_if.rs:14:5 | LL | / if x == "hello" { LL | | if y == "world" { @@ -17,7 +17,7 @@ LL + } | error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:15:5 + --> $DIR/collapsible_if.rs:20:5 | LL | / if x == "hello" || x == "world" { LL | | if y == "world" || y == "hello" { @@ -34,7 +34,7 @@ LL + } | error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:21:5 + --> $DIR/collapsible_if.rs:26:5 | LL | / if x == "hello" && x == "world" { LL | | if y == "world" || y == "hello" { @@ -51,7 +51,7 @@ LL + } | error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:27:5 + --> $DIR/collapsible_if.rs:32:5 | LL | / if x == "hello" || x == "world" { LL | | if y == "world" && y == "hello" { @@ -68,7 +68,7 @@ LL + } | error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:33:5 + --> $DIR/collapsible_if.rs:38:5 | LL | / if x == "hello" && x == "world" { LL | | if y == "world" && y == "hello" { @@ -85,7 +85,7 @@ LL + } | error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:39:5 + --> $DIR/collapsible_if.rs:44:5 | LL | / if 42 == 1337 { LL | | if 'a' != 'A' { @@ -102,7 +102,7 @@ LL + } | error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:95:5 + --> $DIR/collapsible_if.rs:100:5 | LL | / if x == "hello" { LL | | if y == "world" { // Collapsible @@ -119,7 +119,7 @@ LL + } | error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:154:5 + --> $DIR/collapsible_if.rs:159:5 | LL | / if matches!(true, true) { LL | | if matches!(true, true) {} @@ -127,7 +127,7 @@ LL | | } | |_____^ help: collapse nested if block: `if matches!(true, true) && matches!(true, true) {}` error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:159:5 + --> $DIR/collapsible_if.rs:164:5 | LL | / if matches!(true, true) && truth() { LL | | if matches!(true, true) {} diff --git a/tests/ui/dbg_macro.rs b/tests/ui/dbg_macro.rs index 8701e3cd2..10788d404 100644 --- a/tests/ui/dbg_macro.rs +++ b/tests/ui/dbg_macro.rs @@ -4,6 +4,7 @@ fn foo(n: u32) -> u32 { if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n } } +fn bar(_: ()) {} fn factorial(n: u32) -> u32 { if dbg!(n <= 1) { @@ -21,6 +22,32 @@ fn main() { dbg!(1, 2, 3, 4, 5); } +fn issue9914() { + macro_rules! foo { + ($x:expr) => { + $x; + }; + } + macro_rules! foo2 { + ($x:expr) => { + $x; + }; + } + macro_rules! expand_to_dbg { + () => { + dbg!(); + }; + } + + dbg!(); + #[allow(clippy::let_unit_value)] + let _ = dbg!(); + bar(dbg!()); + foo!(dbg!()); + foo2!(foo!(dbg!())); + expand_to_dbg!(); +} + mod issue7274 { trait Thing<'b> { fn foo(&self); diff --git a/tests/ui/dbg_macro.stderr b/tests/ui/dbg_macro.stderr index ddb5f1342..530e76633 100644 --- a/tests/ui/dbg_macro.stderr +++ b/tests/ui/dbg_macro.stderr @@ -11,7 +11,7 @@ LL | if let Some(n) = n.checked_sub(4) { n } else { n } | ~~~~~~~~~~~~~~~~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:9:8 + --> $DIR/dbg_macro.rs:10:8 | LL | if dbg!(n <= 1) { | ^^^^^^^^^^^^ @@ -22,7 +22,7 @@ LL | if n <= 1 { | ~~~~~~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:10:9 + --> $DIR/dbg_macro.rs:11:9 | LL | dbg!(1) | ^^^^^^^ @@ -33,7 +33,7 @@ LL | 1 | error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:12:9 + --> $DIR/dbg_macro.rs:13:9 | LL | dbg!(n * factorial(n - 1)) | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -44,7 +44,7 @@ LL | n * factorial(n - 1) | error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:17:5 + --> $DIR/dbg_macro.rs:18:5 | LL | dbg!(42); | ^^^^^^^^ @@ -55,7 +55,7 @@ LL | 42; | ~~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:18:5 + --> $DIR/dbg_macro.rs:19:5 | LL | dbg!(dbg!(dbg!(42))); | ^^^^^^^^^^^^^^^^^^^^ @@ -66,7 +66,7 @@ LL | dbg!(dbg!(42)); | ~~~~~~~~~~~~~~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:19:14 + --> $DIR/dbg_macro.rs:20:14 | LL | foo(3) + dbg!(factorial(4)); | ^^^^^^^^^^^^^^^^^^ @@ -77,7 +77,7 @@ LL | foo(3) + factorial(4); | ~~~~~~~~~~~~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:20:5 + --> $DIR/dbg_macro.rs:21:5 | LL | dbg!(1, 2, dbg!(3, 4)); | ^^^^^^^^^^^^^^^^^^^^^^ @@ -88,7 +88,7 @@ LL | (1, 2, dbg!(3, 4)); | ~~~~~~~~~~~~~~~~~~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:21:5 + --> $DIR/dbg_macro.rs:22:5 | LL | dbg!(1, 2, 3, 4, 5); | ^^^^^^^^^^^^^^^^^^^ @@ -99,7 +99,63 @@ LL | (1, 2, 3, 4, 5); | ~~~~~~~~~~~~~~~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:41:9 + --> $DIR/dbg_macro.rs:42:5 + | +LL | dbg!(); + | ^^^^^^^ + | +help: remove the invocation before committing it to a version control system + | +LL - dbg!(); +LL + + | + +error: the `dbg!` macro is intended as a debugging tool + --> $DIR/dbg_macro.rs:44:13 + | +LL | let _ = dbg!(); + | ^^^^^^ + | +help: remove the invocation before committing it to a version control system + | +LL | let _ = (); + | ~~ + +error: the `dbg!` macro is intended as a debugging tool + --> $DIR/dbg_macro.rs:45:9 + | +LL | bar(dbg!()); + | ^^^^^^ + | +help: remove the invocation before committing it to a version control system + | +LL | bar(()); + | ~~ + +error: the `dbg!` macro is intended as a debugging tool + --> $DIR/dbg_macro.rs:46:10 + | +LL | foo!(dbg!()); + | ^^^^^^ + | +help: remove the invocation before committing it to a version control system + | +LL | foo!(()); + | ~~ + +error: the `dbg!` macro is intended as a debugging tool + --> $DIR/dbg_macro.rs:47:16 + | +LL | foo2!(foo!(dbg!())); + | ^^^^^^ + | +help: remove the invocation before committing it to a version control system + | +LL | foo2!(foo!(())); + | ~~ + +error: the `dbg!` macro is intended as a debugging tool + --> $DIR/dbg_macro.rs:68:9 | LL | dbg!(2); | ^^^^^^^ @@ -110,7 +166,7 @@ LL | 2; | ~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:47:5 + --> $DIR/dbg_macro.rs:74:5 | LL | dbg!(1); | ^^^^^^^ @@ -121,7 +177,7 @@ LL | 1; | ~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:52:5 + --> $DIR/dbg_macro.rs:79:5 | LL | dbg!(1); | ^^^^^^^ @@ -132,7 +188,7 @@ LL | 1; | ~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:58:9 + --> $DIR/dbg_macro.rs:85:9 | LL | dbg!(1); | ^^^^^^^ @@ -142,5 +198,5 @@ help: remove the invocation before committing it to a version control system LL | 1; | ~ -error: aborting due to 13 previous errors +error: aborting due to 18 previous errors diff --git a/tests/ui/default_constructed_unit_structs.fixed b/tests/ui/default_constructed_unit_structs.fixed index 4c2d1ea48..e1012f38b 100644 --- a/tests/ui/default_constructed_unit_structs.fixed +++ b/tests/ui/default_constructed_unit_structs.fixed @@ -105,6 +105,7 @@ fn main() { // should lint let _ = PhantomData::; let _: PhantomData = PhantomData; + let _: PhantomData = std::marker::PhantomData; let _ = UnitStruct; // should not lint @@ -116,4 +117,21 @@ fn main() { let _ = EmptyStruct::default(); let _ = FakeDefault::default(); let _ = ::default(); + + macro_rules! in_macro { + ($i:ident) => {{ + let _ = UnitStruct::default(); + let _ = $i::default(); + }}; + } + + in_macro!(UnitStruct); + + macro_rules! struct_from_macro { + () => { + UnitStruct + }; + } + + let _ = ::default(); } diff --git a/tests/ui/default_constructed_unit_structs.rs b/tests/ui/default_constructed_unit_structs.rs index 850793dd5..c7b4313db 100644 --- a/tests/ui/default_constructed_unit_structs.rs +++ b/tests/ui/default_constructed_unit_structs.rs @@ -105,6 +105,7 @@ fn main() { // should lint let _ = PhantomData::::default(); let _: PhantomData = PhantomData::default(); + let _: PhantomData = std::marker::PhantomData::default(); let _ = UnitStruct::default(); // should not lint @@ -116,4 +117,21 @@ fn main() { let _ = EmptyStruct::default(); let _ = FakeDefault::default(); let _ = ::default(); + + macro_rules! in_macro { + ($i:ident) => {{ + let _ = UnitStruct::default(); + let _ = $i::default(); + }}; + } + + in_macro!(UnitStruct); + + macro_rules! struct_from_macro { + () => { + UnitStruct + }; + } + + let _ = ::default(); } diff --git a/tests/ui/default_constructed_unit_structs.stderr b/tests/ui/default_constructed_unit_structs.stderr index 4058943d0..61a32fb10 100644 --- a/tests/ui/default_constructed_unit_structs.stderr +++ b/tests/ui/default_constructed_unit_structs.stderr @@ -25,10 +25,16 @@ LL | let _: PhantomData = PhantomData::default(); | ^^^^^^^^^^^ help: remove this call to `default` error: use of `default` to create a unit struct - --> $DIR/default_constructed_unit_structs.rs:108:23 + --> $DIR/default_constructed_unit_structs.rs:108:55 + | +LL | let _: PhantomData = std::marker::PhantomData::default(); + | ^^^^^^^^^^^ help: remove this call to `default` + +error: use of `default` to create a unit struct + --> $DIR/default_constructed_unit_structs.rs:109:23 | LL | let _ = UnitStruct::default(); | ^^^^^^^^^^^ help: remove this call to `default` -error: aborting due to 5 previous errors +error: aborting due to 6 previous errors diff --git a/tests/ui/empty_line_after_doc_comments.rs b/tests/ui/empty_line_after_doc_comments.rs new file mode 100644 index 000000000..e843770f5 --- /dev/null +++ b/tests/ui/empty_line_after_doc_comments.rs @@ -0,0 +1,132 @@ +//@aux-build:proc_macro_attr.rs +#![warn(clippy::empty_line_after_doc_comments)] +#![allow(clippy::assertions_on_constants)] +#![feature(custom_inner_attributes)] +#![rustfmt::skip] + +#[macro_use] +extern crate proc_macro_attr; + +mod some_mod { + //! This doc comment should *NOT* produce a warning + + mod some_inner_mod { + fn some_noop() {} + } +} + +/// This should produce a warning + +fn with_doc_and_newline() { assert!(true)} + +// This should *NOT* produce a warning +#[crate_type = "lib"] + +/// some comment +fn with_one_newline_and_comment() { assert!(true) } + +// This should *NOT* produce a warning +#[crate_type = "lib"] +/// some comment +fn with_no_newline_and_comment() { assert!(true) } + + +// This should *NOT* produce a warning +#[crate_type = "lib"] + +fn with_one_newline() { assert!(true) } + +// This should *NOT* produce a warning +#[crate_type = "lib"] + + +fn with_two_newlines() { assert!(true) } + + +// This should *NOT* produce a warning +#[crate_type = "lib"] + +enum Baz { + One, + Two +} + +// This should *NOT* produce a warning +#[crate_type = "lib"] + +struct Foo { + one: isize, + two: isize +} + +// This should *NOT* produce a warning +#[crate_type = "lib"] + +mod foo { +} + +/// This doc comment should produce a warning + +/** This is also a doc comment and should produce a warning + */ + +// This should *NOT* produce a warning +#[allow(non_camel_case_types)] +#[allow(missing_docs)] +#[allow(missing_docs)] +fn three_attributes() { assert!(true) } + +// This should *NOT* produce a warning +#[doc = " +Returns the escaped value of the textual representation of + +"] +pub fn function() -> bool { + true +} + +// This should *NOT* produce a warning +#[derive(Clone, Copy)] +pub enum FooFighter { + Bar1, + + Bar2, + + Bar3, + + Bar4 +} + +// This should *NOT* produce a warning because the empty line is inside a block comment +#[crate_type = "lib"] +/* + +*/ +pub struct S; + +// This should *NOT* produce a warning +#[crate_type = "lib"] +/* test */ +pub struct T; + +// This should *NOT* produce a warning +// See https://github.com/rust-lang/rust-clippy/issues/5567 +#[fake_async_trait] +pub trait Bazz { + fn foo() -> Vec { + let _i = ""; + + + + vec![] + } +} + +#[derive(Clone, Copy)] +#[dummy(string = "first line + +second line +")] +pub struct Args; + +fn main() {} diff --git a/tests/ui/empty_line_after_doc_comments.stderr b/tests/ui/empty_line_after_doc_comments.stderr new file mode 100644 index 000000000..2ca1b5167 --- /dev/null +++ b/tests/ui/empty_line_after_doc_comments.stderr @@ -0,0 +1,36 @@ +error: found an empty line after a doc comment. Perhaps you need to use `//!` to make a comment on a module, remove the empty line, or make a regular comment with `//`? + --> $DIR/empty_line_after_doc_comments.rs:18:1 + | +LL | / /// This should produce a warning +LL | | +LL | | fn with_doc_and_newline() { assert!(true)} + | |_ + | + = note: `-D clippy::empty-line-after-doc-comments` implied by `-D warnings` + +error: found an empty line after a doc comment. Perhaps you need to use `//!` to make a comment on a module, remove the empty line, or make a regular comment with `//`? + --> $DIR/empty_line_after_doc_comments.rs:68:1 + | +LL | / /// This doc comment should produce a warning +LL | | +LL | | /** This is also a doc comment and should produce a warning +LL | | */ +... | +LL | | #[allow(missing_docs)] +LL | | fn three_attributes() { assert!(true) } + | |_ + +error: found an empty line after a doc comment. Perhaps you need to use `//!` to make a comment on a module, remove the empty line, or make a regular comment with `//`? + --> $DIR/empty_line_after_doc_comments.rs:70:1 + | +LL | / /** This is also a doc comment and should produce a warning +LL | | */ +LL | | +LL | | // This should *NOT* produce a warning +... | +LL | | #[allow(missing_docs)] +LL | | fn three_attributes() { assert!(true) } + | |_ + +error: aborting due to 3 previous errors + diff --git a/tests/ui/float_arithmetic.rs b/tests/ui/float_arithmetic.rs index 60fa7569e..a928c35e8 100644 --- a/tests/ui/float_arithmetic.rs +++ b/tests/ui/float_arithmetic.rs @@ -1,4 +1,4 @@ -#![warn(clippy::integer_arithmetic, clippy::float_arithmetic)] +#![warn(clippy::arithmetic_side_effects, clippy::float_arithmetic)] #![allow( unused, clippy::shadow_reuse, diff --git a/tests/ui/integer_arithmetic.rs b/tests/ui/integer_arithmetic.rs deleted file mode 100644 index ab9b6094c..000000000 --- a/tests/ui/integer_arithmetic.rs +++ /dev/null @@ -1,109 +0,0 @@ -//@aux-build:proc_macro_derive.rs - -#![warn(clippy::integer_arithmetic, clippy::float_arithmetic)] -#![allow(clippy::no_effect, clippy::unnecessary_operation, clippy::op_ref)] - -extern crate proc_macro_derive; - -#[derive(proc_macro_derive::ShadowDerive)] -pub struct Nothing; - -#[rustfmt::skip] -fn main() { - let mut i = 1i32; - let mut var1 = 13i32; - let mut var2 = -1i32; - 1 + i; - i * 2; - 1 % - i / 2; // no error, this is part of the expression in the preceding line - i - 2 + 2 - i; - -i; - i >> 1; - i << 1; - - // no error, overflows are checked by `overflowing_literals` - -1; - -(-1); - - i & 1; // no wrapping - i | 1; - i ^ 1; - - i += 1; - i -= 1; - i *= 2; - i /= 2; - i /= 0; - i /= -1; - i /= var1; - i /= var2; - i %= 2; - i %= 0; - i %= -1; - i %= var1; - i %= var2; - i <<= 3; - i >>= 2; - - // no errors - i |= 1; - i &= 1; - i ^= i; - - // No errors for the following items because they are constant expressions - enum Foo { - Bar = -2, - } - struct Baz([i32; 1 + 1]); - union Qux { - field: [i32; 1 + 1], - } - type Alias = [i32; 1 + 1]; - - const FOO: i32 = -2; - static BAR: i32 = -2; - - let _: [i32; 1 + 1] = [0, 0]; - - let _: [i32; 1 + 1] = { - let a: [i32; 1 + 1] = [0, 0]; - a - }; - - trait Trait { - const ASSOC: i32 = 1 + 1; - } - - impl Trait for Foo { - const ASSOC: i32 = { - let _: [i32; 1 + 1]; - fn foo() {} - 1 + 1 - }; - } -} - -// warn on references as well! (#5328) -pub fn int_arith_ref() { - 3 + &1; - &3 + 1; - &3 + &1; -} - -pub fn foo(x: &i32) -> i32 { - let a = 5; - a + x -} - -pub fn bar(x: &i32, y: &i32) -> i32 { - x + y -} - -pub fn baz(x: i32, y: &i32) -> i32 { - x + y -} - -pub fn qux(x: i32, y: i32) -> i32 { - (&x + &y) -} diff --git a/tests/ui/integer_arithmetic.stderr b/tests/ui/integer_arithmetic.stderr deleted file mode 100644 index add3b6b90..000000000 --- a/tests/ui/integer_arithmetic.stderr +++ /dev/null @@ -1,169 +0,0 @@ -error: this operation will panic at runtime - --> $DIR/integer_arithmetic.rs:37:5 - | -LL | i /= 0; - | ^^^^^^ attempt to divide `_` by zero - | - = note: `#[deny(unconditional_panic)]` on by default - -error: this operation will panic at runtime - --> $DIR/integer_arithmetic.rs:42:5 - | -LL | i %= 0; - | ^^^^^^ attempt to calculate the remainder of `_` with a divisor of zero - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:16:5 - | -LL | 1 + i; - | ^^^^^ - | - = note: `-D clippy::integer-arithmetic` implied by `-D warnings` - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:17:5 - | -LL | i * 2; - | ^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:18:5 - | -LL | / 1 % -LL | | i / 2; // no error, this is part of the expression in the preceding line - | |_____^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:20:5 - | -LL | i - 2 + 2 - i; - | ^^^^^^^^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:21:5 - | -LL | -i; - | ^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:22:5 - | -LL | i >> 1; - | ^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:23:5 - | -LL | i << 1; - | ^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:33:5 - | -LL | i += 1; - | ^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:34:5 - | -LL | i -= 1; - | ^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:35:5 - | -LL | i *= 2; - | ^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:38:11 - | -LL | i /= -1; - | ^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:39:5 - | -LL | i /= var1; - | ^^^^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:40:5 - | -LL | i /= var2; - | ^^^^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:43:11 - | -LL | i %= -1; - | ^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:44:5 - | -LL | i %= var1; - | ^^^^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:45:5 - | -LL | i %= var2; - | ^^^^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:46:5 - | -LL | i <<= 3; - | ^^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:47:5 - | -LL | i >>= 2; - | ^^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:89:5 - | -LL | 3 + &1; - | ^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:90:5 - | -LL | &3 + 1; - | ^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:91:5 - | -LL | &3 + &1; - | ^^^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:96:5 - | -LL | a + x - | ^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:100:5 - | -LL | x + y - | ^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:104:5 - | -LL | x + y - | ^^^^^ - -error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:108:5 - | -LL | (&x + &y) - | ^^^^^^^^^ - -error: aborting due to 27 previous errors - diff --git a/tests/ui/let_underscore_untyped.rs b/tests/ui/let_underscore_untyped.rs index 8486137d3..2c313ff35 100644 --- a/tests/ui/let_underscore_untyped.rs +++ b/tests/ui/let_underscore_untyped.rs @@ -1,6 +1,12 @@ +//@aux-build: proc_macros.rs + #![allow(unused)] #![warn(clippy::let_underscore_untyped)] +extern crate proc_macros; +use proc_macros::with_span; + +use clippy_utils::is_from_proc_macro; use std::future::Future; use std::{boxed::Box, fmt::Display}; @@ -32,6 +38,14 @@ fn g() -> impl Fn() { || {} } +with_span!( + span + + fn dont_lint_proc_macro() { + let _ = a(); + } +); + fn main() { let _ = a(); let _ = b(1); @@ -40,6 +54,7 @@ fn main() { let _ = e(); let _ = f(); let _ = g(); + let closure = || {}; _ = a(); _ = b(1); diff --git a/tests/ui/let_underscore_untyped.stderr b/tests/ui/let_underscore_untyped.stderr index 6844cb998..bbf2508af 100644 --- a/tests/ui/let_underscore_untyped.stderr +++ b/tests/ui/let_underscore_untyped.stderr @@ -1,60 +1,60 @@ error: non-binding `let` without a type annotation - --> $DIR/let_underscore_untyped.rs:36:5 + --> $DIR/let_underscore_untyped.rs:50:5 | LL | let _ = a(); | ^^^^^^^^^^^^ | help: consider adding a type annotation - --> $DIR/let_underscore_untyped.rs:36:10 + --> $DIR/let_underscore_untyped.rs:50:10 | LL | let _ = a(); | ^ = note: `-D clippy::let-underscore-untyped` implied by `-D warnings` error: non-binding `let` without a type annotation - --> $DIR/let_underscore_untyped.rs:37:5 + --> $DIR/let_underscore_untyped.rs:51:5 | LL | let _ = b(1); | ^^^^^^^^^^^^^ | help: consider adding a type annotation - --> $DIR/let_underscore_untyped.rs:37:10 + --> $DIR/let_underscore_untyped.rs:51:10 | LL | let _ = b(1); | ^ error: non-binding `let` without a type annotation - --> $DIR/let_underscore_untyped.rs:39:5 + --> $DIR/let_underscore_untyped.rs:53:5 | LL | let _ = d(&1); | ^^^^^^^^^^^^^^ | help: consider adding a type annotation - --> $DIR/let_underscore_untyped.rs:39:10 + --> $DIR/let_underscore_untyped.rs:53:10 | LL | let _ = d(&1); | ^ error: non-binding `let` without a type annotation - --> $DIR/let_underscore_untyped.rs:40:5 + --> $DIR/let_underscore_untyped.rs:54:5 | LL | let _ = e(); | ^^^^^^^^^^^^ | help: consider adding a type annotation - --> $DIR/let_underscore_untyped.rs:40:10 + --> $DIR/let_underscore_untyped.rs:54:10 | LL | let _ = e(); | ^ error: non-binding `let` without a type annotation - --> $DIR/let_underscore_untyped.rs:41:5 + --> $DIR/let_underscore_untyped.rs:55:5 | LL | let _ = f(); | ^^^^^^^^^^^^ | help: consider adding a type annotation - --> $DIR/let_underscore_untyped.rs:41:10 + --> $DIR/let_underscore_untyped.rs:55:10 | LL | let _ = f(); | ^ diff --git a/tests/ui/manual_let_else.rs b/tests/ui/manual_let_else.rs index d175597a4..3996d775f 100644 --- a/tests/ui/manual_let_else.rs +++ b/tests/ui/manual_let_else.rs @@ -8,6 +8,12 @@ )] #![warn(clippy::manual_let_else)] +enum Variant { + A(usize, usize), + B(usize), + C, +} + fn g() -> Option<()> { None } @@ -135,6 +141,15 @@ fn fire() { }; } create_binding_if_some!(w, g()); + + fn e() -> Variant { + Variant::A(0, 0) + } + + // Should not be renamed + let v = if let Variant::A(a, 0) = e() { a } else { return }; + // Should be renamed + let v = if let Variant::B(b) = e() { b } else { return }; } fn not_fire() { diff --git a/tests/ui/manual_let_else.stderr b/tests/ui/manual_let_else.stderr index 52aac6bc6..f6f56f7b0 100644 --- a/tests/ui/manual_let_else.stderr +++ b/tests/ui/manual_let_else.stderr @@ -1,13 +1,13 @@ error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:18:5 + --> $DIR/manual_let_else.rs:24:5 | LL | let v = if let Some(v_some) = g() { v_some } else { return }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { return };` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { return };` | = note: `-D clippy::manual-let-else` implied by `-D warnings` error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:19:5 + --> $DIR/manual_let_else.rs:25:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -18,13 +18,13 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + return; LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:25:5 + --> $DIR/manual_let_else.rs:31:5 | LL | / let v = if let Some(v) = g() { LL | | // Blocks around the identity should have no impact @@ -45,25 +45,25 @@ LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:38:9 + --> $DIR/manual_let_else.rs:44:9 | LL | let v = if let Some(v_some) = g() { v_some } else { continue }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { continue };` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { continue };` error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:39:9 + --> $DIR/manual_let_else.rs:45:9 | LL | let v = if let Some(v_some) = g() { v_some } else { break }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { break };` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { break };` error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:43:5 + --> $DIR/manual_let_else.rs:49:5 | LL | let v = if let Some(v_some) = g() { v_some } else { panic!() }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { panic!() };` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { panic!() };` error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:46:5 + --> $DIR/manual_let_else.rs:52:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -74,13 +74,13 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + std::process::abort() LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:53:5 + --> $DIR/manual_let_else.rs:59:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -91,13 +91,13 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + if true { return } else { panic!() } LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:60:5 + --> $DIR/manual_let_else.rs:66:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -109,14 +109,14 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + if true {} LL + panic!(); LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:70:5 + --> $DIR/manual_let_else.rs:76:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -129,7 +129,7 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + match () { LL + _ if panic!() => {}, LL + _ => panic!(), @@ -138,13 +138,13 @@ LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:80:5 + --> $DIR/manual_let_else.rs:86:5 | LL | let v = if let Some(v_some) = g() { v_some } else { if panic!() {} }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v_some) = g() else { if panic!() {} };` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { if panic!() {} };` error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:83:5 + --> $DIR/manual_let_else.rs:89:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -157,7 +157,7 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + match panic!() { LL + _ => {}, LL + } @@ -165,7 +165,7 @@ LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:92:5 + --> $DIR/manual_let_else.rs:98:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -178,7 +178,7 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { if true { +LL ~ let Some(v) = g() else { if true { LL + return; LL + } else { LL + panic!("diverge"); @@ -186,7 +186,7 @@ LL + } }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:101:5 + --> $DIR/manual_let_else.rs:107:5 | LL | / let v = if let Some(v_some) = g() { LL | | v_some @@ -199,7 +199,7 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g() else { +LL ~ let Some(v) = g() else { LL + match (g(), g()) { LL + (Some(_), None) => return, LL + (None, Some(_)) => { @@ -215,7 +215,7 @@ LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:118:5 + --> $DIR/manual_let_else.rs:124:5 | LL | / let (v, w) = if let Some(v_some) = g().map(|v| (v, 42)) { LL | | v_some @@ -226,13 +226,13 @@ LL | | }; | help: consider writing | -LL ~ let Some(v_some) = g().map(|v| (v, 42)) else { +LL ~ let Some((v, w)) = g().map(|v| (v, 42)) else { LL + return; LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:125:5 + --> $DIR/manual_let_else.rs:131:5 | LL | / let v = if let (Some(v_some), w_some) = (g(), 0) { LL | | (w_some, v_some) @@ -249,10 +249,10 @@ LL + }; | error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:134:13 + --> $DIR/manual_let_else.rs:140:13 | LL | let $n = if let Some(v) = $e { v } else { return }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { return };` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some($n) = g() else { return };` ... LL | create_binding_if_some!(w, g()); | ------------------------------- in this macro invocation @@ -260,13 +260,25 @@ LL | create_binding_if_some!(w, g()); = note: this error originates in the macro `create_binding_if_some` (in Nightly builds, run with -Z macro-backtrace for more info) error: this could be rewritten as `let...else` - --> $DIR/manual_let_else.rs:247:5 + --> $DIR/manual_let_else.rs:150:5 + | +LL | let v = if let Variant::A(a, 0) = e() { a } else { return }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Variant::A(a, 0) = e() else { return };` + +error: this could be rewritten as `let...else` + --> $DIR/manual_let_else.rs:152:5 + | +LL | let v = if let Variant::B(b) = e() { b } else { return }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Variant::B(v) = e() else { return };` + +error: this could be rewritten as `let...else` + --> $DIR/manual_let_else.rs:262:5 | LL | / let _ = match ff { LL | | Some(value) => value, LL | | _ => macro_call!(), LL | | }; - | |______^ help: consider writing: `let Some(value) = ff else { macro_call!() };` + | |______^ help: consider writing: `let Some(_) = ff else { macro_call!() };` -error: aborting due to 18 previous errors +error: aborting due to 20 previous errors diff --git a/tests/ui/manual_let_else_match.stderr b/tests/ui/manual_let_else_match.stderr index 7abaa0b85..bacc14dc9 100644 --- a/tests/ui/manual_let_else_match.stderr +++ b/tests/ui/manual_let_else_match.stderr @@ -5,7 +5,7 @@ LL | / let v = match g() { LL | | Some(v_some) => v_some, LL | | None => return, LL | | }; - | |______^ help: consider writing: `let Some(v_some) = g() else { return };` + | |______^ help: consider writing: `let Some(v) = g() else { return };` | = note: `-D clippy::manual-let-else` implied by `-D warnings` @@ -16,7 +16,7 @@ LL | / let v = match g() { LL | | Some(v_some) => v_some, LL | | _ => return, LL | | }; - | |______^ help: consider writing: `let Some(v_some) = g() else { return };` + | |______^ help: consider writing: `let Some(v) = g() else { return };` error: this could be rewritten as `let...else` --> $DIR/manual_let_else_match.rs:44:9 diff --git a/tests/ui/manual_next_back.fixed b/tests/ui/manual_next_back.fixed new file mode 100644 index 000000000..e8a47063a --- /dev/null +++ b/tests/ui/manual_next_back.fixed @@ -0,0 +1,36 @@ +//@run-rustfix + +#![allow(unused)] +#![warn(clippy::manual_next_back)] + +struct FakeIter(std::ops::Range); + +impl FakeIter { + fn rev(self) -> Self { + self + } + + fn next(&self) {} +} + +impl DoubleEndedIterator for FakeIter { + fn next_back(&mut self) -> Option { + self.0.next_back() + } +} + +impl Iterator for FakeIter { + type Item = i32; + fn next(&mut self) -> Option { + self.0.next() + } +} + +fn main() { + // should not lint + FakeIter(0..10).rev().next(); + + // should lint + let _ = (0..10).next_back().unwrap(); + let _ = "something".bytes().next_back(); +} diff --git a/tests/ui/manual_next_back.rs b/tests/ui/manual_next_back.rs new file mode 100644 index 000000000..9ec892422 --- /dev/null +++ b/tests/ui/manual_next_back.rs @@ -0,0 +1,36 @@ +//@run-rustfix + +#![allow(unused)] +#![warn(clippy::manual_next_back)] + +struct FakeIter(std::ops::Range); + +impl FakeIter { + fn rev(self) -> Self { + self + } + + fn next(&self) {} +} + +impl DoubleEndedIterator for FakeIter { + fn next_back(&mut self) -> Option { + self.0.next_back() + } +} + +impl Iterator for FakeIter { + type Item = i32; + fn next(&mut self) -> Option { + self.0.next() + } +} + +fn main() { + // should not lint + FakeIter(0..10).rev().next(); + + // should lint + let _ = (0..10).rev().next().unwrap(); + let _ = "something".bytes().rev().next(); +} diff --git a/tests/ui/manual_next_back.stderr b/tests/ui/manual_next_back.stderr new file mode 100644 index 000000000..94ccaa9e4 --- /dev/null +++ b/tests/ui/manual_next_back.stderr @@ -0,0 +1,16 @@ +error: manual backwards iteration + --> $DIR/manual_next_back.rs:34:20 + | +LL | let _ = (0..10).rev().next().unwrap(); + | ^^^^^^^^^^^^^ help: use: `.next_back()` + | + = note: `-D clippy::manual-next-back` implied by `-D warnings` + +error: manual backwards iteration + --> $DIR/manual_next_back.rs:35:32 + | +LL | let _ = "something".bytes().rev().next(); + | ^^^^^^^^^^^^^ help: use: `.next_back()` + +error: aborting due to 2 previous errors + diff --git a/tests/ui/match_expr_like_matches_macro.fixed b/tests/ui/match_expr_like_matches_macro.fixed index 7215660da..60f590661 100644 --- a/tests/ui/match_expr_like_matches_macro.fixed +++ b/tests/ui/match_expr_like_matches_macro.fixed @@ -15,7 +15,7 @@ fn main() { let _y = matches!(x, Some(0)); // Lint - let _w = matches!(x, Some(_)); + let _w = x.is_some(); // Turn into is_none let _z = x.is_none(); diff --git a/tests/ui/match_expr_like_matches_macro.stderr b/tests/ui/match_expr_like_matches_macro.stderr index 46f67ef49..b72fe10b7 100644 --- a/tests/ui/match_expr_like_matches_macro.stderr +++ b/tests/ui/match_expr_like_matches_macro.stderr @@ -10,7 +10,7 @@ LL | | }; | = note: `-D clippy::match-like-matches-macro` implied by `-D warnings` -error: match expression looks like `matches!` macro +error: redundant pattern matching, consider using `is_some()` --> $DIR/match_expr_like_matches_macro.rs:21:14 | LL | let _w = match x { @@ -18,7 +18,9 @@ LL | let _w = match x { LL | | Some(_) => true, LL | | _ => false, LL | | }; - | |_____^ help: try this: `matches!(x, Some(_))` + | |_____^ help: try this: `x.is_some()` + | + = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings` error: redundant pattern matching, consider using `is_none()` --> $DIR/match_expr_like_matches_macro.rs:27:14 @@ -29,8 +31,6 @@ LL | | Some(_) => false, LL | | None => true, LL | | }; | |_____^ help: try this: `x.is_none()` - | - = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings` error: match expression looks like `matches!` macro --> $DIR/match_expr_like_matches_macro.rs:33:15 diff --git a/tests/ui/match_same_arms.rs b/tests/ui/match_same_arms.rs index 0b9342c9c..3914b4546 100644 --- a/tests/ui/match_same_arms.rs +++ b/tests/ui/match_same_arms.rs @@ -53,4 +53,84 @@ mod issue4244 { } } -fn main() {} +macro_rules! m { + (foo) => {}; + (bar) => {}; +} +macro_rules! foo { + () => { + 1 + }; +} +macro_rules! bar { + () => { + 1 + }; +} + +fn main() { + let x = 0; + let _ = match 0 { + 0 => { + m!(foo); + x + }, + 1 => { + m!(bar); + x + }, + _ => 1, + }; + + let _ = match 0 { + 0 => { + m!(foo); + 0 + }, + 1 => { + m!(bar); + 0 + }, + _ => 1, + }; + + let _ = match 0 { + 0 => { + let mut x = 0; + #[cfg(not_enabled)] + { + x = 5; + } + #[cfg(not(not_enabled))] + { + x = 6; + } + x + }, + 1 => { + let mut x = 0; + #[cfg(also_not_enabled)] + { + x = 5; + } + #[cfg(not(also_not_enabled))] + { + x = 6; + } + x + }, + _ => 0, + }; + + let _ = match 0 { + 0 => foo!(), + 1 => bar!(), + _ => 1, + }; + + let _ = match 0 { + 0 => cfg!(not_enabled), + 1 => cfg!(also_not_enabled), + _ => false, + }; +} diff --git a/tests/ui/match_same_arms2.rs b/tests/ui/match_same_arms2.rs index 82b2c433d..60b2975be 100644 --- a/tests/ui/match_same_arms2.rs +++ b/tests/ui/match_same_arms2.rs @@ -239,4 +239,10 @@ fn main() { 3 => core::convert::identity::(todo!()), _ => 5, }; + + let _ = match 0 { + 0 => cfg!(not_enable), + 1 => cfg!(not_enable), + _ => false, + }; } diff --git a/tests/ui/match_same_arms2.stderr b/tests/ui/match_same_arms2.stderr index 06cd43000..8fb461bd2 100644 --- a/tests/ui/match_same_arms2.stderr +++ b/tests/ui/match_same_arms2.stderr @@ -192,5 +192,20 @@ note: other arm here LL | Some(Bar { x: 0, y: 5, .. }) => 1, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 12 previous errors +error: this match arm has an identical body to another arm + --> $DIR/match_same_arms2.rs:245:9 + | +LL | 1 => cfg!(not_enable), + | -^^^^^^^^^^^^^^^^^^^^ + | | + | help: try merging the arm patterns: `1 | 0` + | + = help: or try changing either arm body +note: other arm here + --> $DIR/match_same_arms2.rs:244:9 + | +LL | 0 => cfg!(not_enable), + | ^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 13 previous errors diff --git a/tests/ui/needless_bool/fixable.fixed b/tests/ui/needless_bool/fixable.fixed index f860852e7..bf1911881 100644 --- a/tests/ui/needless_bool/fixable.fixed +++ b/tests/ui/needless_bool/fixable.fixed @@ -63,6 +63,13 @@ fn main() { needless_bool2(x); needless_bool3(x); needless_bool_condition(); + + if a == b { + true + } else { + // Do not lint as this comment might be important + false + }; } fn bool_ret3(x: bool) -> bool { diff --git a/tests/ui/needless_bool/fixable.rs b/tests/ui/needless_bool/fixable.rs index 6680dab5b..a6c465d4f 100644 --- a/tests/ui/needless_bool/fixable.rs +++ b/tests/ui/needless_bool/fixable.rs @@ -99,6 +99,13 @@ fn main() { needless_bool2(x); needless_bool3(x); needless_bool_condition(); + + if a == b { + true + } else { + // Do not lint as this comment might be important + false + }; } fn bool_ret3(x: bool) -> bool { diff --git a/tests/ui/needless_bool/fixable.stderr b/tests/ui/needless_bool/fixable.stderr index d2c48376f..fa906374f 100644 --- a/tests/ui/needless_bool/fixable.stderr +++ b/tests/ui/needless_bool/fixable.stderr @@ -91,7 +91,7 @@ LL | | }; | |_____^ help: you can reduce it to: `a < b` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:105:5 + --> $DIR/fixable.rs:112:5 | LL | / if x { LL | | return true; @@ -101,7 +101,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return x` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:113:5 + --> $DIR/fixable.rs:120:5 | LL | / if x { LL | | return false; @@ -111,7 +111,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return !x` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:121:5 + --> $DIR/fixable.rs:128:5 | LL | / if x && y { LL | | return true; @@ -121,7 +121,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return x && y` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:129:5 + --> $DIR/fixable.rs:136:5 | LL | / if x && y { LL | | return false; @@ -131,7 +131,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return !(x && y)` error: equality checks against true are unnecessary - --> $DIR/fixable.rs:137:8 + --> $DIR/fixable.rs:144:8 | LL | if x == true {}; | ^^^^^^^^^ help: try simplifying it as shown: `x` @@ -139,25 +139,25 @@ LL | if x == true {}; = note: `-D clippy::bool-comparison` implied by `-D warnings` error: equality checks against false can be replaced by a negation - --> $DIR/fixable.rs:141:8 + --> $DIR/fixable.rs:148:8 | LL | if x == false {}; | ^^^^^^^^^^ help: try simplifying it as shown: `!x` error: equality checks against true are unnecessary - --> $DIR/fixable.rs:151:8 + --> $DIR/fixable.rs:158:8 | LL | if x == true {}; | ^^^^^^^^^ help: try simplifying it as shown: `x` error: equality checks against false can be replaced by a negation - --> $DIR/fixable.rs:152:8 + --> $DIR/fixable.rs:159:8 | LL | if x == false {}; | ^^^^^^^^^^ help: try simplifying it as shown: `!x` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:161:12 + --> $DIR/fixable.rs:168:12 | LL | } else if returns_bool() { | ____________^ @@ -168,7 +168,7 @@ LL | | }; | |_____^ help: you can reduce it to: `{ !returns_bool() }` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:174:5 + --> $DIR/fixable.rs:181:5 | LL | / if unsafe { no(4) } & 1 != 0 { LL | | true @@ -178,13 +178,13 @@ LL | | }; | |_____^ help: you can reduce it to: `(unsafe { no(4) } & 1 != 0)` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:179:30 + --> $DIR/fixable.rs:186:30 | LL | let _brackets_unneeded = if unsafe { no(4) } & 1 != 0 { true } else { false }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `unsafe { no(4) } & 1 != 0` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:182:9 + --> $DIR/fixable.rs:189:9 | LL | if unsafe { no(4) } & 1 != 0 { true } else { false } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `(unsafe { no(4) } & 1 != 0)` diff --git a/tests/ui/needless_collect.fixed b/tests/ui/needless_collect.fixed index 024c22de2..b7e80af50 100644 --- a/tests/ui/needless_collect.fixed +++ b/tests/ui/needless_collect.fixed @@ -62,4 +62,16 @@ fn main() { let _ = sample.iter().next().is_none(); let _ = sample.iter().any(|x| x == &0); + + #[allow(clippy::double_parens)] + { + Vec::::new().extend((0..10)); + foo((0..10)); + bar((0..10).collect::>(), (0..10)); + baz((0..10), (), ('a'..='z')) + } } + +fn foo(_: impl IntoIterator) {} +fn bar>(_: Vec, _: I) {} +fn baz>(_: I, _: (), _: impl IntoIterator) {} diff --git a/tests/ui/needless_collect.rs b/tests/ui/needless_collect.rs index 7ed7babec..680b6fa5b 100644 --- a/tests/ui/needless_collect.rs +++ b/tests/ui/needless_collect.rs @@ -62,4 +62,16 @@ fn main() { let _ = sample.iter().collect::>().is_empty(); let _ = sample.iter().collect::>().contains(&&0); + + #[allow(clippy::double_parens)] + { + Vec::::new().extend((0..10).collect::>()); + foo((0..10).collect::>()); + bar((0..10).collect::>(), (0..10).collect::>()); + baz((0..10), (), ('a'..='z').collect::>()) + } } + +fn foo(_: impl IntoIterator) {} +fn bar>(_: Vec, _: I) {} +fn baz>(_: I, _: (), _: impl IntoIterator) {} diff --git a/tests/ui/needless_collect.stderr b/tests/ui/needless_collect.stderr index 584d2a1d8..ad22a7b05 100644 --- a/tests/ui/needless_collect.stderr +++ b/tests/ui/needless_collect.stderr @@ -90,5 +90,29 @@ error: avoid using `collect()` when not needed LL | let _ = sample.iter().collect::>().contains(&&0); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == &0)` -error: aborting due to 15 previous errors +error: avoid using `collect()` when not needed + --> $DIR/needless_collect.rs:68:40 + | +LL | Vec::::new().extend((0..10).collect::>()); + | ^^^^^^^^^^^^^^^^^^^^ help: remove this call + +error: avoid using `collect()` when not needed + --> $DIR/needless_collect.rs:69:20 + | +LL | foo((0..10).collect::>()); + | ^^^^^^^^^^^^^^^^^^^^ help: remove this call + +error: avoid using `collect()` when not needed + --> $DIR/needless_collect.rs:70:49 + | +LL | bar((0..10).collect::>(), (0..10).collect::>()); + | ^^^^^^^^^^^^^^^^^^^^ help: remove this call + +error: avoid using `collect()` when not needed + --> $DIR/needless_collect.rs:71:37 + | +LL | baz((0..10), (), ('a'..='z').collect::>()) + | ^^^^^^^^^^^^^^^^^^^^ help: remove this call + +error: aborting due to 19 previous errors diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed index ee4e5007d..d49ae5d86 100644 --- a/tests/ui/needless_return.fixed +++ b/tests/ui/needless_return.fixed @@ -231,8 +231,9 @@ fn needless_return_macro() -> String { } fn issue_9361() -> i32 { - #[allow(clippy::integer_arithmetic)] - return 1 + 2; + let n = 1; + #[allow(clippy::arithmetic_side_effects)] + return n + n; } fn issue8336(x: i32) -> bool { diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs index cd999db4f..367638261 100644 --- a/tests/ui/needless_return.rs +++ b/tests/ui/needless_return.rs @@ -239,8 +239,9 @@ fn needless_return_macro() -> String { } fn issue_9361() -> i32 { - #[allow(clippy::integer_arithmetic)] - return 1 + 2; + let n = 1; + #[allow(clippy::arithmetic_side_effects)] + return n + n; } fn issue8336(x: i32) -> bool { diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr index 87d0cd3e1..05f6038cd 100644 --- a/tests/ui/needless_return.stderr +++ b/tests/ui/needless_return.stderr @@ -328,7 +328,7 @@ LL | return format!("Hello {}", "world!"); = help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:249:9 + --> $DIR/needless_return.rs:250:9 | LL | return true; | ^^^^^^^^^^^ @@ -336,7 +336,7 @@ LL | return true; = help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:251:9 + --> $DIR/needless_return.rs:252:9 | LL | return false; | ^^^^^^^^^^^^ @@ -344,7 +344,7 @@ LL | return false; = help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:258:13 + --> $DIR/needless_return.rs:259:13 | LL | return 10; | ^^^^^^^^^ @@ -352,7 +352,7 @@ LL | return 10; = help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:261:13 + --> $DIR/needless_return.rs:262:13 | LL | return 100; | ^^^^^^^^^^ @@ -360,7 +360,7 @@ LL | return 100; = help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:269:9 + --> $DIR/needless_return.rs:270:9 | LL | return 0; | ^^^^^^^^ @@ -368,7 +368,7 @@ LL | return 0; = help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:276:13 + --> $DIR/needless_return.rs:277:13 | LL | return *(x as *const isize); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -376,7 +376,7 @@ LL | return *(x as *const isize); = help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:278:13 + --> $DIR/needless_return.rs:279:13 | LL | return !*(x as *const isize); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -384,7 +384,7 @@ LL | return !*(x as *const isize); = help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:285:20 + --> $DIR/needless_return.rs:286:20 | LL | let _ = 42; | ____________________^ @@ -395,7 +395,7 @@ LL | | return; = help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:292:20 + --> $DIR/needless_return.rs:293:20 | LL | let _ = 42; return; | ^^^^^^^ @@ -403,7 +403,7 @@ LL | let _ = 42; return; = help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:304:9 + --> $DIR/needless_return.rs:305:9 | LL | return Ok(format!("ok!")); | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -411,7 +411,7 @@ LL | return Ok(format!("ok!")); = help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:306:9 + --> $DIR/needless_return.rs:307:9 | LL | return Err(format!("err!")); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -419,7 +419,7 @@ LL | return Err(format!("err!")); = help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:312:9 + --> $DIR/needless_return.rs:313:9 | LL | return if true { 1 } else { 2 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -427,7 +427,7 @@ LL | return if true { 1 } else { 2 }; = help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:316:9 + --> $DIR/needless_return.rs:317:9 | LL | return if b1 { 0 } else { 1 } | if b2 { 2 } else { 3 } | if b3 { 4 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/non_minimal_cfg.fixed b/tests/ui/non_minimal_cfg.fixed new file mode 100644 index 000000000..430caafb3 --- /dev/null +++ b/tests/ui/non_minimal_cfg.fixed @@ -0,0 +1,17 @@ +//@run-rustfix + +#![allow(unused)] + +#[cfg(windows)] +fn hermit() {} + +#[cfg(windows)] +fn wasi() {} + +#[cfg(all(unix, not(windows)))] +fn the_end() {} + +#[cfg(any())] +fn any() {} + +fn main() {} diff --git a/tests/ui/non_minimal_cfg.rs b/tests/ui/non_minimal_cfg.rs new file mode 100644 index 000000000..a38ce1c21 --- /dev/null +++ b/tests/ui/non_minimal_cfg.rs @@ -0,0 +1,17 @@ +//@run-rustfix + +#![allow(unused)] + +#[cfg(all(windows))] +fn hermit() {} + +#[cfg(any(windows))] +fn wasi() {} + +#[cfg(all(any(unix), all(not(windows))))] +fn the_end() {} + +#[cfg(any())] +fn any() {} + +fn main() {} diff --git a/tests/ui/non_minimal_cfg.stderr b/tests/ui/non_minimal_cfg.stderr new file mode 100644 index 000000000..cdfd728aa --- /dev/null +++ b/tests/ui/non_minimal_cfg.stderr @@ -0,0 +1,28 @@ +error: unneeded sub `cfg` when there is only one condition + --> $DIR/non_minimal_cfg.rs:5:7 + | +LL | #[cfg(all(windows))] + | ^^^^^^^^^^^^ help: try: `windows` + | + = note: `-D clippy::non-minimal-cfg` implied by `-D warnings` + +error: unneeded sub `cfg` when there is only one condition + --> $DIR/non_minimal_cfg.rs:8:7 + | +LL | #[cfg(any(windows))] + | ^^^^^^^^^^^^ help: try: `windows` + +error: unneeded sub `cfg` when there is only one condition + --> $DIR/non_minimal_cfg.rs:11:11 + | +LL | #[cfg(all(any(unix), all(not(windows))))] + | ^^^^^^^^^ help: try: `unix` + +error: unneeded sub `cfg` when there is only one condition + --> $DIR/non_minimal_cfg.rs:11:22 + | +LL | #[cfg(all(any(unix), all(not(windows))))] + | ^^^^^^^^^^^^^^^^^ help: try: `not(windows)` + +error: aborting due to 4 previous errors + diff --git a/tests/ui/non_minimal_cfg2.rs b/tests/ui/non_minimal_cfg2.rs new file mode 100644 index 000000000..a4c6abce3 --- /dev/null +++ b/tests/ui/non_minimal_cfg2.rs @@ -0,0 +1,6 @@ +#![allow(unused)] + +#[cfg(all())] +fn all() {} + +fn main() {} diff --git a/tests/ui/non_minimal_cfg2.stderr b/tests/ui/non_minimal_cfg2.stderr new file mode 100644 index 000000000..2a9a36fbc --- /dev/null +++ b/tests/ui/non_minimal_cfg2.stderr @@ -0,0 +1,10 @@ +error: unneeded sub `cfg` when there is no condition + --> $DIR/non_minimal_cfg2.rs:3:7 + | +LL | #[cfg(all())] + | ^^^^^ + | + = note: `-D clippy::non-minimal-cfg` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/tests/ui/option_if_let_else.fixed b/tests/ui/option_if_let_else.fixed index 57f341e02..2b8ce5477 100644 --- a/tests/ui/option_if_let_else.fixed +++ b/tests/ui/option_if_let_else.fixed @@ -92,6 +92,15 @@ fn pattern_to_vec(pattern: &str) -> Vec { .collect::>() } +// #10335 +fn test_result_impure_else(variable: Result) { + variable.map_or_else(|_| { + println!("Err"); + }, |binding| { + println!("Ok {binding}"); + }) +} + enum DummyEnum { One(u8), Two, @@ -113,6 +122,7 @@ fn main() { unop_bad(&None, None); let _ = longer_body(None); test_map_or_else(None); + test_result_impure_else(Ok(42)); let _ = negative_tests(None); let _ = impure_else(None); diff --git a/tests/ui/option_if_let_else.rs b/tests/ui/option_if_let_else.rs index 19f9f7045..cfbec8cb2 100644 --- a/tests/ui/option_if_let_else.rs +++ b/tests/ui/option_if_let_else.rs @@ -115,6 +115,15 @@ fn pattern_to_vec(pattern: &str) -> Vec { .collect::>() } +// #10335 +fn test_result_impure_else(variable: Result) { + if let Ok(binding) = variable { + println!("Ok {binding}"); + } else { + println!("Err"); + } +} + enum DummyEnum { One(u8), Two, @@ -136,6 +145,7 @@ fn main() { unop_bad(&None, None); let _ = longer_body(None); test_map_or_else(None); + test_result_impure_else(Ok(42)); let _ = negative_tests(None); let _ = impure_else(None); diff --git a/tests/ui/option_if_let_else.stderr b/tests/ui/option_if_let_else.stderr index f5e4affb6..91d52fc79 100644 --- a/tests/ui/option_if_let_else.stderr +++ b/tests/ui/option_if_let_else.stderr @@ -152,14 +152,33 @@ LL | | vec![s.to_string()] LL | | } | |_____________^ help: try: `s.find('.').map_or_else(|| vec![s.to_string()], |idx| vec![s[..idx].to_string(), s[idx..].to_string()])` +error: use Option::map_or_else instead of an if let/else + --> $DIR/option_if_let_else.rs:120:5 + | +LL | / if let Ok(binding) = variable { +LL | | println!("Ok {binding}"); +LL | | } else { +LL | | println!("Err"); +LL | | } + | |_____^ + | +help: try + | +LL ~ variable.map_or_else(|_| { +LL + println!("Err"); +LL + }, |binding| { +LL + println!("Ok {binding}"); +LL + }) + | + error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:133:13 + --> $DIR/option_if_let_else.rs:142:13 | LL | let _ = if let Some(x) = optional { x + 2 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:142:13 + --> $DIR/option_if_let_else.rs:152:13 | LL | let _ = if let Some(x) = Some(0) { | _____________^ @@ -181,13 +200,13 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:170:13 + --> $DIR/option_if_let_else.rs:180:13 | LL | let _ = if let Some(x) = Some(0) { s.len() + x } else { s.len() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(0).map_or(s.len(), |x| s.len() + x)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:174:13 + --> $DIR/option_if_let_else.rs:184:13 | LL | let _ = if let Some(x) = Some(0) { | _____________^ @@ -207,7 +226,7 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:213:13 + --> $DIR/option_if_let_else.rs:223:13 | LL | let _ = match s { | _____________^ @@ -217,7 +236,7 @@ LL | | }; | |_____^ help: try: `s.map_or(1, |string| string.len())` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:217:13 + --> $DIR/option_if_let_else.rs:227:13 | LL | let _ = match Some(10) { | _____________^ @@ -227,7 +246,7 @@ LL | | }; | |_____^ help: try: `Some(10).map_or(5, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:223:13 + --> $DIR/option_if_let_else.rs:233:13 | LL | let _ = match res { | _____________^ @@ -237,7 +256,7 @@ LL | | }; | |_____^ help: try: `res.map_or(1, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:227:13 + --> $DIR/option_if_let_else.rs:237:13 | LL | let _ = match res { | _____________^ @@ -247,10 +266,10 @@ LL | | }; | |_____^ help: try: `res.map_or(1, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:231:13 + --> $DIR/option_if_let_else.rs:241:13 | LL | let _ = if let Ok(a) = res { a + 1 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `res.map_or(5, |a| a + 1)` -error: aborting due to 20 previous errors +error: aborting due to 21 previous errors diff --git a/tests/ui/partialeq_to_none.fixed b/tests/ui/partialeq_to_none.fixed index 81a716bd2..2df87a26d 100644 --- a/tests/ui/partialeq_to_none.fixed +++ b/tests/ui/partialeq_to_none.fixed @@ -1,5 +1,6 @@ //@run-rustfix #![warn(clippy::partialeq_to_none)] +#![allow(clippy::eq_op)] struct Foobar; diff --git a/tests/ui/partialeq_to_none.rs b/tests/ui/partialeq_to_none.rs index f454715fa..df6233b9a 100644 --- a/tests/ui/partialeq_to_none.rs +++ b/tests/ui/partialeq_to_none.rs @@ -1,5 +1,6 @@ //@run-rustfix #![warn(clippy::partialeq_to_none)] +#![allow(clippy::eq_op)] struct Foobar; diff --git a/tests/ui/partialeq_to_none.stderr b/tests/ui/partialeq_to_none.stderr index d06ab7aee..4f84862a2 100644 --- a/tests/ui/partialeq_to_none.stderr +++ b/tests/ui/partialeq_to_none.stderr @@ -1,5 +1,5 @@ error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:14:8 + --> $DIR/partialeq_to_none.rs:15:8 | LL | if f != None { "yay" } else { "nay" } | ^^^^^^^^^ help: use `Option::is_some()` instead: `f.is_some()` @@ -7,55 +7,55 @@ LL | if f != None { "yay" } else { "nay" } = note: `-D clippy::partialeq-to-none` implied by `-D warnings` error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:44:13 + --> $DIR/partialeq_to_none.rs:45:13 | LL | let _ = x == None; | ^^^^^^^^^ help: use `Option::is_none()` instead: `x.is_none()` error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:45:13 + --> $DIR/partialeq_to_none.rs:46:13 | LL | let _ = x != None; | ^^^^^^^^^ help: use `Option::is_some()` instead: `x.is_some()` error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:46:13 + --> $DIR/partialeq_to_none.rs:47:13 | LL | let _ = None == x; | ^^^^^^^^^ help: use `Option::is_none()` instead: `x.is_none()` error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:47:13 + --> $DIR/partialeq_to_none.rs:48:13 | LL | let _ = None != x; | ^^^^^^^^^ help: use `Option::is_some()` instead: `x.is_some()` error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:49:8 + --> $DIR/partialeq_to_none.rs:50:8 | LL | if foobar() == None {} | ^^^^^^^^^^^^^^^^ help: use `Option::is_none()` instead: `foobar().is_none()` error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:51:8 + --> $DIR/partialeq_to_none.rs:52:8 | LL | if bar().ok() != None {} | ^^^^^^^^^^^^^^^^^^ help: use `Option::is_some()` instead: `bar().ok().is_some()` error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:53:13 + --> $DIR/partialeq_to_none.rs:54:13 | LL | let _ = Some(1 + 2) != None; | ^^^^^^^^^^^^^^^^^^^ help: use `Option::is_some()` instead: `Some(1 + 2).is_some()` error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:55:13 + --> $DIR/partialeq_to_none.rs:56:13 | LL | let _ = { Some(0) } == None; | ^^^^^^^^^^^^^^^^^^^ help: use `Option::is_none()` instead: `{ Some(0) }.is_none()` error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:57:13 + --> $DIR/partialeq_to_none.rs:58:13 | LL | let _ = { | _____________^ @@ -77,31 +77,31 @@ LL ~ }.is_some(); | error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:67:13 + --> $DIR/partialeq_to_none.rs:68:13 | LL | let _ = optref() == &&None; | ^^^^^^^^^^^^^^^^^^ help: use `Option::is_none()` instead: `optref().is_none()` error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:68:13 + --> $DIR/partialeq_to_none.rs:69:13 | LL | let _ = &&None != optref(); | ^^^^^^^^^^^^^^^^^^ help: use `Option::is_some()` instead: `optref().is_some()` error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:69:13 + --> $DIR/partialeq_to_none.rs:70:13 | LL | let _ = **optref() == None; | ^^^^^^^^^^^^^^^^^^ help: use `Option::is_none()` instead: `optref().is_none()` error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:70:13 + --> $DIR/partialeq_to_none.rs:71:13 | LL | let _ = &None != *optref(); | ^^^^^^^^^^^^^^^^^^ help: use `Option::is_some()` instead: `optref().is_some()` error: binary comparison to literal `Option::None` - --> $DIR/partialeq_to_none.rs:73:13 + --> $DIR/partialeq_to_none.rs:74:13 | LL | let _ = None != *x; | ^^^^^^^^^^ help: use `Option::is_some()` instead: `(*x).is_some()` diff --git a/tests/ui/redundant_pattern_matching_option.fixed b/tests/ui/redundant_pattern_matching_option.fixed index d62f7d26a..accdf1da9 100644 --- a/tests/ui/redundant_pattern_matching_option.fixed +++ b/tests/ui/redundant_pattern_matching_option.fixed @@ -46,6 +46,7 @@ fn main() { let _ = if opt.is_some() { true } else { false }; issue6067(); + issue10726(); let _ = if gen_opt().is_some() { 1 @@ -88,3 +89,21 @@ fn issue7921() { if (&None::<()>).is_none() {} if (&None::<()>).is_none() {} } + +fn issue10726() { + let x = Some(42); + + x.is_some(); + + x.is_none(); + + x.is_none(); + + x.is_some(); + + // Don't lint + match x { + Some(21) => true, + _ => false, + }; +} diff --git a/tests/ui/redundant_pattern_matching_option.rs b/tests/ui/redundant_pattern_matching_option.rs index d64294265..ec684bdf7 100644 --- a/tests/ui/redundant_pattern_matching_option.rs +++ b/tests/ui/redundant_pattern_matching_option.rs @@ -55,6 +55,7 @@ fn main() { let _ = if let Some(_) = opt { true } else { false }; issue6067(); + issue10726(); let _ = if let Some(_) = gen_opt() { 1 @@ -103,3 +104,33 @@ fn issue7921() { if let None = *(&None::<()>) {} if let None = *&None::<()> {} } + +fn issue10726() { + let x = Some(42); + + match x { + Some(_) => true, + _ => false, + }; + + match x { + None => true, + _ => false, + }; + + match x { + Some(_) => false, + _ => true, + }; + + match x { + None => false, + _ => true, + }; + + // Don't lint + match x { + Some(21) => true, + _ => false, + }; +} diff --git a/tests/ui/redundant_pattern_matching_option.stderr b/tests/ui/redundant_pattern_matching_option.stderr index 7c5a047e4..a69eb3905 100644 --- a/tests/ui/redundant_pattern_matching_option.stderr +++ b/tests/ui/redundant_pattern_matching_option.stderr @@ -77,49 +77,49 @@ LL | let _ = if let Some(_) = opt { true } else { false }; | -------^^^^^^^------ help: try this: `if opt.is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching_option.rs:59:20 + --> $DIR/redundant_pattern_matching_option.rs:60:20 | LL | let _ = if let Some(_) = gen_opt() { | -------^^^^^^^------------ help: try this: `if gen_opt().is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching_option.rs:61:19 + --> $DIR/redundant_pattern_matching_option.rs:62:19 | LL | } else if let None = gen_opt() { | -------^^^^------------ help: try this: `if gen_opt().is_none()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching_option.rs:67:12 + --> $DIR/redundant_pattern_matching_option.rs:68:12 | LL | if let Some(..) = gen_opt() {} | -------^^^^^^^^------------ help: try this: `if gen_opt().is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching_option.rs:82:12 + --> $DIR/redundant_pattern_matching_option.rs:83:12 | LL | if let Some(_) = Some(42) {} | -------^^^^^^^----------- help: try this: `if Some(42).is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching_option.rs:84:12 + --> $DIR/redundant_pattern_matching_option.rs:85:12 | LL | if let None = None::<()> {} | -------^^^^------------- help: try this: `if None::<()>.is_none()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching_option.rs:86:15 + --> $DIR/redundant_pattern_matching_option.rs:87:15 | LL | while let Some(_) = Some(42) {} | ----------^^^^^^^----------- help: try this: `while Some(42).is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching_option.rs:88:15 + --> $DIR/redundant_pattern_matching_option.rs:89:15 | LL | while let None = None::<()> {} | ----------^^^^------------- help: try this: `while None::<()>.is_none()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching_option.rs:90:5 + --> $DIR/redundant_pattern_matching_option.rs:91:5 | LL | / match Some(42) { LL | | Some(_) => true, @@ -128,7 +128,7 @@ LL | | }; | |_____^ help: try this: `Some(42).is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching_option.rs:95:5 + --> $DIR/redundant_pattern_matching_option.rs:96:5 | LL | / match None::<()> { LL | | Some(_) => false, @@ -137,16 +137,52 @@ LL | | }; | |_____^ help: try this: `None::<()>.is_none()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching_option.rs:103:12 + --> $DIR/redundant_pattern_matching_option.rs:104:12 | LL | if let None = *(&None::<()>) {} | -------^^^^----------------- help: try this: `if (&None::<()>).is_none()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching_option.rs:104:12 + --> $DIR/redundant_pattern_matching_option.rs:105:12 | LL | if let None = *&None::<()> {} | -------^^^^--------------- help: try this: `if (&None::<()>).is_none()` -error: aborting due to 22 previous errors +error: redundant pattern matching, consider using `is_some()` + --> $DIR/redundant_pattern_matching_option.rs:111:5 + | +LL | / match x { +LL | | Some(_) => true, +LL | | _ => false, +LL | | }; + | |_____^ help: try this: `x.is_some()` + +error: redundant pattern matching, consider using `is_none()` + --> $DIR/redundant_pattern_matching_option.rs:116:5 + | +LL | / match x { +LL | | None => true, +LL | | _ => false, +LL | | }; + | |_____^ help: try this: `x.is_none()` + +error: redundant pattern matching, consider using `is_none()` + --> $DIR/redundant_pattern_matching_option.rs:121:5 + | +LL | / match x { +LL | | Some(_) => false, +LL | | _ => true, +LL | | }; + | |_____^ help: try this: `x.is_none()` + +error: redundant pattern matching, consider using `is_some()` + --> $DIR/redundant_pattern_matching_option.rs:126:5 + | +LL | / match x { +LL | | None => false, +LL | | _ => true, +LL | | }; + | |_____^ help: try this: `x.is_some()` + +error: aborting due to 26 previous errors diff --git a/tests/ui/redundant_pattern_matching_result.fixed b/tests/ui/redundant_pattern_matching_result.fixed index c48d15229..e4032ae44 100644 --- a/tests/ui/redundant_pattern_matching_result.fixed +++ b/tests/ui/redundant_pattern_matching_result.fixed @@ -43,6 +43,7 @@ fn main() { issue5504(); issue6067(); issue6065(); + issue10726(); let _ = if gen_res().is_ok() { 1 @@ -107,3 +108,28 @@ const fn issue6067() { Err::(42).is_err(); } + +fn issue10726() { + // This is optional, but it makes the examples easier + let x: Result = Ok(42); + + x.is_ok(); + + x.is_err(); + + x.is_err(); + + x.is_ok(); + + // Don't lint + match x { + Err(16) => false, + _ => true, + }; + + // Don't lint + match x { + Ok(16) => false, + _ => true, + }; +} diff --git a/tests/ui/redundant_pattern_matching_result.rs b/tests/ui/redundant_pattern_matching_result.rs index 26f37d169..39eb10df8 100644 --- a/tests/ui/redundant_pattern_matching_result.rs +++ b/tests/ui/redundant_pattern_matching_result.rs @@ -55,6 +55,7 @@ fn main() { issue5504(); issue6067(); issue6065(); + issue10726(); let _ = if let Ok(_) = gen_res() { 1 @@ -125,3 +126,40 @@ const fn issue6067() { Err(_) => true, }; } + +fn issue10726() { + // This is optional, but it makes the examples easier + let x: Result = Ok(42); + + match x { + Ok(_) => true, + _ => false, + }; + + match x { + Ok(_) => false, + _ => true, + }; + + match x { + Err(_) => true, + _ => false, + }; + + match x { + Err(_) => false, + _ => true, + }; + + // Don't lint + match x { + Err(16) => false, + _ => true, + }; + + // Don't lint + match x { + Ok(16) => false, + _ => true, + }; +} diff --git a/tests/ui/redundant_pattern_matching_result.stderr b/tests/ui/redundant_pattern_matching_result.stderr index d6a46babb..5893ae4dc 100644 --- a/tests/ui/redundant_pattern_matching_result.stderr +++ b/tests/ui/redundant_pattern_matching_result.stderr @@ -73,67 +73,67 @@ LL | let _ = if let Ok(_) = Ok::(4) { true } else { false }; | -------^^^^^--------------------- help: try this: `if Ok::(4).is_ok()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching_result.rs:59:20 + --> $DIR/redundant_pattern_matching_result.rs:60:20 | LL | let _ = if let Ok(_) = gen_res() { | -------^^^^^------------ help: try this: `if gen_res().is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching_result.rs:61:19 + --> $DIR/redundant_pattern_matching_result.rs:62:19 | LL | } else if let Err(_) = gen_res() { | -------^^^^^^------------ help: try this: `if gen_res().is_err()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching_result.rs:84:19 + --> $DIR/redundant_pattern_matching_result.rs:85:19 | LL | while let Some(_) = r#try!(result_opt()) {} | ----------^^^^^^^----------------------- help: try this: `while r#try!(result_opt()).is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching_result.rs:85:16 + --> $DIR/redundant_pattern_matching_result.rs:86:16 | LL | if let Some(_) = r#try!(result_opt()) {} | -------^^^^^^^----------------------- help: try this: `if r#try!(result_opt()).is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching_result.rs:91:12 + --> $DIR/redundant_pattern_matching_result.rs:92:12 | LL | if let Some(_) = m!() {} | -------^^^^^^^------- help: try this: `if m!().is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching_result.rs:92:15 + --> $DIR/redundant_pattern_matching_result.rs:93:15 | LL | while let Some(_) = m!() {} | ----------^^^^^^^------- help: try this: `while m!().is_some()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching_result.rs:110:12 + --> $DIR/redundant_pattern_matching_result.rs:111:12 | LL | if let Ok(_) = Ok::(42) {} | -------^^^^^--------------------- help: try this: `if Ok::(42).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching_result.rs:112:12 + --> $DIR/redundant_pattern_matching_result.rs:113:12 | LL | if let Err(_) = Err::(42) {} | -------^^^^^^---------------------- help: try this: `if Err::(42).is_err()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching_result.rs:114:15 + --> $DIR/redundant_pattern_matching_result.rs:115:15 | LL | while let Ok(_) = Ok::(10) {} | ----------^^^^^--------------------- help: try this: `while Ok::(10).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching_result.rs:116:15 + --> $DIR/redundant_pattern_matching_result.rs:117:15 | LL | while let Err(_) = Ok::(10) {} | ----------^^^^^^--------------------- help: try this: `while Ok::(10).is_err()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching_result.rs:118:5 + --> $DIR/redundant_pattern_matching_result.rs:119:5 | LL | / match Ok::(42) { LL | | Ok(_) => true, @@ -142,7 +142,7 @@ LL | | }; | |_____^ help: try this: `Ok::(42).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching_result.rs:123:5 + --> $DIR/redundant_pattern_matching_result.rs:124:5 | LL | / match Err::(42) { LL | | Ok(_) => false, @@ -150,5 +150,41 @@ LL | | Err(_) => true, LL | | }; | |_____^ help: try this: `Err::(42).is_err()` -error: aborting due to 22 previous errors +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching_result.rs:134:5 + | +LL | / match x { +LL | | Ok(_) => true, +LL | | _ => false, +LL | | }; + | |_____^ help: try this: `x.is_ok()` + +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching_result.rs:139:5 + | +LL | / match x { +LL | | Ok(_) => false, +LL | | _ => true, +LL | | }; + | |_____^ help: try this: `x.is_err()` + +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching_result.rs:144:5 + | +LL | / match x { +LL | | Err(_) => true, +LL | | _ => false, +LL | | }; + | |_____^ help: try this: `x.is_err()` + +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching_result.rs:149:5 + | +LL | / match x { +LL | | Err(_) => false, +LL | | _ => true, +LL | | }; + | |_____^ help: try this: `x.is_ok()` + +error: aborting due to 26 previous errors diff --git a/tests/ui/ref_patterns.rs b/tests/ui/ref_patterns.rs new file mode 100644 index 000000000..c51e0bc76 --- /dev/null +++ b/tests/ui/ref_patterns.rs @@ -0,0 +1,19 @@ +#![allow(unused)] +#![warn(clippy::ref_patterns)] + +fn use_in_pattern() { + let opt = Some(5); + match opt { + None => {}, + Some(ref opt) => {}, + } +} + +fn use_in_binding() { + let x = 5; + let ref y = x; +} + +fn use_in_parameter(ref x: i32) {} + +fn main() {} diff --git a/tests/ui/ref_patterns.stderr b/tests/ui/ref_patterns.stderr new file mode 100644 index 000000000..aa0077826 --- /dev/null +++ b/tests/ui/ref_patterns.stderr @@ -0,0 +1,27 @@ +error: usage of ref pattern + --> $DIR/ref_patterns.rs:8:14 + | +LL | Some(ref opt) => {}, + | ^^^^^^^ + | + = help: consider using `&` for clarity instead + = note: `-D clippy::ref-patterns` implied by `-D warnings` + +error: usage of ref pattern + --> $DIR/ref_patterns.rs:14:9 + | +LL | let ref y = x; + | ^^^^^ + | + = help: consider using `&` for clarity instead + +error: usage of ref pattern + --> $DIR/ref_patterns.rs:17:21 + | +LL | fn use_in_parameter(ref x: i32) {} + | ^^^^^ + | + = help: consider using `&` for clarity instead + +error: aborting due to 3 previous errors + diff --git a/tests/ui/regex.rs b/tests/ui/regex.rs index ab8ac97a0..a5f79b139 100644 --- a/tests/ui/regex.rs +++ b/tests/ui/regex.rs @@ -34,8 +34,10 @@ fn syntax_error() { let set_error = RegexSet::new(&[OPENING_PAREN, r"[a-z]+\.(com|org|net)"]); let bset_error = BRegexSet::new(&[OPENING_PAREN, r"[a-z]+\.(com|org|net)"]); + // These following three cases are considering valid since regex-1.8.0 let raw_string_error = Regex::new(r"[...\/...]"); let raw_string_error = Regex::new(r#"[...\/...]"#); + let _ = Regex::new(r"(?hi)").unwrap(); let escaped_string_span = Regex::new("\\b\\c"); diff --git a/tests/ui/regex.stderr b/tests/ui/regex.stderr index c2440f39e..6b8a772e7 100644 --- a/tests/ui/regex.stderr +++ b/tests/ui/regex.stderr @@ -82,23 +82,11 @@ error: regex parse error: LL | let bset_error = BRegexSet::new(&[OPENING_PAREN, r"[a-z]+/.(com|org|net)"]); | ^^^^^^^^^^^^^ -error: regex syntax error: unrecognized escape sequence - --> $DIR/regex.rs:37:45 - | -LL | let raw_string_error = Regex::new(r"[...//...]"); - | ^^ - -error: regex syntax error: unrecognized escape sequence - --> $DIR/regex.rs:38:46 - | -LL | let raw_string_error = Regex::new(r#"[...//...]"#); - | ^^ - error: regex parse error: /b/c ^^ error: unrecognized escape sequence - --> $DIR/regex.rs:40:42 + --> $DIR/regex.rs:42:42 | LL | let escaped_string_span = Regex::new("/b/c"); | ^^^^^^^^ @@ -106,13 +94,13 @@ LL | let escaped_string_span = Regex::new("/b/c"); = help: consider using a raw string literal: `r".."` error: regex syntax error: duplicate flag - --> $DIR/regex.rs:42:34 + --> $DIR/regex.rs:44:34 | LL | let aux_span = Regex::new("(?ixi)"); | ^ ^ error: trivial regex - --> $DIR/regex.rs:46:33 + --> $DIR/regex.rs:48:33 | LL | let trivial_eq = Regex::new("^foobar$"); | ^^^^^^^^^^ @@ -120,7 +108,7 @@ LL | let trivial_eq = Regex::new("^foobar$"); = help: consider using `==` on `str`s error: trivial regex - --> $DIR/regex.rs:48:48 + --> $DIR/regex.rs:50:48 | LL | let trivial_eq_builder = RegexBuilder::new("^foobar$"); | ^^^^^^^^^^ @@ -128,7 +116,7 @@ LL | let trivial_eq_builder = RegexBuilder::new("^foobar$"); = help: consider using `==` on `str`s error: trivial regex - --> $DIR/regex.rs:50:42 + --> $DIR/regex.rs:52:42 | LL | let trivial_starts_with = Regex::new("^foobar"); | ^^^^^^^^^ @@ -136,7 +124,7 @@ LL | let trivial_starts_with = Regex::new("^foobar"); = help: consider using `str::starts_with` error: trivial regex - --> $DIR/regex.rs:52:40 + --> $DIR/regex.rs:54:40 | LL | let trivial_ends_with = Regex::new("foobar$"); | ^^^^^^^^^ @@ -144,7 +132,7 @@ LL | let trivial_ends_with = Regex::new("foobar$"); = help: consider using `str::ends_with` error: trivial regex - --> $DIR/regex.rs:54:39 + --> $DIR/regex.rs:56:39 | LL | let trivial_contains = Regex::new("foobar"); | ^^^^^^^^ @@ -152,7 +140,7 @@ LL | let trivial_contains = Regex::new("foobar"); = help: consider using `str::contains` error: trivial regex - --> $DIR/regex.rs:56:39 + --> $DIR/regex.rs:58:39 | LL | let trivial_contains = Regex::new(NOT_A_REAL_REGEX); | ^^^^^^^^^^^^^^^^ @@ -160,7 +148,7 @@ LL | let trivial_contains = Regex::new(NOT_A_REAL_REGEX); = help: consider using `str::contains` error: trivial regex - --> $DIR/regex.rs:58:40 + --> $DIR/regex.rs:60:40 | LL | let trivial_backslash = Regex::new("a/.b"); | ^^^^^^^ @@ -168,7 +156,7 @@ LL | let trivial_backslash = Regex::new("a/.b"); = help: consider using `str::contains` error: trivial regex - --> $DIR/regex.rs:61:36 + --> $DIR/regex.rs:63:36 | LL | let trivial_empty = Regex::new(""); | ^^ @@ -176,7 +164,7 @@ LL | let trivial_empty = Regex::new(""); = help: the regex is unlikely to be useful as it is error: trivial regex - --> $DIR/regex.rs:63:36 + --> $DIR/regex.rs:65:36 | LL | let trivial_empty = Regex::new("^"); | ^^^ @@ -184,7 +172,7 @@ LL | let trivial_empty = Regex::new("^"); = help: the regex is unlikely to be useful as it is error: trivial regex - --> $DIR/regex.rs:65:36 + --> $DIR/regex.rs:67:36 | LL | let trivial_empty = Regex::new("^$"); | ^^^^ @@ -192,12 +180,12 @@ LL | let trivial_empty = Regex::new("^$"); = help: consider using `str::is_empty` error: trivial regex - --> $DIR/regex.rs:67:44 + --> $DIR/regex.rs:69:44 | LL | let binary_trivial_empty = BRegex::new("^$"); | ^^^^ | = help: consider using `str::is_empty` -error: aborting due to 25 previous errors +error: aborting due to 23 previous errors diff --git a/tests/ui/rename.fixed b/tests/ui/rename.fixed index 9036f8926..7c2acf43f 100644 --- a/tests/ui/rename.fixed +++ b/tests/ui/rename.fixed @@ -16,6 +16,7 @@ #![allow(clippy::mixed_read_write_in_expression)] #![allow(clippy::useless_conversion)] #![allow(clippy::match_result_ok)] +#![allow(clippy::arithmetic_side_effects)] #![allow(clippy::overly_complex_bool_expr)] #![allow(clippy::new_without_default)] #![allow(clippy::bind_instead_of_map)] @@ -58,6 +59,7 @@ #![warn(clippy::mixed_read_write_in_expression)] #![warn(clippy::useless_conversion)] #![warn(clippy::match_result_ok)] +#![warn(clippy::arithmetic_side_effects)] #![warn(clippy::overly_complex_bool_expr)] #![warn(clippy::new_without_default)] #![warn(clippy::bind_instead_of_map)] diff --git a/tests/ui/rename.rs b/tests/ui/rename.rs index 43cabe810..8d334b0d0 100644 --- a/tests/ui/rename.rs +++ b/tests/ui/rename.rs @@ -16,6 +16,7 @@ #![allow(clippy::mixed_read_write_in_expression)] #![allow(clippy::useless_conversion)] #![allow(clippy::match_result_ok)] +#![allow(clippy::arithmetic_side_effects)] #![allow(clippy::overly_complex_bool_expr)] #![allow(clippy::new_without_default)] #![allow(clippy::bind_instead_of_map)] @@ -58,6 +59,7 @@ #![warn(clippy::eval_order_dependence)] #![warn(clippy::identity_conversion)] #![warn(clippy::if_let_some_result)] +#![warn(clippy::integer_arithmetic)] #![warn(clippy::logic_bug)] #![warn(clippy::new_without_default_derive)] #![warn(clippy::option_and_then_some)] diff --git a/tests/ui/rename.stderr b/tests/ui/rename.stderr index 1ad7cf412..fbf8d3d7e 100644 --- a/tests/ui/rename.stderr +++ b/tests/ui/rename.stderr @@ -1,5 +1,5 @@ error: lint `clippy::almost_complete_letter_range` has been renamed to `clippy::almost_complete_range` - --> $DIR/rename.rs:48:9 + --> $DIR/rename.rs:49:9 | LL | #![warn(clippy::almost_complete_letter_range)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::almost_complete_range` @@ -7,280 +7,286 @@ LL | #![warn(clippy::almost_complete_letter_range)] = note: `-D renamed-and-removed-lints` implied by `-D warnings` error: lint `clippy::blacklisted_name` has been renamed to `clippy::disallowed_names` - --> $DIR/rename.rs:49:9 + --> $DIR/rename.rs:50:9 | LL | #![warn(clippy::blacklisted_name)] | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::disallowed_names` error: lint `clippy::block_in_if_condition_expr` has been renamed to `clippy::blocks_in_if_conditions` - --> $DIR/rename.rs:50:9 + --> $DIR/rename.rs:51:9 | LL | #![warn(clippy::block_in_if_condition_expr)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::blocks_in_if_conditions` error: lint `clippy::block_in_if_condition_stmt` has been renamed to `clippy::blocks_in_if_conditions` - --> $DIR/rename.rs:51:9 + --> $DIR/rename.rs:52:9 | LL | #![warn(clippy::block_in_if_condition_stmt)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::blocks_in_if_conditions` error: lint `clippy::box_vec` has been renamed to `clippy::box_collection` - --> $DIR/rename.rs:52:9 + --> $DIR/rename.rs:53:9 | LL | #![warn(clippy::box_vec)] | ^^^^^^^^^^^^^^^ help: use the new name: `clippy::box_collection` error: lint `clippy::const_static_lifetime` has been renamed to `clippy::redundant_static_lifetimes` - --> $DIR/rename.rs:53:9 + --> $DIR/rename.rs:54:9 | LL | #![warn(clippy::const_static_lifetime)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::redundant_static_lifetimes` error: lint `clippy::cyclomatic_complexity` has been renamed to `clippy::cognitive_complexity` - --> $DIR/rename.rs:54:9 + --> $DIR/rename.rs:55:9 | LL | #![warn(clippy::cyclomatic_complexity)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::cognitive_complexity` error: lint `clippy::derive_hash_xor_eq` has been renamed to `clippy::derived_hash_with_manual_eq` - --> $DIR/rename.rs:55:9 + --> $DIR/rename.rs:56:9 | LL | #![warn(clippy::derive_hash_xor_eq)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::derived_hash_with_manual_eq` error: lint `clippy::disallowed_method` has been renamed to `clippy::disallowed_methods` - --> $DIR/rename.rs:56:9 + --> $DIR/rename.rs:57:9 | LL | #![warn(clippy::disallowed_method)] | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::disallowed_methods` error: lint `clippy::disallowed_type` has been renamed to `clippy::disallowed_types` - --> $DIR/rename.rs:57:9 + --> $DIR/rename.rs:58:9 | LL | #![warn(clippy::disallowed_type)] | ^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::disallowed_types` error: lint `clippy::eval_order_dependence` has been renamed to `clippy::mixed_read_write_in_expression` - --> $DIR/rename.rs:58:9 + --> $DIR/rename.rs:59:9 | LL | #![warn(clippy::eval_order_dependence)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::mixed_read_write_in_expression` error: lint `clippy::identity_conversion` has been renamed to `clippy::useless_conversion` - --> $DIR/rename.rs:59:9 + --> $DIR/rename.rs:60:9 | LL | #![warn(clippy::identity_conversion)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::useless_conversion` error: lint `clippy::if_let_some_result` has been renamed to `clippy::match_result_ok` - --> $DIR/rename.rs:60:9 + --> $DIR/rename.rs:61:9 | LL | #![warn(clippy::if_let_some_result)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::match_result_ok` +error: lint `clippy::integer_arithmetic` has been renamed to `clippy::arithmetic_side_effects` + --> $DIR/rename.rs:62:9 + | +LL | #![warn(clippy::integer_arithmetic)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::arithmetic_side_effects` + error: lint `clippy::logic_bug` has been renamed to `clippy::overly_complex_bool_expr` - --> $DIR/rename.rs:61:9 + --> $DIR/rename.rs:63:9 | LL | #![warn(clippy::logic_bug)] | ^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::overly_complex_bool_expr` error: lint `clippy::new_without_default_derive` has been renamed to `clippy::new_without_default` - --> $DIR/rename.rs:62:9 + --> $DIR/rename.rs:64:9 | LL | #![warn(clippy::new_without_default_derive)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::new_without_default` error: lint `clippy::option_and_then_some` has been renamed to `clippy::bind_instead_of_map` - --> $DIR/rename.rs:63:9 + --> $DIR/rename.rs:65:9 | LL | #![warn(clippy::option_and_then_some)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::bind_instead_of_map` error: lint `clippy::option_expect_used` has been renamed to `clippy::expect_used` - --> $DIR/rename.rs:64:9 + --> $DIR/rename.rs:66:9 | LL | #![warn(clippy::option_expect_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::expect_used` error: lint `clippy::option_map_unwrap_or` has been renamed to `clippy::map_unwrap_or` - --> $DIR/rename.rs:65:9 + --> $DIR/rename.rs:67:9 | LL | #![warn(clippy::option_map_unwrap_or)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::map_unwrap_or` error: lint `clippy::option_map_unwrap_or_else` has been renamed to `clippy::map_unwrap_or` - --> $DIR/rename.rs:66:9 + --> $DIR/rename.rs:68:9 | LL | #![warn(clippy::option_map_unwrap_or_else)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::map_unwrap_or` error: lint `clippy::option_unwrap_used` has been renamed to `clippy::unwrap_used` - --> $DIR/rename.rs:67:9 + --> $DIR/rename.rs:69:9 | LL | #![warn(clippy::option_unwrap_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::unwrap_used` error: lint `clippy::ref_in_deref` has been renamed to `clippy::needless_borrow` - --> $DIR/rename.rs:68:9 + --> $DIR/rename.rs:70:9 | LL | #![warn(clippy::ref_in_deref)] | ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::needless_borrow` error: lint `clippy::result_expect_used` has been renamed to `clippy::expect_used` - --> $DIR/rename.rs:69:9 + --> $DIR/rename.rs:71:9 | LL | #![warn(clippy::result_expect_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::expect_used` error: lint `clippy::result_map_unwrap_or_else` has been renamed to `clippy::map_unwrap_or` - --> $DIR/rename.rs:70:9 + --> $DIR/rename.rs:72:9 | LL | #![warn(clippy::result_map_unwrap_or_else)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::map_unwrap_or` error: lint `clippy::result_unwrap_used` has been renamed to `clippy::unwrap_used` - --> $DIR/rename.rs:71:9 + --> $DIR/rename.rs:73:9 | LL | #![warn(clippy::result_unwrap_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::unwrap_used` error: lint `clippy::single_char_push_str` has been renamed to `clippy::single_char_add_str` - --> $DIR/rename.rs:72:9 + --> $DIR/rename.rs:74:9 | LL | #![warn(clippy::single_char_push_str)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::single_char_add_str` error: lint `clippy::stutter` has been renamed to `clippy::module_name_repetitions` - --> $DIR/rename.rs:73:9 + --> $DIR/rename.rs:75:9 | LL | #![warn(clippy::stutter)] | ^^^^^^^^^^^^^^^ help: use the new name: `clippy::module_name_repetitions` error: lint `clippy::to_string_in_display` has been renamed to `clippy::recursive_format_impl` - --> $DIR/rename.rs:74:9 + --> $DIR/rename.rs:76:9 | LL | #![warn(clippy::to_string_in_display)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::recursive_format_impl` error: lint `clippy::zero_width_space` has been renamed to `clippy::invisible_characters` - --> $DIR/rename.rs:75:9 + --> $DIR/rename.rs:77:9 | LL | #![warn(clippy::zero_width_space)] | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::invisible_characters` error: lint `clippy::clone_double_ref` has been renamed to `suspicious_double_ref_op` - --> $DIR/rename.rs:76:9 + --> $DIR/rename.rs:78:9 | LL | #![warn(clippy::clone_double_ref)] | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `suspicious_double_ref_op` error: lint `clippy::drop_bounds` has been renamed to `drop_bounds` - --> $DIR/rename.rs:77:9 + --> $DIR/rename.rs:79:9 | LL | #![warn(clippy::drop_bounds)] | ^^^^^^^^^^^^^^^^^^^ help: use the new name: `drop_bounds` error: lint `clippy::drop_copy` has been renamed to `drop_copy` - --> $DIR/rename.rs:78:9 + --> $DIR/rename.rs:80:9 | LL | #![warn(clippy::drop_copy)] | ^^^^^^^^^^^^^^^^^ help: use the new name: `drop_copy` error: lint `clippy::drop_ref` has been renamed to `drop_ref` - --> $DIR/rename.rs:79:9 + --> $DIR/rename.rs:81:9 | LL | #![warn(clippy::drop_ref)] | ^^^^^^^^^^^^^^^^ help: use the new name: `drop_ref` error: lint `clippy::for_loop_over_option` has been renamed to `for_loops_over_fallibles` - --> $DIR/rename.rs:80:9 + --> $DIR/rename.rs:82:9 | LL | #![warn(clippy::for_loop_over_option)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles` error: lint `clippy::for_loop_over_result` has been renamed to `for_loops_over_fallibles` - --> $DIR/rename.rs:81:9 + --> $DIR/rename.rs:83:9 | LL | #![warn(clippy::for_loop_over_result)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles` error: lint `clippy::for_loops_over_fallibles` has been renamed to `for_loops_over_fallibles` - --> $DIR/rename.rs:82:9 + --> $DIR/rename.rs:84:9 | LL | #![warn(clippy::for_loops_over_fallibles)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles` error: lint `clippy::forget_copy` has been renamed to `forget_copy` - --> $DIR/rename.rs:83:9 + --> $DIR/rename.rs:85:9 | LL | #![warn(clippy::forget_copy)] | ^^^^^^^^^^^^^^^^^^^ help: use the new name: `forget_copy` error: lint `clippy::forget_ref` has been renamed to `forget_ref` - --> $DIR/rename.rs:84:9 + --> $DIR/rename.rs:86:9 | LL | #![warn(clippy::forget_ref)] | ^^^^^^^^^^^^^^^^^^ help: use the new name: `forget_ref` error: lint `clippy::into_iter_on_array` has been renamed to `array_into_iter` - --> $DIR/rename.rs:85:9 + --> $DIR/rename.rs:87:9 | LL | #![warn(clippy::into_iter_on_array)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `array_into_iter` error: lint `clippy::invalid_atomic_ordering` has been renamed to `invalid_atomic_ordering` - --> $DIR/rename.rs:86:9 + --> $DIR/rename.rs:88:9 | LL | #![warn(clippy::invalid_atomic_ordering)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `invalid_atomic_ordering` error: lint `clippy::invalid_ref` has been renamed to `invalid_value` - --> $DIR/rename.rs:87:9 + --> $DIR/rename.rs:89:9 | LL | #![warn(clippy::invalid_ref)] | ^^^^^^^^^^^^^^^^^^^ help: use the new name: `invalid_value` error: lint `clippy::let_underscore_drop` has been renamed to `let_underscore_drop` - --> $DIR/rename.rs:88:9 + --> $DIR/rename.rs:90:9 | LL | #![warn(clippy::let_underscore_drop)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `let_underscore_drop` error: lint `clippy::mem_discriminant_non_enum` has been renamed to `enum_intrinsics_non_enums` - --> $DIR/rename.rs:89:9 + --> $DIR/rename.rs:91:9 | LL | #![warn(clippy::mem_discriminant_non_enum)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `enum_intrinsics_non_enums` error: lint `clippy::panic_params` has been renamed to `non_fmt_panics` - --> $DIR/rename.rs:90:9 + --> $DIR/rename.rs:92:9 | LL | #![warn(clippy::panic_params)] | ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `non_fmt_panics` error: lint `clippy::positional_named_format_parameters` has been renamed to `named_arguments_used_positionally` - --> $DIR/rename.rs:91:9 + --> $DIR/rename.rs:93:9 | LL | #![warn(clippy::positional_named_format_parameters)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `named_arguments_used_positionally` error: lint `clippy::temporary_cstring_as_ptr` has been renamed to `temporary_cstring_as_ptr` - --> $DIR/rename.rs:92:9 + --> $DIR/rename.rs:94:9 | LL | #![warn(clippy::temporary_cstring_as_ptr)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `temporary_cstring_as_ptr` error: lint `clippy::unknown_clippy_lints` has been renamed to `unknown_lints` - --> $DIR/rename.rs:93:9 + --> $DIR/rename.rs:95:9 | LL | #![warn(clippy::unknown_clippy_lints)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `unknown_lints` error: lint `clippy::unused_label` has been renamed to `unused_labels` - --> $DIR/rename.rs:94:9 + --> $DIR/rename.rs:96:9 | LL | #![warn(clippy::unused_label)] | ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `unused_labels` -error: aborting due to 47 previous errors +error: aborting due to 48 previous errors diff --git a/tests/ui/trait_duplication_in_bounds.fixed b/tests/ui/trait_duplication_in_bounds.fixed index eef8024b1..fdac0e4cb 100644 --- a/tests/ui/trait_duplication_in_bounds.fixed +++ b/tests/ui/trait_duplication_in_bounds.fixed @@ -2,6 +2,8 @@ #![deny(clippy::trait_duplication_in_bounds)] #![allow(unused)] +use std::any::Any; + fn bad_foo(arg0: T, argo1: U) { unimplemented!(); } @@ -109,4 +111,12 @@ fn qualified_path(arg0: T) { unimplemented!(); } +fn good_trait_object(arg0: &(dyn Any + Send)) { + unimplemented!(); +} + +fn bad_trait_object(arg0: &(dyn Any + Send)) { + unimplemented!(); +} + fn main() {} diff --git a/tests/ui/trait_duplication_in_bounds.rs b/tests/ui/trait_duplication_in_bounds.rs index a7a1caf28..a0300da55 100644 --- a/tests/ui/trait_duplication_in_bounds.rs +++ b/tests/ui/trait_duplication_in_bounds.rs @@ -2,6 +2,8 @@ #![deny(clippy::trait_duplication_in_bounds)] #![allow(unused)] +use std::any::Any; + fn bad_foo(arg0: T, argo1: U) { unimplemented!(); } @@ -109,4 +111,12 @@ fn qualified_path(arg0: T) { unimplemented!(); } +fn good_trait_object(arg0: &(dyn Any + Send)) { + unimplemented!(); +} + +fn bad_trait_object(arg0: &(dyn Any + Send + Send)) { + unimplemented!(); +} + fn main() {} diff --git a/tests/ui/trait_duplication_in_bounds.stderr b/tests/ui/trait_duplication_in_bounds.stderr index af800ba78..539b6114c 100644 --- a/tests/ui/trait_duplication_in_bounds.stderr +++ b/tests/ui/trait_duplication_in_bounds.stderr @@ -1,5 +1,5 @@ error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:5:15 + --> $DIR/trait_duplication_in_bounds.rs:7:15 | LL | fn bad_foo(arg0: T, argo1: U) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy` @@ -11,46 +11,52 @@ LL | #![deny(clippy::trait_duplication_in_bounds)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: these where clauses contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:11:8 + --> $DIR/trait_duplication_in_bounds.rs:13:8 | LL | T: Clone + Clone + Clone + Copy, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy` error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:39:26 + --> $DIR/trait_duplication_in_bounds.rs:41:26 | LL | trait BadSelfTraitBound: Clone + Clone + Clone { | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone` error: these where clauses contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:46:15 + --> $DIR/trait_duplication_in_bounds.rs:48:15 | LL | Self: Clone + Clone + Clone; | ^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone` error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:60:24 + --> $DIR/trait_duplication_in_bounds.rs:62:24 | LL | trait BadTraitBound { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy` error: these where clauses contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:67:12 + --> $DIR/trait_duplication_in_bounds.rs:69:12 | LL | T: Clone + Clone + Clone + Copy, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Clone + Copy` error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:100:19 + --> $DIR/trait_duplication_in_bounds.rs:102:19 | LL | fn bad_generic + GenericTrait + GenericTrait>(arg0: T) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `GenericTrait + GenericTrait` error: these bounds contain repeated elements - --> $DIR/trait_duplication_in_bounds.rs:108:22 + --> $DIR/trait_duplication_in_bounds.rs:110:22 | LL | fn qualified_path(arg0: T) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::clone::Clone + foo::Clone` -error: aborting due to 8 previous errors +error: this trait bound is already specified in trait declaration + --> $DIR/trait_duplication_in_bounds.rs:118:33 + | +LL | fn bad_trait_object(arg0: &(dyn Any + Send + Send)) { + | ^^^^^^^^^^^^^^^^^ help: try: `Any + Send` + +error: aborting due to 9 previous errors diff --git a/tests/ui/useless_conversion.fixed b/tests/ui/useless_conversion.fixed index 01eb6c5b0..c16caa38f 100644 --- a/tests/ui/useless_conversion.fixed +++ b/tests/ui/useless_conversion.fixed @@ -33,6 +33,11 @@ fn test_issue_3913() -> Result<(), std::io::Error> { Ok(()) } +fn dont_lint_on_type_alias() { + type A = i32; + _ = A::from(0i32); +} + fn dont_lint_into_iter_on_immutable_local_implementing_iterator_in_expr() { let text = "foo\r\nbar\n\nbaz\n"; let lines = text.lines(); @@ -106,6 +111,7 @@ fn main() { test_questionmark().unwrap(); test_issue_3913().unwrap(); + dont_lint_on_type_alias(); dont_lint_into_iter_on_immutable_local_implementing_iterator_in_expr(); lint_into_iter_on_mutable_local_implementing_iterator_in_expr(); lint_into_iter_on_expr_implementing_iterator(); diff --git a/tests/ui/useless_conversion.rs b/tests/ui/useless_conversion.rs index 34b43a629..c75a2bce4 100644 --- a/tests/ui/useless_conversion.rs +++ b/tests/ui/useless_conversion.rs @@ -33,6 +33,11 @@ fn test_issue_3913() -> Result<(), std::io::Error> { Ok(()) } +fn dont_lint_on_type_alias() { + type A = i32; + _ = A::from(0i32); +} + fn dont_lint_into_iter_on_immutable_local_implementing_iterator_in_expr() { let text = "foo\r\nbar\n\nbaz\n"; let lines = text.lines(); @@ -106,6 +111,7 @@ fn main() { test_questionmark().unwrap(); test_issue_3913().unwrap(); + dont_lint_on_type_alias(); dont_lint_into_iter_on_immutable_local_implementing_iterator_in_expr(); lint_into_iter_on_mutable_local_implementing_iterator_in_expr(); lint_into_iter_on_expr_implementing_iterator(); diff --git a/tests/ui/useless_conversion.stderr b/tests/ui/useless_conversion.stderr index be067c684..4dca3aac5 100644 --- a/tests/ui/useless_conversion.stderr +++ b/tests/ui/useless_conversion.stderr @@ -23,97 +23,97 @@ LL | let _: i32 = 0i32.into(); | ^^^^^^^^^^^ help: consider removing `.into()`: `0i32` error: useless conversion to the same type: `std::str::Lines<'_>` - --> $DIR/useless_conversion.rs:45:22 + --> $DIR/useless_conversion.rs:50:22 | LL | if Some("ok") == lines.into_iter().next() {} | ^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `lines` error: useless conversion to the same type: `std::str::Lines<'_>` - --> $DIR/useless_conversion.rs:50:21 + --> $DIR/useless_conversion.rs:55:21 | LL | let mut lines = text.lines().into_iter(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `text.lines()` error: useless conversion to the same type: `std::str::Lines<'_>` - --> $DIR/useless_conversion.rs:56:22 + --> $DIR/useless_conversion.rs:61:22 | LL | if Some("ok") == text.lines().into_iter().next() {} | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `text.lines()` error: useless conversion to the same type: `std::ops::Range` - --> $DIR/useless_conversion.rs:62:13 + --> $DIR/useless_conversion.rs:67:13 | LL | let _ = NUMBERS.into_iter().next(); | ^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `NUMBERS` error: useless conversion to the same type: `std::ops::Range` - --> $DIR/useless_conversion.rs:67:17 + --> $DIR/useless_conversion.rs:72:17 | LL | let mut n = NUMBERS.into_iter(); | ^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `NUMBERS` error: useless conversion to the same type: `std::string::String` - --> $DIR/useless_conversion.rs:128:21 + --> $DIR/useless_conversion.rs:134:21 | LL | let _: String = "foo".to_string().into(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"foo".to_string()` error: useless conversion to the same type: `std::string::String` - --> $DIR/useless_conversion.rs:129:21 + --> $DIR/useless_conversion.rs:135:21 | LL | let _: String = From::from("foo".to_string()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `From::from()`: `"foo".to_string()` error: useless conversion to the same type: `std::string::String` - --> $DIR/useless_conversion.rs:130:13 + --> $DIR/useless_conversion.rs:136:13 | LL | let _ = String::from("foo".to_string()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `"foo".to_string()` error: useless conversion to the same type: `std::string::String` - --> $DIR/useless_conversion.rs:131:13 + --> $DIR/useless_conversion.rs:137:13 | LL | let _ = String::from(format!("A: {:04}", 123)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `format!("A: {:04}", 123)` error: useless conversion to the same type: `std::str::Lines<'_>` - --> $DIR/useless_conversion.rs:132:13 + --> $DIR/useless_conversion.rs:138:13 | LL | let _ = "".lines().into_iter(); | ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `"".lines()` error: useless conversion to the same type: `std::vec::IntoIter` - --> $DIR/useless_conversion.rs:133:13 + --> $DIR/useless_conversion.rs:139:13 | LL | let _ = vec![1, 2, 3].into_iter().into_iter(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2, 3].into_iter()` error: useless conversion to the same type: `std::string::String` - --> $DIR/useless_conversion.rs:134:21 + --> $DIR/useless_conversion.rs:140:21 | LL | let _: String = format!("Hello {}", "world").into(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `format!("Hello {}", "world")` error: useless conversion to the same type: `i32` - --> $DIR/useless_conversion.rs:139:13 + --> $DIR/useless_conversion.rs:145:13 | LL | let _ = i32::from(a + b) * 3; | ^^^^^^^^^^^^^^^^ help: consider removing `i32::from()`: `(a + b)` error: useless conversion to the same type: `Foo<'a'>` - --> $DIR/useless_conversion.rs:145:23 + --> $DIR/useless_conversion.rs:151:23 | LL | let _: Foo<'a'> = s2.into(); | ^^^^^^^^^ help: consider removing `.into()`: `s2` error: useless conversion to the same type: `Foo<'a'>` - --> $DIR/useless_conversion.rs:147:13 + --> $DIR/useless_conversion.rs:153:13 | LL | let _ = Foo::<'a'>::from(s3); | ^^^^^^^^^^^^^^^^^^^^ help: consider removing `Foo::<'a'>::from()`: `s3` error: useless conversion to the same type: `std::vec::IntoIter>` - --> $DIR/useless_conversion.rs:149:13 + --> $DIR/useless_conversion.rs:155:13 | LL | let _ = vec![s4, s4, s4].into_iter().into_iter(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![s4, s4, s4].into_iter()` diff --git a/tests/ui/wildcard_imports_cfgtest.rs b/tests/ui/wildcard_imports_cfgtest.rs new file mode 100644 index 000000000..203c4e15b --- /dev/null +++ b/tests/ui/wildcard_imports_cfgtest.rs @@ -0,0 +1,19 @@ +//@compile-flags: --test + +#![warn(clippy::wildcard_imports)] +#![allow(unused, clippy::unnecessary_wraps, clippy::let_unit_value)] + +// Test for #10580, the lint should ignore it because of the crate's cfg test flag. + +fn foofoo() {} + +mod outer { + mod inner { + use super::super::*; + fn barbar() { + let _ = foofoo(); + } + } +} + +fn main() {} diff --git a/triagebot.toml b/triagebot.toml index 3f8f6a7b9..c40b71f6c 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -17,9 +17,9 @@ contributing_url = "https://github.com/rust-lang/rust-clippy/blob/master/CONTRIB [assign.owners] "/.github" = ["@flip1995"] +"/book" = ["@flip1995"] "/util/gh-pages" = ["@xFrednet"] "*" = [ - "@flip1995", "@Manishearth", "@llogiq", "@giraffate",