diff options
28 files changed, 393 insertions, 288 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f89f74e745..5e00dec2e77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2772,6 +2772,7 @@ Released 2018-09-13 [`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push [`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some [`self_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_assignment +[`self_named_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructor [`semicolon_if_nothing_returned`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned [`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse [`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs index a8c527fe2e3..17ce3cd809f 100644 --- a/clippy_lints/src/assign_ops.rs +++ b/clippy_lints/src/assign_ops.rs @@ -1,8 +1,8 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet_opt; use clippy_utils::ty::implements_trait; -use clippy_utils::{eq_expr_value, get_trait_def_id, trait_ref_of_method}; -use clippy_utils::{higher, paths, sugg}; +use clippy_utils::{binop_traits, sugg}; +use clippy_utils::{eq_expr_value, trait_ref_of_method}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; @@ -85,71 +85,34 @@ impl<'tcx> LateLintPass<'tcx> for AssignOps { let lint = |assignee: &hir::Expr<'_>, rhs: &hir::Expr<'_>| { let ty = cx.typeck_results().expr_ty(assignee); let rty = cx.typeck_results().expr_ty(rhs); - macro_rules! ops { - ($op:expr, - $cx:expr, - $ty:expr, - $rty:expr, - $($trait_name:ident),+) => { - match $op { - $(hir::BinOpKind::$trait_name => { - let [krate, module] = paths::OPS_MODULE; - let path: [&str; 3] = [krate, module, concat!(stringify!($trait_name), "Assign")]; - let trait_id = if let Some(trait_id) = get_trait_def_id($cx, &path) { - trait_id - } else { - return; // useless if the trait doesn't exist - }; - // check that we are not inside an `impl AssignOp` of this exact operation - let parent_fn = cx.tcx.hir().get_parent_item(e.hir_id); - if_chain! { - if let Some(trait_ref) = trait_ref_of_method(cx, parent_fn); - if trait_ref.path.res.def_id() == trait_id; - then { return; } + if_chain! { + if let Some((_, lang_item)) = binop_traits(op.node); + if let Ok(trait_id) = cx.tcx.lang_items().require(lang_item); + let parent_fn = cx.tcx.hir().get_parent_item(e.hir_id); + if trait_ref_of_method(cx, parent_fn) + .map_or(true, |t| t.path.res.def_id() != trait_id); + if implements_trait(cx, ty, trait_id, &[rty.into()]); + then { + span_lint_and_then( + cx, + ASSIGN_OP_PATTERN, + expr.span, + "manual implementation of an assign operation", + |diag| { + if let (Some(snip_a), Some(snip_r)) = + (snippet_opt(cx, assignee.span), snippet_opt(cx, rhs.span)) + { + diag.span_suggestion( + expr.span, + "replace it with", + format!("{} {}= {}", snip_a, op.node.as_str(), snip_r), + Applicability::MachineApplicable, + ); } - implements_trait($cx, $ty, trait_id, &[$rty]) - },)* - _ => false, - } + }, + ); } } - if ops!( - op.node, - cx, - ty, - rty.into(), - Add, - Sub, - Mul, - Div, - Rem, - And, - Or, - BitAnd, - BitOr, - BitXor, - Shr, - Shl - ) { - span_lint_and_then( - cx, - ASSIGN_OP_PATTERN, - expr.span, - "manual implementation of an assign operation", - |diag| { - if let (Some(snip_a), Some(snip_r)) = - (snippet_opt(cx, assignee.span), snippet_opt(cx, rhs.span)) - { - diag.span_suggestion( - expr.span, - "replace it with", - format!("{} {}= {}", snip_a, op.node.as_str(), snip_r), - Applicability::MachineApplicable, - ); - } - }, - ); - } }; let mut visitor = ExprVisitor { @@ -206,7 +169,7 @@ fn lint_misrefactored_assign_op( if let (Some(snip_a), Some(snip_r)) = (snippet_opt(cx, assignee.span), snippet_opt(cx, rhs_other.span)) { let a = &sugg::Sugg::hir(cx, assignee, ".."); let r = &sugg::Sugg::hir(cx, rhs, ".."); - let long = format!("{} = {}", snip_a, sugg::make_binop(higher::binop(op.node), a, r)); + let long = format!("{} = {}", snip_a, sugg::make_binop(op.node.into(), a, r)); diag.span_suggestion( expr.span, &format!( diff --git a/clippy_lints/src/eq_op.rs b/clippy_lints/src/eq_op.rs index a3a8e748d99..d39cabfb282 100644 --- a/clippy_lints/src/eq_op.rs +++ b/clippy_lints/src/eq_op.rs @@ -102,7 +102,7 @@ impl<'tcx> LateLintPass<'tcx> for EqOp { if macro_with_not_op(&left.kind) || macro_with_not_op(&right.kind) { return; } - if is_useless_with_eq_exprs(higher::binop(op.node)) && eq_expr_value(cx, left, right) { + if is_useless_with_eq_exprs(op.node.into()) && eq_expr_value(cx, left, right) { span_lint( cx, EQ_OP, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1af3a215f44..f8d877bcae0 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -330,6 +330,7 @@ mod regex; mod repeat_once; mod returns; mod self_assignment; +mod self_named_constructor; mod semicolon_if_nothing_returned; mod serde_api; mod shadow; @@ -900,6 +901,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: returns::LET_AND_RETURN, returns::NEEDLESS_RETURN, self_assignment::SELF_ASSIGNMENT, + self_named_constructor::SELF_NAMED_CONSTRUCTOR, semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED, serde_api::SERDE_API_MISUSE, shadow::SHADOW_REUSE, @@ -1406,6 +1408,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(returns::LET_AND_RETURN), LintId::of(returns::NEEDLESS_RETURN), LintId::of(self_assignment::SELF_ASSIGNMENT), + LintId::of(self_named_constructor::SELF_NAMED_CONSTRUCTOR), LintId::of(serde_api::SERDE_API_MISUSE), LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS), LintId::of(size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT), @@ -1559,6 +1562,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES), LintId::of(returns::LET_AND_RETURN), LintId::of(returns::NEEDLESS_RETURN), + LintId::of(self_named_constructor::SELF_NAMED_CONSTRUCTOR), LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS), LintId::of(tabs_in_doc_comments::TABS_IN_DOC_COMMENTS), LintId::of(to_digit_is_some::TO_DIGIT_IS_SOME), @@ -2101,6 +2105,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: 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); + store.register_late_pass(move || box self_named_constructor::SelfNamedConstructor); + } #[rustfmt::skip] diff --git a/clippy_lints/src/self_named_constructor.rs b/clippy_lints/src/self_named_constructor.rs new file mode 100644 index 00000000000..da991e1d90c --- /dev/null +++ b/clippy_lints/src/self_named_constructor.rs @@ -0,0 +1,91 @@ +use clippy_utils::diagnostics::span_lint; +use clippy_utils::return_ty; +use clippy_utils::ty::{contains_adt_constructor, contains_ty}; +use rustc_hir::{Impl, ImplItem, ImplItemKind, ItemKind, Node}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// **What it does:** Warns when constructors have the same name as their types. + /// + /// **Why is this bad?** Repeating the name of the type is redundant. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust,ignore + /// struct Foo {} + /// + /// impl Foo { + /// pub fn foo() -> Foo { + /// Foo {} + /// } + /// } + /// ``` + /// Use instead: + /// ```rust,ignore + /// struct Foo {} + /// + /// impl Foo { + /// pub fn new() -> Foo { + /// Foo {} + /// } + /// } + /// ``` + pub SELF_NAMED_CONSTRUCTOR, + style, + "method should not have the same name as the type it is implemented for" +} + +declare_lint_pass!(SelfNamedConstructor => [SELF_NAMED_CONSTRUCTOR]); + +impl<'tcx> LateLintPass<'tcx> for SelfNamedConstructor { + fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) { + match impl_item.kind { + ImplItemKind::Fn(ref sig, _) => { + if sig.decl.implicit_self.has_implicit_self() { + return; + } + }, + _ => return, + } + + let parent = cx.tcx.hir().get_parent_item(impl_item.hir_id()); + let item = cx.tcx.hir().expect_item(parent); + let self_ty = cx.tcx.type_of(item.def_id); + let ret_ty = return_ty(cx, impl_item.hir_id()); + + // Do not check trait impls + if matches!(item.kind, ItemKind::Impl(Impl { of_trait: Some(_), .. })) { + return; + } + + // Ensure method is constructor-like + if let Some(self_adt) = self_ty.ty_adt_def() { + if !contains_adt_constructor(ret_ty, self_adt) { + return; + } + } else if !contains_ty(ret_ty, self_ty) { + return; + } + + if_chain! { + if let Some(self_def) = self_ty.ty_adt_def(); + if let Some(self_local_did) = self_def.did.as_local(); + let self_id = cx.tcx.hir().local_def_id_to_hir_id(self_local_did); + if let Some(Node::Item(x)) = cx.tcx.hir().find(self_id); + let type_name = x.ident.name.as_str().to_lowercase(); + if impl_item.ident.name.as_str() == type_name || impl_item.ident.name.as_str().replace("_", "") == type_name; + + then { + span_lint( + cx, + SELF_NAMED_CONSTRUCTOR, + impl_item.span, + &format!("constructor `{}` has the same name as the type", impl_item.ident.name), + ); + } + } + } +} diff --git a/clippy_lints/src/suspicious_trait_impl.rs b/clippy_lints/src/suspicious_trait_impl.rs index 2203ab57b10..f2bffd55321 100644 --- a/clippy_lints/src/suspicious_trait_impl.rs +++ b/clippy_lints/src/suspicious_trait_impl.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint; -use clippy_utils::{get_trait_def_id, paths, trait_ref_of_method}; +use clippy_utils::{binop_traits, trait_ref_of_method, BINOP_TRAITS, OP_ASSIGN_TRAITS}; use if_chain::if_chain; use rustc_hir as hir; use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; @@ -55,135 +55,48 @@ declare_lint_pass!(SuspiciousImpl => [SUSPICIOUS_ARITHMETIC_IMPL, SUSPICIOUS_OP_ impl<'tcx> LateLintPass<'tcx> for SuspiciousImpl { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { - if let hir::ExprKind::Binary(binop, _, _) | hir::ExprKind::AssignOp(binop, ..) = expr.kind { - match binop.node { - hir::BinOpKind::Eq - | hir::BinOpKind::Lt - | hir::BinOpKind::Le - | hir::BinOpKind::Ne - | hir::BinOpKind::Ge - | hir::BinOpKind::Gt => return, - _ => {}, - } + if_chain! { + if let hir::ExprKind::Binary(binop, _, _) | hir::ExprKind::AssignOp(binop, ..) = expr.kind; + if let Some((binop_trait_lang, op_assign_trait_lang)) = binop_traits(binop.node); + if let Ok(binop_trait_id) = cx.tcx.lang_items().require(binop_trait_lang); + if let Ok(op_assign_trait_id) = cx.tcx.lang_items().require(op_assign_trait_lang); // Check for more than one binary operation in the implemented function // Linting when multiple operations are involved can result in false positives let parent_fn = cx.tcx.hir().get_parent_item(expr.hir_id); - if_chain! { - if let hir::Node::ImplItem(impl_item) = cx.tcx.hir().get(parent_fn); - if let hir::ImplItemKind::Fn(_, body_id) = impl_item.kind; - then { - let body = cx.tcx.hir().body(body_id); - let mut visitor = BinaryExprVisitor { nb_binops: 0 }; - walk_expr(&mut visitor, &body.value); - if visitor.nb_binops > 1 { - return; - } - } - } - - if let Some(impl_trait) = check_binop( - cx, - expr, - binop.node, - &[ - "Add", "Sub", "Mul", "Div", "Rem", "BitAnd", "BitOr", "BitXor", "Shl", "Shr", - ], - &[ - hir::BinOpKind::Add, - hir::BinOpKind::Sub, - hir::BinOpKind::Mul, - hir::BinOpKind::Div, - hir::BinOpKind::Rem, - hir::BinOpKind::BitAnd, - hir::BinOpKind::BitOr, - hir::BinOpKind::BitXor, - hir::BinOpKind::Shl, - hir::BinOpKind::Shr, - ], - ) { - span_lint( - cx, - SUSPICIOUS_ARITHMETIC_IMPL, - binop.span, - &format!("suspicious use of binary operator in `{}` impl", impl_trait), - ); - } - - if let Some(impl_trait) = check_binop( - cx, - expr, - binop.node, - &[ - "AddAssign", - "SubAssign", - "MulAssign", - "DivAssign", - "BitAndAssign", - "BitOrAssign", - "BitXorAssign", - "RemAssign", - "ShlAssign", - "ShrAssign", - ], - &[ - hir::BinOpKind::Add, - hir::BinOpKind::Sub, - hir::BinOpKind::Mul, - hir::BinOpKind::Div, - hir::BinOpKind::BitAnd, - hir::BinOpKind::BitOr, - hir::BinOpKind::BitXor, - hir::BinOpKind::Rem, - hir::BinOpKind::Shl, - hir::BinOpKind::Shr, - ], - ) { + if let hir::Node::ImplItem(impl_item) = cx.tcx.hir().get(parent_fn); + if let hir::ImplItemKind::Fn(_, body_id) = impl_item.kind; + let body = cx.tcx.hir().body(body_id); + let parent_fn = cx.tcx.hir().get_parent_item(expr.hir_id); + if let Some(trait_ref) = trait_ref_of_method(cx, parent_fn); + let trait_id = trait_ref.path.res.def_id(); + if ![binop_trait_id, op_assign_trait_id].contains(&trait_id); + if let Some(&(_, lint)) = [ + (&BINOP_TRAITS, SUSPICIOUS_ARITHMETIC_IMPL), + (&OP_ASSIGN_TRAITS, SUSPICIOUS_OP_ASSIGN_IMPL), + ] + .iter() + .find(|&(ts, _)| ts.iter().any(|&t| Ok(trait_id) == cx.tcx.lang_items().require(t))); + if count_binops(&body.value) == 1; + then { span_lint( cx, - SUSPICIOUS_OP_ASSIGN_IMPL, + lint, binop.span, - &format!("suspicious use of binary operator in `{}` impl", impl_trait), + &format!("suspicious use of `{}` in `{}` impl", binop.node.as_str(), cx.tcx.item_name(trait_id)), ); } } } } -fn check_binop( - cx: &LateContext<'_>, - expr: &hir::Expr<'_>, - binop: hir::BinOpKind, - traits: &[&'static str], - expected_ops: &[hir::BinOpKind], -) -> Option<&'static str> { - let mut trait_ids = vec![]; - let [krate, module] = paths::OPS_MODULE; - - for &t in traits { - let path = [krate, module, t]; - if let Some(trait_id) = get_trait_def_id(cx, &path) { - trait_ids.push(trait_id); - } else { - return None; - } - } - - // Get the actually implemented trait - let parent_fn = cx.tcx.hir().get_parent_item(expr.hir_id); - - if_chain! { - if let Some(trait_ref) = trait_ref_of_method(cx, parent_fn); - if let Some(idx) = trait_ids.iter().position(|&tid| tid == trait_ref.path.res.def_id()); - if binop != expected_ops[idx]; - then{ - return Some(traits[idx]) - } - } - - None +fn count_binops(expr: &hir::Expr<'_>) -> u32 { + let mut visitor = BinaryExprVisitor::default(); + visitor.visit_expr(expr); + visitor.nb_binops } +#[derive(Default)] struct BinaryExprVisitor { nb_binops: u32, } diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index 3e3e472e99f..f32f1109b08 100644 --- a/clippy_utils/src/higher.rs +++ b/clippy_utils/src/higher.rs @@ -11,31 +11,6 @@ use rustc_hir::{BorrowKind, Expr, ExprKind, StmtKind, UnOp}; use rustc_lint::LateContext; use rustc_span::{sym, ExpnKind, Span, Symbol}; -/// Converts a hir binary operator to the corresponding `ast` type. -#[must_use] -pub fn binop(op: hir::BinOpKind) -> ast::BinOpKind { - match op { - hir::BinOpKind::Eq => ast::BinOpKind::Eq, - hir::BinOpKind::Ge => ast::BinOpKind::Ge, - hir::BinOpKind::Gt => ast::BinOpKind::Gt, - hir::BinOpKind::Le => ast::BinOpKind::Le, - hir::BinOpKind::Lt => ast::BinOpKind::Lt, - hir::BinOpKind::Ne => ast::BinOpKind::Ne, - hir::BinOpKind::Or => ast::BinOpKind::Or, - hir::BinOpKind::Add => ast::BinOpKind::Add, - hir::BinOpKind::And => ast::BinOpKind::And, - hir::BinOpKind::BitAnd => ast::BinOpKind::BitAnd, - hir::BinOpKind::BitOr => ast::BinOpKind::BitOr, - hir::BinOpKind::BitXor => ast::BinOpKind::BitXor, - hir::BinOpKind::Div => ast::BinOpKind::Div, - hir::BinOpKind::Mul => ast::BinOpKind::Mul, - hir::BinOpKind::Rem => ast::BinOpKind::Rem, - hir::BinOpKind::Shl => ast::BinOpKind::Shl, - hir::BinOpKind::Shr => ast::BinOpKind::Shr, - hir::BinOpKind::Sub => ast::BinOpKind::Sub, - } -} - /// Represent a range akin to `ast::ExprKind::Range`. #[derive(Debug, Copy, Clone)] pub struct Range<'a> { diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 6db221ab0fd..c255dafe32d 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -909,12 +909,8 @@ pub fn is_integer_const(cx: &LateContext<'_>, e: &Expr<'_>, value: u128) -> bool if is_integer_literal(e, value) { return true; } - let map = cx.tcx.hir(); - let parent_item = map.get_parent_item(e.hir_id); - if let Some((Constant::Int(v), _)) = map - .maybe_body_owned_by(parent_item) - .and_then(|body_id| constant(cx, cx.tcx.typeck_body(body_id), e)) - { + let enclosing_body = cx.tcx.hir().local_def_id(cx.tcx.hir().enclosing_body_owner(e.hir_id)); + if let Some((Constant::Int(v), _)) = constant(cx, cx.tcx.typeck(enclosing_body), e) { value == v } else { false @@ -1710,3 +1706,34 @@ pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool { matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().contains("test") } + +macro_rules! op_utils { + ($($name:ident $assign:ident)*) => { + /// Binary operation traits like `LangItem::Add` + pub static BINOP_TRAITS: &[LangItem] = &[$(LangItem::$name,)*]; + + /// Operator-Assign traits like `LangItem::AddAssign` + pub static OP_ASSIGN_TRAITS: &[LangItem] = &[$(LangItem::$assign,)*]; + + /// Converts `BinOpKind::Add` to `(LangItem::Add, LangItem::AddAssign)`, for example + pub fn binop_traits(kind: hir::BinOpKind) -> Option<(LangItem, LangItem)> { + match kind { + $(hir::BinOpKind::$name => Some((LangItem::$name, LangItem::$assign)),)* + _ => None, + } + } + }; +} + +op_utils! { + Add AddAssign + Sub SubAssign + Mul MulAssign + Div DivAssign + Rem RemAssign + BitXor BitXorAssign + BitAnd BitAndAssign + BitOr BitOrAssign + Shl ShlAssign + Shr ShrAssign +} diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs index efc0ec50fdc..3bd75b10e90 100644 --- a/clippy_utils/src/sugg.rs +++ b/clippy_utils/src/sugg.rs @@ -154,7 +154,7 @@ impl<'a> Sugg<'a> { | hir::ExprKind::Err => Sugg::NonParen(snippet), hir::ExprKind::Assign(..) => Sugg::BinOp(AssocOp::Assign, snippet), hir::ExprKind::AssignOp(op, ..) => Sugg::BinOp(hirbinop2assignop(op), snippet), - hir::ExprKind::Binary(op, ..) => Sugg::BinOp(AssocOp::from_ast_binop(higher::binop(op.node)), snippet), + hir::ExprKind::Binary(op, ..) => Sugg::BinOp(AssocOp::from_ast_binop(op.node.into()), snippet), hir::ExprKind::Cast(..) => Sugg::BinOp(AssocOp::As, snippet), hir::ExprKind::Type(..) => Sugg::BinOp(AssocOp::Colon, snippet), } diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index 3f5c5604d43..523d55219ab 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -257,10 +257,12 @@ pub fn is_type_diagnostic_item(cx: &LateContext<'_>, ty: Ty<'_>, diag_item: Symb } } -/// Checks if the type is equal to a lang item +/// Checks if the type is equal to a lang item. +/// +/// Returns `false` if the `LangItem` is not defined. pub fn is_type_lang_item(cx: &LateContext<'_>, ty: Ty<'_>, lang_item: hir::LangItem) -> bool { match ty.kind() { - ty::Adt(adt, _) => cx.tcx.lang_items().require(lang_item).unwrap() == adt.did, + ty::Adt(adt, _) => cx.tcx.lang_items().require(lang_item).map_or(false, |li| li == adt.did), _ => false, } } diff --git a/tests/ui/crashes/ice-6179.rs b/tests/ui/crashes/ice-6179.rs index f8c866a49aa..8d9a1af8ff1 100644 --- a/tests/ui/crashes/ice-6179.rs +++ b/tests/ui/crashes/ice-6179.rs @@ -7,7 +7,7 @@ struct Foo {} impl Foo { - fn foo() -> Self { + fn new() -> Self { impl Foo { fn bar() {} } diff --git a/tests/ui/crashes/ice-7340.rs b/tests/ui/crashes/ice-7340.rs new file mode 100644 index 00000000000..7d2351d606f --- /dev/null +++ b/tests/ui/crashes/ice-7340.rs @@ -0,0 +1,6 @@ +#![allow(clippy::no_effect)] + +fn main() { + const CONSTANT: usize = 8; + [1; 1 % CONSTANT]; +} diff --git a/tests/ui/crashes/ice-7410.rs b/tests/ui/crashes/ice-7410.rs new file mode 100644 index 00000000000..aaa422d88c3 --- /dev/null +++ b/tests/ui/crashes/ice-7410.rs @@ -0,0 +1,31 @@ +// compile-flags: -Clink-arg=-nostartfiles +// ignore-macos +// ignore-windows + +#![feature(lang_items, start, libc)] +#![no_std] +#![allow(clippy::redundant_pattern_matching)] + +use core::panic::PanicInfo; + +struct S; + +impl Drop for S { + fn drop(&mut self) {} +} + +#[start] +fn main(argc: isize, argv: *const *const u8) -> isize { + if let Some(_) = Some(S) { + } else { + } + 0 +} + +#[panic_handler] +fn panic(_info: &PanicInfo) -> ! { + loop {} +} + +#[lang = "eh_personality"] +extern "C" fn eh_personality() {} diff --git a/tests/ui/issue_4266.rs b/tests/ui/issue_4266.rs index 8a9d5a3d1d5..cc699b79e43 100644 --- a/tests/ui/issue_4266.rs +++ b/tests/ui/issue_4266.rs @@ -25,7 +25,9 @@ async fn all_to_one<'a>(a: &'a str, _b: &'a str) -> &'a str { struct Foo; impl Foo { // ok - pub async fn foo(&mut self) {} + pub async fn new(&mut self) -> Self { + Foo {} + } } // rust-lang/rust#61115 diff --git a/tests/ui/missing-doc-impl.rs b/tests/ui/missing-doc-impl.rs index bfa9ef01b0e..d5724bf661c 100644 --- a/tests/ui/missing-doc-impl.rs +++ b/tests/ui/missing-doc-impl.rs @@ -59,7 +59,9 @@ pub trait E: Sized { } impl Foo { - pub fn foo() {} + pub fn new() -> Self { + Foo { a: 0, b: 0 } + } fn bar() {} } diff --git a/tests/ui/missing-doc-impl.stderr b/tests/ui/missing-doc-impl.stderr index d33d512475b..bda63d66a17 100644 --- a/tests/ui/missing-doc-impl.stderr +++ b/tests/ui/missing-doc-impl.stderr @@ -78,23 +78,25 @@ LL | type AssociatedTypeDef = Self; error: missing documentation for an associated function --> $DIR/missing-doc-impl.rs:62:5 | -LL | pub fn foo() {} - | ^^^^^^^^^^^^^^^ +LL | / pub fn new() -> Self { +LL | | Foo { a: 0, b: 0 } +LL | | } + | |_____^ error: missing documentation for an associated function - --> $DIR/missing-doc-impl.rs:63:5 + --> $DIR/missing-doc-impl.rs:65:5 | LL | fn bar() {} | ^^^^^^^^^^^ error: missing documentation for an associated function - --> $DIR/missing-doc-impl.rs:67:5 + --> $DIR/missing-doc-impl.rs:69:5 | LL | pub fn foo() {} | ^^^^^^^^^^^^^^^ error: missing documentation for an associated function - --> $DIR/missing-doc-impl.rs:71:5 + --> $DIR/missing-doc-impl.rs:73:5 | LL | / fn foo2() -> u32 { LL | | 1 diff --git a/tests/ui/missing_const_for_fn/cant_be_const.rs b/tests/ui/missing_const_for_fn/cant_be_const.rs index 7cda1aaa3c2..6d2cbb6ad96 100644 --- a/tests/ui/missing_const_for_fn/cant_be_const.rs +++ b/tests/ui/missing_const_for_fn/cant_be_const.rs @@ -84,7 +84,7 @@ mod with_drop { impl A { // This can not be const because the type implements `Drop`. - pub fn a(self) -> B { + pub fn b(self) -> B { B } } diff --git a/tests/ui/needless_bool/fixable.fixed b/tests/ui/needless_bool/fixable.fixed index 567dbc54100..5917ffc3e12 100644 --- a/tests/ui/needless_bool/fixable.fixed +++ b/tests/ui/needless_bool/fixable.fixed @@ -6,7 +6,8 @@ dead_code, clippy::no_effect, clippy::if_same_then_else, - clippy::needless_return + clippy::needless_return, + clippy::self_named_constructor )] use std::cell::Cell; diff --git a/tests/ui/needless_bool/fixable.rs b/tests/ui/needless_bool/fixable.rs index 10126ad4dbb..d26dcb9fcc3 100644 --- a/tests/ui/needless_bool/fixable.rs +++ b/tests/ui/needless_bool/fixable.rs @@ -6,7 +6,8 @@ dead_code, clippy::no_effect, clippy::if_same_then_else, - clippy::needless_return + clippy::needless_return, + clippy::self_named_constructor )] use std::cell::Cell; diff --git a/tests/ui/needless_bool/fixable.stderr b/tests/ui/needless_bool/fixable.stderr index 25abfb2a472..8026d643c44 100644 --- a/tests/ui/needless_bool/fixable.stderr +++ b/tests/ui/needless_bool/fixable.stderr @@ -1,5 +1,5 @@ error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:39:5 + --> $DIR/fixable.rs:40:5 | LL | / if x { LL | | true @@ -11,7 +11,7 @@ LL | | }; = note: `-D clippy::needless-bool` implied by `-D warnings` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:44:5 + --> $DIR/fixable.rs:45:5 | LL | / if x { LL | | false @@ -21,7 +21,7 @@ LL | | }; | |_____^ help: you can reduce it to: `!x` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:49:5 + --> $DIR/fixable.rs:50:5 | LL | / if x && y { LL | | false @@ -31,7 +31,7 @@ LL | | }; | |_____^ help: you can reduce it to: `!(x && y)` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:69:5 + --> $DIR/fixable.rs:70:5 | LL | / if x { LL | | return true; @@ -41,7 +41,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return x` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:77:5 + --> $DIR/fixable.rs:78:5 | LL | / if x { LL | | return false; @@ -51,7 +51,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return !x` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:85:5 + --> $DIR/fixable.rs:86:5 | LL | / if x && y { LL | | return true; @@ -61,7 +61,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return x && y` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:93:5 + --> $DIR/fixable.rs:94:5 | LL | / if x && y { LL | | return false; @@ -71,7 +71,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return !(x && y)` error: equality checks against true are unnecessary - --> $DIR/fixable.rs:101:8 + --> $DIR/fixable.rs:102:8 | LL | if x == true {}; | ^^^^^^^^^ help: try simplifying it as shown: `x` @@ -79,25 +79,25 @@ LL | if x == true {}; = note: `-D clippy::bool-comparison` implied by `-D warnings` error: equality checks against false can be replaced by a negation - --> $DIR/fixable.rs:105:8 + --> $DIR/fixable.rs:106:8 | LL | if x == false {}; | ^^^^^^^^^^ help: try simplifying it as shown: `!x` error: equality checks against true are unnecessary - --> $DIR/fixable.rs:115:8 + --> $DIR/fixable.rs:116:8 | LL | if x == true {}; | ^^^^^^^^^ help: try simplifying it as shown: `x` error: equality checks against false can be replaced by a negation - --> $DIR/fixable.rs:116:8 + --> $DIR/fixable.rs:117:8 | LL | if x == false {}; | ^^^^^^^^^^ help: try simplifying it as shown: `!x` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:125:12 + --> $DIR/fixable.rs:126:12 | LL | } else if returns_bool() { | ____________^ diff --git a/tests/ui/self_named_constructor.rs b/tests/ui/self_named_constructor.rs new file mode 100644 index 00000000000..7658b86a8d6 --- /dev/null +++ b/tests/ui/self_named_constructor.rs @@ -0,0 +1,59 @@ +#![warn(clippy::self_named_constructor)] + +struct ShouldSpawn; +struct ShouldNotSpawn; + +impl ShouldSpawn { + pub fn should_spawn() -> ShouldSpawn { + ShouldSpawn + } + + fn should_not_spawn() -> ShouldNotSpawn { + ShouldNotSpawn + } +} + +impl ShouldNotSpawn { + pub fn new() -> ShouldNotSpawn { + ShouldNotSpawn + } +} + +struct ShouldNotSpawnWithTrait; + +trait ShouldNotSpawnTrait { + type Item; +} + +impl ShouldNotSpawnTrait for ShouldNotSpawnWithTrait { + type Item = Self; +} + +impl ShouldNotSpawnWithTrait { + pub fn should_not_spawn_with_trait() -> impl ShouldNotSpawnTrait<Item = Self> { + ShouldNotSpawnWithTrait + } +} + +// Same trait name and same type name should not spawn the lint +#[derive(Default)] +pub struct Default; + +trait TraitSameTypeName { + fn should_not_spawn() -> Self; +} +impl TraitSameTypeName for ShouldNotSpawn { + fn should_not_spawn() -> Self { + ShouldNotSpawn + } +} + +struct SelfMethodShouldNotSpawn; + +impl SelfMethodShouldNotSpawn { + fn self_method_should_not_spawn(self) -> Self { + SelfMethodShouldNotSpawn + } +} + +fn main() {} diff --git a/tests/ui/self_named_constructor.stderr b/tests/ui/self_named_constructor.stderr new file mode 100644 index 00000000000..1e2c34ac2f7 --- /dev/null +++ b/tests/ui/self_named_constructor.stderr @@ -0,0 +1,12 @@ +error: constructor `should_spawn` has the same name as the type + --> $DIR/self_named_constructor.rs:7:5 + | +LL | / pub fn should_spawn() -> ShouldSpawn { +LL | | ShouldSpawn +LL | | } + | |_____^ + | + = note: `-D clippy::self-named-constructor` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/tests/ui/suspicious_arithmetic_impl.stderr b/tests/ui/suspicious_arithmetic_impl.stderr index 63fc9ecb79a..ced1305874e 100644 --- a/tests/ui/suspicious_arithmetic_impl.stderr +++ b/tests/ui/suspicious_arithmetic_impl.stderr @@ -1,4 +1,4 @@ -error: suspicious use of binary operator in `Add` impl +error: suspicious use of `-` in `Add` impl --> $DIR/suspicious_arithmetic_impl.rs:13:20 | LL | Foo(self.0 - other.0) @@ -6,7 +6,7 @@ LL | Foo(self.0 - other.0) | = note: `-D clippy::suspicious-arithmetic-impl` implied by `-D warnings` -error: suspicious use of binary operator in `AddAssign` impl +error: suspicious use of `-` in `AddAssign` impl --> $DIR/suspicious_arithmetic_impl.rs:19:23 | LL | *self = *self - other; @@ -14,43 +14,43 @@ LL | *self = *self - other; | = note: `-D clippy::suspicious-op-assign-impl` implied by `-D warnings` -error: suspicious use of binary operator in `MulAssign` impl +error: suspicious use of `/` in `MulAssign` impl --> $DIR/suspicious_arithmetic_impl.rs:32:16 | LL | self.0 /= other.0; | ^^ -error: suspicious use of binary operator in `Rem` impl +error: suspicious use of `/` in `Rem` impl --> $DIR/suspicious_arithmetic_impl.rs:70:20 | LL | Foo(self.0 / other.0) | ^ -error: suspicious use of binary operator in `BitAnd` impl +error: suspicious use of `|` in `BitAnd` impl --> $DIR/suspicious_arithmetic_impl.rs:78:20 | LL | Foo(self.0 | other.0) | ^ -error: suspicious use of binary operator in `BitOr` impl +error: suspicious use of `^` in `BitOr` impl --> $DIR/suspicious_arithmetic_impl.rs:86:20 | LL | Foo(self.0 ^ other.0) | ^ -error: suspicious use of binary operator in `BitXor` impl +error: suspicious use of `&` in `BitXor` impl --> $DIR/suspicious_arithmetic_impl.rs:94:20 | LL | Foo(self.0 & other.0) | ^ -error: suspicious use of binary operator in `Shl` impl +error: suspicious use of `>>` in `Shl` impl --> $DIR/suspicious_arithmetic_impl.rs:102:20 | LL | Foo(self.0 >> other.0) | ^^ -error: suspicious use of binary operator in `Shr` impl +error: suspicious use of `<<` in `Shr` impl --> $DIR/suspicious_arithmetic_impl.rs:110:20 | LL | Foo(self.0 << other.0) diff --git a/tests/ui/unit_arg.rs b/tests/ui/unit_arg.rs index 938cc3c7859..df0fdaccb34 100644 --- a/tests/ui/unit_arg.rs +++ b/tests/ui/unit_arg.rs @@ -6,7 +6,8 @@ clippy::unused_unit, clippy::unnecessary_wraps, clippy::or_fun_call, - clippy::needless_question_mark + clippy::needless_question_mark, + clippy::self_named_constructor )] use std::fmt::Debug; diff --git a/tests/ui/unit_arg.stderr b/tests/ui/unit_arg.stderr index 354fd51cd6b..8155c4ae110 100644 --- a/tests/ui/unit_arg.stderr +++ b/tests/ui/unit_arg.stderr @@ -1,5 +1,5 @@ error: passing a unit value to a function - --> $DIR/unit_arg.rs:55:5 + --> $DIR/unit_arg.rs:56:5 | LL | / foo({ LL | | 1; @@ -20,7 +20,7 @@ LL | foo(()); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:58:5 + --> $DIR/unit_arg.rs:59:5 | LL | foo(foo(1)); | ^^^^^^^^^^^ @@ -32,7 +32,7 @@ LL | foo(()); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:59:5 + --> $DIR/unit_arg.rs:60:5 | LL | / foo({ LL | | foo(1); @@ -54,7 +54,7 @@ LL | foo(()); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:64:5 + --> $DIR/unit_arg.rs:65:5 | LL | / b.bar({ LL | | 1; @@ -74,7 +74,7 @@ LL | b.bar(()); | error: passing unit values to a function - --> $DIR/unit_arg.rs:67:5 + --> $DIR/unit_arg.rs:68:5 | LL | taking_multiple_units(foo(0), foo(1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -87,7 +87,7 @@ LL | taking_multiple_units((), ()); | error: passing unit values to a function - --> $DIR/unit_arg.rs:68:5 + --> $DIR/unit_arg.rs:69:5 | LL | / taking_multiple_units(foo(0), { LL | | foo(1); @@ -110,7 +110,7 @@ LL | taking_multiple_units((), ()); | error: passing unit values to a function - --> $DIR/unit_arg.rs:72:5 + --> $DIR/unit_arg.rs:73:5 | LL | / taking_multiple_units( LL | | { @@ -140,7 +140,7 @@ LL | foo(2); ... error: passing a unit value to a function - --> $DIR/unit_arg.rs:83:13 + --> $DIR/unit_arg.rs:84:13 | LL | None.or(Some(foo(2))); | ^^^^^^^^^^^^ @@ -154,7 +154,7 @@ LL | }); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:86:5 + --> $DIR/unit_arg.rs:87:5 | LL | foo(foo(())); | ^^^^^^^^^^^^ @@ -166,7 +166,7 @@ LL | foo(()); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:123:5 + --> $DIR/unit_arg.rs:124:5 | LL | Some(foo(1)) | ^^^^^^^^^^^^ diff --git a/tests/ui/use_self.fixed b/tests/ui/use_self.fixed index e2c28542efc..23fc7632511 100644 --- a/tests/ui/use_self.fixed +++ b/tests/ui/use_self.fixed @@ -4,7 +4,12 @@ #![warn(clippy::use_self)] #![allow(dead_code)] -#![allow(clippy::should_implement_trait, clippy::upper_case_acronyms, clippy::from_over_into)] +#![allow( + clippy::should_implement_trait, + clippy::upper_case_acronyms, + clippy::from_over_into, + clippy::self_named_constructor +)] #[macro_use] extern crate proc_macro_derive; diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs index 3cd99b9f5cd..bb46a339923 100644 --- a/tests/ui/use_self.rs +++ b/tests/ui/use_self.rs @@ -4,7 +4,12 @@ #![warn(clippy::use_self)] #![allow(dead_code)] -#![allow(clippy::should_implement_trait, clippy::upper_case_acronyms, clippy::from_over_into)] +#![allow( + clippy::should_implement_trait, + clippy::upper_case_acronyms, + clippy::from_over_into, + clippy::self_named_constructor +)] #[macro_use] extern crate proc_macro_derive; diff --git a/tests/ui/use_self.stderr b/tests/ui/use_self.stderr index 6ac26c9e5a9..e14368a11aa 100644 --- a/tests/ui/use_self.stderr +++ b/tests/ui/use_self.stderr @@ -1,5 +1,5 @@ error: unnecessary structure name repetition - --> $DIR/use_self.rs:18:21 + --> $DIR/use_self.rs:23:21 | LL | fn new() -> Foo { | ^^^ help: use the applicable keyword: `Self` @@ -7,163 +7,163 @@ LL | fn new() -> Foo { = note: `-D clippy::use-self` implied by `-D warnings` error: unnecessary structure name repetition - --> $DIR/use_self.rs:19:13 + --> $DIR/use_self.rs:24:13 | LL | Foo {} | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:21:22 + --> $DIR/use_self.rs:26:22 | LL | fn test() -> Foo { | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:22:13 + --> $DIR/use_self.rs:27:13 | LL | Foo::new() | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:27:25 + --> $DIR/use_self.rs:32:25 | LL | fn default() -> Foo { | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:28:13 + --> $DIR/use_self.rs:33:13 | LL | Foo::new() | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:93:24 + --> $DIR/use_self.rs:98:24 | LL | fn bad(foos: &[Foo]) -> impl Iterator<Item = &Foo> { | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:93:55 + --> $DIR/use_self.rs:98:55 | LL | fn bad(foos: &[Foo]) -> impl Iterator<Item = &Foo> { | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:108:13 + --> $DIR/use_self.rs:113:13 | LL | TS(0) | ^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:143:29 + --> $DIR/use_self.rs:148:29 | LL | fn bar() -> Bar { | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:144:21 + --> $DIR/use_self.rs:149:21 | LL | Bar { foo: Foo {} } | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:155:21 + --> $DIR/use_self.rs:160:21 | LL | fn baz() -> Foo { | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:156:13 + --> $DIR/use_self.rs:161:13 | LL | Foo {} | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:173:21 + --> $DIR/use_self.rs:178:21 | LL | let _ = Enum::B(42); | ^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:174:21 + --> $DIR/use_self.rs:179:21 | LL | let _ = Enum::C { field: true }; | ^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:175:21 + --> $DIR/use_self.rs:180:21 | LL | let _ = Enum::A; | ^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:217:13 + --> $DIR/use_self.rs:222:13 | LL | nested::A::fun_1(); | ^^^^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:218:13 + --> $DIR/use_self.rs:223:13 | LL | nested::A::A; | ^^^^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:220:13 + --> $DIR/use_self.rs:225:13 | LL | nested::A {}; | ^^^^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:239:13 + --> $DIR/use_self.rs:244:13 | LL | TestStruct::from_something() | ^^^^^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:253:25 + --> $DIR/use_self.rs:258:25 | LL | async fn g() -> S { | ^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:254:13 + --> $DIR/use_self.rs:259:13 | LL | S {} | ^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:258:16 + --> $DIR/use_self.rs:263:16 | LL | &p[S::A..S::B] | ^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:258:22 + --> $DIR/use_self.rs:263:22 | LL | &p[S::A..S::B] | ^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:281:29 + --> $DIR/use_self.rs:286:29 | LL | fn foo(value: T) -> Foo<T> { | ^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:282:13 + --> $DIR/use_self.rs:287:13 | LL | Foo::<T> { value } | ^^^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:454:13 + --> $DIR/use_self.rs:459:13 | LL | A::new::<submod::B>(submod::B {}) | ^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:491:13 + --> $DIR/use_self.rs:496:13 | LL | S2::new() | ^^ help: use the applicable keyword: `Self` |