From 3596d4498808d44e0923a547fe2f1ec7774fffb9 Mon Sep 17 00:00:00 2001 From: Philipp Krones Date: Sat, 16 Dec 2023 14:12:50 +0100 Subject: [PATCH] Merge commit 'a859e5cc1ce100df22346a1005da30532d04de59' into clippyup --- .github/workflows/clippy_bors.yml | 2 +- CHANGELOG.md | 5 + clippy_dev/src/lint.rs | 10 +- clippy_dev/src/serve.rs | 4 +- clippy_lints/Cargo.toml | 2 +- clippy_lints/src/as_conversions.rs | 9 +- clippy_lints/src/blocks_in_conditions.rs | 137 +++++++ clippy_lints/src/blocks_in_if_conditions.rs | 139 ------- clippy_lints/src/borrow_deref_ref.rs | 4 +- clippy_lints/src/casts/as_ptr_cast_mut.rs | 9 +- clippy_lints/src/casts/ptr_as_ptr.rs | 55 ++- clippy_lints/src/declared_lints.rs | 6 +- clippy_lints/src/doc/markdown.rs | 16 +- clippy_lints/src/explicit_write.rs | 2 +- clippy_lints/src/implied_bounds_in_impls.rs | 7 +- clippy_lints/src/ineffective_open_options.rs | 95 +++++ clippy_lints/src/lib.rs | 13 +- clippy_lints/src/loops/infinite_loop.rs | 125 ++++++ clippy_lints/src/loops/mod.rs | 47 ++- clippy_lints/src/loops/needless_range_loop.rs | 3 +- clippy_lints/src/manual_float_methods.rs | 4 +- clippy_lints/src/manual_string_new.rs | 22 +- clippy_lints/src/methods/mod.rs | 2 +- .../src/methods/unnecessary_lazy_eval.rs | 6 +- .../src/methods/unnecessary_to_owned.rs | 31 +- .../src/missing_asserts_for_indexing.rs | 23 +- .../src/mixed_read_write_in_expression.rs | 4 +- .../src/needless_borrows_for_generic_args.rs | 12 +- clippy_lints/src/needless_if.rs | 2 +- clippy_lints/src/no_effect.rs | 34 +- .../src/operators/arithmetic_side_effects.rs | 25 +- clippy_lints/src/renamed_lints.rs | 5 +- clippy_lints/src/repeat_vec_with_capacity.rs | 114 ++++++ clippy_lints/src/same_name_method.rs | 46 +-- clippy_lints/src/single_call_fn.rs | 2 +- clippy_lints/src/uninhabited_references.rs | 84 ++++ .../src/unnecessary_map_on_constructor.rs | 2 +- .../internal_lints/metadata_collector.rs | 4 +- .../internal_lints/unnecessary_def_path.rs | 6 +- clippy_lints/src/vec.rs | 155 ++++---- clippy_utils/src/ast_utils.rs | 17 +- clippy_utils/src/check_proc_macro.rs | 80 ++-- clippy_utils/src/ty.rs | 55 ++- lintcheck/src/main.rs | 6 +- rust-toolchain | 2 +- src/main.rs | 2 +- tests/headers.rs | 7 +- .../module_style/fail_mod_remap/Cargo.stderr | 2 +- .../module_style/fail_no_mod/Cargo.stderr | 2 +- .../multiple_crate_versions/fail/Cargo.stderr | 2 +- .../wildcard_dependencies/fail/Cargo.stderr | 2 +- .../excessive_nesting/excessive_nesting.rs | 2 +- tests/ui/bind_instead_of_map_multipart.fixed | 2 +- tests/ui/bind_instead_of_map_multipart.rs | 2 +- ...tions.fixed => blocks_in_conditions.fixed} | 26 +- ..._conditions.rs => blocks_in_conditions.rs} | 26 +- ...ons.stderr => blocks_in_conditions.stderr} | 33 +- ...ure.rs => blocks_in_conditions_closure.rs} | 21 +- ...rr => blocks_in_conditions_closure.stderr} | 21 +- tests/ui/doc/doc-fixable.fixed | 3 + tests/ui/doc/doc-fixable.rs | 3 + tests/ui/doc/doc-fixable.stderr | 24 +- tests/ui/doc_unsafe.rs | 2 +- tests/ui/ineffective_open_options.fixed | 41 ++ tests/ui/ineffective_open_options.rs | 41 ++ tests/ui/ineffective_open_options.stderr | 17 + tests/ui/infallible_destructuring_match.fixed | 2 +- tests/ui/infallible_destructuring_match.rs | 2 +- tests/ui/infinite_loops.rs | 366 ++++++++++++++++++ tests/ui/infinite_loops.stderr | 259 +++++++++++++ tests/ui/manual_filter.fixed | 2 +- tests/ui/manual_filter.rs | 2 +- tests/ui/missing_asserts_for_indexing.fixed | 15 + tests/ui/missing_asserts_for_indexing.rs | 15 + tests/ui/missing_asserts_for_indexing.stderr | 54 ++- .../needless_borrows_for_generic_args.fixed | 15 + tests/ui/needless_borrows_for_generic_args.rs | 15 + tests/ui/needless_if.fixed | 2 +- tests/ui/needless_if.rs | 2 +- tests/ui/needless_late_init.fixed | 2 +- tests/ui/needless_late_init.rs | 2 +- tests/ui/needless_pass_by_ref_mut.rs | 13 + tests/ui/needless_pass_by_ref_mut.stderr | 62 ++- tests/ui/no_effect.rs | 31 ++ tests/ui/no_effect.stderr | 58 +-- tests/ui/ptr_as_ptr.fixed | 115 ++++++ tests/ui/ptr_as_ptr.rs | 115 ++++++ tests/ui/ptr_as_ptr.stderr | 146 ++++++- tests/ui/regex.rs | 4 + tests/ui/rename.fixed | 7 +- tests/ui/rename.rs | 3 +- tests/ui/rename.stderr | 122 +++--- tests/ui/repeat_vec_with_capacity.fixed | 38 ++ tests/ui/repeat_vec_with_capacity.rs | 38 ++ tests/ui/repeat_vec_with_capacity.stderr | 40 ++ tests/ui/uninhabited_references.rs | 22 ++ tests/ui/uninhabited_references.stderr | 39 ++ tests/ui/unnecessary_operation.fixed | 19 + tests/ui/unnecessary_operation.rs | 19 + tests/ui/unnecessary_operation.stderr | 38 +- tests/ui/unnecessary_to_owned.fixed | 23 +- tests/ui/unnecessary_to_owned.rs | 23 +- tests/ui/unnecessary_to_owned.stderr | 176 +++++---- tests/ui/unnecessary_unsafety_doc.rs | 2 +- tests/ui/vec.fixed | 34 ++ tests/ui/vec.rs | 34 ++ tests/ui/vec.stderr | 8 +- triagebot.toml | 1 + 108 files changed, 3063 insertions(+), 636 deletions(-) create mode 100644 clippy_lints/src/blocks_in_conditions.rs delete mode 100644 clippy_lints/src/blocks_in_if_conditions.rs create mode 100644 clippy_lints/src/ineffective_open_options.rs create mode 100644 clippy_lints/src/loops/infinite_loop.rs create mode 100644 clippy_lints/src/repeat_vec_with_capacity.rs create mode 100644 clippy_lints/src/uninhabited_references.rs rename tests/ui/{blocks_in_if_conditions.fixed => blocks_in_conditions.fixed} (55%) rename tests/ui/{blocks_in_if_conditions.rs => blocks_in_conditions.rs} (56%) rename tests/ui/{blocks_in_if_conditions.stderr => blocks_in_conditions.stderr} (55%) rename tests/ui/{blocks_in_if_conditions_closure.rs => blocks_in_conditions_closure.rs} (76%) rename tests/ui/{blocks_in_if_conditions_closure.stderr => blocks_in_conditions_closure.stderr} (54%) create mode 100644 tests/ui/ineffective_open_options.fixed create mode 100644 tests/ui/ineffective_open_options.rs create mode 100644 tests/ui/ineffective_open_options.stderr create mode 100644 tests/ui/infinite_loops.rs create mode 100644 tests/ui/infinite_loops.stderr create mode 100644 tests/ui/repeat_vec_with_capacity.fixed create mode 100644 tests/ui/repeat_vec_with_capacity.rs create mode 100644 tests/ui/repeat_vec_with_capacity.stderr create mode 100644 tests/ui/uninhabited_references.rs create mode 100644 tests/ui/uninhabited_references.stderr diff --git a/.github/workflows/clippy_bors.yml b/.github/workflows/clippy_bors.yml index f67233dec..73c255507 100644 --- a/.github/workflows/clippy_bors.yml +++ b/.github/workflows/clippy_bors.yml @@ -206,6 +206,7 @@ jobs: max-parallel: 6 matrix: integration: + - 'matthiaskrgr/clippy_ci_panic_test' - 'rust-lang/cargo' - 'rust-lang/chalk' - 'rust-lang/rustfmt' @@ -220,7 +221,6 @@ jobs: - 'rust-itertools/itertools' - 'rust-lang-nursery/failure' - 'rust-lang/log' - - 'matthiaskrgr/clippy_ci_panic_test' runs-on: ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e9b755ca..70aff7f60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4946,6 +4946,7 @@ Released 2018-09-13 [`blanket_clippy_restriction_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#blanket_clippy_restriction_lints [`block_in_if_condition_expr`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_in_if_condition_expr [`block_in_if_condition_stmt`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_in_if_condition_stmt +[`blocks_in_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_conditions [`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions [`bool_assert_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_assert_comparison [`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison @@ -5145,9 +5146,11 @@ Released 2018-09-13 [`index_refutable_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#index_refutable_slice [`indexing_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing [`ineffective_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_bit_mask +[`ineffective_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_open_options [`inefficient_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_to_string [`infallible_destructuring_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#infallible_destructuring_match [`infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_iter +[`infinite_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_loop [`inherent_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string [`inherent_to_string_shadow_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string_shadow_display [`init_numbered_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#init_numbered_fields @@ -5462,6 +5465,7 @@ Released 2018-09-13 [`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 +[`repeat_vec_with_capacity`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_vec_with_capacity [`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts [`reserve_after_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#reserve_after_initialization [`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs @@ -5582,6 +5586,7 @@ Released 2018-09-13 [`undropped_manually_drops`]: https://rust-lang.github.io/rust-clippy/master/index.html#undropped_manually_drops [`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc [`unimplemented`]: https://rust-lang.github.io/rust-clippy/master/index.html#unimplemented +[`uninhabited_references`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninhabited_references [`uninit_assumed_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_assumed_init [`uninit_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_vec [`uninlined_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args diff --git a/clippy_dev/src/lint.rs b/clippy_dev/src/lint.rs index a19be1bca..906a97278 100644 --- a/clippy_dev/src/lint.rs +++ b/clippy_dev/src/lint.rs @@ -1,6 +1,6 @@ use crate::{cargo_clippy_path, exit_if_err}; -use std::fs; use std::process::{self, Command}; +use std::{env, fs}; pub fn run<'a>(path: &str, args: impl Iterator) { let is_file = match fs::metadata(path) { @@ -13,7 +13,7 @@ pub fn run<'a>(path: &str, args: impl Iterator) { if is_file { exit_if_err( - Command::new("cargo") + Command::new(env::var("CARGO").unwrap_or("cargo".into())) .args(["run", "--bin", "clippy-driver", "--"]) .args(["-L", "./target/debug"]) .args(["-Z", "no-codegen"]) @@ -23,7 +23,11 @@ pub fn run<'a>(path: &str, args: impl Iterator) { .status(), ); } else { - exit_if_err(Command::new("cargo").arg("build").status()); + exit_if_err( + Command::new(env::var("CARGO").unwrap_or("cargo".into())) + .arg("build") + .status(), + ); let status = Command::new(cargo_clippy_path()) .arg("clippy") diff --git a/clippy_dev/src/serve.rs b/clippy_dev/src/serve.rs index 535c25e69..ea925f670 100644 --- a/clippy_dev/src/serve.rs +++ b/clippy_dev/src/serve.rs @@ -2,8 +2,8 @@ use std::ffi::OsStr; use std::num::ParseIntError; use std::path::Path; use std::process::Command; -use std::thread; use std::time::{Duration, SystemTime}; +use std::{env, thread}; /// # Panics /// @@ -16,7 +16,7 @@ pub fn run(port: u16, lint: Option<&String>) -> ! { loop { if mtime("util/gh-pages/lints.json") < mtime("clippy_lints/src") { - Command::new("cargo") + Command::new(env::var("CARGO").unwrap_or("cargo".into())) .arg("collect-metadata") .spawn() .unwrap() diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index a9375214b..ad8b7ded4 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -16,7 +16,7 @@ clippy_utils = { path = "../clippy_utils" } declare_clippy_lint = { path = "../declare_clippy_lint" } itertools = "0.11" quine-mc_cluskey = "0.2" -regex-syntax = "0.7" +regex-syntax = "0.8" serde = { version = "1.0", features = ["derive"] } serde_json = { version = "1.0", optional = true } tempfile = { version = "3.3.0", optional = true } diff --git a/clippy_lints/src/as_conversions.rs b/clippy_lints/src/as_conversions.rs index e052d36f1..e3daf75c3 100644 --- a/clippy_lints/src/as_conversions.rs +++ b/clippy_lints/src/as_conversions.rs @@ -48,11 +48,10 @@ declare_lint_pass!(AsConversions => [AS_CONVERSIONS]); impl<'tcx> LateLintPass<'tcx> for AsConversions { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { - if in_external_macro(cx.sess(), expr.span) || is_from_proc_macro(cx, expr) { - return; - } - - if let ExprKind::Cast(_, _) = expr.kind { + if let ExprKind::Cast(_, _) = expr.kind + && !in_external_macro(cx.sess(), expr.span) + && !is_from_proc_macro(cx, expr) + { span_lint_and_help( cx, AS_CONVERSIONS, diff --git a/clippy_lints/src/blocks_in_conditions.rs b/clippy_lints/src/blocks_in_conditions.rs new file mode 100644 index 000000000..1417e230a --- /dev/null +++ b/clippy_lints/src/blocks_in_conditions.rs @@ -0,0 +1,137 @@ +use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg}; +use clippy_utils::source::snippet_block_with_applicability; +use clippy_utils::ty::implements_trait; +use clippy_utils::visitors::{for_each_expr, Descend}; +use clippy_utils::{get_parent_expr, higher}; +use core::ops::ControlFlow; +use rustc_errors::Applicability; +use rustc_hir::{BlockCheckMode, Expr, ExprKind, MatchSource}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_session::declare_lint_pass; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// Checks for `if` conditions that use blocks containing an + /// expression, statements or conditions that use closures with blocks. + /// + /// ### Why is this bad? + /// Style, using blocks in the condition makes it hard to read. + /// + /// ### Examples + /// ```no_run + /// # fn somefunc() -> bool { true }; + /// if { true } { /* ... */ } + /// + /// if { let x = somefunc(); x } { /* ... */ } + /// ``` + /// + /// Use instead: + /// ```no_run + /// # fn somefunc() -> bool { true }; + /// if true { /* ... */ } + /// + /// let res = { let x = somefunc(); x }; + /// if res { /* ... */ } + /// ``` + #[clippy::version = "1.45.0"] + pub BLOCKS_IN_CONDITIONS, + style, + "useless or complex blocks that can be eliminated in conditions" +} + +declare_lint_pass!(BlocksInConditions => [BLOCKS_IN_CONDITIONS]); + +const BRACED_EXPR_MESSAGE: &str = "omit braces around single expression condition"; + +impl<'tcx> LateLintPass<'tcx> for BlocksInConditions { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if in_external_macro(cx.sess(), expr.span) { + return; + } + + let Some((cond, keyword, desc)) = higher::If::hir(expr) + .map(|hif| (hif.cond, "if", "an `if` condition")) + .or(if let ExprKind::Match(match_ex, _, MatchSource::Normal) = expr.kind { + Some((match_ex, "match", "a `match` scrutinee")) + } else { + None + }) + else { + return; + }; + let complex_block_message = &format!( + "in {desc}, avoid complex blocks or closures with blocks; \ + instead, move the block or closure higher and bind it with a `let`", + ); + + if let ExprKind::Block(block, _) = &cond.kind { + if block.rules == BlockCheckMode::DefaultBlock { + if block.stmts.is_empty() { + if let Some(ex) = &block.expr { + // don't dig into the expression here, just suggest that they remove + // the block + if expr.span.from_expansion() || ex.span.from_expansion() { + return; + } + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + BLOCKS_IN_CONDITIONS, + cond.span, + BRACED_EXPR_MESSAGE, + "try", + snippet_block_with_applicability(cx, ex.span, "..", Some(expr.span), &mut applicability) + .to_string(), + applicability, + ); + } + } else { + let span = block.expr.as_ref().map_or_else(|| block.stmts[0].span, |e| e.span); + if span.from_expansion() || expr.span.from_expansion() { + return; + } + // move block higher + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + BLOCKS_IN_CONDITIONS, + expr.span.with_hi(cond.span.hi()), + complex_block_message, + "try", + format!( + "let res = {}; {keyword} res", + snippet_block_with_applicability(cx, block.span, "..", Some(expr.span), &mut applicability), + ), + applicability, + ); + } + } + } else { + let _: Option = for_each_expr(cond, |e| { + if let ExprKind::Closure(closure) = e.kind { + // do not lint if the closure is called using an iterator (see #1141) + if let Some(parent) = get_parent_expr(cx, e) + && let ExprKind::MethodCall(_, self_arg, _, _) = &parent.kind + && let caller = cx.typeck_results().expr_ty(self_arg) + && let Some(iter_id) = cx.tcx.get_diagnostic_item(sym::Iterator) + && implements_trait(cx, caller, iter_id, &[]) + { + return ControlFlow::Continue(Descend::No); + } + + let body = cx.tcx.hir().body(closure.body); + let ex = &body.value; + if let ExprKind::Block(block, _) = ex.kind { + if !body.value.span.from_expansion() && !block.stmts.is_empty() { + span_lint(cx, BLOCKS_IN_CONDITIONS, ex.span, complex_block_message); + return ControlFlow::Continue(Descend::No); + } + } + } + ControlFlow::Continue(Descend::Yes) + }); + } + } +} diff --git a/clippy_lints/src/blocks_in_if_conditions.rs b/clippy_lints/src/blocks_in_if_conditions.rs deleted file mode 100644 index 692309629..000000000 --- a/clippy_lints/src/blocks_in_if_conditions.rs +++ /dev/null @@ -1,139 +0,0 @@ -use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg}; -use clippy_utils::source::snippet_block_with_applicability; -use clippy_utils::ty::implements_trait; -use clippy_utils::visitors::{for_each_expr, Descend}; -use clippy_utils::{get_parent_expr, higher}; -use core::ops::ControlFlow; -use rustc_errors::Applicability; -use rustc_hir::{BlockCheckMode, Expr, ExprKind}; -use rustc_lint::{LateContext, LateLintPass, LintContext}; -use rustc_middle::lint::in_external_macro; -use rustc_session::declare_lint_pass; -use rustc_span::sym; - -declare_clippy_lint! { - /// ### What it does - /// Checks for `if` conditions that use blocks containing an - /// expression, statements or conditions that use closures with blocks. - /// - /// ### Why is this bad? - /// Style, using blocks in the condition makes it hard to read. - /// - /// ### Examples - /// ```no_run - /// # fn somefunc() -> bool { true }; - /// if { true } { /* ... */ } - /// - /// if { let x = somefunc(); x } { /* ... */ } - /// ``` - /// - /// Use instead: - /// ```no_run - /// # fn somefunc() -> bool { true }; - /// if true { /* ... */ } - /// - /// let res = { let x = somefunc(); x }; - /// if res { /* ... */ } - /// ``` - #[clippy::version = "1.45.0"] - pub BLOCKS_IN_IF_CONDITIONS, - style, - "useless or complex blocks that can be eliminated in conditions" -} - -declare_lint_pass!(BlocksInIfConditions => [BLOCKS_IN_IF_CONDITIONS]); - -const BRACED_EXPR_MESSAGE: &str = "omit braces around single expression condition"; -const COMPLEX_BLOCK_MESSAGE: &str = "in an `if` condition, avoid complex blocks or closures with blocks; \ - instead, move the block or closure higher and bind it with a `let`"; - -impl<'tcx> LateLintPass<'tcx> for BlocksInIfConditions { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if in_external_macro(cx.sess(), expr.span) { - return; - } - if let Some(higher::If { cond, .. }) = higher::If::hir(expr) { - if let ExprKind::Block(block, _) = &cond.kind { - if block.rules == BlockCheckMode::DefaultBlock { - if block.stmts.is_empty() { - if let Some(ex) = &block.expr { - // don't dig into the expression here, just suggest that they remove - // the block - if expr.span.from_expansion() || ex.span.from_expansion() { - return; - } - let mut applicability = Applicability::MachineApplicable; - span_lint_and_sugg( - cx, - BLOCKS_IN_IF_CONDITIONS, - cond.span, - BRACED_EXPR_MESSAGE, - "try", - format!( - "{}", - snippet_block_with_applicability( - cx, - ex.span, - "..", - Some(expr.span), - &mut applicability - ) - ), - applicability, - ); - } - } else { - let span = block.expr.as_ref().map_or_else(|| block.stmts[0].span, |e| e.span); - if span.from_expansion() || expr.span.from_expansion() { - return; - } - // move block higher - let mut applicability = Applicability::MachineApplicable; - span_lint_and_sugg( - cx, - BLOCKS_IN_IF_CONDITIONS, - expr.span.with_hi(cond.span.hi()), - COMPLEX_BLOCK_MESSAGE, - "try", - format!( - "let res = {}; if res", - snippet_block_with_applicability( - cx, - block.span, - "..", - Some(expr.span), - &mut applicability - ), - ), - applicability, - ); - } - } - } else { - let _: Option = for_each_expr(cond, |e| { - if let ExprKind::Closure(closure) = e.kind { - // do not lint if the closure is called using an iterator (see #1141) - if let Some(parent) = get_parent_expr(cx, e) - && let ExprKind::MethodCall(_, self_arg, _, _) = &parent.kind - && let caller = cx.typeck_results().expr_ty(self_arg) - && let Some(iter_id) = cx.tcx.get_diagnostic_item(sym::Iterator) - && implements_trait(cx, caller, iter_id, &[]) - { - return ControlFlow::Continue(Descend::No); - } - - let body = cx.tcx.hir().body(closure.body); - let ex = &body.value; - if let ExprKind::Block(block, _) = ex.kind { - if !body.value.span.from_expansion() && !block.stmts.is_empty() { - span_lint(cx, BLOCKS_IN_IF_CONDITIONS, ex.span, COMPLEX_BLOCK_MESSAGE); - return ControlFlow::Continue(Descend::No); - } - } - } - ControlFlow::Continue(Descend::Yes) - }); - } - } - } -} diff --git a/clippy_lints/src/borrow_deref_ref.rs b/clippy_lints/src/borrow_deref_ref.rs index d3d4f3c41..0ca4a0e06 100644 --- a/clippy_lints/src/borrow_deref_ref.rs +++ b/clippy_lints/src/borrow_deref_ref.rs @@ -57,7 +57,6 @@ impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef { && !matches!(deref_target.kind, ExprKind::Unary(UnOp::Deref, ..)) && let ref_ty = cx.typeck_results().expr_ty(deref_target) && let ty::Ref(_, inner_ty, Mutability::Not) = ref_ty.kind() - && !is_from_proc_macro(cx, e) { if let Some(parent_expr) = get_parent_expr(cx, e) { if matches!(parent_expr.kind, ExprKind::Unary(UnOp::Deref, ..)) @@ -75,6 +74,9 @@ impl<'tcx> LateLintPass<'tcx> for BorrowDerefRef { return; } } + if is_from_proc_macro(cx, e) { + return; + } span_lint_and_then( cx, diff --git a/clippy_lints/src/casts/as_ptr_cast_mut.rs b/clippy_lints/src/casts/as_ptr_cast_mut.rs index b55cd8833..8bfb7383f 100644 --- a/clippy_lints/src/casts/as_ptr_cast_mut.rs +++ b/clippy_lints/src/casts/as_ptr_cast_mut.rs @@ -9,11 +9,10 @@ use rustc_middle::ty::{self, Ty, TypeAndMut}; use super::AS_PTR_CAST_MUT; pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_to: Ty<'_>) { - if let ty::RawPtr( - TypeAndMut { - mutbl: Mutability::Mut, ty: ptrty, - }, - ) = cast_to.kind() + if let ty::RawPtr(TypeAndMut { + mutbl: Mutability::Mut, + ty: ptrty, + }) = cast_to.kind() && let ty::RawPtr(TypeAndMut { mutbl: Mutability::Not, .. }) = cx.typeck_results().node_type(cast_expr.hir_id).kind() diff --git a/clippy_lints/src/casts/ptr_as_ptr.rs b/clippy_lints/src/casts/ptr_as_ptr.rs index 0c555c1ac..35e36e9ef 100644 --- a/clippy_lints/src/casts/ptr_as_ptr.rs +++ b/clippy_lints/src/casts/ptr_as_ptr.rs @@ -3,12 +3,29 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; use clippy_utils::sugg::Sugg; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, Mutability, TyKind}; +use rustc_hir::{Expr, ExprKind, Mutability, QPath, TyKind}; +use rustc_hir_pretty::qpath_to_string; use rustc_lint::LateContext; use rustc_middle::ty::{self, TypeAndMut}; +use rustc_span::sym; use super::PTR_AS_PTR; +enum OmitFollowedCastReason<'a> { + None, + Null(&'a QPath<'a>), + NullMut(&'a QPath<'a>), +} + +impl OmitFollowedCastReason<'_> { + fn corresponding_item(&self) -> Option<&QPath<'_>> { + match self { + OmitFollowedCastReason::None => None, + OmitFollowedCastReason::Null(x) | OmitFollowedCastReason::NullMut(x) => Some(*x), + } + } +} + pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, msrv: &Msrv) { if !msrv.meets(msrvs::POINTER_CAST) { return; @@ -25,7 +42,6 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, msrv: &Msrv) { && to_pointee_ty.is_sized(cx.tcx, cx.param_env) { let mut app = Applicability::MachineApplicable; - let cast_expr_sugg = Sugg::hir_with_applicability(cx, cast_expr, "_", &mut app); let turbofish = match &cast_to_hir_ty.kind { TyKind::Infer => String::new(), TyKind::Ptr(mut_ty) => { @@ -41,13 +57,44 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, msrv: &Msrv) { _ => return, }; + // following `cast` does not compile because it fails to infer what type is expected + // as type argument to `std::ptr::ptr_null` or `std::ptr::ptr_null_mut`, so + // we omit following `cast`: + let omit_cast = if let ExprKind::Call(func, []) = cast_expr.kind + && let ExprKind::Path(ref qpath @ QPath::Resolved(None, path)) = func.kind + { + let method_defid = path.res.def_id(); + if cx.tcx.is_diagnostic_item(sym::ptr_null, method_defid) { + OmitFollowedCastReason::Null(qpath) + } else if cx.tcx.is_diagnostic_item(sym::ptr_null_mut, method_defid) { + OmitFollowedCastReason::NullMut(qpath) + } else { + OmitFollowedCastReason::None + } + } else { + OmitFollowedCastReason::None + }; + + let (help, final_suggestion) = if let Some(method) = omit_cast.corresponding_item() { + // don't force absolute path + let method = qpath_to_string(method); + ("try call directly", format!("{method}{turbofish}()")) + } else { + let cast_expr_sugg = Sugg::hir_with_applicability(cx, cast_expr, "_", &mut app); + + ( + "try `pointer::cast`, a safer alternative", + format!("{}.cast{turbofish}()", cast_expr_sugg.maybe_par()), + ) + }; + span_lint_and_sugg( cx, PTR_AS_PTR, expr.span, "`as` casting between raw pointers without changing its mutability", - "try `pointer::cast`, a safer alternative", - format!("{}.cast{turbofish}()", cast_expr_sugg.maybe_par()), + help, + final_suggestion, app, ); } diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 3627096ed..1220eb890 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -63,7 +63,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE_INFO, crate::await_holding_invalid::AWAIT_HOLDING_LOCK_INFO, crate::await_holding_invalid::AWAIT_HOLDING_REFCELL_REF_INFO, - crate::blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS_INFO, + crate::blocks_in_conditions::BLOCKS_IN_CONDITIONS_INFO, crate::bool_assert_comparison::BOOL_ASSERT_COMPARISON_INFO, crate::bool_to_int_with_if::BOOL_TO_INT_WITH_IF_INFO, crate::booleans::NONMINIMAL_BOOL_INFO, @@ -215,6 +215,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::index_refutable_slice::INDEX_REFUTABLE_SLICE_INFO, crate::indexing_slicing::INDEXING_SLICING_INFO, crate::indexing_slicing::OUT_OF_BOUNDS_INDEXING_INFO, + crate::ineffective_open_options::INEFFECTIVE_OPEN_OPTIONS_INFO, crate::infinite_iter::INFINITE_ITER_INFO, crate::infinite_iter::MAYBE_INFINITE_ITER_INFO, crate::inherent_impl::MULTIPLE_INHERENT_IMPL_INFO, @@ -265,6 +266,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::loops::EXPLICIT_INTO_ITER_LOOP_INFO, crate::loops::EXPLICIT_ITER_LOOP_INFO, crate::loops::FOR_KV_MAP_INFO, + crate::loops::INFINITE_LOOP_INFO, crate::loops::ITER_NEXT_LOOP_INFO, crate::loops::MANUAL_FIND_INFO, crate::loops::MANUAL_FLATTEN_INFO, @@ -598,6 +600,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::reference::DEREF_ADDROF_INFO, crate::regex::INVALID_REGEX_INFO, crate::regex::TRIVIAL_REGEX_INFO, + crate::repeat_vec_with_capacity::REPEAT_VEC_WITH_CAPACITY_INFO, crate::reserve_after_initialization::RESERVE_AFTER_INITIALIZATION_INFO, crate::return_self_not_must_use::RETURN_SELF_NOT_MUST_USE_INFO, crate::returns::LET_AND_RETURN_INFO, @@ -678,6 +681,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::unicode::INVISIBLE_CHARACTERS_INFO, crate::unicode::NON_ASCII_LITERAL_INFO, crate::unicode::UNICODE_NOT_NFC_INFO, + crate::uninhabited_references::UNINHABITED_REFERENCES_INFO, crate::uninit_vec::UNINIT_VEC_INFO, crate::unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD_INFO, crate::unit_types::LET_UNIT_VALUE_INFO, diff --git a/clippy_lints/src/doc/markdown.rs b/clippy_lints/src/doc/markdown.rs index c0b11eb0d..a58219c29 100644 --- a/clippy_lints/src/doc/markdown.rs +++ b/clippy_lints/src/doc/markdown.rs @@ -9,11 +9,21 @@ use url::Url; use crate::doc::DOC_MARKDOWN; pub fn check(cx: &LateContext<'_>, valid_idents: &FxHashSet, text: &str, span: Span) { - for word in text.split(|c: char| c.is_whitespace() || c == '\'') { + for orig_word in text.split(|c: char| c.is_whitespace() || c == '\'') { // Trim punctuation as in `some comment (see foo::bar).` // ^^ // Or even as in `_foo bar_` which is emphasized. Also preserve `::` as a prefix/suffix. - let mut word = word.trim_matches(|c: char| !c.is_alphanumeric() && c != ':'); + let trim_pattern = |c: char| !c.is_alphanumeric() && c != ':'; + let mut word = orig_word.trim_end_matches(trim_pattern); + + // If word is immediately followed by `()`, claw it back. + if let Some(tmp_word) = orig_word.get(..word.len() + 2) + && tmp_word.ends_with("()") + { + word = tmp_word; + } + + word = word.trim_start_matches(trim_pattern); // Remove leading or trailing single `:` which may be part of a sentence. if word.starts_with(':') && !word.starts_with("::") { @@ -84,7 +94,7 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span) { return; } - if has_underscore(word) || word.contains("::") || is_camel_case(word) { + if has_underscore(word) || word.contains("::") || is_camel_case(word) || word.ends_with("()") { let mut applicability = Applicability::MachineApplicable; span_lint_and_then( diff --git a/clippy_lints/src/explicit_write.rs b/clippy_lints/src/explicit_write.rs index f1366c434..e8c1e5db3 100644 --- a/clippy_lints/src/explicit_write.rs +++ b/clippy_lints/src/explicit_write.rs @@ -15,7 +15,7 @@ declare_clippy_lint! { /// replaced with `(e)print!()` / `(e)println!()` /// /// ### Why is this bad? - /// Using `(e)println! is clearer and more concise + /// Using `(e)println!` is clearer and more concise /// /// ### Example /// ```no_run diff --git a/clippy_lints/src/implied_bounds_in_impls.rs b/clippy_lints/src/implied_bounds_in_impls.rs index 4811691c8..0f5a9ea5d 100644 --- a/clippy_lints/src/implied_bounds_in_impls.rs +++ b/clippy_lints/src/implied_bounds_in_impls.rs @@ -194,7 +194,12 @@ fn is_same_generics<'tcx>( .enumerate() .skip(1) // skip `Self` implicit arg .all(|(arg_index, arg)| { - if [implied_by_generics.host_effect_index, implied_generics.host_effect_index].contains(&Some(arg_index)) { + if [ + implied_by_generics.host_effect_index, + implied_generics.host_effect_index, + ] + .contains(&Some(arg_index)) + { // skip host effect params in determining whether generics are same return true; } diff --git a/clippy_lints/src/ineffective_open_options.rs b/clippy_lints/src/ineffective_open_options.rs new file mode 100644 index 000000000..955f90d42 --- /dev/null +++ b/clippy_lints/src/ineffective_open_options.rs @@ -0,0 +1,95 @@ +use crate::methods::method_call; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::peel_blocks; +use rustc_ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_session::declare_lint_pass; +use rustc_span::{sym, BytePos, Span}; + +declare_clippy_lint! { + /// ### What it does + /// Checks if both `.write(true)` and `.append(true)` methods are called + /// on a same `OpenOptions`. + /// + /// ### Why is this bad? + /// `.append(true)` already enables `write(true)`, making this one + /// superflous. + /// + /// ### Example + /// ```no_run + /// # use std::fs::OpenOptions; + /// let _ = OpenOptions::new() + /// .write(true) + /// .append(true) + /// .create(true) + /// .open("file.json"); + /// ``` + /// Use instead: + /// ```no_run + /// # use std::fs::OpenOptions; + /// let _ = OpenOptions::new() + /// .append(true) + /// .create(true) + /// .open("file.json"); + /// ``` + #[clippy::version = "1.76.0"] + pub INEFFECTIVE_OPEN_OPTIONS, + suspicious, + "usage of both `write(true)` and `append(true)` on same `OpenOptions`" +} + +declare_lint_pass!(IneffectiveOpenOptions => [INEFFECTIVE_OPEN_OPTIONS]); + +fn index_if_arg_is_boolean(args: &[Expr<'_>], call_span: Span) -> Option { + if let [arg] = args + && let ExprKind::Lit(lit) = peel_blocks(arg).kind + && lit.node == LitKind::Bool(true) + { + // The `.` is not included in the span so we cheat a little bit to include it as well. + Some(call_span.with_lo(call_span.lo() - BytePos(1))) + } else { + None + } +} + +impl<'tcx> LateLintPass<'tcx> for IneffectiveOpenOptions { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + let Some(("open", mut receiver, [_arg], _, _)) = method_call(expr) else { + return; + }; + let receiver_ty = cx.typeck_results().expr_ty(receiver); + match receiver_ty.peel_refs().kind() { + ty::Adt(adt, _) if cx.tcx.is_diagnostic_item(sym::FsOpenOptions, adt.did()) => {}, + _ => return, + } + + let mut append = None; + let mut write = None; + + while let Some((name, recv, args, _, span)) = method_call(receiver) { + if name == "append" { + append = index_if_arg_is_boolean(args, span); + } else if name == "write" { + write = index_if_arg_is_boolean(args, span); + } + receiver = recv; + } + + if let Some(write_span) = write + && append.is_some() + { + span_lint_and_sugg( + cx, + INEFFECTIVE_OPEN_OPTIONS, + write_span, + "unnecessary use of `.write(true)` because there is `.append(true)`", + "remove `.write(true)`", + String::new(), + Applicability::MachineApplicable, + ); + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1c59b2df8..7758d6a58 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -50,6 +50,8 @@ extern crate clippy_utils; #[macro_use] extern crate declare_clippy_lint; +use std::collections::BTreeMap; + use rustc_data_structures::fx::FxHashSet; use rustc_lint::{Lint, LintId}; @@ -74,7 +76,7 @@ mod assertions_on_result_states; mod async_yields_async; mod attrs; mod await_holding_invalid; -mod blocks_in_if_conditions; +mod blocks_in_conditions; mod bool_assert_comparison; mod bool_to_int_with_if; mod booleans; @@ -153,6 +155,7 @@ mod implied_bounds_in_impls; mod inconsistent_struct_constructor; mod index_refutable_slice; mod indexing_slicing; +mod ineffective_open_options; mod infinite_iter; mod inherent_impl; mod inherent_to_string; @@ -289,6 +292,7 @@ mod ref_option_ref; mod ref_patterns; mod reference; mod regex; +mod repeat_vec_with_capacity; mod reserve_after_initialization; mod return_self_not_must_use; mod returns; @@ -325,6 +329,7 @@ mod tuple_array_conversions; mod types; mod undocumented_unsafe_blocks; mod unicode; +mod uninhabited_references; mod uninit_vec; mod unit_return_expecting_ord; mod unit_types; @@ -653,7 +658,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::>::default()); store.register_late_pass(|_| Box::new(len_zero::LenZero)); store.register_late_pass(|_| Box::new(attrs::Attributes)); - store.register_late_pass(|_| Box::new(blocks_in_if_conditions::BlocksInIfConditions)); + store.register_late_pass(|_| Box::new(blocks_in_conditions::BlocksInConditions)); store.register_late_pass(|_| Box::new(unicode::Unicode)); store.register_late_pass(|_| Box::new(uninit_vec::UninitVec)); store.register_late_pass(|_| Box::new(unit_return_expecting_ord::UnitReturnExpectingOrd)); @@ -722,6 +727,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { Box::new(vec::UselessVec { too_large_for_stack, msrv: msrv(), + span_to_lint_map: BTreeMap::new(), }) }); store.register_late_pass(|_| Box::new(panic_unimplemented::PanicUnimplemented)); @@ -1069,6 +1075,9 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(iter_without_into_iter::IterWithoutIntoIter)); store.register_late_pass(|_| Box::new(iter_over_hash_type::IterOverHashType)); store.register_late_pass(|_| Box::new(impl_hash_with_borrow_str_and_bytes::ImplHashWithBorrowStrBytes)); + store.register_late_pass(|_| Box::new(repeat_vec_with_capacity::RepeatVecWithCapacity)); + store.register_late_pass(|_| Box::new(uninhabited_references::UninhabitedReferences)); + store.register_late_pass(|_| Box::new(ineffective_open_options::IneffectiveOpenOptions)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/loops/infinite_loop.rs b/clippy_lints/src/loops/infinite_loop.rs new file mode 100644 index 000000000..9b88dd76e --- /dev/null +++ b/clippy_lints/src/loops/infinite_loop.rs @@ -0,0 +1,125 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::{fn_def_id, is_lint_allowed}; +use hir::intravisit::{walk_expr, Visitor}; +use hir::{Expr, ExprKind, FnRetTy, FnSig, Node}; +use rustc_ast::Label; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::LateContext; + +use super::INFINITE_LOOP; + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &Expr<'_>, + loop_block: &'tcx hir::Block<'_>, + label: Option