diff options
45 files changed, 918 insertions, 175 deletions
diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index b9921301197..a9d42159c4b 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 93198aabdb5..30a156c925b 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 14f20212add..514706d64c8 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 71d71d10359..f42928c2cd1 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 0bc2f49f5e9..7d25b6a2b79 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 ebf5b58a586..dafa3f3a139 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 @@ -4978,6 +4980,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 6745e15c006..d712d3e6750 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 <!-- REUSE-IgnoreStart --> -Copyright 2014-2022 The Rust Project Developers +Copyright 2014-2023 The Rust Project Developers Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or [https://www.apache.org/licenses/LICENSE-2.0](https://www.apache.org/licenses/LICENSE-2.0)> or the MIT license diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 751c262673b..93d88391a79 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -178,6 +178,52 @@ declare_clippy_lint! { 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. /// /// ### Why is this bad? @@ -604,6 +650,7 @@ impl_lint_pass!(EarlyAttributes => [ DEPRECATED_CFG_ATTR, MISMATCHED_TARGET_OS, EMPTY_LINE_AFTER_OUTER_ATTR, + EMPTY_LINE_AFTER_DOC_COMMENTS, ]); impl EarlyLintPass for EarlyAttributes { @@ -619,10 +666,16 @@ impl EarlyLintPass for EarlyAttributes { 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 +692,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); } } } diff --git a/clippy_lints/src/borrow_deref_ref.rs b/clippy_lints/src/borrow_deref_ref.rs index c4520d00392..814108ed8a7 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/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 79d0f6f3607..a3a6f1746bc 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -48,6 +48,7 @@ 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, @@ -351,6 +352,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, @@ -539,6 +541,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 e529d81a7e9..9bd7a0dc0f3 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/lib.rs b/clippy_lints/src/lib.rs index 3517842a01e..38f4ecc75b6 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 /// @@ -388,7 +392,7 @@ pub fn read_conf(sess: &Session, path: &io::Result<(Option<PathBuf>, Vec<String> conf } -#[derive(Default)] +#[derive(Default)] //~ ERROR no such field struct RegistrationGroups { all: Vec<LintId>, cargo: Vec<LintId>, @@ -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 bba9bb445a7..09b2032e20f 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/methods/manual_next_back.rs b/clippy_lints/src/methods/manual_next_back.rs new file mode 100644 index 00000000000..5f3fec53827 --- /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 06b88e34d24..cb86917464b 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; @@ -3193,6 +3194,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 +3345,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 +3702,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), _ => {}, } } diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 3752b9a946f..303f0125690 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 71281a0b40b..62af42a3961 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/arithmetic_side_effects.rs b/clippy_lints/src/operators/arithmetic_side_effects.rs index f72595987ee..c80e2ff7057 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"]; @@ -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/ref_patterns.rs b/clippy_lints/src/ref_patterns.rs new file mode 100644 index 00000000000..b1530eed1c1 --- /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/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index b5f11b4acae..4ccda15068b 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::<Vec<_>>() + .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/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 5f05d971fce..f6de66bb514 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<ClippyConfiguration> { + pub fn get_configuration_metadata() -> Vec<ClippyConfiguration> { vec![ $( { diff --git a/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/clippy_lints/src/utils/internal_lints/metadata_collector.rs index 3d0d4a52511..7a1cd3effae 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<String>, - 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<ClippyConfiguration> { - 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>, 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<String> = 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 dc647af264c..d3ea7cafa80 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<String>, + 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<ClippyConfiguration> { + 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>, 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<String> = 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<P>(&mut self, predicate: P) -> Option<Vec<usize>> + where + P: FnMut(&Self::Item) -> bool; +} + +impl<I> FindAll for I +where + I: Iterator, +{ + fn find_all<P>(&mut self, mut predicate: P) -> Option<Vec<usize>> + where + P: FnMut(&Self::Item) -> bool, + { + let mut occurences = Vec::<usize>::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/wildcard_imports.rs b/clippy_lints/src/wildcard_imports.rs index 36f910c983f..a9089fba3c5 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/tests/ui/arithmetic_side_effects.rs b/tests/ui/arithmetic_side_effects.rs index ab408bdf261..f95af1017bc 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 165e4bc8272..75526461792 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 66c8d69bef9..e319d365f7e 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 d72de37c69f..1e47cda6796 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/default_constructed_unit_structs.fixed b/tests/ui/default_constructed_unit_structs.fixed index 4c2d1ea48e1..e1012f38bba 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::<usize>; let _: PhantomData<i32> = PhantomData; + let _: PhantomData<i32> = std::marker::PhantomData; let _ = UnitStruct; // should not lint @@ -116,4 +117,21 @@ fn main() { let _ = EmptyStruct::default(); let _ = FakeDefault::default(); let _ = <FakeDefault as Default>::default(); + + macro_rules! in_macro { + ($i:ident) => {{ + let _ = UnitStruct::default(); + let _ = $i::default(); + }}; + } + + in_macro!(UnitStruct); + + macro_rules! struct_from_macro { + () => { + UnitStruct + }; + } + + let _ = <struct_from_macro!()>::default(); } diff --git a/tests/ui/default_constructed_unit_structs.rs b/tests/ui/default_constructed_unit_structs.rs index 850793dd5de..c7b4313dbf0 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::<usize>::default(); let _: PhantomData<i32> = PhantomData::default(); + let _: PhantomData<i32> = std::marker::PhantomData::default(); let _ = UnitStruct::default(); // should not lint @@ -116,4 +117,21 @@ fn main() { let _ = EmptyStruct::default(); let _ = FakeDefault::default(); let _ = <FakeDefault as Default>::default(); + + macro_rules! in_macro { + ($i:ident) => {{ + let _ = UnitStruct::default(); + let _ = $i::default(); + }}; + } + + in_macro!(UnitStruct); + + macro_rules! struct_from_macro { + () => { + UnitStruct + }; + } + + let _ = <struct_from_macro!()>::default(); } diff --git a/tests/ui/default_constructed_unit_structs.stderr b/tests/ui/default_constructed_unit_structs.stderr index 4058943d087..61a32fb10e5 100644 --- a/tests/ui/default_constructed_unit_structs.stderr +++ b/tests/ui/default_constructed_unit_structs.stderr @@ -25,10 +25,16 @@ LL | let _: PhantomData<i32> = 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<i32> = 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 00000000000..e843770f578 --- /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<u8> { + 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 00000000000..2ca1b51679e --- /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/manual_next_back.fixed b/tests/ui/manual_next_back.fixed new file mode 100644 index 00000000000..e8a47063ad6 --- /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<i32>); + +impl FakeIter { + fn rev(self) -> Self { + self + } + + fn next(&self) {} +} + +impl DoubleEndedIterator for FakeIter { + fn next_back(&mut self) -> Option<Self::Item> { + self.0.next_back() + } +} + +impl Iterator for FakeIter { + type Item = i32; + fn next(&mut self) -> Option<Self::Item> { + 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 00000000000..9ec89242241 --- /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<i32>); + +impl FakeIter { + fn rev(self) -> Self { + self + } + + fn next(&self) {} +} + +impl DoubleEndedIterator for FakeIter { + fn next_back(&mut self) -> Option<Self::Item> { + self.0.next_back() + } +} + +impl Iterator for FakeIter { + type Item = i32; + fn next(&mut self) -> Option<Self::Item> { + 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 00000000000..94ccaa9e4cc --- /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/needless_bool/fixable.fixed b/tests/ui/needless_bool/fixable.fixed index f860852e7b7..bf1911881c8 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 6680dab5b6d..a6c465d4fbd 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 d2c48376f76..fa906374fb3 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/ref_patterns.rs b/tests/ui/ref_patterns.rs new file mode 100644 index 00000000000..c51e0bc76ef --- /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 00000000000..aa007782683 --- /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/trait_duplication_in_bounds.fixed b/tests/ui/trait_duplication_in_bounds.fixed index eef8024b131..fdac0e4cb1e 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<T: Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) { unimplemented!(); } @@ -109,4 +111,12 @@ fn qualified_path<T: std::clone::Clone + foo::Clone>(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 a7a1caf2880..a0300da5555 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<T: Clone + Clone + Clone + Copy, U: Clone + Copy>(arg0: T, argo1: U) { unimplemented!(); } @@ -109,4 +111,12 @@ fn qualified_path<T: std::clone::Clone + Clone + foo::Clone>(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 af800ba7888..539b6114ca3 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<T: Clone + Clone + Clone + Copy, U: Clone + Copy>(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<T: Clone + Clone + Clone + Copy, U: Clone + Copy> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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<T: GenericTrait<u64> + GenericTrait<u32> + GenericTrait<u64>>(arg0: T) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `GenericTrait<u64> + GenericTrait<u32>` 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<T: std::clone::Clone + Clone + foo::Clone>(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/wildcard_imports_cfgtest.rs b/tests/ui/wildcard_imports_cfgtest.rs new file mode 100644 index 00000000000..203c4e15b50 --- /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() {} |