From a9e9b7f9b2ad52035fe7b57bb0fc8ba62d649c33 Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Wed, 14 Jul 2021 02:21:08 -0400 Subject: [PATCH 1/4] ExprUseVisitor::Delegate consume only when moving --- clippy_lints/src/escape.rs | 9 +++------ clippy_lints/src/loops/mut_range_bound.rs | 4 ++-- clippy_lints/src/needless_pass_by_value.rs | 6 ++---- clippy_utils/src/usage.rs | 4 ++-- 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs index 3581ab419..5f400d079 100644 --- a/clippy_lints/src/escape.rs +++ b/clippy_lints/src/escape.rs @@ -11,7 +11,7 @@ use rustc_span::source_map::Span; use rustc_span::symbol::kw; use rustc_target::abi::LayoutOf; use rustc_target::spec::abi::Abi; -use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; +use rustc_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; #[derive(Copy, Clone)] pub struct BoxedLocal { @@ -133,13 +133,10 @@ fn is_argument(map: rustc_middle::hir::map::Map<'_>, id: HirId) -> bool { } impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { - fn consume(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId, mode: ConsumeMode) { + fn consume(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId) { if cmt.place.projections.is_empty() { if let PlaceBase::Local(lid) = cmt.place.base { - if let ConsumeMode::Move = mode { - // moved out or in. clearly can't be localized - self.set.remove(&lid); - } + self.set.remove(&lid); let map = &self.cx.tcx.hir(); if let Some(Node::Binding(_)) = map.find(cmt.hir_id) { if self.set.contains(&lid) { diff --git a/clippy_lints/src/loops/mut_range_bound.rs b/clippy_lints/src/loops/mut_range_bound.rs index d07b5a93b..1e54a1e2d 100644 --- a/clippy_lints/src/loops/mut_range_bound.rs +++ b/clippy_lints/src/loops/mut_range_bound.rs @@ -7,7 +7,7 @@ use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::LateContext; use rustc_middle::{mir::FakeReadCause, ty}; use rustc_span::source_map::Span; -use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; +use rustc_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>, body: &Expr<'_>) { if let Some(higher::Range { @@ -82,7 +82,7 @@ struct MutatePairDelegate<'a, 'tcx> { } impl<'tcx> Delegate<'tcx> for MutatePairDelegate<'_, 'tcx> { - fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId, _: ConsumeMode) {} + fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {} fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, diag_expr_id: HirId, bk: ty::BorrowKind) { if let ty::BorrowKind::MutBorrow = bk { diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index e33a33e23..57fd03f4e 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -326,10 +326,8 @@ impl MovedVariablesCtxt { } impl<'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt { - fn consume(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, _: HirId, mode: euv::ConsumeMode) { - if let euv::ConsumeMode::Move = mode { - self.move_common(cmt); - } + fn consume(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, _: HirId) { + self.move_common(cmt); } fn borrow(&mut self, _: &euv::PlaceWithHirId<'tcx>, _: HirId, _: ty::BorrowKind) {} diff --git a/clippy_utils/src/usage.rs b/clippy_utils/src/usage.rs index 2c55021ac..82e4b3000 100644 --- a/clippy_utils/src/usage.rs +++ b/clippy_utils/src/usage.rs @@ -10,7 +10,7 @@ use rustc_lint::LateContext; use rustc_middle::hir::map::Map; use rustc_middle::mir::FakeReadCause; use rustc_middle::ty; -use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; +use rustc_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; /// Returns a set of mutated local variable IDs, or `None` if mutations could not be determined. pub fn mutated_variables<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) -> Option { @@ -67,7 +67,7 @@ impl<'tcx> MutVarsDelegate { } impl<'tcx> Delegate<'tcx> for MutVarsDelegate { - fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId, _: ConsumeMode) {} + fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {} fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId, bk: ty::BorrowKind) { if let ty::BorrowKind::MutBorrow = bk { From 1d084b13a58efac06c478ae721b5666e76fe3214 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 15 Jul 2021 10:44:10 +0200 Subject: [PATCH 2/4] Merge commit '54a20a02ecd0e1352a871aa0990bcc8b8b03173e' into clippyup --- CHANGELOG.md | 2 + clippy_lints/src/cargo_common_metadata.rs | 5 +- clippy_lints/src/copies.rs | 14 +- clippy_lints/src/default_numeric_fallback.rs | 26 ++- clippy_lints/src/dereference.rs | 4 +- clippy_lints/src/derive.rs | 4 +- clippy_lints/src/doc.rs | 9 +- clippy_lints/src/eta_reduction.rs | 20 +- clippy_lints/src/explicit_write.rs | 117 ++++------- clippy_lints/src/format.rs | 192 +++++++----------- clippy_lints/src/inherent_impl.rs | 6 +- clippy_lints/src/len_zero.rs | 4 +- clippy_lints/src/lib.rs | 10 +- clippy_lints/src/manual_map.rs | 6 +- clippy_lints/src/matches.rs | 4 +- clippy_lints/src/methods/expect_fun_call.rs | 72 ++----- clippy_lints/src/multiple_crate_versions.rs | 4 +- clippy_lints/src/mut_key.rs | 4 +- clippy_lints/src/needless_bool.rs | 3 + clippy_lints/src/nonstandard_macro_braces.rs | 14 +- clippy_lints/src/panic_unimplemented.rs | 4 +- clippy_lints/src/ptr.rs | 4 +- clippy_lints/src/redundant_clone.rs | 108 +++++++++- clippy_lints/src/strings.rs | 4 +- clippy_lints/src/strlen_on_c_strings.rs | 82 ++++++++ clippy_lints/src/types/mod.rs | 37 +++- clippy_lints/src/types/rc_mutex.rs | 27 +++ .../src/types/redundant_allocation.rs | 153 ++++++++------ clippy_lints/src/unicode.rs | 6 +- clippy_lints/src/unused_unit.rs | 4 + clippy_lints/src/use_self.rs | 17 +- clippy_lints/src/utils/internal_lints.rs | 16 +- .../internal_lints/metadata_collector.rs | 4 +- clippy_lints/src/wildcard_dependencies.rs | 5 +- clippy_utils/src/higher.rs | 108 +++++++++- clippy_utils/src/lib.rs | 21 +- clippy_utils/src/numeric_literal.rs | 3 + clippy_utils/src/paths.rs | 5 +- clippy_utils/src/ty.rs | 22 +- clippy_utils/src/usage.rs | 47 +++++ rust-toolchain | 2 +- .../auxiliary/proc_macro_derive.rs | 18 ++ .../conf_nonstandard_macro_braces.rs | 10 +- .../conf_nonstandard_macro_braces.stderr | 28 +-- .../branches_sharing_code/false_positives.rs | 28 +++ tests/ui/crashes/ice-7423.rs | 13 ++ tests/ui/default_numeric_fallback_f64.fixed | 174 ++++++++++++++++ tests/ui/default_numeric_fallback_f64.rs | 174 ++++++++++++++++ tests/ui/default_numeric_fallback_f64.stderr | 147 ++++++++++++++ tests/ui/default_numeric_fallback_i32.fixed | 173 ++++++++++++++++ ...ack.rs => default_numeric_fallback_i32.rs} | 24 ++- ...rr => default_numeric_fallback_i32.stderr} | 60 +++--- tests/ui/doc/unbalanced_ticks.rs | 7 + tests/ui/eta.fixed | 16 ++ tests/ui/eta.rs | 16 ++ tests/ui/eta.stderr | 26 ++- tests/ui/explicit_write_non_rustfix.stderr | 3 +- tests/ui/format.fixed | 2 + tests/ui/format.rs | 2 + tests/ui/panicking_macros.rs | 16 ++ tests/ui/rc_mutex.rs | 34 ++++ tests/ui/rc_mutex.stderr | 28 +++ tests/ui/redundant_allocation.fixed | 48 ----- tests/ui/redundant_allocation.rs | 68 +++++-- tests/ui/redundant_allocation.stderr | 158 ++++++++++---- tests/ui/redundant_allocation_fixable.fixed | 75 +++++++ tests/ui/redundant_allocation_fixable.rs | 75 +++++++ tests/ui/redundant_allocation_fixable.stderr | 99 +++++++++ tests/ui/redundant_clone.fixed | 28 +++ tests/ui/redundant_clone.rs | 28 +++ tests/ui/redundant_clone.stderr | 20 +- tests/ui/strlen_on_c_strings.rs | 16 ++ tests/ui/strlen_on_c_strings.stderr | 25 +++ 73 files changed, 2228 insertions(+), 610 deletions(-) create mode 100644 clippy_lints/src/strlen_on_c_strings.rs create mode 100644 clippy_lints/src/types/rc_mutex.rs create mode 100644 tests/ui-toml/nonstandard_macro_braces/auxiliary/proc_macro_derive.rs create mode 100644 tests/ui/branches_sharing_code/false_positives.rs create mode 100644 tests/ui/crashes/ice-7423.rs create mode 100644 tests/ui/default_numeric_fallback_f64.fixed create mode 100644 tests/ui/default_numeric_fallback_f64.rs create mode 100644 tests/ui/default_numeric_fallback_f64.stderr create mode 100644 tests/ui/default_numeric_fallback_i32.fixed rename tests/ui/{default_numeric_fallback.rs => default_numeric_fallback_i32.rs} (90%) rename tests/ui/{default_numeric_fallback.stderr => default_numeric_fallback_i32.stderr} (75%) create mode 100644 tests/ui/rc_mutex.rs create mode 100644 tests/ui/rc_mutex.stderr delete mode 100644 tests/ui/redundant_allocation.fixed create mode 100644 tests/ui/redundant_allocation_fixable.fixed create mode 100644 tests/ui/redundant_allocation_fixable.rs create mode 100644 tests/ui/redundant_allocation_fixable.stderr create mode 100644 tests/ui/strlen_on_c_strings.rs create mode 100644 tests/ui/strlen_on_c_strings.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 421614057..9f89f74e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2744,6 +2744,7 @@ Released 2018-09-13 [`range_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_step_by_zero [`range_zip_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_zip_with_len [`rc_buffer`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer +[`rc_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex [`redundant_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation [`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone [`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure @@ -2797,6 +2798,7 @@ Released 2018-09-13 [`string_from_utf8_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_from_utf8_as_bytes [`string_lit_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes [`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string +[`strlen_on_c_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#strlen_on_c_strings [`struct_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#struct_excessive_bools [`suboptimal_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#suboptimal_flops [`suspicious_arithmetic_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_arithmetic_impl diff --git a/clippy_lints/src/cargo_common_metadata.rs b/clippy_lints/src/cargo_common_metadata.rs index 8097a1c83..21c7b2448 100644 --- a/clippy_lints/src/cargo_common_metadata.rs +++ b/clippy_lints/src/cargo_common_metadata.rs @@ -2,8 +2,7 @@ use std::path::PathBuf; -use clippy_utils::diagnostics::span_lint; -use clippy_utils::run_lints; +use clippy_utils::{diagnostics::span_lint, is_lint_allowed}; use rustc_hir::{hir_id::CRATE_HIR_ID, Crate}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -85,7 +84,7 @@ fn is_empty_vec(value: &[String]) -> bool { impl LateLintPass<'_> for CargoCommonMetadata { fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) { - if !run_lints(cx, &[CARGO_COMMON_METADATA], CRATE_HIR_ID) { + if is_lint_allowed(cx, CARGO_COMMON_METADATA, CRATE_HIR_ID) { return; } diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index 376a14b81..9cbcde597 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_then}; use clippy_utils::source::{first_line_of_span, indent_of, reindent_multiline, snippet, snippet_opt}; use clippy_utils::{ both, count_eq, eq_expr_value, get_enclosing_block, get_parent_expr, if_sequence, in_macro, is_else_clause, - run_lints, search_same, ContainsName, SpanlessEq, SpanlessHash, + is_lint_allowed, search_same, ContainsName, SpanlessEq, SpanlessHash, }; use if_chain::if_chain; use rustc_data_structures::fx::FxHashSet; @@ -120,7 +120,10 @@ declare_clippy_lint! { /// /// **Why is this bad?** Duplicate code is less maintainable. /// - /// **Known problems:** Hopefully none. + /// **Known problems:** + /// * The lint doesn't check if the moved expressions modify values that are beeing used in + /// the if condition. The suggestion can in that case modify the behavior of the program. + /// See [rust-clippy#7452](https://github.com/rust-lang/rust-clippy/issues/7452) /// /// **Example:** /// ```ignore @@ -337,8 +340,8 @@ fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> Option< if block_expr_eq; if l_stmts.len() == r_stmts.len(); if l_stmts.len() == current_start_eq; - if run_lints(cx, &[IF_SAME_THEN_ELSE], win[0].hir_id); - if run_lints(cx, &[IF_SAME_THEN_ELSE], win[1].hir_id); + if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win[0].hir_id); + if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win[1].hir_id); then { span_lint_and_note( cx, @@ -358,8 +361,7 @@ fn scan_block_for_eq(cx: &LateContext<'tcx>, blocks: &[&Block<'tcx>]) -> Option< expr_eq &= block_expr_eq; } - let has_expr = blocks[0].expr.is_some(); - if has_expr && !expr_eq { + if !expr_eq { end_eq = 0; } diff --git a/clippy_lints/src/default_numeric_fallback.rs b/clippy_lints/src/default_numeric_fallback.rs index a125376bf..e719a1b0a 100644 --- a/clippy_lints/src/default_numeric_fallback.rs +++ b/clippy_lints/src/default_numeric_fallback.rs @@ -1,5 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet; +use clippy_utils::numeric_literal; +use clippy_utils::source::snippet_opt; use if_chain::if_chain; use rustc_ast::ast::{LitFloatType, LitIntType, LitKind}; use rustc_errors::Applicability; @@ -78,16 +79,25 @@ impl<'a, 'tcx> NumericFallbackVisitor<'a, 'tcx> { if let Some(ty_bound) = self.ty_bounds.last(); if matches!(lit.node, LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed)); - if !ty_bound.is_integral(); + if !ty_bound.is_numeric(); then { - let suffix = match lit_ty.kind() { - ty::Int(IntTy::I32) => "i32", - ty::Float(FloatTy::F64) => "f64", + let (suffix, is_float) = match lit_ty.kind() { + ty::Int(IntTy::I32) => ("i32", false), + ty::Float(FloatTy::F64) => ("f64", true), // Default numeric fallback never results in other types. _ => return, }; - let sugg = format!("{}_{}", snippet(self.cx, lit.span, ""), suffix); + let src = if let Some(src) = snippet_opt(self.cx, lit.span) { + src + } else { + match lit.node { + LitKind::Int(src, _) => format!("{}", src), + LitKind::Float(src, _) => format!("{}", src), + _ => return, + } + }; + let sugg = numeric_literal::format(&src, Some(suffix), is_float); span_lint_and_sugg( self.cx, DEFAULT_NUMERIC_FALLBACK, @@ -219,10 +229,10 @@ enum TyBound<'tcx> { } impl<'tcx> TyBound<'tcx> { - fn is_integral(self) -> bool { + fn is_numeric(self) -> bool { match self { TyBound::Any => true, - TyBound::Ty(t) => t.is_integral(), + TyBound::Ty(t) => t.is_numeric(), TyBound::Nothing => false, } } diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index 1415f8e23..682003f9c 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_context; use clippy_utils::ty::peel_mid_ty_refs; -use clippy_utils::{get_parent_node, in_macro, is_allowed}; +use clippy_utils::{get_parent_node, in_macro, is_lint_allowed}; use rustc_ast::util::parser::PREC_PREFIX; use rustc_errors::Applicability; use rustc_hir::{BorrowKind, Expr, ExprKind, HirId, MatchSource, Mutability, Node, UnOp}; @@ -107,7 +107,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { match kind { RefOp::Method(target_mut) - if !is_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id) + if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id) && is_linted_explicit_deref_position(parent, expr.hir_id, expr.span) => { self.state = Some(( diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index 3ac20fd98..7aafaf713 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note, span_lint_and_then}; use clippy_utils::paths; use clippy_utils::ty::{implements_trait, is_copy}; -use clippy_utils::{get_trait_def_id, is_allowed, is_automatically_derived, match_def_path}; +use clippy_utils::{get_trait_def_id, is_automatically_derived, is_lint_allowed, match_def_path}; use if_chain::if_chain; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{walk_expr, walk_fn, walk_item, FnKind, NestedVisitorMap, Visitor}; @@ -362,7 +362,7 @@ fn check_unsafe_derive_deserialize<'tcx>( if let ty::Adt(def, _) = ty.kind(); if let Some(local_def_id) = def.did.as_local(); let adt_hir_id = cx.tcx.hir().local_def_id_to_hir_id(local_def_id); - if !is_allowed(cx, UNSAFE_DERIVE_DESERIALIZE, adt_hir_id); + if !is_lint_allowed(cx, UNSAFE_DERIVE_DESERIALIZE, adt_hir_id); if cx.tcx.inherent_impls(def.did) .iter() .map(|imp_did| item_from_def_id(cx, *imp_did)) diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index d6e3e92fc..0c19988a9 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -550,7 +550,7 @@ fn check_doc<'a, Events: Iterator, Range { let (begin, span) = get_current_span(spans, range.start); paragraph_span = paragraph_span.with_hi(span.hi()); - ticks_unbalanced |= text.contains('`'); + ticks_unbalanced |= text.contains('`') && !in_code; if Some(&text) == in_link.as_ref() || ticks_unbalanced { // Probably a link of the form `` // Which are represented as a link to "http://example.com" with @@ -595,7 +595,7 @@ fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) { let handler = Handler::with_emitter(false, None, box emitter); let sess = ParseSess::with_span_handler(handler, sm); - let mut parser = match maybe_new_parser_from_source_str(&sess, filename, code.into()) { + let mut parser = match maybe_new_parser_from_source_str(&sess, filename, code) { Ok(p) => p, Err(errs) => { for mut err in errs { @@ -653,7 +653,10 @@ fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) { // Because of the global session, we need to create a new session in a different thread with // the edition we need. let text = text.to_owned(); - if thread::spawn(move || has_needless_main(text, edition)).join().expect("thread::spawn failed") { + if thread::spawn(move || has_needless_main(text, edition)) + .join() + .expect("thread::spawn failed") + { span_lint(cx, NEEDLESS_DOCTEST_MAIN, span, "needless `fn main` in doctest"); } } diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs index 8d066f305..667eb8eb2 100644 --- a/clippy_lints/src/eta_reduction.rs +++ b/clippy_lints/src/eta_reduction.rs @@ -1,15 +1,16 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; -use clippy_utils::higher; use clippy_utils::higher::VecArgs; use clippy_utils::source::snippet_opt; use clippy_utils::ty::{implements_trait, type_is_unsafe_function}; +use clippy_utils::usage::UsedAfterExprVisitor; +use clippy_utils::{get_enclosing_loop_or_closure, higher}; use clippy_utils::{is_adjusted, iter_input_pats}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{def_id, Expr, ExprKind, Param, PatKind, QPath}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; -use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{self, ClosureKind, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { @@ -86,7 +87,7 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction { } } -fn check_closure(cx: &LateContext<'_>, expr: &Expr<'_>) { +fn check_closure<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if let ExprKind::Closure(_, decl, eid, _, _) = expr.kind { let body = cx.tcx.hir().body(eid); let ex = &body.value; @@ -131,7 +132,18 @@ fn check_closure(cx: &LateContext<'_>, expr: &Expr<'_>) { then { span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure", |diag| { - if let Some(snippet) = snippet_opt(cx, caller.span) { + if let Some(mut snippet) = snippet_opt(cx, caller.span) { + if_chain! { + if let ty::Closure(_, substs) = fn_ty.kind(); + if let ClosureKind::FnMut = substs.as_closure().kind(); + if UsedAfterExprVisitor::is_found(cx, caller) + || get_enclosing_loop_or_closure(cx.tcx, expr).is_some(); + + then { + // Mutable closure is used after current expr; we cannot consume it. + snippet = format!("&mut {}", snippet); + } + } diag.span_suggestion( expr.span, "replace the closure with the function itself", diff --git a/clippy_lints/src/explicit_write.rs b/clippy_lints/src/explicit_write.rs index da4936ff2..667242948 100644 --- a/clippy_lints/src/explicit_write.rs +++ b/clippy_lints/src/explicit_write.rs @@ -1,9 +1,9 @@ -use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg}; +use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; +use clippy_utils::higher::FormatArgsExpn; use clippy_utils::{is_expn_of, match_function_call, paths}; use if_chain::if_chain; -use rustc_ast::ast::LitKind; use rustc_errors::Applicability; -use rustc_hir::{BorrowKind, Expr, ExprKind}; +use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -34,29 +34,26 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitWrite { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if_chain! { // match call to unwrap - if let ExprKind::MethodCall(unwrap_fun, _, unwrap_args, _) = expr.kind; + if let ExprKind::MethodCall(unwrap_fun, _, [write_call], _) = expr.kind; if unwrap_fun.ident.name == sym::unwrap; // match call to write_fmt - if !unwrap_args.is_empty(); - if let ExprKind::MethodCall(write_fun, _, write_args, _) = - unwrap_args[0].kind; + if let ExprKind::MethodCall(write_fun, _, [write_recv, write_arg], _) = write_call.kind; if write_fun.ident.name == sym!(write_fmt); // match calls to std::io::stdout() / std::io::stderr () - if !write_args.is_empty(); - if let Some(dest_name) = if match_function_call(cx, &write_args[0], &paths::STDOUT).is_some() { + if let Some(dest_name) = if match_function_call(cx, write_recv, &paths::STDOUT).is_some() { Some("stdout") - } else if match_function_call(cx, &write_args[0], &paths::STDERR).is_some() { + } else if match_function_call(cx, write_recv, &paths::STDERR).is_some() { Some("stderr") } else { None }; + if let Some(format_args) = FormatArgsExpn::parse(write_arg); then { - let write_span = unwrap_args[0].span; let calling_macro = // ordering is important here, since `writeln!` uses `write!` internally - if is_expn_of(write_span, "writeln").is_some() { + if is_expn_of(write_call.span, "writeln").is_some() { Some("writeln") - } else if is_expn_of(write_span, "write").is_some() { + } else if is_expn_of(write_call.span, "write").is_some() { Some("write") } else { None @@ -70,82 +67,40 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitWrite { // We need to remove the last trailing newline from the string because the // underlying `fmt::write` function doesn't know whether `println!` or `print!` was // used. - if let Some(mut write_output) = write_output_string(write_args) { + let (used, sugg_mac) = if let Some(macro_name) = calling_macro { + ( + format!("{}!({}(), ...)", macro_name, dest_name), + macro_name.replace("write", "print"), + ) + } else { + ( + format!("{}().write_fmt(...)", dest_name), + "print".into(), + ) + }; + let msg = format!("use of `{}.unwrap()`", used); + if let [write_output] = *format_args.format_string_symbols { + let mut write_output = write_output.to_string(); if write_output.ends_with('\n') { write_output.pop(); } - if let Some(macro_name) = calling_macro { - span_lint_and_sugg( - cx, - EXPLICIT_WRITE, - expr.span, - &format!( - "use of `{}!({}(), ...).unwrap()`", - macro_name, - dest_name - ), - "try this", - format!("{}{}!(\"{}\")", prefix, macro_name.replace("write", "print"), write_output.escape_default()), - Applicability::MachineApplicable - ); - } else { - span_lint_and_sugg( - cx, - EXPLICIT_WRITE, - expr.span, - &format!("use of `{}().write_fmt(...).unwrap()`", dest_name), - "try this", - format!("{}print!(\"{}\")", prefix, write_output.escape_default()), - Applicability::MachineApplicable - ); - } + let sugg = format!("{}{}!(\"{}\")", prefix, sugg_mac, write_output.escape_default()); + span_lint_and_sugg( + cx, + EXPLICIT_WRITE, + expr.span, + &msg, + "try this", + sugg, + Applicability::MachineApplicable + ); } else { // We don't have a proper suggestion - if let Some(macro_name) = calling_macro { - span_lint( - cx, - EXPLICIT_WRITE, - expr.span, - &format!( - "use of `{}!({}(), ...).unwrap()`. Consider using `{}{}!` instead", - macro_name, - dest_name, - prefix, - macro_name.replace("write", "print") - ) - ); - } else { - span_lint( - cx, - EXPLICIT_WRITE, - expr.span, - &format!("use of `{}().write_fmt(...).unwrap()`. Consider using `{}print!` instead", dest_name, prefix), - ); - } + let help = format!("consider using `{}{}!` instead", prefix, sugg_mac); + span_lint_and_help(cx, EXPLICIT_WRITE, expr.span, &msg, None, &help); } - } } } } - -// Extract the output string from the given `write_args`. -fn write_output_string(write_args: &[Expr<'_>]) -> Option { - if_chain! { - // Obtain the string that should be printed - if write_args.len() > 1; - if let ExprKind::Call(_, output_args) = write_args[1].kind; - if !output_args.is_empty(); - if let ExprKind::AddrOf(BorrowKind::Ref, _, output_string_expr) = output_args[0].kind; - if let ExprKind::Array(string_exprs) = output_string_expr.kind; - // we only want to provide an automatic suggestion for simple (non-format) strings - if string_exprs.len() == 1; - if let ExprKind::Lit(ref lit) = string_exprs[0].kind; - if let LitKind::Str(ref write_output, _) = lit.node; - then { - return Some(write_output.to_string()) - } - } - None -} diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index c2b055ed6..ca3490d8e 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -1,17 +1,16 @@ -use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::paths; -use clippy_utils::source::{snippet, snippet_opt}; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::higher::FormatExpn; +use clippy_utils::last_path_segment; +use clippy_utils::source::{snippet_opt, snippet_with_applicability}; use clippy_utils::sugg::Sugg; -use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{is_expn_of, last_path_segment, match_def_path, match_function_call}; use if_chain::if_chain; -use rustc_ast::ast::LitKind; use rustc_errors::Applicability; -use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, MatchSource, PatKind}; -use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_hir::{BorrowKind, Expr, ExprKind, QPath}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::source_map::Span; -use rustc_span::sym; +use rustc_span::symbol::kw; +use rustc_span::{sym, Span}; declare_clippy_lint! { /// **What it does:** Checks for the use of `format!("string literal with no @@ -44,130 +43,78 @@ declare_lint_pass!(UselessFormat => [USELESS_FORMAT]); impl<'tcx> LateLintPass<'tcx> for UselessFormat { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - let span = match is_expn_of(expr.span, "format") { - Some(s) if !s.from_expansion() => s, + let FormatExpn { call_site, format_args } = match FormatExpn::parse(expr) { + Some(e) if !e.call_site.from_expansion() => e, _ => return, }; - // Operate on the only argument of `alloc::fmt::format`. - if let Some(sugg) = on_new_v1(cx, expr) { - span_useless_format(cx, span, "consider using `.to_string()`", sugg); - } else if let Some(sugg) = on_new_v1_fmt(cx, expr) { - span_useless_format(cx, span, "consider using `.to_string()`", sugg); - } + let mut applicability = Applicability::MachineApplicable; + if format_args.value_args.is_empty() { + if_chain! { + if let [e] = &*format_args.format_string_parts; + if let ExprKind::Lit(lit) = &e.kind; + if let Some(s_src) = snippet_opt(cx, lit.span); + then { + // Simulate macro expansion, converting {{ and }} to { and }. + let s_expand = s_src.replace("{{", "{").replace("}}", "}"); + let sugg = format!("{}.to_string()", s_expand); + span_useless_format(cx, call_site, sugg, applicability); + } + } + } else if let [value] = *format_args.value_args { + if_chain! { + if format_args.format_string_symbols == [kw::Empty]; + if match cx.typeck_results().expr_ty(value).peel_refs().kind() { + ty::Adt(adt, _) => cx.tcx.is_diagnostic_item(sym::string_type, adt.did), + ty::Str => true, + _ => false, + }; + if format_args.args.iter().all(|e| is_display_arg(e)); + if format_args.fmt_expr.map_or(true, |e| check_unformatted(e)); + then { + let is_new_string = match value.kind { + ExprKind::Binary(..) => true, + ExprKind::MethodCall(path, ..) => path.ident.name.as_str() == "to_string", + _ => false, + }; + let sugg = if is_new_string { + snippet_with_applicability(cx, value.span, "..", &mut applicability).into_owned() + } else { + let sugg = Sugg::hir_with_applicability(cx, value, "", &mut applicability); + format!("{}.to_string()", sugg.maybe_par()) + }; + span_useless_format(cx, call_site, sugg, applicability); + } + } + }; } } -fn span_useless_format(cx: &T, span: Span, help: &str, mut sugg: String) { - let to_replace = span.source_callsite(); - +fn span_useless_format(cx: &LateContext<'_>, span: Span, mut sugg: String, mut applicability: Applicability) { // The callsite span contains the statement semicolon for some reason. - let snippet = snippet(cx, to_replace, ".."); - if snippet.ends_with(';') { + if snippet_with_applicability(cx, span, "..", &mut applicability).ends_with(';') { sugg.push(';'); } - span_lint_and_then(cx, USELESS_FORMAT, span, "useless use of `format!`", |diag| { - diag.span_suggestion( - to_replace, - help, - sugg, - Applicability::MachineApplicable, // snippet - ); - }); + span_lint_and_sugg( + cx, + USELESS_FORMAT, + span, + "useless use of `format!`", + "consider using `.to_string()`", + sugg, + applicability, + ); } -fn on_argumentv1_new<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>]) -> Option { +fn is_display_arg(expr: &Expr<'_>) -> bool { if_chain! { - if let ExprKind::AddrOf(BorrowKind::Ref, _, format_args) = expr.kind; - if let ExprKind::Array(elems) = arms[0].body.kind; - if elems.len() == 1; - if let Some(args) = match_function_call(cx, &elems[0], &paths::FMT_ARGUMENTV1_NEW); - // matches `core::fmt::Display::fmt` - if args.len() == 2; - if let ExprKind::Path(ref qpath) = args[1].kind; - if let Some(did) = cx.qpath_res(qpath, args[1].hir_id).opt_def_id(); - if match_def_path(cx, did, &paths::DISPLAY_FMT_METHOD); - // check `(arg0,)` in match block - if let PatKind::Tuple(pats, None) = arms[0].pat.kind; - if pats.len() == 1; - then { - let ty = cx.typeck_results().pat_ty(pats[0]).peel_refs(); - if *ty.kind() != rustc_middle::ty::Str && !is_type_diagnostic_item(cx, ty, sym::string_type) { - return None; - } - if let ExprKind::Lit(ref lit) = format_args.kind { - if let LitKind::Str(ref s, _) = lit.node { - return Some(format!("{:?}.to_string()", s.as_str())); - } - } else { - let sugg = Sugg::hir(cx, format_args, ""); - if let ExprKind::MethodCall(path, _, _, _) = format_args.kind { - if path.ident.name == sym!(to_string) { - return Some(format!("{}", sugg)); - } - } else if let ExprKind::Binary(..) = format_args.kind { - return Some(format!("{}", sugg)); - } - return Some(format!("{}.to_string()", sugg.maybe_par())); - } - } + if let ExprKind::Call(_, [_, fmt]) = expr.kind; + if let ExprKind::Path(QPath::Resolved(_, path)) = fmt.kind; + if let [.., t, _] = path.segments; + if t.ident.name.as_str() == "Display"; + then { true } else { false } } - None -} - -fn on_new_v1<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option { - if_chain! { - if let Some(args) = match_function_call(cx, expr, &paths::FMT_ARGUMENTS_NEW_V1); - if args.len() == 2; - // Argument 1 in `new_v1()` - if let ExprKind::AddrOf(BorrowKind::Ref, _, arr) = args[0].kind; - if let ExprKind::Array(pieces) = arr.kind; - if pieces.len() == 1; - if let ExprKind::Lit(ref lit) = pieces[0].kind; - if let LitKind::Str(ref s, _) = lit.node; - // Argument 2 in `new_v1()` - if let ExprKind::AddrOf(BorrowKind::Ref, _, arg1) = args[1].kind; - if let ExprKind::Match(matchee, arms, MatchSource::Normal) = arg1.kind; - if arms.len() == 1; - if let ExprKind::Tup(tup) = matchee.kind; - then { - // `format!("foo")` expansion contains `match () { () => [], }` - if tup.is_empty() { - if let Some(s_src) = snippet_opt(cx, lit.span) { - // Simulate macro expansion, converting {{ and }} to { and }. - let s_expand = s_src.replace("{{", "{").replace("}}", "}"); - return Some(format!("{}.to_string()", s_expand)); - } - } else if s.as_str().is_empty() { - return on_argumentv1_new(cx, &tup[0], arms); - } - } - } - None -} - -fn on_new_v1_fmt<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option { - if_chain! { - if let Some(args) = match_function_call(cx, expr, &paths::FMT_ARGUMENTS_NEW_V1_FORMATTED); - if args.len() == 3; - if check_unformatted(&args[2]); - // Argument 1 in `new_v1_formatted()` - if let ExprKind::AddrOf(BorrowKind::Ref, _, arr) = args[0].kind; - if let ExprKind::Array(pieces) = arr.kind; - if pieces.len() == 1; - if let ExprKind::Lit(ref lit) = pieces[0].kind; - if let LitKind::Str(..) = lit.node; - // Argument 2 in `new_v1_formatted()` - if let ExprKind::AddrOf(BorrowKind::Ref, _, arg1) = args[1].kind; - if let ExprKind::Match(matchee, arms, MatchSource::Normal) = arg1.kind; - if arms.len() == 1; - if let ExprKind::Tup(tup) = matchee.kind; - then { - return on_argumentv1_new(cx, &tup[0], arms); - } - } - None } /// Checks if the expression matches @@ -184,10 +131,9 @@ fn on_new_v1_fmt<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option) -> bool { if_chain! { if let ExprKind::AddrOf(BorrowKind::Ref, _, expr) = expr.kind; - if let ExprKind::Array(exprs) = expr.kind; - if exprs.len() == 1; + if let ExprKind::Array([expr]) = expr.kind; // struct `core::fmt::rt::v1::Argument` - if let ExprKind::Struct(_, fields, _) = exprs[0].kind; + if let ExprKind::Struct(_, fields, _) = expr.kind; if let Some(format_field) = fields.iter().find(|f| f.ident.name == sym::format); // struct `core::fmt::rt::v1::FormatSpec` if let ExprKind::Struct(_, fields, _) = format_field.expr.kind; diff --git a/clippy_lints/src/inherent_impl.rs b/clippy_lints/src/inherent_impl.rs index ee41c4aea..9641784eb 100644 --- a/clippy_lints/src/inherent_impl.rs +++ b/clippy_lints/src/inherent_impl.rs @@ -1,7 +1,7 @@ //! lint on inherent implementations use clippy_utils::diagnostics::span_lint_and_note; -use clippy_utils::{in_macro, is_allowed}; +use clippy_utils::{in_macro, is_lint_allowed}; use rustc_data_structures::fx::FxHashMap; use rustc_hir::{def_id::LocalDefId, Crate, Item, ItemKind, Node}; use rustc_lint::{LateContext, LateLintPass}; @@ -59,7 +59,7 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl { .filter(|(&id, impls)| { impls.len() > 1 // Check for `#[allow]` on the type definition - && !is_allowed( + && !is_lint_allowed( cx, MULTIPLE_INHERENT_IMPL, cx.tcx.hir().local_def_id_to_hir_id(id), @@ -123,7 +123,7 @@ fn get_impl_span(cx: &LateContext<'_>, id: LocalDefId) -> Option { .. }) = cx.tcx.hir().get(id) { - (!in_macro(span) && impl_item.generics.params.is_empty() && !is_allowed(cx, MULTIPLE_INHERENT_IMPL, id)) + (!in_macro(span) && impl_item.generics.params.is_empty() && !is_lint_allowed(cx, MULTIPLE_INHERENT_IMPL, id)) .then(|| span) } else { None diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index d69187f67..892b3af0b 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::{get_item_name, get_parent_as_impl, is_allowed}; +use clippy_utils::{get_item_name, get_parent_as_impl, is_lint_allowed}; use if_chain::if_chain; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; @@ -139,7 +139,7 @@ impl<'tcx> LateLintPass<'tcx> for LenZero { if let Some(ty_id) = cx.qpath_res(ty_path, imp.self_ty.hir_id).opt_def_id(); if let Some(local_id) = ty_id.as_local(); let ty_hir_id = cx.tcx.hir().local_def_id_to_hir_id(local_id); - if !is_allowed(cx, LEN_WITHOUT_IS_EMPTY, ty_hir_id); + if !is_lint_allowed(cx, LEN_WITHOUT_IS_EMPTY, ty_hir_id); if let Some(output) = parse_len_output(cx, cx.tcx.fn_sig(item.def_id).skip_binder()); then { let (name, kind) = match cx.tcx.hir().find(ty_hir_id) { diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index c29b3e7c7..1af3a215f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -338,6 +338,7 @@ mod size_of_in_element_count; mod slow_vector_initialization; mod stable_sort_primitive; mod strings; +mod strlen_on_c_strings; mod suspicious_operation_groupings; mod suspicious_trait_impl; mod swap; @@ -914,6 +915,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: strings::STRING_LIT_AS_BYTES, strings::STRING_TO_STRING, strings::STR_TO_STRING, + strlen_on_c_strings::STRLEN_ON_C_STRINGS, suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS, suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL, suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL, @@ -944,6 +946,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: types::LINKEDLIST, types::OPTION_OPTION, types::RC_BUFFER, + types::RC_MUTEX, types::REDUNDANT_ALLOCATION, types::TYPE_COMPLEXITY, types::VEC_BOX, @@ -1042,6 +1045,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(strings::STRING_TO_STRING), LintId::of(strings::STR_TO_STRING), LintId::of(types::RC_BUFFER), + LintId::of(types::RC_MUTEX), LintId::of(unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS), LintId::of(unwrap_in_result::UNWRAP_IN_RESULT), LintId::of(verbose_file_reads::VERBOSE_FILE_READS), @@ -1375,7 +1379,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES), LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS), - LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES), LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS), LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP), LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL), @@ -1409,6 +1412,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(slow_vector_initialization::SLOW_VECTOR_INITIALIZATION), LintId::of(stable_sort_primitive::STABLE_SORT_PRIMITIVE), LintId::of(strings::STRING_FROM_UTF8_AS_BYTES), + LintId::of(strlen_on_c_strings::STRLEN_ON_C_STRINGS), LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL), LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL), LintId::of(swap::ALMOST_SWAPPED), @@ -1546,7 +1550,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST), LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES), - LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES), LintId::of(ptr::CMP_NULL), LintId::of(ptr::PTR_ARG), LintId::of(ptr_eq::PTR_EQ), @@ -1639,6 +1642,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(reference::REF_IN_DEREF), LintId::of(repeat_once::REPEAT_ONCE), LintId::of(strings::STRING_FROM_UTF8_AS_BYTES), + LintId::of(strlen_on_c_strings::STRLEN_ON_C_STRINGS), LintId::of(swap::MANUAL_SWAP), LintId::of(temporary_assignment::TEMPORARY_ASSIGNMENT), LintId::of(transmute::CROSSPOINTER_TRANSMUTE), @@ -1791,6 +1795,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(missing_const_for_fn::MISSING_CONST_FOR_FN), LintId::of(mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL), LintId::of(mutex_atomic::MUTEX_INTEGER), + LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES), LintId::of(path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE), LintId::of(redundant_pub_crate::REDUNDANT_PUB_CRATE), LintId::of(regex::TRIVIAL_REGEX), @@ -2095,6 +2100,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || box missing_enforced_import_rename::ImportRename::new(import_renames.clone())); let scripts = conf.allowed_scripts.clone(); store.register_early_pass(move || box disallowed_script_idents::DisallowedScriptIdents::new(&scripts)); + store.register_late_pass(|| box strlen_on_c_strings::StrlenOnCStrings); } #[rustfmt::skip] diff --git a/clippy_lints/src/manual_map.rs b/clippy_lints/src/manual_map.rs index 97e4a983f..563d5cdb5 100644 --- a/clippy_lints/src/manual_map.rs +++ b/clippy_lints/src/manual_map.rs @@ -3,7 +3,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable}; use clippy_utils::{ - can_move_expr_to_closure, in_constant, is_allowed, is_else_clause, is_lang_ctor, path_to_local_id, + can_move_expr_to_closure, in_constant, is_else_clause, is_lang_ctor, is_lint_allowed, path_to_local_id, peel_hir_expr_refs, }; use rustc_ast::util::parser::PREC_POSTFIX; @@ -102,7 +102,7 @@ impl LateLintPass<'_> for ManualMap { // These two lints will go back and forth with each other. if cx.typeck_results().expr_ty(some_expr) == cx.tcx.types.unit - && !is_allowed(cx, OPTION_MAP_UNIT_FN, expr.hir_id) + && !is_lint_allowed(cx, OPTION_MAP_UNIT_FN, expr.hir_id) { return; } @@ -146,7 +146,7 @@ impl LateLintPass<'_> for ManualMap { }, _ => { if path_to_local_id(some_expr, id) - && !is_allowed(cx, MATCH_AS_REF, expr.hir_id) + && !is_lint_allowed(cx, MATCH_AS_REF, expr.hir_id) && binding_ref.is_some() { return; diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index f1e3492c4..986469a2b 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -7,7 +7,7 @@ use clippy_utils::sugg::Sugg; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type, peel_mid_ty_refs}; use clippy_utils::visitors::LocalUsedVisitor; use clippy_utils::{ - get_parent_expr, in_macro, is_allowed, is_expn_of, is_lang_ctor, is_refutable, is_wild, meets_msrv, msrvs, + get_parent_expr, in_macro, is_expn_of, is_lang_ctor, is_lint_allowed, is_refutable, is_wild, meets_msrv, msrvs, path_to_local, path_to_local_id, peel_hir_pat_refs, peel_n_hir_expr_refs, recurse_or_patterns, remove_blocks, strip_pat_refs, }; @@ -707,7 +707,7 @@ fn check_single_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], exp }; let ty = cx.typeck_results().expr_ty(ex); - if *ty.kind() != ty::Bool || is_allowed(cx, MATCH_BOOL, ex.hir_id) { + if *ty.kind() != ty::Bool || is_lint_allowed(cx, MATCH_BOOL, ex.hir_id) { check_single_match_single_pattern(cx, ex, arms, expr, els); check_single_match_opt_like(cx, ex, arms, expr, ty, els); } diff --git a/clippy_lints/src/methods/expect_fun_call.rs b/clippy_lints/src/methods/expect_fun_call.rs index 03cb41697..f8ee31a00 100644 --- a/clippy_lints/src/methods/expect_fun_call.rs +++ b/clippy_lints/src/methods/expect_fun_call.rs @@ -1,8 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::is_expn_of; -use clippy_utils::source::{snippet, snippet_with_applicability}; +use clippy_utils::higher::FormatExpn; +use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::is_type_diagnostic_item; -use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -94,27 +93,6 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_span: Spa } } - fn generate_format_arg_snippet( - cx: &LateContext<'_>, - a: &hir::Expr<'_>, - applicability: &mut Applicability, - ) -> Vec { - if_chain! { - if let hir::ExprKind::AddrOf(hir::BorrowKind::Ref, _, format_arg) = a.kind; - if let hir::ExprKind::Match(format_arg_expr, _, _) = format_arg.kind; - if let hir::ExprKind::Tup(format_arg_expr_tup) = format_arg_expr.kind; - - then { - format_arg_expr_tup - .iter() - .map(|a| snippet_with_applicability(cx, a.span, "..", applicability).into_owned()) - .collect() - } else { - unreachable!() - } - } - } - fn is_call(node: &hir::ExprKind<'_>) -> bool { match node { hir::ExprKind::AddrOf(hir::BorrowKind::Ref, _, expr) => { @@ -150,36 +128,22 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_span: Spa let mut applicability = Applicability::MachineApplicable; //Special handling for `format!` as arg_root - if_chain! { - if let hir::ExprKind::Block(block, None) = &arg_root.kind; - if block.stmts.len() == 1; - if let hir::StmtKind::Local(local) = &block.stmts[0].kind; - if let Some(arg_root) = &local.init; - if let hir::ExprKind::Call(inner_fun, inner_args) = arg_root.kind; - if is_expn_of(inner_fun.span, "format").is_some() && inner_args.len() == 1; - if let hir::ExprKind::Call(_, format_args) = &inner_args[0].kind; - then { - let fmt_spec = &format_args[0]; - let fmt_args = &format_args[1]; - - let mut args = vec![snippet(cx, fmt_spec.span, "..").into_owned()]; - - args.extend(generate_format_arg_snippet(cx, fmt_args, &mut applicability)); - - let sugg = args.join(", "); - - span_lint_and_sugg( - cx, - EXPECT_FUN_CALL, - span_replace_word, - &format!("use of `{}` followed by a function call", name), - "try this", - format!("unwrap_or_else({} panic!({}))", closure_args, sugg), - applicability, - ); - - return; - } + if let Some(format_expn) = FormatExpn::parse(arg_root) { + let span = match *format_expn.format_args.value_args { + [] => format_expn.format_args.format_string_span, + [.., last] => format_expn.format_args.format_string_span.to(last.span), + }; + let sugg = snippet_with_applicability(cx, span, "..", &mut applicability); + span_lint_and_sugg( + cx, + EXPECT_FUN_CALL, + span_replace_word, + &format!("use of `{}` followed by a function call", name), + "try this", + format!("unwrap_or_else({} panic!({}))", closure_args, sugg), + applicability, + ); + return; } let mut arg_root_snippet: Cow<'_, _> = snippet_with_applicability(cx, arg_root.span, "..", &mut applicability); diff --git a/clippy_lints/src/multiple_crate_versions.rs b/clippy_lints/src/multiple_crate_versions.rs index 584daa5e1..f5ce3e325 100644 --- a/clippy_lints/src/multiple_crate_versions.rs +++ b/clippy_lints/src/multiple_crate_versions.rs @@ -1,7 +1,7 @@ //! lint on multiple versions of a crate being used use clippy_utils::diagnostics::span_lint; -use clippy_utils::run_lints; +use clippy_utils::is_lint_allowed; use rustc_hir::def_id::LOCAL_CRATE; use rustc_hir::{Crate, CRATE_HIR_ID}; use rustc_lint::{LateContext, LateLintPass}; @@ -39,7 +39,7 @@ declare_lint_pass!(MultipleCrateVersions => [MULTIPLE_CRATE_VERSIONS]); impl LateLintPass<'_> for MultipleCrateVersions { fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) { - if !run_lints(cx, &[MULTIPLE_CRATE_VERSIONS], CRATE_HIR_ID) { + if is_lint_allowed(cx, MULTIPLE_CRATE_VERSIONS, CRATE_HIR_ID) { return; } diff --git a/clippy_lints/src/mut_key.rs b/clippy_lints/src/mut_key.rs index 6c87136e5..4dbbb14c5 100644 --- a/clippy_lints/src/mut_key.rs +++ b/clippy_lints/src/mut_key.rs @@ -120,8 +120,8 @@ fn is_mutable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span) -> bo }, Tuple(..) => ty.tuple_fields().any(|ty| is_mutable_type(cx, ty, span)), Adt(..) => { - cx.tcx.layout_of(cx.param_env.and(ty)).is_ok() - && !ty.has_escaping_bound_vars() + !ty.has_escaping_bound_vars() + && cx.tcx.layout_of(cx.param_env.and(ty)).is_ok() && !ty.is_freeze(cx.tcx.at(span), cx.param_env) }, _ => false, diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs index 3b3736fd3..780690548 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -71,6 +71,9 @@ declare_lint_pass!(NeedlessBool => [NEEDLESS_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() { + return; + } if let ExprKind::If(pred, then_block, Some(else_expr)) = e.kind { let reduce = |ret, not| { let mut applicability = Applicability::MachineApplicable; diff --git a/clippy_lints/src/nonstandard_macro_braces.rs b/clippy_lints/src/nonstandard_macro_braces.rs index 1adad5be6..043e7fa30 100644 --- a/clippy_lints/src/nonstandard_macro_braces.rs +++ b/clippy_lints/src/nonstandard_macro_braces.rs @@ -31,7 +31,7 @@ declare_clippy_lint! { /// vec![1, 2, 3]; /// ``` pub NONSTANDARD_MACRO_BRACES, - style, + nursery, "check consistent use of braces in macro" } @@ -92,14 +92,18 @@ impl EarlyLintPass for MacroBraces { } } -fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, this: &'a MacroBraces) -> Option> { +fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, mac_braces: &'a MacroBraces) -> Option> { if_chain! { if in_macro(span); - if let Some((name, braces)) = find_matching_macro(span, &this.macro_braces); + if let Some((name, braces)) = find_matching_macro(span, &mac_braces.macro_braces); if let Some(snip) = snippet_opt(cx, span.ctxt().outer_expn_data().call_site); - let c = snip.replace(" ", ""); // make formatting consistent + // we must check only invocation sites + // https://github.com/rust-lang/rust-clippy/issues/7422 + if snip.starts_with(name); + // make formatting consistent + let c = snip.replace(" ", ""); if !c.starts_with(&format!("{}!{}", name, braces.0)); - if !this.done.contains(&span.ctxt().outer_expn_data().call_site); + if !mac_braces.done.contains(&span.ctxt().outer_expn_data().call_site); then { Some((name, braces, snip)) } else { diff --git a/clippy_lints/src/panic_unimplemented.rs b/clippy_lints/src/panic_unimplemented.rs index 1a680e760..dc28874c1 100644 --- a/clippy_lints/src/panic_unimplemented.rs +++ b/clippy_lints/src/panic_unimplemented.rs @@ -74,7 +74,9 @@ declare_lint_pass!(PanicUnimplemented => [UNIMPLEMENTED, UNREACHABLE, TODO, PANI impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if match_panic_call(cx, expr).is_some() && is_expn_of(expr.span, "debug_assert").is_none() { + if match_panic_call(cx, expr).is_some() + && (is_expn_of(expr.span, "debug_assert").is_none() && is_expn_of(expr.span, "assert").is_none()) + { let span = get_outer_span(expr); if is_expn_of(expr.span, "unimplemented").is_some() { span_lint( diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index 12c444368..dba3b1805 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -4,7 +4,7 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_the use clippy_utils::ptr::get_spans; use clippy_utils::source::snippet_opt; use clippy_utils::ty::{is_type_diagnostic_item, match_type, walk_ptrs_hir_ty}; -use clippy_utils::{expr_path_res, is_allowed, match_any_def_paths, paths}; +use clippy_utils::{expr_path_res, is_lint_allowed, match_any_def_paths, paths}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{ @@ -246,7 +246,7 @@ fn check_fn(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_id: HirId, opt_body_id: for (idx, (arg, ty)) in decl.inputs.iter().zip(fn_ty.inputs()).enumerate() { // Honor the allow attribute on parameters. See issue 5644. if let Some(body) = &body { - if is_allowed(cx, PTR_ARG, body.params[idx].hir_id) { + if is_lint_allowed(cx, PTR_ARG, body.params[idx].hir_id) { continue; } } diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs index 119ec2152..7ba7ff3a3 100644 --- a/clippy_lints/src/redundant_clone.rs +++ b/clippy_lints/src/redundant_clone.rs @@ -12,6 +12,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::mir::{ self, traversal, visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor as _}, + Mutability, }; use rustc_middle::ty::{self, fold::TypeVisitor, Ty}; use rustc_mir::dataflow::{Analysis, AnalysisDomain, GenKill, GenKillAnalysis, ResultsCursor}; @@ -87,13 +88,18 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone { let mir = cx.tcx.optimized_mir(def_id.to_def_id()); + let possible_origin = { + let mut vis = PossibleOriginVisitor::new(mir); + vis.visit_body(mir); + vis.into_map(cx) + }; let maybe_storage_live_result = MaybeStorageLive .into_engine(cx.tcx, mir) .pass_name("redundant_clone") .iterate_to_fixpoint() .into_results_cursor(mir); let mut possible_borrower = { - let mut vis = PossibleBorrowerVisitor::new(cx, mir); + let mut vis = PossibleBorrowerVisitor::new(cx, mir, possible_origin); vis.visit_body(mir); vis.into_map(cx, maybe_storage_live_result) }; @@ -509,14 +515,20 @@ struct PossibleBorrowerVisitor<'a, 'tcx> { possible_borrower: TransitiveRelation, body: &'a mir::Body<'tcx>, cx: &'a LateContext<'tcx>, + possible_origin: FxHashMap>, } impl<'a, 'tcx> PossibleBorrowerVisitor<'a, 'tcx> { - fn new(cx: &'a LateContext<'tcx>, body: &'a mir::Body<'tcx>) -> Self { + fn new( + cx: &'a LateContext<'tcx>, + body: &'a mir::Body<'tcx>, + possible_origin: FxHashMap>, + ) -> Self { Self { possible_borrower: TransitiveRelation::default(), cx, body, + possible_origin, } } @@ -585,21 +597,105 @@ impl<'a, 'tcx> mir::visit::Visitor<'tcx> for PossibleBorrowerVisitor<'a, 'tcx> { .. } = &terminator.kind { + // TODO add doc // If the call returns something with lifetimes, // let's conservatively assume the returned value contains lifetime of all the arguments. // For example, given `let y: Foo<'a> = foo(x)`, `y` is considered to be a possible borrower of `x`. - if ContainsRegion.visit_ty(self.body.local_decls[*dest].ty).is_continue() { - return; - } + + let mut immutable_borrowers = vec![]; + let mut mutable_borrowers = vec![]; for op in args { match op { mir::Operand::Copy(p) | mir::Operand::Move(p) => { - self.possible_borrower.add(p.local, *dest); + if let ty::Ref(_, _, Mutability::Mut) = self.body.local_decls[p.local].ty.kind() { + mutable_borrowers.push(p.local); + } else { + immutable_borrowers.push(p.local); + } }, mir::Operand::Constant(..) => (), } } + + let mut mutable_variables: Vec = mutable_borrowers + .iter() + .filter_map(|r| self.possible_origin.get(r)) + .flat_map(HybridBitSet::iter) + .collect(); + + if ContainsRegion.visit_ty(self.body.local_decls[*dest].ty).is_break() { + mutable_variables.push(*dest); + } + + for y in mutable_variables { + for x in &immutable_borrowers { + self.possible_borrower.add(*x, y); + } + for x in &mutable_borrowers { + self.possible_borrower.add(*x, y); + } + } + } + } +} + +/// Collect possible borrowed for every `&mut` local. +/// For exampel, `_1 = &mut _2` generate _1: {_2,...} +/// Known Problems: not sure all borrowed are tracked +struct PossibleOriginVisitor<'a, 'tcx> { + possible_origin: TransitiveRelation, + body: &'a mir::Body<'tcx>, +} + +impl<'a, 'tcx> PossibleOriginVisitor<'a, 'tcx> { + fn new(body: &'a mir::Body<'tcx>) -> Self { + Self { + possible_origin: TransitiveRelation::default(), + body, + } + } + + fn into_map(self, cx: &LateContext<'tcx>) -> FxHashMap> { + let mut map = FxHashMap::default(); + for row in (1..self.body.local_decls.len()).map(mir::Local::from_usize) { + if is_copy(cx, self.body.local_decls[row].ty) { + continue; + } + + let borrowers = self.possible_origin.reachable_from(&row); + if !borrowers.is_empty() { + let mut bs = HybridBitSet::new_empty(self.body.local_decls.len()); + for &c in borrowers { + if c != mir::Local::from_usize(0) { + bs.insert(c); + } + } + + if !bs.is_empty() { + map.insert(row, bs); + } + } + } + map + } +} + +impl<'a, 'tcx> mir::visit::Visitor<'tcx> for PossibleOriginVisitor<'a, 'tcx> { + fn visit_assign(&mut self, place: &mir::Place<'tcx>, rvalue: &mir::Rvalue<'_>, _location: mir::Location) { + let lhs = place.local; + match rvalue { + // Only consider `&mut`, which can modify origin place + mir::Rvalue::Ref(_, rustc_middle::mir::BorrowKind::Mut { .. }, borrowed) | + // _2: &mut _; + // _3 = move _2 + mir::Rvalue::Use(mir::Operand::Move(borrowed)) | + // _3 = move _2 as &mut _; + mir::Rvalue::Cast(_, mir::Operand::Move(borrowed), _) + => { + self.possible_origin.add(lhs, borrowed.local); + }, + _ => {}, } } } diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs index 9d91b53e1..958e46212 100644 --- a/clippy_lints/src/strings.rs +++ b/clippy_lints/src/strings.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sug use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::SpanlessEq; -use clippy_utils::{get_parent_expr, is_allowed, match_function_call, method_calls, paths}; +use clippy_utils::{get_parent_expr, is_lint_allowed, match_function_call, method_calls, paths}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, LangItem, QPath}; @@ -124,7 +124,7 @@ impl<'tcx> LateLintPass<'tcx> for StringAdd { ) = e.kind { if is_string(cx, left) { - if !is_allowed(cx, STRING_ADD_ASSIGN, e.hir_id) { + if !is_lint_allowed(cx, STRING_ADD_ASSIGN, e.hir_id) { let parent = get_parent_expr(cx, e); if let Some(p) = parent { if let ExprKind::Assign(target, _, _) = p.kind { diff --git a/clippy_lints/src/strlen_on_c_strings.rs b/clippy_lints/src/strlen_on_c_strings.rs new file mode 100644 index 000000000..2ccf3a379 --- /dev/null +++ b/clippy_lints/src/strlen_on_c_strings.rs @@ -0,0 +1,82 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::in_macro; +use clippy_utils::paths; +use clippy_utils::source::snippet_with_macro_callsite; +use clippy_utils::ty::{is_type_diagnostic_item, is_type_ref_to_diagnostic_item}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::{sym, Symbol}; + +declare_clippy_lint! { + /// **What it does:** Checks for usage of `libc::strlen` on a `CString` or `CStr` value, + /// and suggest calling `as_bytes().len()` or `to_bytes().len()` respectively instead. + /// + /// **Why is this bad?** This avoids calling an unsafe `libc` function. + /// Currently, it also avoids calculating the length. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust, ignore + /// use std::ffi::CString; + /// let cstring = CString::new("foo").expect("CString::new failed"); + /// let len = unsafe { libc::strlen(cstring.as_ptr()) }; + /// ``` + /// Use instead: + /// ```rust, no_run + /// use std::ffi::CString; + /// let cstring = CString::new("foo").expect("CString::new failed"); + /// let len = cstring.as_bytes().len(); + /// ``` + pub STRLEN_ON_C_STRINGS, + complexity, + "using `libc::strlen` on a `CString` or `CStr` value, while `as_bytes().len()` or `to_bytes().len()` respectively can be used instead" +} + +declare_lint_pass!(StrlenOnCStrings => [STRLEN_ON_C_STRINGS]); + +impl LateLintPass<'tcx> for StrlenOnCStrings { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { + if in_macro(expr.span) { + return; + } + + if_chain! { + if let hir::ExprKind::Call(func, [recv]) = expr.kind; + if let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = func.kind; + + if (&paths::LIBC_STRLEN).iter().map(|x| Symbol::intern(x)).eq( + path.segments.iter().map(|seg| seg.ident.name)); + if let hir::ExprKind::MethodCall(path, _, args, _) = recv.kind; + if args.len() == 1; + if !args.iter().any(|e| e.span.from_expansion()); + if path.ident.name == sym::as_ptr; + then { + let cstring = &args[0]; + let ty = cx.typeck_results().expr_ty(cstring); + let val_name = snippet_with_macro_callsite(cx, cstring.span, ".."); + let sugg = if is_type_diagnostic_item(cx, ty, sym::cstring_type){ + format!("{}.as_bytes().len()", val_name) + } else if is_type_ref_to_diagnostic_item(cx, ty, sym::CStr){ + format!("{}.to_bytes().len()", val_name) + } else { + return; + }; + + span_lint_and_sugg( + cx, + STRLEN_ON_C_STRINGS, + expr.span, + "using `libc::strlen` on a `CString` or `CStr` value", + "try this (you might also need to get rid of `unsafe` block in some cases):", + sugg, + Applicability::Unspecified // Sometimes unnecessary `unsafe` block + ); + } + } + } +} diff --git a/clippy_lints/src/types/mod.rs b/clippy_lints/src/types/mod.rs index 70b9e8ade..7d629b545 100644 --- a/clippy_lints/src/types/mod.rs +++ b/clippy_lints/src/types/mod.rs @@ -3,6 +3,7 @@ mod box_vec; mod linked_list; mod option_option; mod rc_buffer; +mod rc_mutex; mod redundant_allocation; mod type_complexity; mod utils; @@ -177,8 +178,8 @@ declare_clippy_lint! { declare_clippy_lint! { /// **What it does:** Checks for use of redundant allocations anywhere in the code. /// - /// **Why is this bad?** Expressions such as `Rc<&T>`, `Rc>`, `Rc>`, `Box<&T>` - /// add an unnecessary level of indirection. + /// **Why is this bad?** Expressions such as `Rc<&T>`, `Rc>`, `Rc>`, `Rc>`, Arc<&T>`, `Arc>`, + /// `Arc>`, `Arc>`, `Box<&T>`, `Box>`, `Box>`, `Box>`, add an unnecessary level of indirection. /// /// **Known problems:** None. /// @@ -250,12 +251,41 @@ declare_clippy_lint! { "usage of very complex types that might be better factored into `type` definitions" } +declare_clippy_lint! { + /// **What it does:** Checks for `Rc>`. + /// + /// **Why is this bad?** `Rc` is used in single thread and `Mutex` is used in multi thread. + /// Consider using `Rc>` in single thread or `Arc>` in multi thread. + /// + /// **Known problems:** Sometimes combining generic types can lead to the requirement that a + /// type use Rc in conjunction with Mutex. We must consider those cases false positives, but + /// alas they are quite hard to rule out. Luckily they are also rare. + /// + /// **Example:** + /// ```rust,ignore + /// use std::rc::Rc; + /// use std::sync::Mutex; + /// fn foo(interned: Rc>) { ... } + /// ``` + /// + /// Better: + /// + /// ```rust,ignore + /// use std::rc::Rc; + /// use std::cell::RefCell + /// fn foo(interned: Rc>) { ... } + /// ``` + pub RC_MUTEX, + restriction, + "usage of `Rc>`" +} + pub struct Types { vec_box_size_threshold: u64, type_complexity_threshold: u64, } -impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, TYPE_COMPLEXITY]); +impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]); impl<'tcx> LateLintPass<'tcx> for Types { fn check_fn(&mut self, cx: &LateContext<'_>, _: FnKind<'_>, decl: &FnDecl<'_>, _: &Body<'_>, _: Span, id: HirId) { @@ -375,6 +405,7 @@ impl Types { triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold); triggered |= option_option::check(cx, hir_ty, qpath, def_id); triggered |= linked_list::check(cx, hir_ty, def_id); + triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id); if triggered { return; diff --git a/clippy_lints/src/types/rc_mutex.rs b/clippy_lints/src/types/rc_mutex.rs new file mode 100644 index 000000000..bd7a0ee64 --- /dev/null +++ b/clippy_lints/src/types/rc_mutex.rs @@ -0,0 +1,27 @@ +use clippy_utils::diagnostics::span_lint; +use clippy_utils::is_ty_param_diagnostic_item; +use if_chain::if_chain; +use rustc_hir::{self as hir, def_id::DefId, QPath}; +use rustc_lint::LateContext; +use rustc_span::symbol::sym; + +use super::RC_MUTEX; + +pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_>, def_id: DefId) -> bool { + if_chain! { + if cx.tcx.is_diagnostic_item(sym::Rc, def_id) ; + if let Some(_) = is_ty_param_diagnostic_item(cx, qpath, sym!(mutex_type)) ; + + then{ + span_lint( + cx, + RC_MUTEX, + hir_ty.span, + "found `Rc>`. Consider using `Rc>` or `Arc>` instead", + ); + return true; + } + } + + false +} diff --git a/clippy_lints/src/types/redundant_allocation.rs b/clippy_lints/src/types/redundant_allocation.rs index c0c1f3405..8e83dcbf7 100644 --- a/clippy_lints/src/types/redundant_allocation.rs +++ b/clippy_lints/src/types/redundant_allocation.rs @@ -1,5 +1,5 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet_with_applicability; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::{get_qpath_generic_tys, is_ty_param_diagnostic_item, is_ty_param_lang_item}; use rustc_errors::Applicability; use rustc_hir::{self as hir, def_id::DefId, LangItem, QPath, TyKind}; @@ -9,74 +9,99 @@ use rustc_span::symbol::sym; use super::{utils, REDUNDANT_ALLOCATION}; pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, qpath: &QPath<'_>, def_id: DefId) -> bool { - if Some(def_id) == cx.tcx.lang_items().owned_box() { - if let Some(span) = utils::match_borrows_parameter(cx, qpath) { - let mut applicability = Applicability::MachineApplicable; - span_lint_and_sugg( - cx, - REDUNDANT_ALLOCATION, - hir_ty.span, - "usage of `Box<&T>`", - "try", - snippet_with_applicability(cx, span, "..", &mut applicability).to_string(), - applicability, - ); - return true; - } + let outer_sym = if Some(def_id) == cx.tcx.lang_items().owned_box() { + "Box" + } else if cx.tcx.is_diagnostic_item(sym::Rc, def_id) { + "Rc" + } else if cx.tcx.is_diagnostic_item(sym::Arc, def_id) { + "Arc" + } else { + return false; + }; + + if let Some(span) = utils::match_borrows_parameter(cx, qpath) { + let mut applicability = Applicability::MaybeIncorrect; + let generic_snippet = snippet_with_applicability(cx, span, "..", &mut applicability); + span_lint_and_then( + cx, + REDUNDANT_ALLOCATION, + hir_ty.span, + &format!("usage of `{}<{}>`", outer_sym, generic_snippet), + |diag| { + diag.span_suggestion(hir_ty.span, "try", format!("{}", generic_snippet), applicability); + diag.note(&format!( + "`{generic}` is already a pointer, `{outer}<{generic}>` allocates a pointer on the heap", + outer = outer_sym, + generic = generic_snippet + )); + }, + ); + return true; } - if cx.tcx.is_diagnostic_item(sym::Rc, def_id) { - if let Some(ty) = is_ty_param_diagnostic_item(cx, qpath, sym::Rc) { - let mut applicability = Applicability::MachineApplicable; - span_lint_and_sugg( - cx, - REDUNDANT_ALLOCATION, - hir_ty.span, - "usage of `Rc>`", - "try", - snippet_with_applicability(cx, ty.span, "..", &mut applicability).to_string(), - applicability, - ); - true - } else if let Some(ty) = is_ty_param_lang_item(cx, qpath, LangItem::OwnedBox) { - let qpath = match &ty.kind { - TyKind::Path(qpath) => qpath, - _ => return false, - }; - let inner_span = match get_qpath_generic_tys(qpath).next() { - Some(ty) => ty.span, - None => return false, - }; - let mut applicability = Applicability::MachineApplicable; - span_lint_and_sugg( - cx, - REDUNDANT_ALLOCATION, - hir_ty.span, - "usage of `Rc>`", - "try", - format!( - "Rc<{}>", - snippet_with_applicability(cx, inner_span, "..", &mut applicability) - ), - applicability, - ); - true - } else { - utils::match_borrows_parameter(cx, qpath).map_or(false, |span| { - let mut applicability = Applicability::MachineApplicable; - span_lint_and_sugg( - cx, - REDUNDANT_ALLOCATION, + let (inner_sym, ty) = if let Some(ty) = is_ty_param_lang_item(cx, qpath, LangItem::OwnedBox) { + ("Box", ty) + } else if let Some(ty) = is_ty_param_diagnostic_item(cx, qpath, sym::Rc) { + ("Rc", ty) + } else if let Some(ty) = is_ty_param_diagnostic_item(cx, qpath, sym::Arc) { + ("Arc", ty) + } else { + return false; + }; + + let inner_qpath = match &ty.kind { + TyKind::Path(inner_qpath) => inner_qpath, + _ => return false, + }; + let inner_span = match get_qpath_generic_tys(inner_qpath).next() { + Some(ty) => ty.span, + None => return false, + }; + if inner_sym == outer_sym { + let mut applicability = Applicability::MaybeIncorrect; + let generic_snippet = snippet_with_applicability(cx, inner_span, "..", &mut applicability); + span_lint_and_then( + cx, + REDUNDANT_ALLOCATION, + hir_ty.span, + &format!("usage of `{}<{}<{}>>`", outer_sym, inner_sym, generic_snippet), + |diag| { + diag.span_suggestion( hir_ty.span, - "usage of `Rc<&T>`", "try", - snippet_with_applicability(cx, span, "..", &mut applicability).to_string(), + format!("{}<{}>", outer_sym, generic_snippet), applicability, ); - true - }) - } + diag.note(&format!( + "`{inner}<{generic}>` is already on the heap, `{outer}<{inner}<{generic}>>` makes an extra allocation", + outer = outer_sym, + inner = inner_sym, + generic = generic_snippet + )); + }, + ); } else { - false + let generic_snippet = snippet(cx, inner_span, ".."); + span_lint_and_then( + cx, + REDUNDANT_ALLOCATION, + hir_ty.span, + &format!("usage of `{}<{}<{}>>`", outer_sym, inner_sym, generic_snippet), + |diag| { + diag.note(&format!( + "`{inner}<{generic}>` is already on the heap, `{outer}<{inner}<{generic}>>` makes an extra allocation", + outer = outer_sym, + inner = inner_sym, + generic = generic_snippet + )); + diag.help(&format!( + "consider using just `{outer}<{generic}>` or `{inner}<{generic}>`", + outer = outer_sym, + inner = inner_sym, + generic = generic_snippet + )); + }, + ); } + true } diff --git a/clippy_lints/src/unicode.rs b/clippy_lints/src/unicode.rs index 45291a120..2f0a61898 100644 --- a/clippy_lints/src/unicode.rs +++ b/clippy_lints/src/unicode.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::is_allowed; +use clippy_utils::is_lint_allowed; use clippy_utils::source::snippet; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; @@ -114,7 +114,7 @@ fn check_str(cx: &LateContext<'_>, span: Span, id: HirId) { span, "literal non-ASCII character detected", "consider replacing the string with", - if is_allowed(cx, UNICODE_NOT_NFC, id) { + if is_lint_allowed(cx, UNICODE_NOT_NFC, id) { escape(string.chars()) } else { escape(string.nfc()) @@ -122,7 +122,7 @@ fn check_str(cx: &LateContext<'_>, span: Span, id: HirId) { Applicability::MachineApplicable, ); } - if is_allowed(cx, NON_ASCII_LITERAL, id) && string.chars().zip(string.nfc()).any(|(a, b)| a != b) { + if is_lint_allowed(cx, NON_ASCII_LITERAL, id) && string.chars().zip(string.nfc()).any(|(a, b)| a != b) { span_lint_and_sugg( cx, UNICODE_NOT_NFC, diff --git a/clippy_lints/src/unused_unit.rs b/clippy_lints/src/unused_unit.rs index e14945651..ab0cdf75f 100644 --- a/clippy_lints/src/unused_unit.rs +++ b/clippy_lints/src/unused_unit.rs @@ -24,6 +24,10 @@ declare_clippy_lint! { /// () /// } /// ``` + /// is equivalent to + /// ```rust + /// fn return_unit() {} + /// ``` pub UNUSED_UNIT, style, "needless unit expression" diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index 906ac10f4..71117e967 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -87,11 +87,8 @@ const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element"; impl<'tcx> LateLintPass<'tcx> for UseSelf { fn check_item(&mut self, _cx: &LateContext<'_>, item: &Item<'_>) { - if !is_item_interesting(item) { - // This does two things: - // 1) Reduce needless churn on `self.stack` - // 2) Don't push `StackItem::NoCheck` when entering `ItemKind::OpaqueTy`, - // in order to lint `foo() -> impl <..>` + if matches!(item.kind, ItemKind::OpaqueTy(_)) { + // skip over `ItemKind::OpaqueTy` in order to lint `foo() -> impl <..>` return; } // We push the self types of `impl`s on a stack here. Only the top type on the stack is @@ -119,7 +116,7 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { } fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) { - if is_item_interesting(item) { + if !matches!(item.kind, ItemKind::OpaqueTy(_)) { self.stack.pop(); } } @@ -297,11 +294,3 @@ fn lint_path_to_variant(cx: &LateContext<'_>, path: &Path<'_>) { span_lint(cx, span); } } - -fn is_item_interesting(item: &Item<'_>) -> bool { - use rustc_hir::ItemKind::{Const, Enum, Fn, Impl, Static, Struct, Trait, Union}; - matches!( - item.kind, - Impl { .. } | Static(..) | Const(..) | Fn(..) | Enum(..) | Struct(..) | Union(..) | Trait(..) - ) -} diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index b1523e032..668807f49 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -3,8 +3,8 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sug use clippy_utils::source::snippet; use clippy_utils::ty::match_type; use clippy_utils::{ - is_else_clause, is_expn_of, is_expr_path_def_path, match_def_path, method_calls, path_to_res, paths, run_lints, - SpanlessEq, + is_else_clause, is_expn_of, is_expr_path_def_path, is_lint_allowed, match_def_path, method_calls, path_to_res, + paths, SpanlessEq, }; use if_chain::if_chain; use rustc_ast::ast::{Crate as AstCrate, ItemKind, LitKind, ModKind, NodeId}; @@ -353,7 +353,7 @@ impl_lint_pass!(LintWithoutLintPass => [DEFAULT_LINT, LINT_WITHOUT_LINT_PASS]); impl<'tcx> LateLintPass<'tcx> for LintWithoutLintPass { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { - if !run_lints(cx, &[DEFAULT_LINT], item.hir_id()) { + if is_lint_allowed(cx, DEFAULT_LINT, item.hir_id()) { return; } @@ -411,7 +411,7 @@ impl<'tcx> LateLintPass<'tcx> for LintWithoutLintPass { } fn check_crate_post(&mut self, cx: &LateContext<'tcx>, _: &'tcx Crate<'_>) { - if !run_lints(cx, &[LINT_WITHOUT_LINT_PASS], CRATE_HIR_ID) { + if is_lint_allowed(cx, LINT_WITHOUT_LINT_PASS, CRATE_HIR_ID) { return; } @@ -497,7 +497,7 @@ impl_lint_pass!(CompilerLintFunctions => [COMPILER_LINT_FUNCTIONS]); impl<'tcx> LateLintPass<'tcx> for CompilerLintFunctions { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if !run_lints(cx, &[COMPILER_LINT_FUNCTIONS], expr.hir_id) { + if is_lint_allowed(cx, COMPILER_LINT_FUNCTIONS, expr.hir_id) { return; } @@ -526,7 +526,7 @@ declare_lint_pass!(OuterExpnDataPass => [OUTER_EXPN_EXPN_DATA]); impl<'tcx> LateLintPass<'tcx> for OuterExpnDataPass { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { - if !run_lints(cx, &[OUTER_EXPN_EXPN_DATA], expr.hir_id) { + if is_lint_allowed(cx, OUTER_EXPN_EXPN_DATA, expr.hir_id) { return; } @@ -576,7 +576,7 @@ declare_lint_pass!(CollapsibleCalls => [COLLAPSIBLE_SPAN_LINT_CALLS]); impl<'tcx> LateLintPass<'tcx> for CollapsibleCalls { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { - if !run_lints(cx, &[COLLAPSIBLE_SPAN_LINT_CALLS], expr.hir_id) { + if is_lint_allowed(cx, COLLAPSIBLE_SPAN_LINT_CALLS, expr.hir_id) { return; } @@ -757,7 +757,7 @@ declare_lint_pass!(MatchTypeOnDiagItem => [MATCH_TYPE_ON_DIAGNOSTIC_ITEM]); impl<'tcx> LateLintPass<'tcx> for MatchTypeOnDiagItem { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { - if !run_lints(cx, &[MATCH_TYPE_ON_DIAGNOSTIC_ITEM], expr.hir_id) { + if is_lint_allowed(cx, MATCH_TYPE_ON_DIAGNOSTIC_ITEM, expr.hir_id) { return; } diff --git a/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/clippy_lints/src/utils/internal_lints/metadata_collector.rs index e877af09e..3eccc89cd 100644 --- a/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -520,7 +520,9 @@ fn get_lint_group_and_level_or_lint( lint_name: &str, item: &'hir Item<'_>, ) -> Option<(String, &'static str)> { - let result = cx.lint_store.check_lint_name(lint_name, Some(sym::clippy)); + let result = cx + .lint_store + .check_lint_name(cx.sess(), lint_name, Some(sym::clippy), &[]); if let CheckLintNameResult::Tool(Ok(lint_lst)) = result { if let Some(group) = get_lint_group(cx, lint_lst[0]) { if EXCLUDED_LINT_GROUPS.contains(&group.as_str()) { diff --git a/clippy_lints/src/wildcard_dependencies.rs b/clippy_lints/src/wildcard_dependencies.rs index 60c3489a4..1ca1117a4 100644 --- a/clippy_lints/src/wildcard_dependencies.rs +++ b/clippy_lints/src/wildcard_dependencies.rs @@ -1,5 +1,4 @@ -use clippy_utils::diagnostics::span_lint; -use clippy_utils::run_lints; +use clippy_utils::{diagnostics::span_lint, is_lint_allowed}; use rustc_hir::{hir_id::CRATE_HIR_ID, Crate}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -31,7 +30,7 @@ declare_lint_pass!(WildcardDependencies => [WILDCARD_DEPENDENCIES]); impl LateLintPass<'_> for WildcardDependencies { fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) { - if !run_lints(cx, &[WILDCARD_DEPENDENCIES], CRATE_HIR_ID) { + if is_lint_allowed(cx, WILDCARD_DEPENDENCIES, CRATE_HIR_ID) { return; } diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index 8be36756b..3e3e472e9 100644 --- a/clippy_utils/src/higher.rs +++ b/clippy_utils/src/higher.rs @@ -5,11 +5,11 @@ use crate::{is_expn_of, match_def_path, paths}; use if_chain::if_chain; -use rustc_ast::ast; +use rustc_ast::ast::{self, LitKind}; use rustc_hir as hir; use rustc_hir::{BorrowKind, Expr, ExprKind, StmtKind, UnOp}; use rustc_lint::LateContext; -use rustc_span::source_map::Span; +use rustc_span::{sym, ExpnKind, Span, Symbol}; /// Converts a hir binary operator to the corresponding `ast` type. #[must_use] @@ -266,3 +266,107 @@ pub fn extract_assert_macro_args<'tcx>(e: &'tcx Expr<'tcx>) -> Option { + /// Span of `format!(..)` + pub call_site: Span, + /// Inner `format_args!` expansion + pub format_args: FormatArgsExpn<'tcx>, +} + +impl FormatExpn<'tcx> { + /// Parses an expanded `format!` invocation + pub fn parse(expr: &'tcx Expr<'tcx>) -> Option { + if_chain! { + if let ExprKind::Block(block, _) = expr.kind; + if let [stmt] = block.stmts; + if let StmtKind::Local(local) = stmt.kind; + if let Some(init) = local.init; + if let ExprKind::Call(_, [format_args]) = init.kind; + let expn_data = expr.span.ctxt().outer_expn_data(); + if let ExpnKind::Macro(_, sym::format) = expn_data.kind; + if let Some(format_args) = FormatArgsExpn::parse(format_args); + then { + Some(FormatExpn { + call_site: expn_data.call_site, + format_args, + }) + } else { + None + } + } + } +} + +/// A parsed `format_args!` expansion +pub struct FormatArgsExpn<'tcx> { + /// Span of the first argument, the format string + pub format_string_span: Span, + /// Values passed after the format string + pub value_args: Vec<&'tcx Expr<'tcx>>, + + /// String literal expressions which represent the format string split by "{}" + pub format_string_parts: &'tcx [Expr<'tcx>], + /// Symbols corresponding to [`format_string_parts`] + pub format_string_symbols: Vec, + /// Expressions like `ArgumentV1::new(arg0, Debug::fmt)` + pub args: &'tcx [Expr<'tcx>], + /// The final argument passed to `Arguments::new_v1_formatted`, if applicable + pub fmt_expr: Option<&'tcx Expr<'tcx>>, +} + +impl FormatArgsExpn<'tcx> { + /// Parses an expanded `format_args!` or `format_args_nl!` invocation + pub fn parse(expr: &'tcx Expr<'tcx>) -> Option { + if_chain! { + if let ExpnKind::Macro(_, name) = expr.span.ctxt().outer_expn_data().kind; + let name = name.as_str(); + if name.ends_with("format_args") || name.ends_with("format_args_nl"); + if let ExprKind::Call(_, args) = expr.kind; + if let Some((strs_ref, args, fmt_expr)) = match args { + // Arguments::new_v1 + [strs_ref, args] => Some((strs_ref, args, None)), + // Arguments::new_v1_formatted + [strs_ref, args, fmt_expr] => Some((strs_ref, args, Some(fmt_expr))), + _ => None, + }; + if let ExprKind::AddrOf(BorrowKind::Ref, _, strs_arr) = strs_ref.kind; + if let ExprKind::Array(format_string_parts) = strs_arr.kind; + if let Some(format_string_symbols) = format_string_parts + .iter() + .map(|e| { + if let ExprKind::Lit(lit) = &e.kind { + if let LitKind::Str(symbol, _style) = lit.node { + return Some(symbol); + } + } + None + }) + .collect(); + if let ExprKind::AddrOf(BorrowKind::Ref, _, args) = args.kind; + if let ExprKind::Match(args, [arm], _) = args.kind; + if let ExprKind::Tup(value_args) = args.kind; + if let Some(value_args) = value_args + .iter() + .map(|e| match e.kind { + ExprKind::AddrOf(_, _, e) => Some(e), + _ => None, + }) + .collect(); + if let ExprKind::Array(args) = arm.body.kind; + then { + Some(FormatArgsExpn { + format_string_span: strs_ref.span, + value_args, + format_string_parts, + format_string_symbols, + args, + fmt_expr, + }) + } else { + None + } + } + } +} diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 2f1047218..6db221ab0 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1163,7 +1163,7 @@ pub fn is_try<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Option<&'tc /// Returns `true` if the lint is allowed in the current context /// /// Useful for skipping long running code when it's unnecessary -pub fn is_allowed(cx: &LateContext<'_>, lint: &'static Lint, id: HirId) -> bool { +pub fn is_lint_allowed(cx: &LateContext<'_>, lint: &'static Lint, id: HirId) -> bool { cx.tcx.lint_level_at_node(lint, id).0 == Level::Allow } @@ -1531,25 +1531,6 @@ pub fn fn_def_id(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { } } -/// This function checks if any of the lints in the slice is enabled for the provided `HirId`. -/// A lint counts as enabled with any of the levels: `Level::Forbid` | `Level::Deny` | `Level::Warn` -/// -/// ```ignore -/// #[deny(clippy::YOUR_AWESOME_LINT)] -/// println!("Hello, World!"); // <- Clippy code: run_lints(cx, &[YOUR_AWESOME_LINT], id) == true -/// -/// #[allow(clippy::YOUR_AWESOME_LINT)] -/// println!("See you soon!"); // <- Clippy code: run_lints(cx, &[YOUR_AWESOME_LINT], id) == false -/// ``` -pub fn run_lints(cx: &LateContext<'_>, lints: &[&'static Lint], id: HirId) -> bool { - lints.iter().any(|lint| { - matches!( - cx.tcx.lint_level_at_node(lint, id), - (Level::Forbid | Level::Deny | Level::Warn, _) - ) - }) -} - /// Returns Option where String is a textual representation of the type encapsulated in the /// slice iff the given expression is a slice of primitives (as defined in the /// `is_recursively_primitive_type` function) and None otherwise. diff --git a/clippy_utils/src/numeric_literal.rs b/clippy_utils/src/numeric_literal.rs index 546706d51..4a28c7dd9 100644 --- a/clippy_utils/src/numeric_literal.rs +++ b/clippy_utils/src/numeric_literal.rs @@ -162,6 +162,9 @@ impl<'a> NumericLiteral<'a> { } if let Some(suffix) = self.suffix { + if output.ends_with('.') { + output.push('0'); + } output.push('_'); output.push_str(suffix); } diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index b913d1ce8..c960eec30 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -38,7 +38,6 @@ pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "defa pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"]; pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"]; pub const DIR_BUILDER: [&str; 3] = ["std", "fs", "DirBuilder"]; -pub const DISPLAY_FMT_METHOD: [&str; 4] = ["core", "fmt", "Display", "fmt"]; pub const DISPLAY_TRAIT: [&str; 3] = ["core", "fmt", "Display"]; pub const DOUBLE_ENDED_ITERATOR: [&str; 4] = ["core", "iter", "traits", "DoubleEndedIterator"]; pub const DROP: [&str; 3] = ["core", "mem", "drop"]; @@ -50,9 +49,6 @@ pub const F32_EPSILON: [&str; 4] = ["core", "f32", "", "EPSILON"]; pub const F64_EPSILON: [&str; 4] = ["core", "f64", "", "EPSILON"]; pub const FILE: [&str; 3] = ["std", "fs", "File"]; pub const FILE_TYPE: [&str; 3] = ["std", "fs", "FileType"]; -pub const FMT_ARGUMENTS_NEW_V1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"]; -pub const FMT_ARGUMENTS_NEW_V1_FORMATTED: [&str; 4] = ["core", "fmt", "Arguments", "new_v1_formatted"]; -pub const FMT_ARGUMENTV1_NEW: [&str; 4] = ["core", "fmt", "ArgumentV1", "new"]; pub const FROM_FROM: [&str; 4] = ["core", "convert", "From", "from"]; pub const FROM_ITERATOR: [&str; 5] = ["core", "iter", "traits", "collect", "FromIterator"]; pub const FROM_ITERATOR_METHOD: [&str; 6] = ["core", "iter", "traits", "collect", "FromIterator", "from_iter"]; @@ -82,6 +78,7 @@ pub const ITER_REPEAT: [&str; 5] = ["core", "iter", "sources", "repeat", "repeat pub const KW_MODULE: [&str; 3] = ["rustc_span", "symbol", "kw"]; #[cfg(feature = "internal-lints")] pub const LATE_CONTEXT: [&str; 2] = ["rustc_lint", "LateContext"]; +pub const LIBC_STRLEN: [&str; 2] = ["libc", "strlen"]; pub const LINKED_LIST: [&str; 4] = ["alloc", "collections", "linked_list", "LinkedList"]; #[cfg(any(feature = "internal-lints", feature = "metadata-collector-lint"))] pub const LINT: [&str; 2] = ["rustc_lint_defs", "Lint"]; diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index 8857e77d8..3f5c5604d 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -14,8 +14,8 @@ use rustc_middle::ty::{self, AdtDef, IntTy, Ty, TypeFoldable, UintTy}; use rustc_span::sym; use rustc_span::symbol::{Ident, Symbol}; use rustc_span::DUMMY_SP; -use rustc_trait_selection::traits::query::normalize::AtExt; use rustc_trait_selection::infer::InferCtxtExt; +use rustc_trait_selection::traits::query::normalize::AtExt; use crate::{match_def_path, must_use_attr}; @@ -129,10 +129,11 @@ pub fn implements_trait<'tcx>( return false; } let ty_params = cx.tcx.mk_substs(ty_params.iter()); - cx.tcx.infer_ctxt().enter(|infcx| - infcx.type_implements_trait(trait_id, ty, ty_params, cx.param_env) - .must_apply_modulo_regions() - ) + cx.tcx.infer_ctxt().enter(|infcx| { + infcx + .type_implements_trait(trait_id, ty, ty_params, cx.param_env) + .must_apply_modulo_regions() + }) } /// Checks whether this type implements `Drop`. @@ -235,6 +236,17 @@ pub fn is_recursively_primitive_type(ty: Ty<'_>) -> bool { } } +/// Checks if the type is a reference equals to a diagnostic item +pub fn is_type_ref_to_diagnostic_item(cx: &LateContext<'_>, ty: Ty<'_>, diag_item: Symbol) -> bool { + match ty.kind() { + ty::Ref(_, ref_ty, _) => match ref_ty.kind() { + ty::Adt(adt, _) => cx.tcx.is_diagnostic_item(diag_item, adt.did), + _ => false, + }, + _ => false, + } +} + /// Checks if the type is equal to a diagnostic item /// /// If you change the signature, remember to update the internal lint `MatchTypeOnDiagItem` diff --git a/clippy_utils/src/usage.rs b/clippy_utils/src/usage.rs index 2c55021ac..182d8cb11 100644 --- a/clippy_utils/src/usage.rs +++ b/clippy_utils/src/usage.rs @@ -199,3 +199,50 @@ pub fn contains_return_break_continue_macro(expression: &Expr<'_>) -> bool { recursive_visitor.visit_expr(expression); recursive_visitor.seen_return_break_continue } + +pub struct UsedAfterExprVisitor<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + definition: HirId, + past_expr: bool, + used_after_expr: bool, +} +impl<'a, 'tcx> UsedAfterExprVisitor<'a, 'tcx> { + pub fn is_found(cx: &'a LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { + utils::path_to_local(expr).map_or(false, |definition| { + let mut visitor = UsedAfterExprVisitor { + cx, + expr, + definition, + past_expr: false, + used_after_expr: false, + }; + utils::get_enclosing_block(cx, definition).map_or(false, |block| { + visitor.visit_block(block); + visitor.used_after_expr + }) + }) + } +} + +impl<'a, 'tcx> intravisit::Visitor<'tcx> for UsedAfterExprVisitor<'a, 'tcx> { + type Map = Map<'tcx>; + + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::OnlyBodies(self.cx.tcx.hir()) + } + + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { + if self.used_after_expr { + return; + } + + if expr.hir_id == self.expr.hir_id { + self.past_expr = true; + } else if self.past_expr && utils::path_to_local_id(expr, self.definition) { + self.used_after_expr = true; + } else { + intravisit::walk_expr(self, expr); + } + } +} diff --git a/rust-toolchain b/rust-toolchain index 2d3c65c1d..bf9cfb61b 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2021-07-01" +channel = "nightly-2021-07-15" components = ["llvm-tools-preview", "rustc-dev", "rust-src"] diff --git a/tests/ui-toml/nonstandard_macro_braces/auxiliary/proc_macro_derive.rs b/tests/ui-toml/nonstandard_macro_braces/auxiliary/proc_macro_derive.rs new file mode 100644 index 000000000..6452189a4 --- /dev/null +++ b/tests/ui-toml/nonstandard_macro_braces/auxiliary/proc_macro_derive.rs @@ -0,0 +1,18 @@ +// compile-flags: --emit=link +// no-prefer-dynamic + +#![crate_type = "proc-macro"] + +extern crate proc_macro; + +use proc_macro::TokenStream; + +#[proc_macro_derive(DeriveSomething)] +pub fn derive(_: TokenStream) -> TokenStream { + "fn _f() -> Vec { vec![] }".parse().unwrap() +} + +#[proc_macro] +pub fn foo_bar(_: TokenStream) -> TokenStream { + "fn issue_7422() { eprintln!(); }".parse().unwrap() +} diff --git a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs index 4ae6864cb..e9f042dde 100644 --- a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs +++ b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs @@ -1,9 +1,17 @@ -// #![warn(clippy::nonstandard_macro_braces)] +// aux-build:proc_macro_derive.rs +#![warn(clippy::nonstandard_macro_braces)] + +extern crate proc_macro_derive; extern crate quote; use quote::quote; +#[derive(proc_macro_derive::DeriveSomething)] +pub struct S; + +proc_macro_derive::foo_bar!(); + #[rustfmt::skip] macro_rules! test { () => { diff --git a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr index 7bcd18295..86063a082 100644 --- a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr +++ b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr @@ -1,54 +1,54 @@ error: use of irregular braces for `vec!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:29:13 + --> $DIR/conf_nonstandard_macro_braces.rs:37:13 | LL | let _ = vec! {1, 2, 3}; | ^^^^^^^^^^^^^^ | = note: `-D clippy::nonstandard-macro-braces` implied by `-D warnings` help: consider writing `vec![1, 2, 3]` - --> $DIR/conf_nonstandard_macro_braces.rs:29:13 + --> $DIR/conf_nonstandard_macro_braces.rs:37:13 | LL | let _ = vec! {1, 2, 3}; | ^^^^^^^^^^^^^^ error: use of irregular braces for `format!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:30:13 + --> $DIR/conf_nonstandard_macro_braces.rs:38:13 | LL | let _ = format!["ugh {} stop being such a good compiler", "hello"]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: consider writing `format!("ugh () stop being such a good compiler", "hello")` - --> $DIR/conf_nonstandard_macro_braces.rs:30:13 + --> $DIR/conf_nonstandard_macro_braces.rs:38:13 | LL | let _ = format!["ugh {} stop being such a good compiler", "hello"]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: use of irregular braces for `quote!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:31:13 + --> $DIR/conf_nonstandard_macro_braces.rs:39:13 | LL | let _ = quote!(let x = 1;); | ^^^^^^^^^^^^^^^^^^ | help: consider writing `quote! {let x = 1;}` - --> $DIR/conf_nonstandard_macro_braces.rs:31:13 + --> $DIR/conf_nonstandard_macro_braces.rs:39:13 | LL | let _ = quote!(let x = 1;); | ^^^^^^^^^^^^^^^^^^ error: use of irregular braces for `quote::quote!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:32:13 + --> $DIR/conf_nonstandard_macro_braces.rs:40:13 | LL | let _ = quote::quote!(match match match); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: consider writing `quote::quote! {match match match}` - --> $DIR/conf_nonstandard_macro_braces.rs:32:13 + --> $DIR/conf_nonstandard_macro_braces.rs:40:13 | LL | let _ = quote::quote!(match match match); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: use of irregular braces for `vec!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:10:9 + --> $DIR/conf_nonstandard_macro_braces.rs:18:9 | LL | vec!{0, 0, 0} | ^^^^^^^^^^^^^ @@ -57,7 +57,7 @@ LL | let _ = test!(); | ------- in this macro invocation | help: consider writing `vec![0, 0, 0]` - --> $DIR/conf_nonstandard_macro_braces.rs:10:9 + --> $DIR/conf_nonstandard_macro_braces.rs:18:9 | LL | vec!{0, 0, 0} | ^^^^^^^^^^^^^ @@ -67,25 +67,25 @@ LL | let _ = test!(); = note: this error originates in the macro `test` (in Nightly builds, run with -Z macro-backtrace for more info) error: use of irregular braces for `type_pos!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:41:12 + --> $DIR/conf_nonstandard_macro_braces.rs:49:12 | LL | let _: type_pos!(usize) = vec![]; | ^^^^^^^^^^^^^^^^ | help: consider writing `type_pos![usize]` - --> $DIR/conf_nonstandard_macro_braces.rs:41:12 + --> $DIR/conf_nonstandard_macro_braces.rs:49:12 | LL | let _: type_pos!(usize) = vec![]; | ^^^^^^^^^^^^^^^^ error: use of irregular braces for `eprint!` macro - --> $DIR/conf_nonstandard_macro_braces.rs:43:5 + --> $DIR/conf_nonstandard_macro_braces.rs:51:5 | LL | eprint!("test if user config overrides defaults"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: consider writing `eprint!["test if user config overrides defaults"];` - --> $DIR/conf_nonstandard_macro_braces.rs:43:5 + --> $DIR/conf_nonstandard_macro_braces.rs:51:5 | LL | eprint!("test if user config overrides defaults"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/branches_sharing_code/false_positives.rs b/tests/ui/branches_sharing_code/false_positives.rs new file mode 100644 index 000000000..7f42df463 --- /dev/null +++ b/tests/ui/branches_sharing_code/false_positives.rs @@ -0,0 +1,28 @@ +#![allow(dead_code)] +#![deny(clippy::if_same_then_else, clippy::branches_sharing_code)] + +// ################################## +// # Issue clippy#7369 +// ################################## +#[derive(Debug)] +pub struct FooBar { + foo: Vec, +} + +impl FooBar { + pub fn bar(&mut self) { + if true { + self.foo.pop(); + } else { + self.baz(); + + self.foo.pop(); + + self.baz() + } + } + + fn baz(&mut self) {} +} + +fn main() {} diff --git a/tests/ui/crashes/ice-7423.rs b/tests/ui/crashes/ice-7423.rs new file mode 100644 index 000000000..31340b012 --- /dev/null +++ b/tests/ui/crashes/ice-7423.rs @@ -0,0 +1,13 @@ +pub trait Trait { + fn f(); +} + +impl Trait for usize { + fn f() { + extern "C" { + fn g() -> usize; + } + } +} + +fn main() {} diff --git a/tests/ui/default_numeric_fallback_f64.fixed b/tests/ui/default_numeric_fallback_f64.fixed new file mode 100644 index 000000000..1b0e7544e --- /dev/null +++ b/tests/ui/default_numeric_fallback_f64.fixed @@ -0,0 +1,174 @@ +// run-rustfix +// aux-build:macro_rules.rs + +#![warn(clippy::default_numeric_fallback)] +#![allow(unused)] +#![allow(clippy::never_loop)] +#![allow(clippy::no_effect)] +#![allow(clippy::unnecessary_operation)] +#![allow(clippy::branches_sharing_code)] +#![allow(clippy::match_single_binding)] + +#[macro_use] +extern crate macro_rules; + +mod basic_expr { + fn test() { + // Should lint unsuffixed literals typed `f64`. + let x = 0.12_f64; + let x = [1.0_f64, 2.0_f64, 3.0_f64]; + let x = if true { (1.0_f64, 2.0_f64) } else { (3.0_f64, 4.0_f64) }; + let x = match 1.0_f64 { + _ => 1.0_f64, + }; + + // Should NOT lint suffixed literals. + let x = 0.12_f64; + + // Should NOT lint literals in init expr if `Local` has a type annotation. + let x: f64 = 0.1; + let x: [f64; 3] = [1., 2., 3.]; + let x: (f64, f64) = if true { (1., 2.) } else { (3., 4.) }; + let x: _ = 1.; + } +} + +mod nested_local { + fn test() { + let x: _ = { + // Should lint this because this literal is not bound to any types. + let y = 1.0_f64; + + // Should NOT lint this because this literal is bound to `_` of outer `Local`. + 1. + }; + + let x: _ = if true { + // Should lint this because this literal is not bound to any types. + let y = 1.0_f64; + + // Should NOT lint this because this literal is bound to `_` of outer `Local`. + 1. + } else { + // Should lint this because this literal is not bound to any types. + let y = 1.0_f64; + + // Should NOT lint this because this literal is bound to `_` of outer `Local`. + 2. + }; + } +} + +mod function_def { + fn ret_f64() -> f64 { + // Even though the output type is specified, + // this unsuffixed literal is linted to reduce heuristics and keep codebase simple. + 1.0_f64 + } + + fn test() { + // Should lint this because return type is inferred to `f64` and NOT bound to a concrete + // type. + let f = || -> _ { 1.0_f64 }; + + // Even though the output type is specified, + // this unsuffixed literal is linted to reduce heuristics and keep codebase simple. + let f = || -> f64 { 1.0_f64 }; + } +} + +mod function_calls { + fn concrete_arg(f: f64) {} + + fn generic_arg(t: T) {} + + fn test() { + // Should NOT lint this because the argument type is bound to a concrete type. + concrete_arg(1.); + + // Should lint this because the argument type is inferred to `f64` and NOT bound to a concrete type. + generic_arg(1.0_f64); + + // Should lint this because the argument type is inferred to `f64` and NOT bound to a concrete type. + let x: _ = generic_arg(1.0_f64); + } +} + +mod struct_ctor { + struct ConcreteStruct { + x: f64, + } + + struct GenericStruct { + x: T, + } + + fn test() { + // Should NOT lint this because the field type is bound to a concrete type. + ConcreteStruct { x: 1. }; + + // Should lint this because the field type is inferred to `f64` and NOT bound to a concrete type. + GenericStruct { x: 1.0_f64 }; + + // Should lint this because the field type is inferred to `f64` and NOT bound to a concrete type. + let _ = GenericStruct { x: 1.0_f64 }; + } +} + +mod enum_ctor { + enum ConcreteEnum { + X(f64), + } + + enum GenericEnum { + X(T), + } + + fn test() { + // Should NOT lint this because the field type is bound to a concrete type. + ConcreteEnum::X(1.); + + // Should lint this because the field type is inferred to `f64` and NOT bound to a concrete type. + GenericEnum::X(1.0_f64); + } +} + +mod method_calls { + struct StructForMethodCallTest {} + + impl StructForMethodCallTest { + fn concrete_arg(&self, f: f64) {} + + fn generic_arg(&self, t: T) {} + } + + fn test() { + let s = StructForMethodCallTest {}; + + // Should NOT lint this because the argument type is bound to a concrete type. + s.concrete_arg(1.); + + // Should lint this because the argument type is bound to a concrete type. + s.generic_arg(1.0_f64); + } +} + +mod in_macro { + macro_rules! internal_macro { + () => { + let x = 22.0_f64; + }; + } + + // Should lint in internal macro. + fn internal() { + internal_macro!(); + } + + // Should NOT lint in external macro. + fn external() { + default_numeric_fallback!(); + } +} + +fn main() {} diff --git a/tests/ui/default_numeric_fallback_f64.rs b/tests/ui/default_numeric_fallback_f64.rs new file mode 100644 index 000000000..e9687777b --- /dev/null +++ b/tests/ui/default_numeric_fallback_f64.rs @@ -0,0 +1,174 @@ +// run-rustfix +// aux-build:macro_rules.rs + +#![warn(clippy::default_numeric_fallback)] +#![allow(unused)] +#![allow(clippy::never_loop)] +#![allow(clippy::no_effect)] +#![allow(clippy::unnecessary_operation)] +#![allow(clippy::branches_sharing_code)] +#![allow(clippy::match_single_binding)] + +#[macro_use] +extern crate macro_rules; + +mod basic_expr { + fn test() { + // Should lint unsuffixed literals typed `f64`. + let x = 0.12; + let x = [1., 2., 3.]; + let x = if true { (1., 2.) } else { (3., 4.) }; + let x = match 1. { + _ => 1., + }; + + // Should NOT lint suffixed literals. + let x = 0.12_f64; + + // Should NOT lint literals in init expr if `Local` has a type annotation. + let x: f64 = 0.1; + let x: [f64; 3] = [1., 2., 3.]; + let x: (f64, f64) = if true { (1., 2.) } else { (3., 4.) }; + let x: _ = 1.; + } +} + +mod nested_local { + fn test() { + let x: _ = { + // Should lint this because this literal is not bound to any types. + let y = 1.; + + // Should NOT lint this because this literal is bound to `_` of outer `Local`. + 1. + }; + + let x: _ = if true { + // Should lint this because this literal is not bound to any types. + let y = 1.; + + // Should NOT lint this because this literal is bound to `_` of outer `Local`. + 1. + } else { + // Should lint this because this literal is not bound to any types. + let y = 1.; + + // Should NOT lint this because this literal is bound to `_` of outer `Local`. + 2. + }; + } +} + +mod function_def { + fn ret_f64() -> f64 { + // Even though the output type is specified, + // this unsuffixed literal is linted to reduce heuristics and keep codebase simple. + 1. + } + + fn test() { + // Should lint this because return type is inferred to `f64` and NOT bound to a concrete + // type. + let f = || -> _ { 1. }; + + // Even though the output type is specified, + // this unsuffixed literal is linted to reduce heuristics and keep codebase simple. + let f = || -> f64 { 1. }; + } +} + +mod function_calls { + fn concrete_arg(f: f64) {} + + fn generic_arg(t: T) {} + + fn test() { + // Should NOT lint this because the argument type is bound to a concrete type. + concrete_arg(1.); + + // Should lint this because the argument type is inferred to `f64` and NOT bound to a concrete type. + generic_arg(1.); + + // Should lint this because the argument type is inferred to `f64` and NOT bound to a concrete type. + let x: _ = generic_arg(1.); + } +} + +mod struct_ctor { + struct ConcreteStruct { + x: f64, + } + + struct GenericStruct { + x: T, + } + + fn test() { + // Should NOT lint this because the field type is bound to a concrete type. + ConcreteStruct { x: 1. }; + + // Should lint this because the field type is inferred to `f64` and NOT bound to a concrete type. + GenericStruct { x: 1. }; + + // Should lint this because the field type is inferred to `f64` and NOT bound to a concrete type. + let _ = GenericStruct { x: 1. }; + } +} + +mod enum_ctor { + enum ConcreteEnum { + X(f64), + } + + enum GenericEnum { + X(T), + } + + fn test() { + // Should NOT lint this because the field type is bound to a concrete type. + ConcreteEnum::X(1.); + + // Should lint this because the field type is inferred to `f64` and NOT bound to a concrete type. + GenericEnum::X(1.); + } +} + +mod method_calls { + struct StructForMethodCallTest {} + + impl StructForMethodCallTest { + fn concrete_arg(&self, f: f64) {} + + fn generic_arg(&self, t: T) {} + } + + fn test() { + let s = StructForMethodCallTest {}; + + // Should NOT lint this because the argument type is bound to a concrete type. + s.concrete_arg(1.); + + // Should lint this because the argument type is bound to a concrete type. + s.generic_arg(1.); + } +} + +mod in_macro { + macro_rules! internal_macro { + () => { + let x = 22.; + }; + } + + // Should lint in internal macro. + fn internal() { + internal_macro!(); + } + + // Should NOT lint in external macro. + fn external() { + default_numeric_fallback!(); + } +} + +fn main() {} diff --git a/tests/ui/default_numeric_fallback_f64.stderr b/tests/ui/default_numeric_fallback_f64.stderr new file mode 100644 index 000000000..961c7cb57 --- /dev/null +++ b/tests/ui/default_numeric_fallback_f64.stderr @@ -0,0 +1,147 @@ +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:18:17 + | +LL | let x = 0.12; + | ^^^^ help: consider adding suffix: `0.12_f64` + | + = note: `-D clippy::default-numeric-fallback` implied by `-D warnings` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:19:18 + | +LL | let x = [1., 2., 3.]; + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:19:22 + | +LL | let x = [1., 2., 3.]; + | ^^ help: consider adding suffix: `2.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:19:26 + | +LL | let x = [1., 2., 3.]; + | ^^ help: consider adding suffix: `3.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:20:28 + | +LL | let x = if true { (1., 2.) } else { (3., 4.) }; + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:20:32 + | +LL | let x = if true { (1., 2.) } else { (3., 4.) }; + | ^^ help: consider adding suffix: `2.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:20:46 + | +LL | let x = if true { (1., 2.) } else { (3., 4.) }; + | ^^ help: consider adding suffix: `3.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:20:50 + | +LL | let x = if true { (1., 2.) } else { (3., 4.) }; + | ^^ help: consider adding suffix: `4.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:21:23 + | +LL | let x = match 1. { + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:22:18 + | +LL | _ => 1., + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:40:21 + | +LL | let y = 1.; + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:48:21 + | +LL | let y = 1.; + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:54:21 + | +LL | let y = 1.; + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:66:9 + | +LL | 1. + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:72:27 + | +LL | let f = || -> _ { 1. }; + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:76:29 + | +LL | let f = || -> f64 { 1. }; + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:90:21 + | +LL | generic_arg(1.); + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:93:32 + | +LL | let x: _ = generic_arg(1.); + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:111:28 + | +LL | GenericStruct { x: 1. }; + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:114:36 + | +LL | let _ = GenericStruct { x: 1. }; + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:132:24 + | +LL | GenericEnum::X(1.); + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:152:23 + | +LL | s.generic_arg(1.); + | ^^ help: consider adding suffix: `1.0_f64` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_f64.rs:159:21 + | +LL | let x = 22.; + | ^^^ help: consider adding suffix: `22.0_f64` +... +LL | internal_macro!(); + | ------------------ in this macro invocation + | + = note: this error originates in the macro `internal_macro` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 23 previous errors + diff --git a/tests/ui/default_numeric_fallback_i32.fixed b/tests/ui/default_numeric_fallback_i32.fixed new file mode 100644 index 000000000..55c082fcb --- /dev/null +++ b/tests/ui/default_numeric_fallback_i32.fixed @@ -0,0 +1,173 @@ +// run-rustfix +// aux-build:macro_rules.rs + +#![warn(clippy::default_numeric_fallback)] +#![allow(unused)] +#![allow(clippy::never_loop)] +#![allow(clippy::no_effect)] +#![allow(clippy::unnecessary_operation)] +#![allow(clippy::branches_sharing_code)] + +#[macro_use] +extern crate macro_rules; + +mod basic_expr { + fn test() { + // Should lint unsuffixed literals typed `i32`. + let x = 22_i32; + let x = [1_i32, 2_i32, 3_i32]; + let x = if true { (1_i32, 2_i32) } else { (3_i32, 4_i32) }; + let x = match 1_i32 { + 1_i32 => 1_i32, + _ => 2_i32, + }; + + // Should NOT lint suffixed literals. + let x = 22_i32; + + // Should NOT lint literals in init expr if `Local` has a type annotation. + let x: [i32; 3] = [1, 2, 3]; + let x: (i32, i32) = if true { (1, 2) } else { (3, 4) }; + let x: _ = 1; + } +} + +mod nested_local { + fn test() { + let x: _ = { + // Should lint this because this literal is not bound to any types. + let y = 1_i32; + + // Should NOT lint this because this literal is bound to `_` of outer `Local`. + 1 + }; + + let x: _ = if true { + // Should lint this because this literal is not bound to any types. + let y = 1_i32; + + // Should NOT lint this because this literal is bound to `_` of outer `Local`. + 1 + } else { + // Should lint this because this literal is not bound to any types. + let y = 1_i32; + + // Should NOT lint this because this literal is bound to `_` of outer `Local`. + 2 + }; + } +} + +mod function_def { + fn ret_i32() -> i32 { + // Even though the output type is specified, + // this unsuffixed literal is linted to reduce heuristics and keep codebase simple. + 1_i32 + } + + fn test() { + // Should lint this because return type is inferred to `i32` and NOT bound to a concrete + // type. + let f = || -> _ { 1_i32 }; + + // Even though the output type is specified, + // this unsuffixed literal is linted to reduce heuristics and keep codebase simple. + let f = || -> i32 { 1_i32 }; + } +} + +mod function_calls { + fn concrete_arg(x: i32) {} + + fn generic_arg(t: T) {} + + fn test() { + // Should NOT lint this because the argument type is bound to a concrete type. + concrete_arg(1); + + // Should lint this because the argument type is inferred to `i32` and NOT bound to a concrete type. + generic_arg(1_i32); + + // Should lint this because the argument type is inferred to `i32` and NOT bound to a concrete type. + let x: _ = generic_arg(1_i32); + } +} + +mod struct_ctor { + struct ConcreteStruct { + x: i32, + } + + struct GenericStruct { + x: T, + } + + fn test() { + // Should NOT lint this because the field type is bound to a concrete type. + ConcreteStruct { x: 1 }; + + // Should lint this because the field type is inferred to `i32` and NOT bound to a concrete type. + GenericStruct { x: 1_i32 }; + + // Should lint this because the field type is inferred to `i32` and NOT bound to a concrete type. + let _ = GenericStruct { x: 1_i32 }; + } +} + +mod enum_ctor { + enum ConcreteEnum { + X(i32), + } + + enum GenericEnum { + X(T), + } + + fn test() { + // Should NOT lint this because the field type is bound to a concrete type. + ConcreteEnum::X(1); + + // Should lint this because the field type is inferred to `i32` and NOT bound to a concrete type. + GenericEnum::X(1_i32); + } +} + +mod method_calls { + struct StructForMethodCallTest {} + + impl StructForMethodCallTest { + fn concrete_arg(&self, x: i32) {} + + fn generic_arg(&self, t: T) {} + } + + fn test() { + let s = StructForMethodCallTest {}; + + // Should NOT lint this because the argument type is bound to a concrete type. + s.concrete_arg(1); + + // Should lint this because the argument type is bound to a concrete type. + s.generic_arg(1_i32); + } +} + +mod in_macro { + macro_rules! internal_macro { + () => { + let x = 22_i32; + }; + } + + // Should lint in internal macro. + fn internal() { + internal_macro!(); + } + + // Should NOT lint in external macro. + fn external() { + default_numeric_fallback!(); + } +} + +fn main() {} diff --git a/tests/ui/default_numeric_fallback.rs b/tests/ui/default_numeric_fallback_i32.rs similarity index 90% rename from tests/ui/default_numeric_fallback.rs rename to tests/ui/default_numeric_fallback_i32.rs index c0625fd1b..e0a4828ce 100644 --- a/tests/ui/default_numeric_fallback.rs +++ b/tests/ui/default_numeric_fallback_i32.rs @@ -1,3 +1,4 @@ +// run-rustfix // aux-build:macro_rules.rs #![warn(clippy::default_numeric_fallback)] @@ -21,15 +22,10 @@ mod basic_expr { _ => 2, }; - // Should lint unsuffixed literals typed `f64`. - let x = 0.12; - // Should NOT lint suffixed literals. let x = 22_i32; - let x = 0.12_f64; // Should NOT lint literals in init expr if `Local` has a type annotation. - let x: f64 = 0.1; let x: [i32; 3] = [1, 2, 3]; let x: (i32, i32) = if true { (1, 2) } else { (3, 4) }; let x: _ = 1; @@ -118,6 +114,24 @@ mod struct_ctor { } } +mod enum_ctor { + enum ConcreteEnum { + X(i32), + } + + enum GenericEnum { + X(T), + } + + fn test() { + // Should NOT lint this because the field type is bound to a concrete type. + ConcreteEnum::X(1); + + // Should lint this because the field type is inferred to `i32` and NOT bound to a concrete type. + GenericEnum::X(1); + } +} + mod method_calls { struct StructForMethodCallTest {} diff --git a/tests/ui/default_numeric_fallback.stderr b/tests/ui/default_numeric_fallback_i32.stderr similarity index 75% rename from tests/ui/default_numeric_fallback.stderr rename to tests/ui/default_numeric_fallback_i32.stderr index 5862cd936..5edf48b20 100644 --- a/tests/ui/default_numeric_fallback.stderr +++ b/tests/ui/default_numeric_fallback_i32.stderr @@ -1,5 +1,5 @@ error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:16:17 + --> $DIR/default_numeric_fallback_i32.rs:17:17 | LL | let x = 22; | ^^ help: consider adding suffix: `22_i32` @@ -7,145 +7,145 @@ LL | let x = 22; = note: `-D clippy::default-numeric-fallback` implied by `-D warnings` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:17:18 + --> $DIR/default_numeric_fallback_i32.rs:18:18 | LL | let x = [1, 2, 3]; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:17:21 + --> $DIR/default_numeric_fallback_i32.rs:18:21 | LL | let x = [1, 2, 3]; | ^ help: consider adding suffix: `2_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:17:24 + --> $DIR/default_numeric_fallback_i32.rs:18:24 | LL | let x = [1, 2, 3]; | ^ help: consider adding suffix: `3_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:18:28 + --> $DIR/default_numeric_fallback_i32.rs:19:28 | LL | let x = if true { (1, 2) } else { (3, 4) }; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:18:31 + --> $DIR/default_numeric_fallback_i32.rs:19:31 | LL | let x = if true { (1, 2) } else { (3, 4) }; | ^ help: consider adding suffix: `2_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:18:44 + --> $DIR/default_numeric_fallback_i32.rs:19:44 | LL | let x = if true { (1, 2) } else { (3, 4) }; | ^ help: consider adding suffix: `3_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:18:47 + --> $DIR/default_numeric_fallback_i32.rs:19:47 | LL | let x = if true { (1, 2) } else { (3, 4) }; | ^ help: consider adding suffix: `4_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:19:23 + --> $DIR/default_numeric_fallback_i32.rs:20:23 | LL | let x = match 1 { | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:20:13 + --> $DIR/default_numeric_fallback_i32.rs:21:13 | LL | 1 => 1, | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:20:18 + --> $DIR/default_numeric_fallback_i32.rs:21:18 | LL | 1 => 1, | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:21:18 + --> $DIR/default_numeric_fallback_i32.rs:22:18 | LL | _ => 2, | ^ help: consider adding suffix: `2_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:25:17 - | -LL | let x = 0.12; - | ^^^^ help: consider adding suffix: `0.12_f64` - -error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:43:21 + --> $DIR/default_numeric_fallback_i32.rs:39:21 | LL | let y = 1; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:51:21 + --> $DIR/default_numeric_fallback_i32.rs:47:21 | LL | let y = 1; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:57:21 + --> $DIR/default_numeric_fallback_i32.rs:53:21 | LL | let y = 1; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:69:9 + --> $DIR/default_numeric_fallback_i32.rs:65:9 | LL | 1 | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:75:27 + --> $DIR/default_numeric_fallback_i32.rs:71:27 | LL | let f = || -> _ { 1 }; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:79:29 + --> $DIR/default_numeric_fallback_i32.rs:75:29 | LL | let f = || -> i32 { 1 }; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:93:21 + --> $DIR/default_numeric_fallback_i32.rs:89:21 | LL | generic_arg(1); | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:96:32 + --> $DIR/default_numeric_fallback_i32.rs:92:32 | LL | let x: _ = generic_arg(1); | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:114:28 + --> $DIR/default_numeric_fallback_i32.rs:110:28 | LL | GenericStruct { x: 1 }; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:117:36 + --> $DIR/default_numeric_fallback_i32.rs:113:36 | LL | let _ = GenericStruct { x: 1 }; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:137:23 + --> $DIR/default_numeric_fallback_i32.rs:131:24 + | +LL | GenericEnum::X(1); + | ^ help: consider adding suffix: `1_i32` + +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback_i32.rs:151:23 | LL | s.generic_arg(1); | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:144:21 + --> $DIR/default_numeric_fallback_i32.rs:158:21 | LL | let x = 22; | ^^ help: consider adding suffix: `22_i32` diff --git a/tests/ui/doc/unbalanced_ticks.rs b/tests/ui/doc/unbalanced_ticks.rs index 78e87bc69..8e8324b30 100644 --- a/tests/ui/doc/unbalanced_ticks.rs +++ b/tests/ui/doc/unbalanced_ticks.rs @@ -34,3 +34,10 @@ fn in_code_block() {} /// - This `item has unbalanced tick marks /// - This item needs backticks_here fn other_markdown() {} + +#[rustfmt::skip] +/// - ```rust +/// /// `lol` +/// pub struct Struct; +/// ``` +fn iss_7421() {} diff --git a/tests/ui/eta.fixed b/tests/ui/eta.fixed index 9e752311c..91b837f9a 100644 --- a/tests/ui/eta.fixed +++ b/tests/ui/eta.fixed @@ -220,3 +220,19 @@ impl std::ops::Deref for Bar { fn test_deref_with_trait_method() { let _ = [Bar].iter().map(|s| s.to_string()).collect::>(); } + +fn mutable_closure_used_again(x: Vec, y: Vec, z: Vec) { + let mut res = Vec::new(); + let mut add_to_res = |n| res.push(n); + x.into_iter().for_each(&mut add_to_res); + y.into_iter().for_each(&mut add_to_res); + z.into_iter().for_each(add_to_res); +} + +fn mutable_closure_in_loop() { + let mut value = 0; + let mut closure = |n| value += n; + for _ in 0..5 { + Some(1).map(&mut closure); + } +} diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs index 44be4628c..1b5370028 100644 --- a/tests/ui/eta.rs +++ b/tests/ui/eta.rs @@ -220,3 +220,19 @@ impl std::ops::Deref for Bar { fn test_deref_with_trait_method() { let _ = [Bar].iter().map(|s| s.to_string()).collect::>(); } + +fn mutable_closure_used_again(x: Vec, y: Vec, z: Vec) { + let mut res = Vec::new(); + let mut add_to_res = |n| res.push(n); + x.into_iter().for_each(|x| add_to_res(x)); + y.into_iter().for_each(|x| add_to_res(x)); + z.into_iter().for_each(|x| add_to_res(x)); +} + +fn mutable_closure_in_loop() { + let mut value = 0; + let mut closure = |n| value += n; + for _ in 0..5 { + Some(1).map(|n| closure(n)); + } +} diff --git a/tests/ui/eta.stderr b/tests/ui/eta.stderr index 8795d3b42..28da89413 100644 --- a/tests/ui/eta.stderr +++ b/tests/ui/eta.stderr @@ -82,5 +82,29 @@ error: redundant closure LL | let a = Some(1u8).map(|a| closure(a)); | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `closure` -error: aborting due to 13 previous errors +error: redundant closure + --> $DIR/eta.rs:227:28 + | +LL | x.into_iter().for_each(|x| add_to_res(x)); + | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res` + +error: redundant closure + --> $DIR/eta.rs:228:28 + | +LL | y.into_iter().for_each(|x| add_to_res(x)); + | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut add_to_res` + +error: redundant closure + --> $DIR/eta.rs:229:28 + | +LL | z.into_iter().for_each(|x| add_to_res(x)); + | ^^^^^^^^^^^^^^^^^ help: replace the closure with the function itself: `add_to_res` + +error: redundant closure + --> $DIR/eta.rs:236:21 + | +LL | Some(1).map(|n| closure(n)); + | ^^^^^^^^^^^^^^ help: replace the closure with the function itself: `&mut closure` + +error: aborting due to 17 previous errors diff --git a/tests/ui/explicit_write_non_rustfix.stderr b/tests/ui/explicit_write_non_rustfix.stderr index 77cadb99b..b94ec6403 100644 --- a/tests/ui/explicit_write_non_rustfix.stderr +++ b/tests/ui/explicit_write_non_rustfix.stderr @@ -1,10 +1,11 @@ -error: use of `writeln!(stderr(), ...).unwrap()`. Consider using `eprintln!` instead +error: use of `writeln!(stderr(), ...).unwrap()` --> $DIR/explicit_write_non_rustfix.rs:7:5 | LL | writeln!(std::io::stderr(), "foo {}", bar).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::explicit-write` implied by `-D warnings` + = help: consider using `eprintln!` instead error: aborting due to previous error diff --git a/tests/ui/format.fixed b/tests/ui/format.fixed index e4cfb005f..5dd64140e 100644 --- a/tests/ui/format.fixed +++ b/tests/ui/format.fixed @@ -69,4 +69,6 @@ fn main() { // Wrap it with braces let v: Vec = vec!["foo".to_string(), "bar".to_string()]; let _s: String = (&*v.join("\n")).to_string(); + + format!("prepend {:+}", "s"); } diff --git a/tests/ui/format.rs b/tests/ui/format.rs index 683957f0f..4599fb520 100644 --- a/tests/ui/format.rs +++ b/tests/ui/format.rs @@ -71,4 +71,6 @@ fn main() { // Wrap it with braces let v: Vec = vec!["foo".to_string(), "bar".to_string()]; let _s: String = format!("{}", &*v.join("\n")); + + format!("prepend {:+}", "s"); } diff --git a/tests/ui/panicking_macros.rs b/tests/ui/panicking_macros.rs index 93b236f74..733806464 100644 --- a/tests/ui/panicking_macros.rs +++ b/tests/ui/panicking_macros.rs @@ -43,6 +43,18 @@ fn core_versions() { unreachable!(); } +fn assert() { + assert!(true); + assert_eq!(true, true); + assert_ne!(true, false); +} + +fn assert_msg() { + assert!(true, "this should not panic"); + assert_eq!(true, true, "this should not panic"); + assert_ne!(true, false, "this should not panic"); +} + fn debug_assert() { debug_assert!(true); debug_assert_eq!(true, true); @@ -61,4 +73,8 @@ fn main() { unimplemented(); unreachable(); core_versions(); + assert(); + assert_msg(); + debug_assert(); + debug_assert_msg(); } diff --git a/tests/ui/rc_mutex.rs b/tests/ui/rc_mutex.rs new file mode 100644 index 000000000..657a3ecf6 --- /dev/null +++ b/tests/ui/rc_mutex.rs @@ -0,0 +1,34 @@ +#![warn(clippy::rc_mutex)] +#![allow(clippy::blacklisted_name)] + +use std::rc::Rc; +use std::sync::Mutex; + +pub struct MyStruct { + foo: Rc>, +} + +pub struct SubT { + foo: T, +} + +pub enum MyEnum { + One, + Two, +} + +pub fn test1(foo: Rc>) {} + +pub fn test2(foo: Rc>) {} + +pub fn test3(foo: Rc>>) {} + +fn main() { + test1(Rc::new(Mutex::new(1))); + test2(Rc::new(Mutex::new(MyEnum::One))); + test3(Rc::new(Mutex::new(SubT { foo: 1 }))); + + let _my_struct = MyStruct { + foo: Rc::new(Mutex::new(1)), + }; +} diff --git a/tests/ui/rc_mutex.stderr b/tests/ui/rc_mutex.stderr new file mode 100644 index 000000000..8e58e2bc2 --- /dev/null +++ b/tests/ui/rc_mutex.stderr @@ -0,0 +1,28 @@ +error: found `Rc>`. Consider using `Rc>` or `Arc>` instead + --> $DIR/rc_mutex.rs:8:10 + | +LL | foo: Rc>, + | ^^^^^^^^^^^^^^ + | + = note: `-D clippy::rc-mutex` implied by `-D warnings` + +error: found `Rc>`. Consider using `Rc>` or `Arc>` instead + --> $DIR/rc_mutex.rs:20:22 + | +LL | pub fn test1(foo: Rc>) {} + | ^^^^^^^^^^^^ + +error: found `Rc>`. Consider using `Rc>` or `Arc>` instead + --> $DIR/rc_mutex.rs:22:19 + | +LL | pub fn test2(foo: Rc>) {} + | ^^^^^^^^^^^^^^^^^ + +error: found `Rc>`. Consider using `Rc>` or `Arc>` instead + --> $DIR/rc_mutex.rs:24:19 + | +LL | pub fn test3(foo: Rc>>) {} + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 4 previous errors + diff --git a/tests/ui/redundant_allocation.fixed b/tests/ui/redundant_allocation.fixed deleted file mode 100644 index 6514fd6d1..000000000 --- a/tests/ui/redundant_allocation.fixed +++ /dev/null @@ -1,48 +0,0 @@ -// run-rustfix -#![warn(clippy::all)] -#![allow(clippy::boxed_local, clippy::needless_pass_by_value)] -#![allow(clippy::blacklisted_name, unused_variables, dead_code)] - -use std::boxed::Box; -use std::rc::Rc; - -pub struct MyStruct {} - -pub struct SubT { - foo: T, -} - -pub enum MyEnum { - One, - Two, -} - -// Rc<&T> - -pub fn test1(foo: &T) {} - -pub fn test2(foo: &MyStruct) {} - -pub fn test3(foo: &MyEnum) {} - -pub fn test4_neg(foo: Rc>) {} - -// Rc> - -pub fn test5(a: Rc) {} - -// Rc> - -pub fn test6(a: Rc) {} - -// Box<&T> - -pub fn test7(foo: &T) {} - -pub fn test8(foo: &MyStruct) {} - -pub fn test9(foo: &MyEnum) {} - -pub fn test10_neg(foo: Box>) {} - -fn main() {} diff --git a/tests/ui/redundant_allocation.rs b/tests/ui/redundant_allocation.rs index 677b3e56d..1b4f2a66c 100644 --- a/tests/ui/redundant_allocation.rs +++ b/tests/ui/redundant_allocation.rs @@ -1,10 +1,7 @@ -// run-rustfix #![warn(clippy::all)] #![allow(clippy::boxed_local, clippy::needless_pass_by_value)] #![allow(clippy::blacklisted_name, unused_variables, dead_code)] - -use std::boxed::Box; -use std::rc::Rc; +#![allow(unused_imports)] pub struct MyStruct {} @@ -17,32 +14,67 @@ pub enum MyEnum { Two, } -// Rc<&T> +mod outer_box { + use crate::MyEnum; + use crate::MyStruct; + use crate::SubT; + use std::boxed::Box; + use std::rc::Rc; + use std::sync::Arc; -pub fn test1(foo: Rc<&T>) {} + pub fn box_test6(foo: Box>) {} -pub fn test2(foo: Rc<&MyStruct>) {} + pub fn box_test7(foo: Box>) {} -pub fn test3(foo: Rc<&MyEnum>) {} + pub fn box_test8() -> Box>> { + unimplemented!(); + } -pub fn test4_neg(foo: Rc>) {} + pub fn box_test9(foo: Box>) -> Box>> { + unimplemented!(); + } +} -// Rc> +mod outer_rc { + use crate::MyEnum; + use crate::MyStruct; + use crate::SubT; + use std::boxed::Box; + use std::rc::Rc; + use std::sync::Arc; -pub fn test5(a: Rc>) {} + pub fn rc_test5(a: Rc>) {} -// Rc> + pub fn rc_test7(a: Rc>) {} -pub fn test6(a: Rc>) {} + pub fn rc_test8() -> Rc>> { + unimplemented!(); + } -// Box<&T> + pub fn rc_test9(foo: Rc>) -> Rc>> { + unimplemented!(); + } +} -pub fn test7(foo: Box<&T>) {} +mod outer_arc { + use crate::MyEnum; + use crate::MyStruct; + use crate::SubT; + use std::boxed::Box; + use std::rc::Rc; + use std::sync::Arc; -pub fn test8(foo: Box<&MyStruct>) {} + pub fn arc_test5(a: Arc>) {} -pub fn test9(foo: Box<&MyEnum>) {} + pub fn arc_test6(a: Arc>) {} -pub fn test10_neg(foo: Box>) {} + pub fn arc_test8() -> Arc>> { + unimplemented!(); + } + + pub fn arc_test9(foo: Arc>) -> Arc>> { + unimplemented!(); + } +} fn main() {} diff --git a/tests/ui/redundant_allocation.stderr b/tests/ui/redundant_allocation.stderr index 92e4f67f5..fdab74eb5 100644 --- a/tests/ui/redundant_allocation.stderr +++ b/tests/ui/redundant_allocation.stderr @@ -1,52 +1,138 @@ -error: usage of `Rc<&T>` - --> $DIR/redundant_allocation.rs:22:22 +error: usage of `Box>` + --> $DIR/redundant_allocation.rs:25:30 | -LL | pub fn test1(foo: Rc<&T>) {} - | ^^^^^^ help: try: `&T` +LL | pub fn box_test6(foo: Box>) {} + | ^^^^^^^^^^ | = note: `-D clippy::redundant-allocation` implied by `-D warnings` + = note: `Rc` is already on the heap, `Box>` makes an extra allocation + = help: consider using just `Box` or `Rc` -error: usage of `Rc<&T>` - --> $DIR/redundant_allocation.rs:24:19 +error: usage of `Box>` + --> $DIR/redundant_allocation.rs:27:30 | -LL | pub fn test2(foo: Rc<&MyStruct>) {} - | ^^^^^^^^^^^^^ help: try: `&MyStruct` - -error: usage of `Rc<&T>` - --> $DIR/redundant_allocation.rs:26:19 +LL | pub fn box_test7(foo: Box>) {} + | ^^^^^^^^^^^ | -LL | pub fn test3(foo: Rc<&MyEnum>) {} - | ^^^^^^^^^^^ help: try: `&MyEnum` + = note: `Arc` is already on the heap, `Box>` makes an extra allocation + = help: consider using just `Box` or `Arc` -error: usage of `Rc>` - --> $DIR/redundant_allocation.rs:32:17 +error: usage of `Box>>` + --> $DIR/redundant_allocation.rs:29:27 | -LL | pub fn test5(a: Rc>) {} - | ^^^^^^^^^^^^ help: try: `Rc` - -error: usage of `Rc>` - --> $DIR/redundant_allocation.rs:36:17 +LL | pub fn box_test8() -> Box>> { + | ^^^^^^^^^^^^^^^^^^^^ | -LL | pub fn test6(a: Rc>) {} - | ^^^^^^^^^^^^^ help: try: `Rc` + = note: `Rc>` is already on the heap, `Box>>` makes an extra allocation + = help: consider using just `Box>` or `Rc>` -error: usage of `Box<&T>` - --> $DIR/redundant_allocation.rs:40:22 +error: usage of `Box>` + --> $DIR/redundant_allocation.rs:33:30 | -LL | pub fn test7(foo: Box<&T>) {} - | ^^^^^^^ help: try: `&T` - -error: usage of `Box<&T>` - --> $DIR/redundant_allocation.rs:42:19 +LL | pub fn box_test9(foo: Box>) -> Box>> { + | ^^^^^^^^^^^ | -LL | pub fn test8(foo: Box<&MyStruct>) {} - | ^^^^^^^^^^^^^^ help: try: `&MyStruct` + = note: `Arc` is already on the heap, `Box>` makes an extra allocation + = help: consider using just `Box` or `Arc` -error: usage of `Box<&T>` - --> $DIR/redundant_allocation.rs:44:19 +error: usage of `Box>>` + --> $DIR/redundant_allocation.rs:33:46 | -LL | pub fn test9(foo: Box<&MyEnum>) {} - | ^^^^^^^^^^^^ help: try: `&MyEnum` +LL | pub fn box_test9(foo: Box>) -> Box>> { + | ^^^^^^^^^^^^^^^^^ + | + = note: `Arc>` is already on the heap, `Box>>` makes an extra allocation + = help: consider using just `Box>` or `Arc>` -error: aborting due to 8 previous errors +error: usage of `Rc>` + --> $DIR/redundant_allocation.rs:46:24 + | +LL | pub fn rc_test5(a: Rc>) {} + | ^^^^^^^^^^^^^ + | + = note: `Box` is already on the heap, `Rc>` makes an extra allocation + = help: consider using just `Rc` or `Box` + +error: usage of `Rc>` + --> $DIR/redundant_allocation.rs:48:24 + | +LL | pub fn rc_test7(a: Rc>) {} + | ^^^^^^^^^^^^^ + | + = note: `Arc` is already on the heap, `Rc>` makes an extra allocation + = help: consider using just `Rc` or `Arc` + +error: usage of `Rc>>` + --> $DIR/redundant_allocation.rs:50:26 + | +LL | pub fn rc_test8() -> Rc>> { + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: `Box>` is already on the heap, `Rc>>` makes an extra allocation + = help: consider using just `Rc>` or `Box>` + +error: usage of `Rc>` + --> $DIR/redundant_allocation.rs:54:29 + | +LL | pub fn rc_test9(foo: Rc>) -> Rc>> { + | ^^^^^^^^^^ + | + = note: `Arc` is already on the heap, `Rc>` makes an extra allocation + = help: consider using just `Rc` or `Arc` + +error: usage of `Rc>>` + --> $DIR/redundant_allocation.rs:54:44 + | +LL | pub fn rc_test9(foo: Rc>) -> Rc>> { + | ^^^^^^^^^^^^^^^^ + | + = note: `Arc>` is already on the heap, `Rc>>` makes an extra allocation + = help: consider using just `Rc>` or `Arc>` + +error: usage of `Arc>` + --> $DIR/redundant_allocation.rs:67:25 + | +LL | pub fn arc_test5(a: Arc>) {} + | ^^^^^^^^^^^^^^ + | + = note: `Box` is already on the heap, `Arc>` makes an extra allocation + = help: consider using just `Arc` or `Box` + +error: usage of `Arc>` + --> $DIR/redundant_allocation.rs:69:25 + | +LL | pub fn arc_test6(a: Arc>) {} + | ^^^^^^^^^^^^^ + | + = note: `Rc` is already on the heap, `Arc>` makes an extra allocation + = help: consider using just `Arc` or `Rc` + +error: usage of `Arc>>` + --> $DIR/redundant_allocation.rs:71:27 + | +LL | pub fn arc_test8() -> Arc>> { + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: `Box>` is already on the heap, `Arc>>` makes an extra allocation + = help: consider using just `Arc>` or `Box>` + +error: usage of `Arc>` + --> $DIR/redundant_allocation.rs:75:30 + | +LL | pub fn arc_test9(foo: Arc>) -> Arc>> { + | ^^^^^^^^^^ + | + = note: `Rc` is already on the heap, `Arc>` makes an extra allocation + = help: consider using just `Arc` or `Rc` + +error: usage of `Arc>>` + --> $DIR/redundant_allocation.rs:75:45 + | +LL | pub fn arc_test9(foo: Arc>) -> Arc>> { + | ^^^^^^^^^^^^^^^^ + | + = note: `Rc>` is already on the heap, `Arc>>` makes an extra allocation + = help: consider using just `Arc>` or `Rc>` + +error: aborting due to 15 previous errors diff --git a/tests/ui/redundant_allocation_fixable.fixed b/tests/ui/redundant_allocation_fixable.fixed new file mode 100644 index 000000000..ef089b25f --- /dev/null +++ b/tests/ui/redundant_allocation_fixable.fixed @@ -0,0 +1,75 @@ +// run-rustfix +#![warn(clippy::all)] +#![allow(clippy::boxed_local, clippy::needless_pass_by_value)] +#![allow(clippy::blacklisted_name, unused_variables, dead_code)] +#![allow(unused_imports)] + +pub struct MyStruct {} + +pub struct SubT { + foo: T, +} + +pub enum MyEnum { + One, + Two, +} + +mod outer_box { + use crate::MyEnum; + use crate::MyStruct; + use crate::SubT; + use std::boxed::Box; + use std::rc::Rc; + use std::sync::Arc; + + pub fn box_test1(foo: &T) {} + + pub fn box_test2(foo: &MyStruct) {} + + pub fn box_test3(foo: &MyEnum) {} + + pub fn box_test4_neg(foo: Box>) {} + + pub fn box_test5(foo: Box) {} +} + +mod outer_rc { + use crate::MyEnum; + use crate::MyStruct; + use crate::SubT; + use std::boxed::Box; + use std::rc::Rc; + use std::sync::Arc; + + pub fn rc_test1(foo: &T) {} + + pub fn rc_test2(foo: &MyStruct) {} + + pub fn rc_test3(foo: &MyEnum) {} + + pub fn rc_test4_neg(foo: Rc>) {} + + pub fn rc_test6(a: Rc) {} +} + +mod outer_arc { + use crate::MyEnum; + use crate::MyStruct; + use crate::SubT; + use std::boxed::Box; + use std::rc::Rc; + use std::sync::Arc; + + pub fn arc_test1(foo: &T) {} + + pub fn arc_test2(foo: &MyStruct) {} + + pub fn arc_test3(foo: &MyEnum) {} + + pub fn arc_test4_neg(foo: Arc>) {} + + pub fn arc_test7(a: Arc) {} +} + +fn main() {} diff --git a/tests/ui/redundant_allocation_fixable.rs b/tests/ui/redundant_allocation_fixable.rs new file mode 100644 index 000000000..fefa87721 --- /dev/null +++ b/tests/ui/redundant_allocation_fixable.rs @@ -0,0 +1,75 @@ +// run-rustfix +#![warn(clippy::all)] +#![allow(clippy::boxed_local, clippy::needless_pass_by_value)] +#![allow(clippy::blacklisted_name, unused_variables, dead_code)] +#![allow(unused_imports)] + +pub struct MyStruct {} + +pub struct SubT { + foo: T, +} + +pub enum MyEnum { + One, + Two, +} + +mod outer_box { + use crate::MyEnum; + use crate::MyStruct; + use crate::SubT; + use std::boxed::Box; + use std::rc::Rc; + use std::sync::Arc; + + pub fn box_test1(foo: Box<&T>) {} + + pub fn box_test2(foo: Box<&MyStruct>) {} + + pub fn box_test3(foo: Box<&MyEnum>) {} + + pub fn box_test4_neg(foo: Box>) {} + + pub fn box_test5(foo: Box>) {} +} + +mod outer_rc { + use crate::MyEnum; + use crate::MyStruct; + use crate::SubT; + use std::boxed::Box; + use std::rc::Rc; + use std::sync::Arc; + + pub fn rc_test1(foo: Rc<&T>) {} + + pub fn rc_test2(foo: Rc<&MyStruct>) {} + + pub fn rc_test3(foo: Rc<&MyEnum>) {} + + pub fn rc_test4_neg(foo: Rc>) {} + + pub fn rc_test6(a: Rc>) {} +} + +mod outer_arc { + use crate::MyEnum; + use crate::MyStruct; + use crate::SubT; + use std::boxed::Box; + use std::rc::Rc; + use std::sync::Arc; + + pub fn arc_test1(foo: Arc<&T>) {} + + pub fn arc_test2(foo: Arc<&MyStruct>) {} + + pub fn arc_test3(foo: Arc<&MyEnum>) {} + + pub fn arc_test4_neg(foo: Arc>) {} + + pub fn arc_test7(a: Arc>) {} +} + +fn main() {} diff --git a/tests/ui/redundant_allocation_fixable.stderr b/tests/ui/redundant_allocation_fixable.stderr new file mode 100644 index 000000000..fdd76ef17 --- /dev/null +++ b/tests/ui/redundant_allocation_fixable.stderr @@ -0,0 +1,99 @@ +error: usage of `Box<&T>` + --> $DIR/redundant_allocation_fixable.rs:26:30 + | +LL | pub fn box_test1(foo: Box<&T>) {} + | ^^^^^^^ help: try: `&T` + | + = note: `-D clippy::redundant-allocation` implied by `-D warnings` + = note: `&T` is already a pointer, `Box<&T>` allocates a pointer on the heap + +error: usage of `Box<&MyStruct>` + --> $DIR/redundant_allocation_fixable.rs:28:27 + | +LL | pub fn box_test2(foo: Box<&MyStruct>) {} + | ^^^^^^^^^^^^^^ help: try: `&MyStruct` + | + = note: `&MyStruct` is already a pointer, `Box<&MyStruct>` allocates a pointer on the heap + +error: usage of `Box<&MyEnum>` + --> $DIR/redundant_allocation_fixable.rs:30:27 + | +LL | pub fn box_test3(foo: Box<&MyEnum>) {} + | ^^^^^^^^^^^^ help: try: `&MyEnum` + | + = note: `&MyEnum` is already a pointer, `Box<&MyEnum>` allocates a pointer on the heap + +error: usage of `Box>` + --> $DIR/redundant_allocation_fixable.rs:34:30 + | +LL | pub fn box_test5(foo: Box>) {} + | ^^^^^^^^^^^ help: try: `Box` + | + = note: `Box` is already on the heap, `Box>` makes an extra allocation + +error: usage of `Rc<&T>` + --> $DIR/redundant_allocation_fixable.rs:45:29 + | +LL | pub fn rc_test1(foo: Rc<&T>) {} + | ^^^^^^ help: try: `&T` + | + = note: `&T` is already a pointer, `Rc<&T>` allocates a pointer on the heap + +error: usage of `Rc<&MyStruct>` + --> $DIR/redundant_allocation_fixable.rs:47:26 + | +LL | pub fn rc_test2(foo: Rc<&MyStruct>) {} + | ^^^^^^^^^^^^^ help: try: `&MyStruct` + | + = note: `&MyStruct` is already a pointer, `Rc<&MyStruct>` allocates a pointer on the heap + +error: usage of `Rc<&MyEnum>` + --> $DIR/redundant_allocation_fixable.rs:49:26 + | +LL | pub fn rc_test3(foo: Rc<&MyEnum>) {} + | ^^^^^^^^^^^ help: try: `&MyEnum` + | + = note: `&MyEnum` is already a pointer, `Rc<&MyEnum>` allocates a pointer on the heap + +error: usage of `Rc>` + --> $DIR/redundant_allocation_fixable.rs:53:24 + | +LL | pub fn rc_test6(a: Rc>) {} + | ^^^^^^^^^^^^ help: try: `Rc` + | + = note: `Rc` is already on the heap, `Rc>` makes an extra allocation + +error: usage of `Arc<&T>` + --> $DIR/redundant_allocation_fixable.rs:64:30 + | +LL | pub fn arc_test1(foo: Arc<&T>) {} + | ^^^^^^^ help: try: `&T` + | + = note: `&T` is already a pointer, `Arc<&T>` allocates a pointer on the heap + +error: usage of `Arc<&MyStruct>` + --> $DIR/redundant_allocation_fixable.rs:66:27 + | +LL | pub fn arc_test2(foo: Arc<&MyStruct>) {} + | ^^^^^^^^^^^^^^ help: try: `&MyStruct` + | + = note: `&MyStruct` is already a pointer, `Arc<&MyStruct>` allocates a pointer on the heap + +error: usage of `Arc<&MyEnum>` + --> $DIR/redundant_allocation_fixable.rs:68:27 + | +LL | pub fn arc_test3(foo: Arc<&MyEnum>) {} + | ^^^^^^^^^^^^ help: try: `&MyEnum` + | + = note: `&MyEnum` is already a pointer, `Arc<&MyEnum>` allocates a pointer on the heap + +error: usage of `Arc>` + --> $DIR/redundant_allocation_fixable.rs:72:25 + | +LL | pub fn arc_test7(a: Arc>) {} + | ^^^^^^^^^^^^^^ help: try: `Arc` + | + = note: `Arc` is already on the heap, `Arc>` makes an extra allocation + +error: aborting due to 12 previous errors + diff --git a/tests/ui/redundant_clone.fixed b/tests/ui/redundant_clone.fixed index f5da703cd..2d7110827 100644 --- a/tests/ui/redundant_clone.fixed +++ b/tests/ui/redundant_clone.fixed @@ -55,6 +55,8 @@ fn main() { issue_5405(); manually_drop(); clone_then_move_cloned(); + hashmap_neg(); + false_negative_5707(); } #[derive(Clone)] @@ -206,3 +208,29 @@ fn clone_then_move_cloned() { let mut x = S(String::new()); x.0.clone().chars().for_each(|_| x.m()); } + +fn hashmap_neg() { + // issue 5707 + use std::collections::HashMap; + use std::path::PathBuf; + + let p = PathBuf::from("/"); + + let mut h: HashMap<&str, &str> = HashMap::new(); + h.insert("orig-p", p.to_str().unwrap()); + + let mut q = p.clone(); + q.push("foo"); + + println!("{:?} {}", h, q.display()); +} + +fn false_negative_5707() { + fn foo(_x: &Alpha, _y: &mut Alpha) {} + + let x = Alpha; + let mut y = Alpha; + foo(&x, &mut y); + let _z = x.clone(); // pr 7346 can't lint on `x` + drop(y); +} diff --git a/tests/ui/redundant_clone.rs b/tests/ui/redundant_clone.rs index fd7f31a1c..bd3d73652 100644 --- a/tests/ui/redundant_clone.rs +++ b/tests/ui/redundant_clone.rs @@ -55,6 +55,8 @@ fn main() { issue_5405(); manually_drop(); clone_then_move_cloned(); + hashmap_neg(); + false_negative_5707(); } #[derive(Clone)] @@ -206,3 +208,29 @@ fn clone_then_move_cloned() { let mut x = S(String::new()); x.0.clone().chars().for_each(|_| x.m()); } + +fn hashmap_neg() { + // issue 5707 + use std::collections::HashMap; + use std::path::PathBuf; + + let p = PathBuf::from("/"); + + let mut h: HashMap<&str, &str> = HashMap::new(); + h.insert("orig-p", p.to_str().unwrap()); + + let mut q = p.clone(); + q.push("foo"); + + println!("{:?} {}", h, q.display()); +} + +fn false_negative_5707() { + fn foo(_x: &Alpha, _y: &mut Alpha) {} + + let x = Alpha; + let mut y = Alpha; + foo(&x, &mut y); + let _z = x.clone(); // pr 7346 can't lint on `x` + drop(y); +} diff --git a/tests/ui/redundant_clone.stderr b/tests/ui/redundant_clone.stderr index 529a6de91..fbc90493a 100644 --- a/tests/ui/redundant_clone.stderr +++ b/tests/ui/redundant_clone.stderr @@ -108,61 +108,61 @@ LL | let _t = tup.0.clone(); | ^^^^^ error: redundant clone - --> $DIR/redundant_clone.rs:63:25 + --> $DIR/redundant_clone.rs:65:25 | LL | if b { (a.clone(), a.clone()) } else { (Alpha, a) } | ^^^^^^^^ help: remove this | note: this value is dropped without further use - --> $DIR/redundant_clone.rs:63:24 + --> $DIR/redundant_clone.rs:65:24 | LL | if b { (a.clone(), a.clone()) } else { (Alpha, a) } | ^ error: redundant clone - --> $DIR/redundant_clone.rs:120:15 + --> $DIR/redundant_clone.rs:122:15 | LL | let _s = s.clone(); | ^^^^^^^^ help: remove this | note: this value is dropped without further use - --> $DIR/redundant_clone.rs:120:14 + --> $DIR/redundant_clone.rs:122:14 | LL | let _s = s.clone(); | ^ error: redundant clone - --> $DIR/redundant_clone.rs:121:15 + --> $DIR/redundant_clone.rs:123:15 | LL | let _t = t.clone(); | ^^^^^^^^ help: remove this | note: this value is dropped without further use - --> $DIR/redundant_clone.rs:121:14 + --> $DIR/redundant_clone.rs:123:14 | LL | let _t = t.clone(); | ^ error: redundant clone - --> $DIR/redundant_clone.rs:131:19 + --> $DIR/redundant_clone.rs:133:19 | LL | let _f = f.clone(); | ^^^^^^^^ help: remove this | note: this value is dropped without further use - --> $DIR/redundant_clone.rs:131:18 + --> $DIR/redundant_clone.rs:133:18 | LL | let _f = f.clone(); | ^ error: redundant clone - --> $DIR/redundant_clone.rs:143:14 + --> $DIR/redundant_clone.rs:145:14 | LL | let y = x.clone().join("matthias"); | ^^^^^^^^ help: remove this | note: cloned value is neither consumed nor mutated - --> $DIR/redundant_clone.rs:143:13 + --> $DIR/redundant_clone.rs:145:13 | LL | let y = x.clone().join("matthias"); | ^^^^^^^^^ diff --git a/tests/ui/strlen_on_c_strings.rs b/tests/ui/strlen_on_c_strings.rs new file mode 100644 index 000000000..21902fa84 --- /dev/null +++ b/tests/ui/strlen_on_c_strings.rs @@ -0,0 +1,16 @@ +#![warn(clippy::strlen_on_c_strings)] +#![allow(dead_code)] +#![feature(rustc_private)] +extern crate libc; + +use std::ffi::{CStr, CString}; + +fn main() { + // CString + let cstring = CString::new("foo").expect("CString::new failed"); + let len = unsafe { libc::strlen(cstring.as_ptr()) }; + + // CStr + let cstr = CStr::from_bytes_with_nul(b"foo\0").expect("CStr::from_bytes_with_nul failed"); + let len = unsafe { libc::strlen(cstr.as_ptr()) }; +} diff --git a/tests/ui/strlen_on_c_strings.stderr b/tests/ui/strlen_on_c_strings.stderr new file mode 100644 index 000000000..a212bd327 --- /dev/null +++ b/tests/ui/strlen_on_c_strings.stderr @@ -0,0 +1,25 @@ +error: using `libc::strlen` on a `CString` or `CStr` value + --> $DIR/strlen_on_c_strings.rs:11:24 + | +LL | let len = unsafe { libc::strlen(cstring.as_ptr()) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::strlen-on-c-strings` implied by `-D warnings` +help: try this (you might also need to get rid of `unsafe` block in some cases): + | +LL | let len = unsafe { cstring.as_bytes().len() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: using `libc::strlen` on a `CString` or `CStr` value + --> $DIR/strlen_on_c_strings.rs:15:24 + | +LL | let len = unsafe { libc::strlen(cstr.as_ptr()) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: try this (you might also need to get rid of `unsafe` block in some cases): + | +LL | let len = unsafe { cstr.to_bytes().len() }; + | ^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + From 81904a413eb1d90d64e2450f075e38d7e0cf1c00 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Wed, 14 Jul 2021 16:17:04 -0500 Subject: [PATCH 3/4] Remove refs from pat slices --- clippy_lints/src/manual_unwrap_or.rs | 4 ++-- clippy_lints/src/matches.rs | 2 +- clippy_lints/src/option_if_let_else.rs | 2 +- clippy_lints/src/pattern_type_mismatch.rs | 2 +- clippy_utils/src/lib.rs | 18 +++++++++--------- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/manual_unwrap_or.rs b/clippy_lints/src/manual_unwrap_or.rs index 18038dd78..9d8d77cf8 100644 --- a/clippy_lints/src/manual_unwrap_or.rs +++ b/clippy_lints/src/manual_unwrap_or.rs @@ -61,13 +61,13 @@ fn lint_manual_unwrap_or<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if let Some((idx, or_arm)) = arms.iter().enumerate().find(|(_, arm)| { match arm.pat.kind { PatKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone), - PatKind::TupleStruct(ref qpath, &[pat], _) => + PatKind::TupleStruct(ref qpath, [pat], _) => matches!(pat.kind, PatKind::Wild) && is_lang_ctor(cx, qpath, ResultErr), _ => false, } }); let unwrap_arm = &arms[1 - idx]; - if let PatKind::TupleStruct(ref qpath, &[unwrap_pat], _) = unwrap_arm.pat.kind; + if let PatKind::TupleStruct(ref qpath, [unwrap_pat], _) = unwrap_arm.pat.kind; if is_lang_ctor(cx, qpath, OptionSome) || is_lang_ctor(cx, qpath, ResultOk); if let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind; if path_to_local_id(unwrap_arm.body, binding_hir_id); diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 986469a2b..6d5ce3373 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -625,7 +625,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { if let PatKind::TupleStruct( QPath::Resolved(None, variant_name), args, _) = arms[0].pat.kind; if args.len() == 1; - if let PatKind::Binding(_, arg, ..) = strip_pat_refs(args[0]).kind; + if let PatKind::Binding(_, arg, ..) = strip_pat_refs(&args[0]).kind; let body = remove_blocks(arms[0].body); if path_to_local_id(body, arg); diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs index b6af4175e..b2be35bdd 100644 --- a/clippy_lints/src/option_if_let_else.rs +++ b/clippy_lints/src/option_if_let_else.rs @@ -132,7 +132,7 @@ fn detect_option_if_let_else<'tcx>( if !is_else_clause(cx.tcx, expr); if arms.len() == 2; if !is_result_ok(cx, cond_expr); // Don't lint on Result::ok because a different lint does it already - if let PatKind::TupleStruct(struct_qpath, &[inner_pat], _) = &arms[0].pat.kind; + if let PatKind::TupleStruct(struct_qpath, [inner_pat], _) = &arms[0].pat.kind; if is_lang_ctor(cx, struct_qpath, OptionSome); if let PatKind::Binding(bind_annotation, _, id, _) = &inner_pat.kind; if !contains_return_break_continue_macro(arms[0].body); diff --git a/clippy_lints/src/pattern_type_mismatch.rs b/clippy_lints/src/pattern_type_mismatch.rs index 9bab78399..ea4065d37 100644 --- a/clippy_lints/src/pattern_type_mismatch.rs +++ b/clippy_lints/src/pattern_type_mismatch.rs @@ -258,7 +258,7 @@ fn get_variant<'a>(adt_def: &'a AdtDef, qpath: &QPath<'_>) -> Option<&'a Variant fn find_first_mismatch_in_tuple<'tcx, I>( cx: &LateContext<'tcx>, - pats: &[&Pat<'_>], + pats: &[Pat<'_>], ty_iter_src: I, ) -> Option<(Span, Mutability, Level)> where diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 6db221ab0..4f0a9f442 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -255,7 +255,7 @@ pub fn in_macro(span: Span) -> bool { } /// Checks if given pattern is a wildcard (`_`) -pub fn is_wild<'tcx>(pat: &impl std::ops::Deref>) -> bool { +pub fn is_wild(pat: &Pat<'_>) -> bool { matches!(pat.kind, PatKind::Wild) } @@ -1023,8 +1023,8 @@ pub fn is_refutable(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool { ) } - fn are_refutable<'a, I: Iterator>>(cx: &LateContext<'_>, mut i: I) -> bool { - i.any(|pat| is_refutable(cx, pat)) + fn are_refutable<'a, I: IntoIterator>>(cx: &LateContext<'_>, i: I) -> bool { + i.into_iter().any(|pat| is_refutable(cx, pat)) } match pat.kind { @@ -1035,23 +1035,23 @@ pub fn is_refutable(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool { PatKind::Path(ref qpath) => is_enum_variant(cx, qpath, pat.hir_id), PatKind::Or(pats) => { // TODO: should be the honest check, that pats is exhaustive set - are_refutable(cx, pats.iter().map(|pat| &**pat)) + are_refutable(cx, pats) }, - PatKind::Tuple(pats, _) => are_refutable(cx, pats.iter().map(|pat| &**pat)), + PatKind::Tuple(pats, _) => are_refutable(cx, pats), PatKind::Struct(ref qpath, fields, _) => { is_enum_variant(cx, qpath, pat.hir_id) || are_refutable(cx, fields.iter().map(|field| &*field.pat)) }, PatKind::TupleStruct(ref qpath, pats, _) => { - is_enum_variant(cx, qpath, pat.hir_id) || are_refutable(cx, pats.iter().map(|pat| &**pat)) + is_enum_variant(cx, qpath, pat.hir_id) || are_refutable(cx, pats) }, - PatKind::Slice(head, ref middle, tail) => { + PatKind::Slice(head, middle, tail) => { match &cx.typeck_results().node_type(pat.hir_id).kind() { rustc_ty::Slice(..) => { // [..] is the only irrefutable slice pattern. !head.is_empty() || middle.is_none() || !tail.is_empty() }, rustc_ty::Array(..) => { - are_refutable(cx, head.iter().chain(middle).chain(tail.iter()).map(|pat| &**pat)) + are_refutable(cx, head.iter().chain(middle).chain(tail.iter())) }, _ => { // unreachable!() @@ -1066,7 +1066,7 @@ pub fn is_refutable(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool { /// the function once on the given pattern. pub fn recurse_or_patterns<'tcx, F: FnMut(&'tcx Pat<'tcx>)>(pat: &'tcx Pat<'tcx>, mut f: F) { if let PatKind::Or(pats) = pat.kind { - pats.iter().copied().for_each(f); + pats.iter().for_each(f); } else { f(pat); } From 21abb5de2746ffa8b4312ef15300164b094c6e3b Mon Sep 17 00:00:00 2001 From: flip1995 Date: Mon, 19 Jul 2021 11:49:03 +0200 Subject: [PATCH 4/4] Bump nightly version -> 2021-07-19 --- rust-toolchain | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-toolchain b/rust-toolchain index bf9cfb61b..3a2005e78 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2021-07-15" +channel = "nightly-2021-07-19" components = ["llvm-tools-preview", "rustc-dev", "rust-src"]