diff --git a/clippy_lints/src/new_without_default.rs b/clippy_lints/src/new_without_default.rs index 7b838fdee951..86c345b025ca 100644 --- a/clippy_lints/src/new_without_default.rs +++ b/clippy_lints/src/new_without_default.rs @@ -17,7 +17,7 @@ use crate::rustc_errors::Applicability; use crate::syntax::source_map::Span; use crate::utils::paths; use crate::utils::sugg::DiagnosticBuilderExt; -use crate::utils::{get_trait_def_id, implements_trait, return_ty, same_tys, span_lint_and_then}; +use crate::utils::{get_trait_def_id, implements_trait, return_ty, same_tys, span_lint_node_and_then}; use if_chain::if_chain; /// **What it does:** Checks for types with a `fn new() -> Self` method and no @@ -165,9 +165,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault { } if let Some(sp) = can_derive_default(self_ty, cx, default_trait_id) { - span_lint_and_then( + span_lint_node_and_then( cx, NEW_WITHOUT_DEFAULT_DERIVE, + id, impl_item.span, &format!( "you should consider deriving a `Default` implementation for `{}`", @@ -183,9 +184,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault { ); }); } else { - span_lint_and_then( + span_lint_node_and_then( cx, NEW_WITHOUT_DEFAULT, + id, impl_item.span, &format!( "you should consider adding a `Default` implementation for `{}`", diff --git a/clippy_lints/src/partialeq_ne_impl.rs b/clippy_lints/src/partialeq_ne_impl.rs index 70c93c5978b6..02935cf773d2 100644 --- a/clippy_lints/src/partialeq_ne_impl.rs +++ b/clippy_lints/src/partialeq_ne_impl.rs @@ -10,7 +10,7 @@ use crate::rustc::hir::*; use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use crate::rustc::{declare_tool_lint, lint_array}; -use crate::utils::{is_automatically_derived, span_lint}; +use crate::utils::{is_automatically_derived, span_lint_node}; use if_chain::if_chain; /// **What it does:** Checks for manual re-implementations of `PartialEq::ne`. @@ -56,10 +56,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { then { for impl_item in impl_items { if impl_item.ident.name == "ne" { - span_lint(cx, - PARTIALEQ_NE_IMPL, - impl_item.span, - "re-implementing `PartialEq::ne` is unnecessary") + span_lint_node( + cx, + PARTIALEQ_NE_IMPL, + impl_item.id.node_id, + impl_item.span, + "re-implementing `PartialEq::ne` is unnecessary", + ); } } } diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index 3a62a3a1526f..057b4850e4f4 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -17,7 +17,7 @@ use if_chain::if_chain; use crate::rustc_errors::Applicability; use crate::utils::paths::*; -use crate::utils::{match_def_path, match_type, span_lint_and_then}; +use crate::utils::{match_def_path, match_type, span_lint_and_then, SpanlessEq}; /// **What it does:** Checks for expressions that could be replaced by the question mark operator /// @@ -64,14 +64,40 @@ impl Pass { /// If it matches, it will suggest to use the question mark operator instead fn check_is_none_and_early_return_none(cx: &LateContext<'_, '_>, expr: &Expr) { if_chain! { - if let ExprKind::If(ref if_expr, ref body, _) = expr.node; - if let ExprKind::MethodCall(ref segment, _, ref args) = if_expr.node; + if let ExprKind::If(if_expr, body, else_) = &expr.node; + if let ExprKind::MethodCall(segment, _, args) = &if_expr.node; if segment.ident.name == "is_none"; if Self::expression_returns_none(cx, body); if let Some(subject) = args.get(0); if Self::is_option(cx, subject); then { + if let Some(else_) = else_ { + if_chain! { + if let ExprKind::Block(block, None) = &else_.node; + if block.stmts.len() == 0; + if let Some(block_expr) = &block.expr; + if SpanlessEq::new(cx).ignore_fn().eq_expr(subject, block_expr); + then { + span_lint_and_then( + cx, + QUESTION_MARK, + expr.span, + "this block may be rewritten with the `?` operator", + |db| { + db.span_suggestion_with_applicability( + expr.span, + "replace_it_with", + format!("Some({}?)", Sugg::hir(cx, subject, "..")), + Applicability::MaybeIncorrect, // snippet + ); + } + ) + } + } + return; + } + span_lint_and_then( cx, QUESTION_MARK, @@ -84,7 +110,7 @@ impl Pass { expr.span, "replace_it_with", format!("{}?;", receiver_str), - Applicability::MachineApplicable, // snippet + Applicability::MaybeIncorrect, // snippet ); } ) @@ -133,9 +159,13 @@ impl Pass { } } - // Check if the block has an implicit return expression - if let Some(ref ret_expr) = block.expr { - return Some(ret_expr.clone()); + // Check for `return` without a semicolon. + if_chain! { + if block.stmts.len() == 0; + if let Some(ExprKind::Ret(Some(ret_expr))) = block.expr.as_ref().map(|e| &e.node); + then { + return Some(ret_expr.clone()); + } } None diff --git a/clippy_lints/src/redundant_field_names.rs b/clippy_lints/src/redundant_field_names.rs index 308f0066b69f..d8d80f2d128f 100644 --- a/clippy_lints/src/redundant_field_names.rs +++ b/clippy_lints/src/redundant_field_names.rs @@ -57,7 +57,10 @@ impl EarlyLintPass for RedundantFieldNames { continue; } if let ExprKind::Path(None, path) = &field.expr.node { - if path.segments.len() == 1 && path.segments[0].ident == field.ident { + if path.segments.len() == 1 + && path.segments[0].ident == field.ident + && path.segments[0].args.is_none() + { span_lint_and_sugg( cx, REDUNDANT_FIELD_NAMES, diff --git a/tests/ui/new_without_default.rs b/tests/ui/new_without_default.rs index a1818e037a70..2e715a6f8ba8 100644 --- a/tests/ui/new_without_default.rs +++ b/tests/ui/new_without_default.rs @@ -139,4 +139,18 @@ impl<'a, T: 'a> OptionRefWrapper<'a, T> { } } +pub struct Allow(Foo); + +impl Allow { + #[allow(clippy::new_without_default)] + pub fn new() -> Self { unimplemented!() } +} + +pub struct AllowDerive; + +impl AllowDerive { + #[allow(clippy::new_without_default_derive)] + pub fn new() -> Self { unimplemented!() } +} + fn main() {} diff --git a/tests/ui/partialeq_ne_impl.rs b/tests/ui/partialeq_ne_impl.rs index 3f9f91c81b1d..fabeee24b305 100644 --- a/tests/ui/partialeq_ne_impl.rs +++ b/tests/ui/partialeq_ne_impl.rs @@ -20,4 +20,12 @@ impl PartialEq for Foo { } } +struct Bar; + +impl PartialEq for Bar { + fn eq(&self, _: &Bar) -> bool { true } + #[allow(clippy::partialeq_ne_impl)] + fn ne(&self, _: &Bar) -> bool { false } +} + fn main() {} diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs index 7f1d06fbd29e..b1edec32eeeb 100644 --- a/tests/ui/question_mark.rs +++ b/tests/ui/question_mark.rs @@ -42,11 +42,22 @@ pub struct SomeStruct { } impl SomeStruct { + #[rustfmt::skip] pub fn func(&self) -> Option { if (self.opt).is_none() { return None; } + if self.opt.is_none() { + return None + } + + let _ = if self.opt.is_none() { + return None; + } else { + self.opt + }; + self.opt } } diff --git a/tests/ui/question_mark.stderr b/tests/ui/question_mark.stderr index d3daaaa92705..c9d5538f36f4 100644 --- a/tests/ui/question_mark.stderr +++ b/tests/ui/question_mark.stderr @@ -9,12 +9,31 @@ error: this block may be rewritten with the `?` operator = note: `-D clippy::question-mark` implied by `-D warnings` error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:46:9 + --> $DIR/question_mark.rs:47:9 | -46 | / if (self.opt).is_none() { -47 | | return None; -48 | | } +47 | / if (self.opt).is_none() { +48 | | return None; +49 | | } | |_________^ help: replace_it_with: `(self.opt)?;` -error: aborting due to 2 previous errors +error: this block may be rewritten with the `?` operator + --> $DIR/question_mark.rs:51:9 + | +51 | / if self.opt.is_none() { +52 | | return None +53 | | } + | |_________^ help: replace_it_with: `self.opt?;` + +error: this block may be rewritten with the `?` operator + --> $DIR/question_mark.rs:55:17 + | +55 | let _ = if self.opt.is_none() { + | _________________^ +56 | | return None; +57 | | } else { +58 | | self.opt +59 | | }; + | |_________^ help: replace_it_with: `Some(self.opt?)` + +error: aborting due to 4 previous errors diff --git a/tests/ui/redundant_field_names.rs b/tests/ui/redundant_field_names.rs index 68adba92f8a2..3d727ee6e6a1 100644 --- a/tests/ui/redundant_field_names.rs +++ b/tests/ui/redundant_field_names.rs @@ -68,3 +68,14 @@ fn main() { let _ = RangeInclusive::new(start, end); let _ = RangeToInclusive { end: end }; } + +fn issue_3476() { + fn foo() { + } + + struct S { + foo: fn(), + } + + S { foo: foo:: }; +} diff --git a/tests/ui/unused_io_amount.rs b/tests/ui/unused_io_amount.rs index a125d0397af8..0ec8ce57ad7f 100644 --- a/tests/ui/unused_io_amount.rs +++ b/tests/ui/unused_io_amount.rs @@ -12,7 +12,7 @@ use std::io; -// FIXME: compiletest doesn't understand errors from macro invocation span + fn try_macro(s: &mut T) -> io::Result<()> { try!(s.write(b"test")); let mut buf = [0u8; 4];