diff options
| author | flip1995 <philipp.krones@embecosm.com> | 2021-08-12 10:58:44 +0200 |
|---|---|---|
| committer | flip1995 <philipp.krones@embecosm.com> | 2021-08-12 10:58:44 +0200 |
| commit | d02016d68634637b3416cc740cbea7100dd06d7c (patch) | |
| tree | 1aac1a0066b57a4fb26c1fbd4b3062c7e2b273b6 /clippy_lints | |
| parent | 652b6a771f279393ceda2e1b52150d86c1528e73 (diff) | |
| parent | 62f4187ed01c3c6ef10485d2549d501fe23ca012 (diff) | |
| download | rust-d02016d68634637b3416cc740cbea7100dd06d7c.tar.gz rust-d02016d68634637b3416cc740cbea7100dd06d7c.zip | |
Merge remote-tracking branch 'upstream/master' into rustup
Diffstat (limited to 'clippy_lints')
39 files changed, 498 insertions, 277 deletions
diff --git a/clippy_lints/src/as_conversions.rs b/clippy_lints/src/as_conversions.rs index 7c39a3e2ce3..0be460d67a7 100644 --- a/clippy_lints/src/as_conversions.rs +++ b/clippy_lints/src/as_conversions.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_help; use rustc_ast::ast::{Expr, ExprKind}; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -47,7 +47,7 @@ declare_lint_pass!(AsConversions => [AS_CONVERSIONS]); impl EarlyLintPass for AsConversions { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { - if in_external_macro(cx.sess(), expr.span) { + if in_external_macro(cx.sess, expr.span) { return; } diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index a3a3603c4c0..75561cfde36 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -230,15 +230,7 @@ impl<'tcx> LateLintPass<'tcx> for DocMarkdown { panic_span: None, }; fpu.visit_expr(&body.value); - lint_for_missing_headers( - cx, - item.def_id, - item.span, - sig, - headers, - Some(body_id), - fpu.panic_span, - ); + lint_for_missing_headers(cx, item.def_id, item.span, sig, headers, Some(body_id), fpu.panic_span); } }, hir::ItemKind::Impl(ref impl_) => { @@ -278,15 +270,7 @@ impl<'tcx> LateLintPass<'tcx> for DocMarkdown { panic_span: None, }; fpu.visit_expr(&body.value); - lint_for_missing_headers( - cx, - item.def_id, - item.span, - sig, - headers, - Some(body_id), - fpu.panic_span, - ); + lint_for_missing_headers(cx, item.def_id, item.span, sig, headers, Some(body_id), fpu.panic_span); } } } diff --git a/clippy_lints/src/else_if_without_else.rs b/clippy_lints/src/else_if_without_else.rs index 0541ac5eccc..b64246515f3 100644 --- a/clippy_lints/src/else_if_without_else.rs +++ b/clippy_lints/src/else_if_without_else.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_help; use rustc_ast::ast::{Expr, ExprKind}; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -49,7 +49,7 @@ declare_lint_pass!(ElseIfWithoutElse => [ELSE_IF_WITHOUT_ELSE]); impl EarlyLintPass for ElseIfWithoutElse { fn check_expr(&mut self, cx: &EarlyContext<'_>, mut item: &Expr) { - if in_external_macro(cx.sess(), item.span) { + if in_external_macro(cx.sess, item.span) { return; } diff --git a/clippy_lints/src/functions/mod.rs b/clippy_lints/src/functions/mod.rs index ce23c0ce4a0..04fc5887e8e 100644 --- a/clippy_lints/src/functions/mod.rs +++ b/clippy_lints/src/functions/mod.rs @@ -251,7 +251,7 @@ impl<'tcx> LateLintPass<'tcx> for Functions { hir_id: hir::HirId, ) { too_many_arguments::check_fn(cx, kind, decl, span, hir_id, self.too_many_arguments_threshold); - too_many_lines::check_fn(cx, span, body, self.too_many_lines_threshold); + too_many_lines::check_fn(cx, kind, span, body, self.too_many_lines_threshold); not_unsafe_ptr_arg_deref::check_fn(cx, kind, decl, body, hir_id); } diff --git a/clippy_lints/src/functions/too_many_lines.rs b/clippy_lints/src/functions/too_many_lines.rs index a666fee1a4a..008ef661b55 100644 --- a/clippy_lints/src/functions/too_many_lines.rs +++ b/clippy_lints/src/functions/too_many_lines.rs @@ -1,4 +1,5 @@ use rustc_hir as hir; +use rustc_hir::intravisit::FnKind; use rustc_lint::{LateContext, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_span::Span; @@ -8,8 +9,16 @@ use clippy_utils::source::snippet_opt; use super::TOO_MANY_LINES; -pub(super) fn check_fn(cx: &LateContext<'_>, span: Span, body: &'tcx hir::Body<'_>, too_many_lines_threshold: u64) { - if in_external_macro(cx.sess(), span) { +pub(super) fn check_fn( + cx: &LateContext<'_>, + kind: FnKind<'tcx>, + span: Span, + body: &'tcx hir::Body<'_>, + too_many_lines_threshold: u64, +) { + // Closures must be contained in a parent body, which will be checked for `too_many_lines`. + // Don't check closures for `too_many_lines` to avoid duplicated lints. + if matches!(kind, FnKind::Closure) || in_external_macro(cx.sess(), span) { return; } diff --git a/clippy_lints/src/if_not_else.rs b/clippy_lints/src/if_not_else.rs index 28db7233d70..3ce91d421ba 100644 --- a/clippy_lints/src/if_not_else.rs +++ b/clippy_lints/src/if_not_else.rs @@ -3,7 +3,7 @@ use clippy_utils::diagnostics::span_lint_and_help; use rustc_ast::ast::{BinOpKind, Expr, ExprKind, UnOp}; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -48,7 +48,7 @@ declare_lint_pass!(IfNotElse => [IF_NOT_ELSE]); impl EarlyLintPass for IfNotElse { fn check_expr(&mut self, cx: &EarlyContext<'_>, item: &Expr) { - if in_external_macro(cx.sess(), item.span) { + if in_external_macro(cx.sess, item.span) { return; } if let ExprKind::If(ref cond, _, Some(ref els)) = item.kind { diff --git a/clippy_lints/src/items_after_statements.rs b/clippy_lints/src/items_after_statements.rs index 429c6ed7d2d..3736d237642 100644 --- a/clippy_lints/src/items_after_statements.rs +++ b/clippy_lints/src/items_after_statements.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint; use rustc_ast::ast::{Block, ItemKind, StmtKind}; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -54,7 +54,7 @@ declare_lint_pass!(ItemsAfterStatements => [ITEMS_AFTER_STATEMENTS]); impl EarlyLintPass for ItemsAfterStatements { fn check_block(&mut self, cx: &EarlyContext<'_>, item: &Block) { - if in_external_macro(cx.sess(), item.span) { + if in_external_macro(cx.sess, item.span) { return; } @@ -68,7 +68,7 @@ impl EarlyLintPass for ItemsAfterStatements { // lint on all further items for stmt in stmts { if let StmtKind::Item(ref it) = *stmt { - if in_external_macro(cx.sess(), it.span) { + if in_external_macro(cx.sess, it.span) { return; } if let ItemKind::MacroDef(..) = it.kind { diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index a2cbfb1a05e..a519ad90df5 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -207,8 +207,7 @@ fn check_trait_items(cx: &LateContext<'_>, visited_trait: &Item<'_>, trait_items } } - if cx.access_levels.is_exported(visited_trait.def_id) - && trait_items.iter().any(|i| is_named_self(cx, i, sym::len)) + if cx.access_levels.is_exported(visited_trait.def_id) && trait_items.iter().any(|i| is_named_self(cx, i, sym::len)) { let mut current_and_super_traits = DefIdSet::default(); fill_trait_set(visited_trait.def_id.to_def_id(), &mut current_and_super_traits, cx); @@ -331,17 +330,15 @@ fn check_for_is_empty( None, None, ), - Some(is_empty) if !cx.access_levels.is_exported(is_empty.def_id.expect_local()) => { - ( - format!( - "{} `{}` has a public `len` method, but a private `is_empty` method", - item_kind, - item_name.as_str(), - ), - Some(cx.tcx.def_span(is_empty.def_id)), - None, - ) - }, + Some(is_empty) if !cx.access_levels.is_exported(is_empty.def_id.expect_local()) => ( + format!( + "{} `{}` has a public `len` method, but a private `is_empty` method", + item_kind, + item_name.as_str(), + ), + Some(cx.tcx.def_span(is_empty.def_id)), + None, + ), Some(is_empty) if !(is_empty.fn_has_self_parameter && check_is_empty_sig(cx.tcx.fn_sig(is_empty.def_id).skip_binder(), self_kind, output)) => diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f49b382c5ea..dbdb4251b3b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -797,6 +797,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: methods::UNNECESSARY_FILTER_MAP, methods::UNNECESSARY_FOLD, methods::UNNECESSARY_LAZY_EVALUATIONS, + methods::UNWRAP_OR_ELSE_DEFAULT, methods::UNWRAP_USED, methods::USELESS_ASREF, methods::WRONG_SELF_CONVENTION, @@ -1341,6 +1342,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(methods::UNNECESSARY_FILTER_MAP), LintId::of(methods::UNNECESSARY_FOLD), LintId::of(methods::UNNECESSARY_LAZY_EVALUATIONS), + LintId::of(methods::UNWRAP_OR_ELSE_DEFAULT), LintId::of(methods::USELESS_ASREF), LintId::of(methods::WRONG_SELF_CONVENTION), LintId::of(methods::ZST_OFFSET), @@ -1535,6 +1537,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(methods::STRING_EXTEND_CHARS), LintId::of(methods::UNNECESSARY_FOLD), LintId::of(methods::UNNECESSARY_LAZY_EVALUATIONS), + LintId::of(methods::UNWRAP_OR_ELSE_DEFAULT), LintId::of(methods::WRONG_SELF_CONVENTION), LintId::of(misc::TOPLEVEL_REF_ARG), LintId::of(misc::ZERO_PTR), diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index 699ddce0cff..0e5121ca3d7 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -10,7 +10,7 @@ use clippy_utils::{ use if_chain::if_chain; use rustc_ast::ast::{Expr, ExprKind, Lit, LitKind}; use rustc_errors::Applicability; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_tool_lint, impl_lint_pass}; use std::iter; @@ -222,7 +222,7 @@ impl_lint_pass!(LiteralDigitGrouping => [ impl EarlyLintPass for LiteralDigitGrouping { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { - if in_external_macro(cx.sess(), expr.span) { + if in_external_macro(cx.sess, expr.span) { return; } @@ -415,7 +415,7 @@ impl_lint_pass!(DecimalLiteralRepresentation => [DECIMAL_LITERAL_REPRESENTATION] impl EarlyLintPass for DecimalLiteralRepresentation { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { - if in_external_macro(cx.sess(), expr.span) { + if in_external_macro(cx.sess, expr.span) { return; } diff --git a/clippy_lints/src/loops/manual_memcpy.rs b/clippy_lints/src/loops/manual_memcpy.rs index a98e2dc1372..7d5ed3ab0a7 100644 --- a/clippy_lints/src/loops/manual_memcpy.rs +++ b/clippy_lints/src/loops/manual_memcpy.rs @@ -204,11 +204,8 @@ struct MinifyingSugg<'a>(Sugg<'a>); impl<'a> MinifyingSugg<'a> { fn as_str(&self) -> &str { - // HACK: Don't sync to Clippy! Required because something with the `or_patterns` feature - // changed and this would now require parentheses. - match &self.0 { - Sugg::NonParen(s) | Sugg::MaybeParen(s) | Sugg::BinOp(_, s) => s.as_ref(), - } + let (Sugg::NonParen(s) | Sugg::MaybeParen(s) | Sugg::BinOp(_, s)) = &self.0; + s.as_ref() } fn into_sugg(self) -> Sugg<'a> { diff --git a/clippy_lints/src/loops/never_loop.rs b/clippy_lints/src/loops/never_loop.rs index e97b7c94170..6d9f6215ed4 100644 --- a/clippy_lints/src/loops/never_loop.rs +++ b/clippy_lints/src/loops/never_loop.rs @@ -1,13 +1,36 @@ +use super::utils::make_iterator_snippet; use super::NEVER_LOOP; -use clippy_utils::diagnostics::span_lint; -use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, Stmt, StmtKind}; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::higher; +use clippy_utils::source::snippet; +use rustc_errors::Applicability; +use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, LoopSource, Node, Pat, Stmt, StmtKind}; use rustc_lint::LateContext; use std::iter::{once, Iterator}; pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let ExprKind::Loop(block, _, _, _) = expr.kind { + if let ExprKind::Loop(block, _, source, _) = expr.kind { match never_loop_block(block, expr.hir_id) { - NeverLoopResult::AlwaysBreak => span_lint(cx, NEVER_LOOP, expr.span, "this loop never actually loops"), + NeverLoopResult::AlwaysBreak => { + span_lint_and_then(cx, NEVER_LOOP, expr.span, "this loop never actually loops", |diag| { + if_chain! { + if let LoopSource::ForLoop = source; + if let Some((_, Node::Expr(parent_match))) = cx.tcx.hir().parent_iter(expr.hir_id).nth(1); + if let Some((pat, iterator, _, for_span)) = higher::for_loop(parent_match); + then { + // Suggests using an `if let` instead. This is `Unspecified` because the + // loop may (probably) contain `break` statements which would be invalid + // in an `if let`. + diag.span_suggestion_verbose( + for_span.with_hi(iterator.span.hi()), + "if you need the first element of the iterator, try writing", + for_to_if_let_sugg(cx, iterator, pat), + Applicability::Unspecified, + ); + } + }; + }); + }, NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (), } } @@ -170,3 +193,14 @@ fn never_loop_expr_branch<'a, T: Iterator<Item = &'a Expr<'a>>>(e: &mut T, main_ e.map(|e| never_loop_expr(e, main_loop_id)) .fold(NeverLoopResult::AlwaysBreak, combine_branches) } + +fn for_to_if_let_sugg(cx: &LateContext<'_>, iterator: &Expr<'_>, pat: &Pat<'_>) -> String { + let pat_snippet = snippet(cx, pat.span, "_"); + let iter_snippet = make_iterator_snippet(cx, iterator, &mut Applicability::Unspecified); + + format!( + "if let Some({pat}) = {iter}.next()", + pat = pat_snippet, + iter = iter_snippet + ) +} diff --git a/clippy_lints/src/loops/while_let_on_iterator.rs b/clippy_lints/src/loops/while_let_on_iterator.rs index d57588716a5..ef822e0cbe5 100644 --- a/clippy_lints/src/loops/while_let_on_iterator.rs +++ b/clippy_lints/src/loops/while_let_on_iterator.rs @@ -7,7 +7,7 @@ use clippy_utils::{ use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor}; -use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, MatchSource, Node, PatKind, QPath, UnOp}; +use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, MatchSource, Mutability, Node, PatKind, QPath, UnOp}; use rustc_lint::LateContext; use rustc_span::{symbol::sym, Span, Symbol}; @@ -48,7 +48,12 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // borrowed mutably. TODO: If the struct can be partially moved from and the struct isn't used // afterwards a mutable borrow of a field isn't necessary. let ref_mut = if !iter_expr.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr, loop_expr) { - "&mut " + if cx.typeck_results().node_type(iter_expr.hir_id).ref_mutability() == Some(Mutability::Mut) { + // Reborrow for mutable references. It may not be possible to get a mutable reference here. + "&mut *" + } else { + "&mut " + } } else { "" }; @@ -69,6 +74,8 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { struct IterExpr { /// The span of the whole expression, not just the path and fields stored here. span: Span, + /// The HIR id of the whole expression, not just the path and fields stored here. + hir_id: HirId, /// The fields used, in order of child to parent. fields: Vec<Symbol>, /// The path being used. @@ -78,12 +85,14 @@ struct IterExpr { /// the expression might have side effects. fn try_parse_iter_expr(cx: &LateContext<'_>, mut e: &Expr<'_>) -> Option<IterExpr> { let span = e.span; + let hir_id = e.hir_id; let mut fields = Vec::new(); loop { match e.kind { ExprKind::Path(ref path) => { break Some(IterExpr { span, + hir_id, fields, path: cx.qpath_res(path, e.hir_id), }); @@ -137,7 +146,7 @@ fn is_expr_same_child_or_parent_field(cx: &LateContext<'_>, expr: &Expr<'_>, fie match expr.kind { ExprKind::Field(base, name) => { if let Some((head_field, tail_fields)) = fields.split_first() { - if name.name == *head_field && is_expr_same_field(cx, base, fields, path_res) { + if name.name == *head_field && is_expr_same_field(cx, base, tail_fields, path_res) { return true; } // Check if the expression is a parent field diff --git a/clippy_lints/src/methods/extend_with_drain.rs b/clippy_lints/src/methods/extend_with_drain.rs index 57e10ce42f8..8829b8c5f4d 100644 --- a/clippy_lints/src/methods/extend_with_drain.rs +++ b/clippy_lints/src/methods/extend_with_drain.rs @@ -16,7 +16,10 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, arg: //check source object if let ExprKind::MethodCall(src_method, _, [drain_vec, drain_arg], _) = &arg.kind; if src_method.ident.as_str() == "drain"; - if let src_ty = cx.typeck_results().expr_ty(drain_vec).peel_refs(); + let src_ty = cx.typeck_results().expr_ty(drain_vec); + //check if actual src type is mutable for code suggestion + let immutable = src_ty.is_mutable_ptr(); + let src_ty = src_ty.peel_refs(); if is_type_diagnostic_item(cx, src_ty, sym::vec_type); //check drain range if let src_ty_range = cx.typeck_results().expr_ty(drain_arg).peel_refs(); @@ -30,8 +33,9 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, arg: "use of `extend` instead of `append` for adding the full range of a second vector", "try this", format!( - "{}.append(&mut {})", + "{}.append({}{})", snippet_with_applicability(cx, recv.span, "..", &mut applicability), + if immutable { "" } else { "&mut " }, snippet_with_applicability(cx, drain_vec.span, "..", &mut applicability) ), applicability, diff --git a/clippy_lints/src/methods/from_iter_instead_of_collect.rs b/clippy_lints/src/methods/from_iter_instead_of_collect.rs index b4188d9ed30..99c03844f49 100644 --- a/clippy_lints/src/methods/from_iter_instead_of_collect.rs +++ b/clippy_lints/src/methods/from_iter_instead_of_collect.rs @@ -1,10 +1,11 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_opt; use clippy_utils::ty::implements_trait; use clippy_utils::{is_expr_path_def_path, paths, sugg}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; -use rustc_lint::{LateContext, LintContext}; +use rustc_lint::LateContext; use rustc_middle::ty::Ty; use rustc_span::sym; @@ -43,7 +44,7 @@ fn extract_turbofish(cx: &LateContext<'_>, expr: &hir::Expr<'_>, ty: Ty<'tcx>) - let call_site = expr.span.source_callsite(); if_chain! { - if let Ok(snippet) = cx.sess().source_map().span_to_snippet(call_site); + if let Some(snippet) = snippet_opt(cx, call_site); let snippet_split = snippet.split("::").collect::<Vec<_>>(); if let Some((_, elements)) = snippet_split.split_last(); diff --git a/clippy_lints/src/methods/map_flatten.rs b/clippy_lints/src/methods/map_flatten.rs index e8ad16bc0de..08d3a7ce92b 100644 --- a/clippy_lints/src/methods/map_flatten.rs +++ b/clippy_lints/src/methods/map_flatten.rs @@ -52,18 +52,32 @@ pub(super) fn check<'tcx>( ); } - // lint if caller of `.map().flatten()` is an Option - if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::option_type) { - let func_snippet = snippet(cx, map_arg.span, ".."); - let hint = format!(".and_then({})", func_snippet); - span_lint_and_sugg( - cx, - MAP_FLATTEN, - expr.span.with_lo(recv.span.hi()), - "called `map(..).flatten()` on an `Option`", - "try using `and_then` instead", - hint, - Applicability::MachineApplicable, - ); - } + // lint if caller of `.map().flatten()` is an Option or Result + let caller_type = match cx.typeck_results().expr_ty(recv).kind() { + ty::Adt(adt, _) => { + if cx.tcx.is_diagnostic_item(sym::option_type, adt.did) { + "Option" + } else if cx.tcx.is_diagnostic_item(sym::result_type, adt.did) { + "Result" + } else { + return; + } + }, + _ => { + return; + }, + }; + + let func_snippet = snippet(cx, map_arg.span, ".."); + let hint = format!(".and_then({})", func_snippet); + let lint_info = format!("called `map(..).flatten()` on an `{}`", caller_type); + span_lint_and_sugg( + cx, + MAP_FLATTEN, + expr.span.with_lo(recv.span.hi()), + &lint_info, + "try using `and_then` instead", + hint, + Applicability::MachineApplicable, + ); } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 1909fabb22f..91606ed3b2b 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -56,6 +56,7 @@ mod uninit_assumed_init; mod unnecessary_filter_map; mod unnecessary_fold; mod unnecessary_lazy_eval; +mod unwrap_or_else_default; mod unwrap_used; mod useless_asref; mod utils; @@ -312,6 +313,31 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does + /// Checks for usages of `_.unwrap_or_else(Default::default)` on `Option` and + /// `Result` values. + /// + /// ### Why is this bad? + /// Readability, these can be written as `_.unwrap_or_default`, which is + /// simpler and more concise. + /// + /// ### Examples + /// ```rust + /// # let x = Some(1); + /// + /// // Bad + /// x.unwrap_or_else(Default::default); + /// x.unwrap_or_else(u32::default); + /// + /// // Good + /// x.unwrap_or_default(); + /// ``` + pub UNWRAP_OR_ELSE_DEFAULT, + style, + "using `.unwrap_or_else(Default::default)`, which is more succinctly expressed as `.unwrap_or_default()`" +} + +declare_clippy_lint! { + /// ### What it does /// Checks for usage of `option.map(_).unwrap_or(_)` or `option.map(_).unwrap_or_else(_)` or /// `result.map(_).unwrap_or_else(_)`. /// @@ -1766,6 +1792,7 @@ impl_lint_pass!(Methods => [ SHOULD_IMPLEMENT_TRAIT, WRONG_SELF_CONVENTION, OK_EXPECT, + UNWRAP_OR_ELSE_DEFAULT, MAP_UNWRAP_OR, RESULT_MAP_OR_INTO_OPTION, OPTION_MAP_OR_NONE, @@ -2172,7 +2199,10 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio }, ("unwrap_or_else", [u_arg]) => match method_call!(recv) { Some(("map", [recv, map_arg], _)) if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, msrv) => {}, - _ => unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or"), + _ => { + unwrap_or_else_default::check(cx, expr, recv, u_arg); + unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or"); + }, }, _ => {}, } diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index ef615b0aa40..c1d22e5d72c 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -1,7 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::eager_or_lazy::is_lazyness_candidate; +use clippy_utils::is_trait_item; use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_macro_callsite}; -use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type}; +use clippy_utils::ty::implements_trait; +use clippy_utils::ty::{is_type_diagnostic_item, match_type}; use clippy_utils::{contains_return, last_path_segment, paths}; use if_chain::if_chain; use rustc_errors::Applicability; @@ -34,15 +36,23 @@ pub(super) fn check<'tcx>( or_has_args: bool, span: Span, ) -> bool { + let is_default_default = || is_trait_item(cx, fun, sym::Default); + + let implements_default = |arg, default_trait_id| { + let arg_ty = cx.typeck_results().expr_ty(arg); + implements_trait(cx, arg_ty, default_trait_id, &[]) + }; + if_chain! { if !or_has_args; if name == "unwrap_or"; if let hir::ExprKind::Path(ref qpath) = fun.kind; - let path = last_path_segment(qpath).ident.name; - if matches!(path, kw::Default | sym::new); - let arg_ty = cx.typeck_results().expr_ty(arg); if let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default); - if implements_trait(cx, arg_ty, default_trait_id, &[]); + let path = last_path_segment(qpath).ident.name; + // needs to target Default::default in particular or be *::new and have a Default impl + // available + if (matches!(path, kw::Default) && is_default_default()) + || (matches!(path, sym::new) && implements_default(arg, default_trait_id)); then { let mut applicability = Applicability::MachineApplicable; diff --git a/clippy_lints/src/methods/unwrap_or_else_default.rs b/clippy_lints/src/methods/unwrap_or_else_default.rs new file mode 100644 index 00000000000..677aa80e1b7 --- /dev/null +++ b/clippy_lints/src/methods/unwrap_or_else_default.rs @@ -0,0 +1,45 @@ +//! Lint for `some_result_or_option.unwrap_or_else(Default::default)` + +use super::UNWRAP_OR_ELSE_DEFAULT; +use clippy_utils::{ + diagnostics::span_lint_and_sugg, is_trait_item, source::snippet_with_applicability, ty::is_type_diagnostic_item, +}; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::LateContext; +use rustc_span::sym; + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx hir::Expr<'_>, + recv: &'tcx hir::Expr<'_>, + u_arg: &'tcx hir::Expr<'_>, +) { + // something.unwrap_or_else(Default::default) + // ^^^^^^^^^- recv ^^^^^^^^^^^^^^^^- u_arg + // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- expr + let recv_ty = cx.typeck_results().expr_ty(recv); + let is_option = is_type_diagnostic_item(cx, recv_ty, sym::option_type); + let is_result = is_type_diagnostic_item(cx, recv_ty, sym::result_type); + + if_chain! { + if is_option || is_result; + if is_trait_item(cx, u_arg, sym::Default); + then { + let mut applicability = Applicability::MachineApplicable; + + span_lint_and_sugg( + cx, + UNWRAP_OR_ELSE_DEFAULT, + expr.span, + "use of `.unwrap_or_else(..)` to construct default value", + "try", + format!( + "{}.unwrap_or_default()", + snippet_with_applicability(cx, recv.span, "..", &mut applicability) + ), + applicability, + ); + } + } +} diff --git a/clippy_lints/src/misc_early/mod.rs b/clippy_lints/src/misc_early/mod.rs index 06fe967dafc..b32feab4ee3 100644 --- a/clippy_lints/src/misc_early/mod.rs +++ b/clippy_lints/src/misc_early/mod.rs @@ -12,7 +12,7 @@ use clippy_utils::source::snippet_opt; use rustc_ast::ast::{Expr, Generics, Lit, LitFloatType, LitIntType, LitKind, NodeId, Pat, PatKind}; use rustc_ast::visit::FnKind; use rustc_data_structures::fx::FxHashMap; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; @@ -307,7 +307,7 @@ impl EarlyLintPass for MiscEarlyLints { } fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { - if in_external_macro(cx.sess(), expr.span) { + if in_external_macro(cx.sess, expr.span) { return; } double_neg::check(cx, expr); diff --git a/clippy_lints/src/misc_early/unneeded_field_pattern.rs b/clippy_lints/src/misc_early/unneeded_field_pattern.rs index 2201cf56d52..fff533167ed 100644 --- a/clippy_lints/src/misc_early/unneeded_field_pattern.rs +++ b/clippy_lints/src/misc_early/unneeded_field_pattern.rs @@ -1,6 +1,7 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_help}; +use clippy_utils::source::snippet_opt; use rustc_ast::ast::{Pat, PatKind}; -use rustc_lint::{EarlyContext, LintContext}; +use rustc_lint::EarlyContext; use super::UNNEEDED_FIELD_PATTERN; @@ -48,7 +49,7 @@ pub(super) fn check(cx: &EarlyContext<'_>, pat: &Pat) { match field.pat.kind { PatKind::Wild => {}, _ => { - if let Ok(n) = cx.sess().source_map().span_to_snippet(field.span) { + if let Some(n) = snippet_opt(cx, field.span) { normal.push(n); } }, diff --git a/clippy_lints/src/missing_doc.rs b/clippy_lints/src/missing_doc.rs index d358e9fb876..8f5aeae85df 100644 --- a/clippy_lints/src/missing_doc.rs +++ b/clippy_lints/src/missing_doc.rs @@ -7,7 +7,8 @@ use clippy_utils::attrs::is_doc_hidden; use clippy_utils::diagnostics::span_lint; -use rustc_ast::ast; +use if_chain::if_chain; +use rustc_ast::ast::{self, MetaItem, MetaItemKind}; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::ty; @@ -55,6 +56,20 @@ impl MissingDoc { *self.doc_hidden_stack.last().expect("empty doc_hidden_stack") } + fn has_include(meta: Option<MetaItem>) -> bool { + if_chain! { + if let Some(meta) = meta; + if let MetaItemKind::List(list) = meta.kind; + if let Some(meta) = list.get(0); + if let Some(name) = meta.ident(); + then { + name.name == sym::include + } else { + false + } + } + } + fn check_missing_docs_attrs( &self, cx: &LateContext<'_>, @@ -80,7 +95,7 @@ impl MissingDoc { let has_doc = attrs .iter() - .any(|a| a.doc_str().is_some()); + .any(|a| a.doc_str().is_some() || Self::has_include(a.meta())); if !has_doc { span_lint( cx, diff --git a/clippy_lints/src/mutable_debug_assertion.rs b/clippy_lints/src/mutable_debug_assertion.rs index 9b44cf9d43a..ee50891cc31 100644 --- a/clippy_lints/src/mutable_debug_assertion.rs +++ b/clippy_lints/src/mutable_debug_assertion.rs @@ -87,10 +87,6 @@ impl<'a, 'tcx> Visitor<'tcx> for MutArgVisitor<'a, 'tcx> { self.found = true; return; }, - ExprKind::If(..) => { - self.found = true; - return; - }, ExprKind::Path(_) => { if let Some(adj) = self.cx.typeck_results().adjustments().get(expr.hir_id) { if adj diff --git a/clippy_lints/src/needless_borrow.rs b/clippy_lints/src/needless_borrow.rs index 3f0b23ee4d3..ba8f9446af8 100644 --- a/clippy_lints/src/needless_borrow.rs +++ b/clippy_lints/src/needless_borrow.rs @@ -27,11 +27,15 @@ declare_clippy_lint! { /// /// ### Example /// ```rust + /// fn fun(_a: &i32) {} + /// /// // Bad /// let x: &i32 = &&&&&&5; + /// fun(&x); /// /// // Good /// let x: &i32 = &5; + /// fun(x); /// ``` pub NEEDLESS_BORROW, style, diff --git a/clippy_lints/src/needless_continue.rs b/clippy_lints/src/needless_continue.rs index 5088b8bb0d3..5a50cc48d61 100644 --- a/clippy_lints/src/needless_continue.rs +++ b/clippy_lints/src/needless_continue.rs @@ -422,7 +422,7 @@ fn check_and_warn<'a>(cx: &EarlyContext<'_>, expr: &'a ast::Expr) { /// /// is transformed to /// -/// ```ignore +/// ```text /// { /// let x = 5; /// ``` diff --git a/clippy_lints/src/needless_for_each.rs b/clippy_lints/src/needless_for_each.rs index d9aa42fe8ee..9a6ddc72ce5 100644 --- a/clippy_lints/src/needless_for_each.rs +++ b/clippy_lints/src/needless_for_each.rs @@ -122,7 +122,7 @@ impl LateLintPass<'_> for NeedlessForEach { /// 2. Detect use of `return` in `Loop` in the closure body. /// /// NOTE: The functionality of this type is similar to -/// [`crate::utilts::visitors::find_all_ret_expressions`], but we can't use +/// [`clippy_utils::visitors::find_all_ret_expressions`], but we can't use /// `find_all_ret_expressions` instead of this type. The reasons are: /// 1. `find_all_ret_expressions` passes the argument of `ExprKind::Ret` to a callback, but what we /// need here is `ExprKind::Ret` itself. diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index e07518b2586..28e9e6f438e 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -3,7 +3,7 @@ use clippy_utils::source::snippet_opt; use clippy_utils::ty::has_drop; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::{BinOpKind, BlockCheckMode, Expr, ExprKind, Stmt, StmtKind, UnsafeSource}; +use rustc_hir::{is_range_literal, BinOpKind, BlockCheckMode, Expr, ExprKind, Stmt, StmtKind, UnsafeSource}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use std::ops::Deref; @@ -68,12 +68,14 @@ fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { ExprKind::Call(callee, args) => { if let ExprKind::Path(ref qpath) = callee.kind { let res = cx.qpath_res(qpath, callee.hir_id); - match res { - Res::Def(DefKind::Struct | DefKind::Variant | DefKind::Ctor(..), ..) => { - !has_drop(cx, cx.typeck_results().expr_ty(expr)) - && args.iter().all(|arg| has_no_effect(cx, arg)) - }, - _ => false, + let def_matched = matches!( + res, + Res::Def(DefKind::Struct | DefKind::Variant | DefKind::Ctor(..), ..) + ); + if def_matched || is_range_literal(expr) { + !has_drop(cx, cx.typeck_results().expr_ty(expr)) && args.iter().all(|arg| has_no_effect(cx, arg)) + } else { + false } } else { false diff --git a/clippy_lints/src/non_expressive_names.rs b/clippy_lints/src/non_expressive_names.rs index 06c431babc2..ac21eb5275f 100644 --- a/clippy_lints/src/non_expressive_names.rs +++ b/clippy_lints/src/non_expressive_names.rs @@ -1,7 +1,6 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; use rustc_ast::ast::{ - Arm, AssocItem, AssocItemKind, Attribute, Block, FnDecl, FnKind, Item, ItemKind, Local, Pat, - PatKind, + Arm, AssocItem, AssocItemKind, Attribute, Block, FnDecl, FnKind, Item, ItemKind, Local, Pat, PatKind, }; use rustc_ast::visit::{walk_block, walk_expr, walk_pat, Visitor}; use rustc_lint::{EarlyContext, EarlyLintPass}; @@ -127,6 +126,7 @@ const ALLOWED_TO_BE_SIMILAR: &[&[&str]] = &[ &["qpath", "path"], &["lit", "lint"], &["wparam", "lparam"], + &["iter", "item"], ]; struct SimilarNamesNameVisitor<'a, 'tcx, 'b>(&'b mut SimilarNamesLocalVisitor<'a, 'tcx>); diff --git a/clippy_lints/src/nonstandard_macro_braces.rs b/clippy_lints/src/nonstandard_macro_braces.rs index dbe9cbe0ded..ca660a9250d 100644 --- a/clippy_lints/src/nonstandard_macro_braces.rs +++ b/clippy_lints/src/nonstandard_macro_braces.rs @@ -7,6 +7,7 @@ use clippy_utils::{diagnostics::span_lint_and_help, in_macro, is_direct_expn_of, use if_chain::if_chain; use rustc_ast::ast; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_hir::def_id::DefId; use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::Span; @@ -91,13 +92,23 @@ impl EarlyLintPass for MacroBraces { } fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, mac_braces: &'a MacroBraces) -> Option<MacroInfo<'a>> { + let unnested_or_local = || { + let nested = in_macro(span.ctxt().outer_expn_data().call_site); + !nested + || span + .macro_backtrace() + .last() + .map_or(false, |e| e.macro_def_id.map_or(false, DefId::is_local)) + }; if_chain! { + // Make sure we are only one level deep otherwise there are to many FP's if in_macro(span); 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); // we must check only invocation sites // https://github.com/rust-lang/rust-clippy/issues/7422 - if snip.starts_with(name); + if snip.starts_with(&format!("{}!", name)); + if unnested_or_local(); // make formatting consistent let c = snip.replace(" ", ""); if !c.starts_with(&format!("{}!{}", name, braces.0)); diff --git a/clippy_lints/src/pass_by_ref_or_value.rs b/clippy_lints/src/pass_by_ref_or_value.rs index 1222a95d4ea..157b18c1f6b 100644 --- a/clippy_lints/src/pass_by_ref_or_value.rs +++ b/clippy_lints/src/pass_by_ref_or_value.rs @@ -2,9 +2,9 @@ use std::cmp; use std::iter; use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::is_self_ty; use clippy_utils::source::snippet; use clippy_utils::ty::is_copy; +use clippy_utils::{is_self, is_self_ty}; use if_chain::if_chain; use rustc_ast::attr; use rustc_errors::Applicability; @@ -170,7 +170,7 @@ impl<'tcx> PassByRefOrValue { if size <= self.ref_min_size; if let hir::TyKind::Rptr(_, MutTy { ty: decl_ty, .. }) = input.kind; then { - let value_type = if is_self_ty(decl_ty) { + let value_type = if fn_body.and_then(|body| body.params.get(index)).map_or(false, is_self) { "self".into() } else { snippet(cx, decl_ty.span, "_").into() diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs index eeeac35a6d5..530b3396abe 100644 --- a/clippy_lints/src/redundant_clone.rs +++ b/clippy_lints/src/redundant_clone.rs @@ -725,7 +725,7 @@ fn rvalue_locals(rvalue: &mir::Rvalue<'_>, mut visit: impl FnMut(mir::Local)) { BinaryOp(_, box (lhs, rhs)) | CheckedBinaryOp(_, box (lhs, rhs)) => { visit_op(lhs); visit_op(rhs); - } + }, _ => (), } } diff --git a/clippy_lints/src/redundant_closure_call.rs b/clippy_lints/src/redundant_closure_call.rs index a79b2fe76e2..7314bce83e0 100644 --- a/clippy_lints/src/redundant_closure_call.rs +++ b/clippy_lints/src/redundant_closure_call.rs @@ -8,7 +8,7 @@ use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::intravisit as hir_visit; use rustc_hir::intravisit::Visitor as HirVisitor; -use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass}; use rustc_middle::hir::map::Map; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -63,7 +63,7 @@ impl<'ast> ast_visit::Visitor<'ast> for ReturnVisitor { impl EarlyLintPass for RedundantClosureCall { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) { - if in_external_macro(cx.sess(), expr.span) { + if in_external_macro(cx.sess, expr.span) { return; } if_chain! { diff --git a/clippy_lints/src/reference.rs b/clippy_lints/src/reference.rs index e0930d69ab9..77b6e60d893 100644 --- a/clippy_lints/src/reference.rs +++ b/clippy_lints/src/reference.rs @@ -54,7 +54,8 @@ impl EarlyLintPass for DerefAddrOf { then { let mut applicability = Applicability::MachineApplicable; let sugg = if e.span.from_expansion() { - if let Ok(macro_source) = cx.sess.source_map().span_to_snippet(e.span) { + #[allow(clippy::option_if_let_else)] + if let Some(macro_source) = snippet_opt(cx, e.span) { // Remove leading whitespace from the given span // e.g: ` $visitor` turns into `$visitor` let trim_leading_whitespaces = |span| { diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs index 4fa8e77a67b..f126908e84b 100644 --- a/clippy_lints/src/swap.rs +++ b/clippy_lints/src/swap.rs @@ -1,15 +1,16 @@ -use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::source::snippet_with_applicability; use clippy_utils::sugg::Sugg; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{differing_macro_contexts, eq_expr_value}; +use clippy_utils::{can_mut_borrow_both, differing_macro_contexts, eq_expr_value}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{Block, Expr, ExprKind, PatKind, QPath, StmtKind}; +use rustc_hir::{BinOpKind, Block, Expr, ExprKind, PatKind, QPath, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::sym; +use rustc_span::source_map::Spanned; +use rustc_span::{sym, Span}; declare_clippy_lint! { /// ### What it does @@ -70,9 +71,67 @@ impl<'tcx> LateLintPass<'tcx> for Swap { fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) { check_manual_swap(cx, block); check_suspicious_swap(cx, block); + check_xor_swap(cx, block); } } +fn generate_swap_warning(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>, span: Span, is_xor_based: bool) { + let mut applicability = Applicability::MachineApplicable; + + if !can_mut_borrow_both(cx, e1, e2) { + if let ExprKind::Index(lhs1, idx1) = e1.kind { + if let ExprKind::Index(lhs2, idx2) = e2.kind { + if eq_expr_value(cx, lhs1, lhs2) { + let ty = cx.typeck_results().expr_ty(lhs1).peel_refs(); + + if matches!(ty.kind(), ty::Slice(_)) + || matches!(ty.kind(), ty::Array(_, _)) + || is_type_diagnostic_item(cx, ty, sym::vec_type) + || is_type_diagnostic_item(cx, ty, sym::vecdeque_type) + { + let slice = Sugg::hir_with_applicability(cx, lhs1, "<slice>", &mut applicability); + span_lint_and_sugg( + cx, + MANUAL_SWAP, + span, + &format!("this looks like you are swapping elements of `{}` manually", slice), + "try", + format!( + "{}.swap({}, {})", + slice.maybe_par(), + snippet_with_applicability(cx, idx1.span, "..", &mut applicability), + snippet_with_applicability(cx, idx2.span, "..", &mut applicability), + ), + applicability, + ); + } + } + } + } + return; + } + + let first = Sugg::hir_with_applicability(cx, e1, "..", &mut applicability); + let second = Sugg::hir_with_applicability(cx, e2, "..", &mut applicability); + span_lint_and_then( + cx, + MANUAL_SWAP, + span, + &format!("this looks like you are swapping `{}` and `{}` manually", first, second), + |diag| { + diag.span_suggestion( + span, + "try", + format!("std::mem::swap({}, {})", first.mut_addr(), second.mut_addr()), + applicability, + ); + if !is_xor_based { + diag.note("or maybe you should use `std::mem::replace`?"); + } + }, + ); +} + /// Implementation of the `MANUAL_SWAP` lint. fn check_manual_swap(cx: &LateContext<'_>, block: &Block<'_>) { for w in block.stmts.windows(3) { @@ -96,121 +155,11 @@ fn check_manual_swap(cx: &LateContext<'_>, block: &Block<'_>) { if eq_expr_value(cx, tmp_init, lhs1); if eq_expr_value(cx, rhs1, lhs2); then { - if let ExprKind::Field(lhs1, _) = lhs1.kind { - if let ExprKind::Field(lhs2, _) = lhs2.kind { - if lhs1.hir_id.owner == lhs2.hir_id.owner { - return; - } - } - } - - let mut applicability = Applicability::MachineApplicable; - - let slice = check_for_slice(cx, lhs1, lhs2); - let (replace, what, sugg) = if let Slice::NotSwappable = slice { - return; - } else if let Slice::Swappable(slice, idx1, idx2) = slice { - if let Some(slice) = Sugg::hir_opt(cx, slice) { - ( - false, - format!(" elements of `{}`", slice), - format!( - "{}.swap({}, {})", - slice.maybe_par(), - snippet_with_applicability(cx, idx1.span, "..", &mut applicability), - snippet_with_applicability(cx, idx2.span, "..", &mut applicability), - ), - ) - } else { - (false, String::new(), String::new()) - } - } else if let (Some(first), Some(second)) = (Sugg::hir_opt(cx, lhs1), Sugg::hir_opt(cx, rhs1)) { - ( - true, - format!(" `{}` and `{}`", first, second), - format!("std::mem::swap({}, {})", first.mut_addr(), second.mut_addr()), - ) - } else { - (true, String::new(), String::new()) - }; - let span = w[0].span.to(second.span); - - span_lint_and_then( - cx, - MANUAL_SWAP, - span, - &format!("this looks like you are swapping{} manually", what), - |diag| { - if !sugg.is_empty() { - diag.span_suggestion( - span, - "try", - sugg, - applicability, - ); - - if replace { - diag.note("or maybe you should use `std::mem::replace`?"); - } - } - } - ); - } - } - } -} - -enum Slice<'a> { - /// `slice.swap(idx1, idx2)` can be used - /// - /// ## Example - /// - /// ```rust - /// # let mut a = vec![0, 1]; - /// let t = a[1]; - /// a[1] = a[0]; - /// a[0] = t; - /// // can be written as - /// a.swap(0, 1); - /// ``` - Swappable(&'a Expr<'a>, &'a Expr<'a>, &'a Expr<'a>), - /// The `swap` function cannot be used. - /// - /// ## Example - /// - /// ```rust - /// # let mut a = [vec![1, 2], vec![3, 4]]; - /// let t = a[0][1]; - /// a[0][1] = a[1][0]; - /// a[1][0] = t; - /// ``` - NotSwappable, - /// Not a slice - None, -} - -/// Checks if both expressions are index operations into "slice-like" types. -fn check_for_slice<'a>(cx: &LateContext<'_>, lhs1: &'a Expr<'_>, lhs2: &'a Expr<'_>) -> Slice<'a> { - if let ExprKind::Index(lhs1, idx1) = lhs1.kind { - if let ExprKind::Index(lhs2, idx2) = lhs2.kind { - if eq_expr_value(cx, lhs1, lhs2) { - let ty = cx.typeck_results().expr_ty(lhs1).peel_refs(); - - if matches!(ty.kind(), ty::Slice(_)) - || matches!(ty.kind(), ty::Array(_, _)) - || is_type_diagnostic_item(cx, ty, sym::vec_type) - || is_type_diagnostic_item(cx, ty, sym::vecdeque_type) - { - return Slice::Swappable(lhs1, idx1, idx2); - } - } else { - return Slice::NotSwappable; + generate_swap_warning(cx, lhs1, lhs2, span, false); } } } - - Slice::None } /// Implementation of the `ALMOST_SWAPPED` lint. @@ -262,3 +211,40 @@ fn check_suspicious_swap(cx: &LateContext<'_>, block: &Block<'_>) { } } } + +/// Implementation of the xor case for `MANUAL_SWAP` lint. +fn check_xor_swap(cx: &LateContext<'_>, block: &Block<'_>) { + for window in block.stmts.windows(3) { + if_chain! { + if let Some((lhs0, rhs0)) = extract_sides_of_xor_assign(&window[0]); + if let Some((lhs1, rhs1)) = extract_sides_of_xor_assign(&window[1]); + if let Some((lhs2, rhs2)) = extract_sides_of_xor_assign(&window[2]); + if eq_expr_value(cx, lhs0, rhs1); + if eq_expr_value(cx, lhs2, rhs1); + if eq_expr_value(cx, lhs1, rhs0); + if eq_expr_value(cx, lhs1, rhs2); + then { + let span = window[0].span.to(window[2].span); + generate_swap_warning(cx, lhs0, rhs0, span, true); + } + }; + } +} + +/// Returns the lhs and rhs of an xor assignment statement. +fn extract_sides_of_xor_assign<'a, 'hir>(stmt: &'a Stmt<'hir>) -> Option<(&'a Expr<'hir>, &'a Expr<'hir>)> { + if let StmtKind::Semi(expr) = stmt.kind { + if let ExprKind::AssignOp( + Spanned { + node: BinOpKind::BitXor, + .. + }, + lhs, + rhs, + ) = expr.kind + { + return Some((lhs, rhs)); + } + } + None +} diff --git a/clippy_lints/src/types/mod.rs b/clippy_lints/src/types/mod.rs index ad7409fe3a9..371bb8b445a 100644 --- a/clippy_lints/src/types/mod.rs +++ b/clippy_lints/src/types/mod.rs @@ -186,7 +186,7 @@ declare_clippy_lint! { /// Checks for use of redundant allocations anywhere in the code. /// /// ### Why is this bad? - /// Expressions such as `Rc<&T>`, `Rc<Rc<T>>`, `Rc<Arc<T>>`, `Rc<Box<T>>`, Arc<&T>`, `Arc<Rc<T>>`, + /// Expressions such as `Rc<&T>`, `Rc<Rc<T>>`, `Rc<Arc<T>>`, `Rc<Box<T>>`, `Arc<&T>`, `Arc<Rc<T>>`, /// `Arc<Arc<T>>`, `Arc<Box<T>>`, `Box<&T>`, `Box<Rc<T>>`, `Box<Arc<T>>`, `Box<Box<T>>`, add an unnecessary level of indirection. /// /// ### Example diff --git a/clippy_lints/src/unnested_or_patterns.rs b/clippy_lints/src/unnested_or_patterns.rs index 9acfbc994b3..c8a231341b7 100644 --- a/clippy_lints/src/unnested_or_patterns.rs +++ b/clippy_lints/src/unnested_or_patterns.rs @@ -35,8 +35,6 @@ declare_clippy_lint! { /// ``` /// Use instead: /// ```rust - /// #![feature(or_patterns)] - /// /// fn main() { /// if let Some(0 | 2) = Some(0) {} /// } diff --git a/clippy_lints/src/unused_unit.rs b/clippy_lints/src/unused_unit.rs index 9ed5e585f84..1164ac4938f 100644 --- a/clippy_lints/src/unused_unit.rs +++ b/clippy_lints/src/unused_unit.rs @@ -1,10 +1,10 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::position_before_rarrow; +use clippy_utils::source::{position_before_rarrow, snippet_opt}; use if_chain::if_chain; use rustc_ast::ast; use rustc_ast::visit::FnKind; use rustc_errors::Applicability; -use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; use rustc_span::BytePos; @@ -125,17 +125,16 @@ fn is_unit_expr(expr: &ast::Expr) -> bool { } fn lint_unneeded_unit_return(cx: &EarlyContext<'_>, ty: &ast::Ty, span: Span) { - let (ret_span, appl) = if let Ok(fn_source) = cx.sess().source_map().span_to_snippet(span.with_hi(ty.span.hi())) { - position_before_rarrow(&fn_source).map_or((ty.span, Applicability::MaybeIncorrect), |rpos| { - ( - #[allow(clippy::cast_possible_truncation)] - ty.span.with_lo(BytePos(span.lo().0 + rpos as u32)), - Applicability::MachineApplicable, - ) - }) - } else { - (ty.span, Applicability::MaybeIncorrect) - }; + let (ret_span, appl) = + snippet_opt(cx, span.with_hi(ty.span.hi())).map_or((ty.span, Applicability::MaybeIncorrect), |fn_source| { + position_before_rarrow(&fn_source).map_or((ty.span, Applicability::MaybeIncorrect), |rpos| { + ( + #[allow(clippy::cast_possible_truncation)] + ty.span.with_lo(BytePos(span.lo().0 + rpos as u32)), + Applicability::MachineApplicable, + ) + }) + }); span_lint_and_sugg( cx, UNUSED_UNIT, diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 44d3d456342..a28b1d78f7d 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -36,13 +36,13 @@ impl TryConf { /// See (rust-clippy#7172) macro_rules! define_Conf { ($( - #[doc = $doc:literal] + $(#[doc = $doc:literal])+ $(#[conf_deprecated($dep:literal)])? ($name:ident: $ty:ty = $default:expr), )*) => { /// Clippy lint configuration pub struct Conf { - $(#[doc = $doc] pub $name: $ty,)* + $($(#[doc = $doc])+ pub $name: $ty,)* } mod defaults { @@ -119,7 +119,7 @@ macro_rules! define_Conf { stringify!($name), stringify!($ty), format!("{:?}", super::defaults::$name()), - $doc, + concat!($($doc, '\n',)*), deprecation_reason, ) }, @@ -132,18 +132,30 @@ macro_rules! define_Conf { // N.B., this macro is parsed by util/lintlib.py define_Conf! { - /// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION. Suppress lints whenever the suggested change would cause breakage for other crates. + /// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION. + /// + /// Suppress lints whenever the suggested change would cause breakage for other crates. (avoid_breaking_exported_api: bool = true), - /// Lint: MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE. The minimum rust version that the project supports + /// Lint: MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE. + /// + /// The minimum rust version that the project supports (msrv: Option<String> = None), - /// Lint: BLACKLISTED_NAME. The list of blacklisted names to lint about. NB: `bar` is not here since it has legitimate uses + /// Lint: BLACKLISTED_NAME. + /// + /// The list of blacklisted names to lint about. NB: `bar` is not here since it has legitimate uses (blacklisted_names: Vec<String> = ["foo", "baz", "quux"].iter().map(ToString::to_string).collect()), - /// Lint: COGNITIVE_COMPLEXITY. The maximum cognitive complexity a function can have + /// Lint: COGNITIVE_COMPLEXITY. + /// + /// The maximum cognitive complexity a function can have (cognitive_complexity_threshold: u64 = 25), - /// DEPRECATED LINT: CYCLOMATIC_COMPLEXITY. Use the Cognitive Complexity lint instead. + /// DEPRECATED LINT: CYCLOMATIC_COMPLEXITY. + /// + /// Use the Cognitive Complexity lint instead. #[conf_deprecated("Please use `cognitive-complexity-threshold` instead")] (cyclomatic_complexity_threshold: Option<u64> = None), - /// Lint: DOC_MARKDOWN. The list of words this lint should not consider as identifiers needing ticks + /// Lint: DOC_MARKDOWN. + /// + /// The list of words this lint should not consider as identifiers needing ticks (doc_valid_idents: Vec<String> = [ "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "DirectX", @@ -164,55 +176,109 @@ define_Conf! { "MinGW", "CamelCase", ].iter().map(ToString::to_string).collect()), - /// Lint: TOO_MANY_ARGUMENTS. The maximum number of argument a function or method can have + /// Lint: TOO_MANY_ARGUMENTS. + /// + /// The maximum number of argument a function or method can have (too_many_arguments_threshold: u64 = 7), - /// Lint: TYPE_COMPLEXITY. The maximum complexity a type can have + /// Lint: TYPE_COMPLEXITY. + /// + /// The maximum complexity a type can have (type_complexity_threshold: u64 = 250), - /// Lint: MANY_SINGLE_CHAR_NAMES. The maximum number of single char bindings a scope may have + /// Lint: MANY_SINGLE_CHAR_NAMES. + /// + /// The maximum number of single char bindings a scope may have (single_char_binding_names_threshold: u64 = 4), - /// Lint: BOXED_LOCAL, USELESS_VEC. The maximum size of objects (in bytes) that will be linted. Larger objects are ok on the heap + /// Lint: BOXED_LOCAL, USELESS_VEC. + /// + /// The maximum size of objects (in bytes) that will be linted. Larger objects are ok on the heap (too_large_for_stack: u64 = 200), - /// Lint: ENUM_VARIANT_NAMES. The minimum number of enum variants for the lints about variant names to trigger + /// Lint: ENUM_VARIANT_NAMES. + /// + /// The minimum number of enum variants for the lints about variant names to trigger (enum_variant_name_threshold: u64 = 3), - /// Lint: LARGE_ENUM_VARIANT. The maximum size of a enum's variant to avoid box suggestion + /// Lint: LARGE_ENUM_VARIANT. + /// + /// The maximum size of a enum's variant to avoid box suggestion (enum_variant_size_threshold: u64 = 200), - /// Lint: VERBOSE_BIT_MASK. The maximum allowed size of a bit mask before suggesting to use 'trailing_zeros' + /// Lint: VERBOSE_BIT_MASK. + /// + /// The maximum allowed size of a bit mask before suggesting to use 'trailing_zeros' (verbose_bit_mask_threshold: u64 = 1), - /// Lint: DECIMAL_LITERAL_REPRESENTATION. The lower bound for linting decimal literals + /// Lint: DECIMAL_LITERAL_REPRESENTATION. + /// + /// The lower bound for linting decimal literals (literal_representation_threshold: u64 = 16384), - /// Lint: TRIVIALLY_COPY_PASS_BY_REF. The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by reference. + /// Lint: TRIVIALLY_COPY_PASS_BY_REF. + /// + /// The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by reference. (trivial_copy_size_limit: Option<u64> = None), - /// Lint: LARGE_TYPE_PASS_BY_MOVE. The minimum size (in bytes) to consider a type for passing by reference instead of by value. + /// Lint: LARGE_TYPE_PASS_BY_MOVE. + /// + /// The minimum size (in bytes) to consider a type for passing by reference instead of by value. (pass_by_value_size_limit: u64 = 256), - /// Lint: TOO_MANY_LINES. The maximum number of lines a function or method can have + /// Lint: TOO_MANY_LINES. + /// + /// The maximum number of lines a function or method can have (too_many_lines_threshold: u64 = 100), - /// Lint: LARGE_STACK_ARRAYS, LARGE_CONST_ARRAYS. The maximum allowed size for arrays on the stack + /// Lint: LARGE_STACK_ARRAYS, LARGE_CONST_ARRAYS. + /// + /// The maximum allowed size for arrays on the stack (array_size_threshold: u64 = 512_000), - /// Lint: VEC_BOX. The size of the boxed type in bytes, where boxing in a `Vec` is allowed + /// Lint: VEC_BOX. + /// + /// The size of the boxed type in bytes, where boxing in a `Vec` is allowed (vec_box_size_threshold: u64 = 4096), - /// Lint: TYPE_REPETITION_IN_BOUNDS. The maximum number of bounds a trait can have to be linted + /// Lint: TYPE_REPETITION_IN_BOUNDS. + /// + /// The maximum number of bounds a trait can have to be linted (max_trait_bounds: u64 = 3), - /// Lint: STRUCT_EXCESSIVE_BOOLS. The maximum number of bool fields a struct can have + /// Lint: STRUCT_EXCESSIVE_BOOLS. + /// + /// The maximum number of bool fields a struct can have (max_struct_bools: u64 = 3), - /// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bool parameters a function can have + /// Lint: FN_PARAMS_EXCESSIVE_BOOLS. + /// + /// The maximum number of bool parameters a function can have (max_fn_params_bools: u64 = 3), - /// Lint: WILDCARD_IMPORTS. Whether to allow certain wildcard imports (prelude, super in tests). + /// Lint: WILDCARD_IMPORTS. + /// + /// Whether to allow certain wildcard imports (prelude, super in tests). (warn_on_all_wildcard_imports: bool = false), - /// Lint: DISALLOWED_METHOD. The list of disallowed methods, written as fully qualified paths. + /// Lint: DISALLOWED_METHOD. + /// + /// The list of disallowed methods, written as fully qualified paths. (disallowed_methods: Vec<String> = Vec::new()), - /// Lint: DISALLOWED_TYPE. The list of disallowed types, written as fully qualified paths. + /// Lint: DISALLOWED_TYPE. + /// + /// The list of disallowed types, written as fully qualified paths. (disallowed_types: Vec<String> = Vec::new()), - /// Lint: UNREADABLE_LITERAL. Should the fraction of a decimal be linted to include separators. + /// Lint: UNREADABLE_LITERAL. + /// + /// Should the fraction of a decimal be linted to include separators. (unreadable_literal_lint_fractions: bool = true), - /// Lint: UPPER_CASE_ACRONYMS. Enables verbose mode. Triggers if there is more than one uppercase char next to each other + /// Lint: UPPER_CASE_ACRONYMS. + /// + /// Enables verbose mode. Triggers if there is more than one uppercase char next to each other (upper_case_acronyms_aggressive: bool = false), - /// Lint: _CARGO_COMMON_METADATA. For internal testing only, ignores the current `publish` settings in the Cargo manifest. + /// Lint: _CARGO_COMMON_METADATA. + /// + /// For internal testing only, ignores the current `publish` settings in the Cargo manifest. (cargo_ignore_publish: bool = false), - /// Lint: NONSTANDARD_MACRO_BRACES. Enforce the named macros always use the braces specified. <br> A `MacroMatcher` can be added like so `{ name = "macro_name", brace = "(" }`. If the macro is could be used with a full path two `MacroMatcher`s have to be added one with the full path `crate_name::macro_name` and one with just the macro name. + /// Lint: NONSTANDARD_MACRO_BRACES. + /// + /// Enforce the named macros always use the braces specified. + /// + /// A `MacroMatcher` can be added like so `{ name = "macro_name", brace = "(" }`. If the macro + /// is could be used with a full path two `MacroMatcher`s have to be added one with the full path + /// `crate_name::macro_name` and one with just the macro name. (standard_macro_braces: Vec<crate::nonstandard_macro_braces::MacroMatcher> = Vec::new()), - /// Lint: MISSING_ENFORCED_IMPORT_RENAMES. The list of imports to always rename, a fully qualified path followed by the rename. + /// Lint: MISSING_ENFORCED_IMPORT_RENAMES. + /// + /// The list of imports to always rename, a fully qualified path followed by the rename. (enforced_import_renames: Vec<crate::utils::conf::Rename> = Vec::new()), - /// Lint: RESTRICTED_SCRIPTS. The list of unicode scripts allowed to be used in the scope. + /// Lint: RESTRICTED_SCRIPTS. + /// + /// The list of unicode scripts allowed to be used in the scope. (allowed_scripts: Vec<String> = vec!["Latin".to_string()]), } diff --git a/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/clippy_lints/src/utils/internal_lints/metadata_collector.rs index 47336459d7d..a48a5385083 100644 --- a/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -82,7 +82,7 @@ This lint has the following configuration variables: /// `default` macro_rules! CONFIGURATION_VALUE_TEMPLATE { () => { - "* {name}: {ty}: {doc} (defaults to `{default}`)\n" + "* {name}: `{ty}`: {doc} (defaults to `{default}`)\n" }; } @@ -344,11 +344,16 @@ fn parse_config_field_doc(doc_comment: &str) -> Option<(Vec<String>, String)> { if let Some(split_pos) = doc_comment.find('.'); then { let mut doc_comment = doc_comment.to_string(); - let documentation = doc_comment.split_off(split_pos); + let mut documentation = doc_comment.split_off(split_pos); + // Extract lints doc_comment.make_ascii_lowercase(); let lints: Vec<String> = doc_comment.split_off(DOC_START.len()).split(", ").map(str::to_string).collect(); + // Format documentation correctly + // split off leading `.` from lint name list and indent for correct formatting + documentation = documentation.trim_start_matches('.').trim().replace("\n ", "\n "); + Some((lints, documentation)) } else { None |
