Auto merge of #5651 - ebroto:names_as_early_passes, r=flip1995

Register redundant_field_names and non_expressive_names as early passes

Similar names was moved to a pre-expansion pass to solve #2927, so I'm avoiding linting on code from expansion, which makes the dogfood (mostly, see below) pass.

I had to change new_without_default though, and although I understand why it was not triggering before, TBH I don't see why the binding inside the nested `if_chain` is being linted now. Any ideas? (it seems legit though as the code can be changed by the user)

changelog: Register redundant_field_names and non_expressive_names as early passes

Fixes #5356
Fixes #5521

Author: bors

Diff for: clippy_lints/src/lib.rs

@@ -346,13 +346,8 @@ mod reexport {
 /// level (i.e `#![cfg_attr(...)]`) will still be expanded even when using a pre-expansion pass.
 ///
 /// Used in `./src/driver.rs`.
-pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore, conf: &Conf) {
+pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore) {
     store.register_pre_expansion_pass(|| box write::Write::default());
-    store.register_pre_expansion_pass(|| box redundant_field_names::RedundantFieldNames);
-    let single_char_binding_names_threshold = conf.single_char_binding_names_threshold;
-    store.register_pre_expansion_pass(move || box non_expressive_names::NonExpressiveNames {
-        single_char_binding_names_threshold,
-    });
     store.register_pre_expansion_pass(|| box attrs::EarlyAttributes);
     store.register_pre_expansion_pass(|| box dbg_macro::DbgMacro);
 }
@@ -1066,6 +1061,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box match_on_vec_items::MatchOnVecItems);
     store.register_early_pass(|| box manual_non_exhaustive::ManualNonExhaustive);
     store.register_late_pass(|| box manual_async_fn::ManualAsyncFn);
+    store.register_early_pass(|| box redundant_field_names::RedundantFieldNames);
+    let single_char_binding_names_threshold = conf.single_char_binding_names_threshold;
+    store.register_early_pass(move || box non_expressive_names::NonExpressiveNames {
+        single_char_binding_names_threshold,
+    });
 
     store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
         LintId::of(&arithmetic::FLOAT_ARITHMETIC),

Diff for: clippy_lints/src/new_without_default.rs

@@ -90,8 +90,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault {
                             return;
                         }
                         if sig.decl.inputs.is_empty() && name == sym!(new) && cx.access_levels.is_reachable(id) {
-                            let self_did = cx.tcx.hir().local_def_id(cx.tcx.hir().get_parent_item(id));
-                            let self_ty = cx.tcx.type_of(self_did);
+                            let self_def_id = cx.tcx.hir().local_def_id(cx.tcx.hir().get_parent_item(id));
+                            let self_ty = cx.tcx.type_of(self_def_id);
                             if_chain! {
                                 if same_tys(cx, self_ty, return_ty(cx, id));
                                 if let Some(default_trait_id) = get_trait_def_id(cx, &paths::DEFAULT_TRAIT);
@@ -112,10 +112,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NewWithoutDefault {
                                     // generics
                                     if_chain! {
                                         if let Some(ref impling_types) = self.impling_types;
-                                        if let Some(self_def) = cx.tcx.type_of(self_did).ty_adt_def();
-                                        if let Some(self_def_id) = self_def.did.as_local();
+                                        if let Some(self_def) = cx.tcx.type_of(self_def_id).ty_adt_def();
+                                        if let Some(self_local_did) = self_def.did.as_local();
                                         then {
-                                            let self_id = cx.tcx.hir().local_def_id_to_hir_id(self_def_id);
+                                            let self_id = cx.tcx.hir().local_def_id_to_hir_id(self_local_did);
                                             if impling_types.contains(&self_id) {
                                                 return;
                                             }

Diff for: clippy_lints/src/non_expressive_names.rs

@@ -5,6 +5,7 @@ use rustc_ast::ast::{
 use rustc_ast::attr;
 use rustc_ast::visit::{walk_block, walk_expr, walk_pat, Visitor};
 use rustc_lint::{EarlyContext, EarlyLintPass};
+use rustc_middle::lint::in_external_macro;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::Span;
 use rustc_span::symbol::{Ident, SymbolStr};
@@ -131,7 +132,11 @@ struct SimilarNamesNameVisitor<'a, 'tcx, 'b>(&'b mut SimilarNamesLocalVisitor<'a
 impl<'a, 'tcx, 'b> Visitor<'tcx> for SimilarNamesNameVisitor<'a, 'tcx, 'b> {
     fn visit_pat(&mut self, pat: &'tcx Pat) {
         match pat.kind {
-            PatKind::Ident(_, ident, _) => self.check_ident(ident),
+            PatKind::Ident(_, ident, _) => {
+                if !pat.span.from_expansion() {
+                    self.check_ident(ident);
+                }
+            },
             PatKind::Struct(_, ref fields, _) => {
                 for field in fields {
                     if !field.is_shorthand {
@@ -354,12 +359,20 @@ impl<'a, 'tcx> Visitor<'tcx> for SimilarNamesLocalVisitor<'a, 'tcx> {
 
 impl EarlyLintPass for NonExpressiveNames {
     fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
+        if in_external_macro(cx.sess, item.span) {
+            return;
+        }
+
         if let ItemKind::Fn(_, ref sig, _, Some(ref blk)) = item.kind {
             do_check(self, cx, &item.attrs, &sig.decl, blk);
         }
     }
 
     fn check_impl_item(&mut self, cx: &EarlyContext<'_>, item: &AssocItem) {
+        if in_external_macro(cx.sess, item.span) {
+            return;
+        }
+
         if let AssocItemKind::Fn(_, ref sig, _, Some(ref blk)) = item.kind {
             do_check(self, cx, &item.attrs, &sig.decl, blk);
         }

Diff for: clippy_lints/src/redundant_field_names.rs

@@ -2,6 +2,7 @@ use crate::utils::span_lint_and_sugg;
 use rustc_ast::ast::{Expr, ExprKind};
 use rustc_errors::Applicability;
 use rustc_lint::{EarlyContext, EarlyLintPass};
+use rustc_middle::lint::in_external_macro;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 
 declare_clippy_lint! {
@@ -36,6 +37,9 @@ declare_lint_pass!(RedundantFieldNames => [REDUNDANT_FIELD_NAMES]);
 
 impl EarlyLintPass for RedundantFieldNames {
     fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
+        if in_external_macro(cx.sess, expr.span) {
+            return;
+        }
         if let ExprKind::Struct(_, ref fields, _) = expr.kind {
             for field in fields {
                 if field.is_shorthand {

Diff for: src/driver.rs

@@ -79,7 +79,7 @@ impl rustc_driver::Callbacks for ClippyCallbacks {
 
             let conf = clippy_lints::read_conf(&[], &sess);
             clippy_lints::register_plugins(&mut lint_store, &sess, &conf);
-            clippy_lints::register_pre_expansion_lints(&mut lint_store, &conf);
+            clippy_lints::register_pre_expansion_lints(&mut lint_store);
             clippy_lints::register_renamed(&mut lint_store);
         }));