Auto merge of #6244 - mikerite:invalid_paths_20201027, r=flip1995

bors · bors · commit 8823684197f2 · 2020-10-27T11:16:21.000Z
New internal lint: Invalid paths Add a new internal lint that detects invalid paths in the `util::paths` and fix some invalid paths found. This commit partially addresses #6047 but the lint would have to be run before running tests to close that issue. changelog: none
diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs
@@ -1,6 +1,6 @@
 use crate::utils::paths;
 use crate::utils::{
-    get_trait_def_id, is_allowed, is_automatically_derived, is_copy, match_path, span_lint_and_help,
+    get_trait_def_id, is_allowed, is_automatically_derived, is_copy, match_def_path, match_path, span_lint_and_help,
     span_lint_and_note, span_lint_and_then,
 };
 use if_chain::if_chain;
@@ -193,10 +193,9 @@ fn check_hash_peq<'tcx>(
     hash_is_automatically_derived: bool,
 ) {
     if_chain! {
-        if match_path(&trait_ref.path, &paths::HASH);
         if let Some(peq_trait_def_id) = cx.tcx.lang_items().eq_trait();
-        if let Some(def_id) = &trait_ref.trait_def_id();
-        if !def_id.is_local();
+        if let Some(def_id) = trait_ref.trait_def_id();
+        if match_def_path(cx, def_id, &paths::HASH);
         then {
             // Look for the PartialEq implementations for `ty`
             cx.tcx.for_each_relevant_impl(peq_trait_def_id, ty, |impl_id| {
@@ -352,7 +351,8 @@ fn check_unsafe_derive_deserialize<'tcx>(
     }
 
     if_chain! {
-        if match_path(&trait_ref.path, &paths::SERDE_DESERIALIZE);
+        if let Some(trait_def_id) = trait_ref.trait_def_id();
+        if match_def_path(cx, trait_def_id, &paths::SERDE_DESERIALIZE);
         if let ty::Adt(def, _) = ty.kind();
         if let Some(local_def_id) = def.did.as_local();
         let adt_hir_id = cx.tcx.hir().local_def_id_to_hir_id(local_def_id);
diff --git a/clippy_lints/src/float_equality_without_abs.rs b/clippy_lints/src/float_equality_without_abs.rs
@@ -1,7 +1,8 @@
-use crate::utils::{match_qpath, paths, span_lint_and_then, sugg};
+use crate::utils::{match_def_path, paths, span_lint_and_then, sugg};
 use if_chain::if_chain;
 use rustc_ast::util::parser::AssocOp;
 use rustc_errors::Applicability;
+use rustc_hir::def::{DefKind, Res};
 use rustc_hir::{BinOpKind, Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty;
@@ -76,7 +77,8 @@ impl<'tcx> LateLintPass<'tcx> for FloatEqualityWithoutAbs {
 
             // right hand side matches either f32::EPSILON or f64::EPSILON
             if let ExprKind::Path(ref epsilon_path) = rhs.kind;
-            if match_qpath(epsilon_path, &paths::F32_EPSILON) || match_qpath(epsilon_path, &paths::F64_EPSILON);
+            if let Res::Def(DefKind::AssocConst, def_id) = cx.qpath_res(epsilon_path, rhs.hir_id);
+            if match_def_path(cx, def_id, &paths::F32_EPSILON) || match_def_path(cx, def_id, &paths::F64_EPSILON);
 
             // values of the substractions on the left hand side are of the type float
             let t_val_l = cx.typeck_results().expr_ty(val_l);
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -892,6 +892,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &utils::internal_lints::COLLAPSIBLE_SPAN_LINT_CALLS,
         &utils::internal_lints::COMPILER_LINT_FUNCTIONS,
         &utils::internal_lints::DEFAULT_LINT,
+        &utils::internal_lints::INVALID_PATHS,
         &utils::internal_lints::LINT_WITHOUT_LINT_PASS,
         &utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM,
         &utils::internal_lints::OUTER_EXPN_EXPN_DATA,
@@ -919,6 +920,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box utils::internal_lints::CompilerLintFunctions::new());
     store.register_late_pass(|| box utils::internal_lints::LintWithoutLintPass::default());
     store.register_late_pass(|| box utils::internal_lints::OuterExpnDataPass);
+    store.register_late_pass(|| box utils::internal_lints::InvalidPaths);
     store.register_late_pass(|| box utils::inspector::DeepCodeInspector);
     store.register_late_pass(|| box utils::author::Author);
     let vec_box_size_threshold = conf.vec_box_size_threshold;
@@ -1280,6 +1282,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&utils::internal_lints::COLLAPSIBLE_SPAN_LINT_CALLS),
         LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS),
         LintId::of(&utils::internal_lints::DEFAULT_LINT),
+        LintId::of(&utils::internal_lints::INVALID_PATHS),
         LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS),
         LintId::of(&utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM),
         LintId::of(&utils::internal_lints::OUTER_EXPN_EXPN_DATA),
diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs
@@ -1,3 +1,4 @@
+use crate::consts::{constant_simple, Constant};
 use crate::utils::{
     is_expn_of, match_def_path, match_qpath, match_type, method_calls, path_to_res, paths, qpath_res, run_lints,
     snippet, span_lint, span_lint_and_help, span_lint_and_sugg, SpanlessEq,
@@ -14,9 +15,11 @@ use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
 use rustc_hir::{Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Node, Path, StmtKind, Ty, TyKind};
 use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass};
 use rustc_middle::hir::map::Map;
+use rustc_middle::ty;
 use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::{Span, Spanned};
 use rustc_span::symbol::{Symbol, SymbolStr};
+use rustc_typeck::hir_ty_to_ty;
 
 use std::borrow::{Borrow, Cow};
 
@@ -229,6 +232,21 @@ declare_clippy_lint! {
     "using `utils::match_type()` instead of `utils::is_type_diagnostic_item()`"
 }
 
+declare_clippy_lint! {
+    /// **What it does:**
+    /// Checks the paths module for invalid paths.
+    ///
+    /// **Why is this bad?**
+    /// It indicates a bug in the code.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:** None.
+    pub INVALID_PATHS,
+    internal,
+    "invalid path"
+}
+
 declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]);
 
 impl EarlyLintPass for ClippyLintsInternal {
@@ -761,3 +779,64 @@ fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Ve
 
     None
 }
+
+// This is not a complete resolver for paths. It works on all the paths currently used in the paths
+// module.  That's all it does and all it needs to do.
+pub fn check_path(cx: &LateContext<'_>, path: &[&str]) -> bool {
+    if path_to_res(cx, path).is_some() {
+        return true;
+    }
+
+    // Some implementations can't be found by `path_to_res`, particularly inherent
+    // implementations of native types. Check lang items.
+    let path_syms: Vec<_> = path.iter().map(|p| Symbol::intern(p)).collect();
+    let lang_items = cx.tcx.lang_items();
+    for lang_item in lang_items.items() {
+        if let Some(def_id) = lang_item {
+            let lang_item_path = cx.get_def_path(*def_id);
+            if path_syms.starts_with(&lang_item_path) {
+                if let [item] = &path_syms[lang_item_path.len()..] {
+                    for child in cx.tcx.item_children(*def_id) {
+                        if child.ident.name == *item {
+                            return true;
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    false
+}
+
+declare_lint_pass!(InvalidPaths => [INVALID_PATHS]);
+
+impl<'tcx> LateLintPass<'tcx> for InvalidPaths {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
+        let local_def_id = &cx.tcx.parent_module(item.hir_id);
+        let mod_name = &cx.tcx.item_name(local_def_id.to_def_id());
+        if_chain! {
+            if mod_name.as_str() == "paths";
+            if let hir::ItemKind::Const(ty, body_id) = item.kind;
+            let ty = hir_ty_to_ty(cx.tcx, ty);
+            if let ty::Array(el_ty, _) = &ty.kind();
+            if let ty::Ref(_, el_ty, _) = &el_ty.kind();
+            if el_ty.is_str();
+            let body = cx.tcx.hir().body(body_id);
+            let typeck_results = cx.tcx.typeck_body(body_id);
+            if let Some(Constant::Vec(path)) = constant_simple(cx, typeck_results, &body.value);
+            let path: Vec<&str> = path.iter().map(|x| {
+                    if let Constant::Str(s) = x {
+                        s.as_str()
+                    } else {
+                        // We checked the type of the constant above
+                        unreachable!()
+                    }
+                }).collect();
+            if !check_path(cx, &path[..]);
+            then {
+                span_lint(cx, CLIPPY_LINTS_INTERNAL, item.span, "invalid path");
+            }
+        }
+    }
+}
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
@@ -268,6 +268,7 @@ pub fn path_to_res(cx: &LateContext<'_>, path: &[&str]) -> Option<def::Res> {
             krate: *krate,
             index: CRATE_DEF_INDEX,
         };
+        let mut current_item = None;
         let mut items = cx.tcx.item_children(krate);
         let mut path_it = path.iter().skip(1).peekable();
 
@@ -277,17 +278,41 @@ pub fn path_to_res(cx: &LateContext<'_>, path: &[&str]) -> Option<def::Res> {
                 None => return None,
             };
 
+            // `get_def_path` seems to generate these empty segments for extern blocks.
+            // We can just ignore them.
+            if segment.is_empty() {
+                continue;
+            }
+
             let result = SmallVec::<[_; 8]>::new();
             for item in mem::replace(&mut items, cx.tcx.arena.alloc_slice(&result)).iter() {
                 if item.ident.name.as_str() == *segment {
                     if path_it.peek().is_none() {
                         return Some(item.res);
                     }
 
+                    current_item = Some(item);
                     items = cx.tcx.item_children(item.res.def_id());
                     break;
                 }
             }
+
+            // The segment isn't a child_item.
+            // Try to find it under an inherent impl.
+            if_chain! {
+                if path_it.peek().is_none();
+                if let Some(current_item) = current_item;
+                let item_def_id = current_item.res.def_id();
+                if cx.tcx.def_kind(item_def_id) == DefKind::Struct;
+                then {
+                    // Bad `find_map` suggestion. See #4193.
+                    #[allow(clippy::find_map)]
+                    return cx.tcx.inherent_impls(item_def_id).iter()
+                        .flat_map(|&impl_def_id| cx.tcx.item_children(impl_def_id))
+                        .find(|item| item.ident.name.as_str() == *segment)
+                        .map(|item| item.res);
+                }
+            }
         }
     } else {
         None
diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs
@@ -32,10 +32,10 @@ pub const DISPLAY_TRAIT: [&str; 3] = ["core", "fmt", "Display"];
 pub const DOUBLE_ENDED_ITERATOR: [&str; 4] = ["core", "iter", "traits", "DoubleEndedIterator"];
 pub const DROP: [&str; 3] = ["core", "mem", "drop"];
 pub const DURATION: [&str; 3] = ["core", "time", "Duration"];
-pub const EARLY_CONTEXT: [&str; 4] = ["rustc", "lint", "context", "EarlyContext"];
+pub const EARLY_CONTEXT: [&str; 2] = ["rustc_lint", "EarlyContext"];
 pub const EXIT: [&str; 3] = ["std", "process", "exit"];
-pub const F32_EPSILON: [&str; 2] = ["f32", "EPSILON"];
-pub const F64_EPSILON: [&str; 2] = ["f64", "EPSILON"];
+pub const F32_EPSILON: [&str; 4] = ["core", "f32", "<impl f32>", "EPSILON"];
+pub const F64_EPSILON: [&str; 4] = ["core", "f64", "<impl f64>", "EPSILON"];
 pub const FILE: [&str; 3] = ["std", "fs", "File"];
 pub const FILE_TYPE: [&str; 3] = ["std", "fs", "FileType"];
 pub const FMT_ARGUMENTS_NEW_V1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"];
@@ -47,7 +47,7 @@ pub const FN_ONCE: [&str; 3] = ["core", "ops", "FnOnce"];
 pub const FROM_FROM: [&str; 4] = ["core", "convert", "From", "from"];
 pub const FROM_TRAIT: [&str; 3] = ["core", "convert", "From"];
 pub const FUTURE_FROM_GENERATOR: [&str; 3] = ["core", "future", "from_generator"];
-pub const HASH: [&str; 2] = ["hash", "Hash"];
+pub const HASH: [&str; 3] = ["core", "hash", "Hash"];
 pub const HASHMAP: [&str; 5] = ["std", "collections", "hash", "map", "HashMap"];
 pub const HASHMAP_ENTRY: [&str; 5] = ["std", "collections", "hash", "map", "Entry"];
 pub const HASHSET: [&str; 5] = ["std", "collections", "hash", "set", "HashSet"];
@@ -58,7 +58,7 @@ pub const INTO_ITERATOR: [&str; 5] = ["core", "iter", "traits", "collect", "Into
 pub const IO_READ: [&str; 3] = ["std", "io", "Read"];
 pub const IO_WRITE: [&str; 3] = ["std", "io", "Write"];
 pub const ITERATOR: [&str; 5] = ["core", "iter", "traits", "iterator", "Iterator"];
-pub const LATE_CONTEXT: [&str; 4] = ["rustc", "lint", "context", "LateContext"];
+pub const LATE_CONTEXT: [&str; 2] = ["rustc_lint", "LateContext"];
 pub const LINKED_LIST: [&str; 4] = ["alloc", "collections", "linked_list", "LinkedList"];
 pub const LINT: [&str; 3] = ["rustc_session", "lint", "Lint"];
 pub const MEM_DISCRIMINANT: [&str; 3] = ["core", "mem", "discriminant"];
@@ -86,8 +86,8 @@ pub const PATH_BUF_AS_PATH: [&str; 4] = ["std", "path", "PathBuf", "as_path"];
 pub const PATH_TO_PATH_BUF: [&str; 4] = ["std", "path", "Path", "to_path_buf"];
 pub const POLL: [&str; 4] = ["core", "task", "poll", "Poll"];
 pub const PTR_EQ: [&str; 3] = ["core", "ptr", "eq"];
-pub const PTR_NULL: [&str; 2] = ["ptr", "null"];
-pub const PTR_NULL_MUT: [&str; 2] = ["ptr", "null_mut"];
+pub const PTR_NULL: [&str; 3] = ["core", "ptr", "null"];
+pub const PTR_NULL_MUT: [&str; 3] = ["core", "ptr", "null_mut"];
 pub const PUSH_STR: [&str; 4] = ["alloc", "string", "String", "push_str"];
 pub const RANGE_ARGUMENT_TRAIT: [&str; 3] = ["core", "ops", "RangeBounds"];
 pub const RC: [&str; 3] = ["alloc", "rc", "Rc"];
@@ -107,7 +107,7 @@ pub const RESULT_ERR: [&str; 4] = ["core", "result", "Result", "Err"];
 pub const RESULT_OK: [&str; 4] = ["core", "result", "Result", "Ok"];
 pub const RWLOCK_READ_GUARD: [&str; 4] = ["std", "sync", "rwlock", "RwLockReadGuard"];
 pub const RWLOCK_WRITE_GUARD: [&str; 4] = ["std", "sync", "rwlock", "RwLockWriteGuard"];
-pub const SERDE_DESERIALIZE: [&str; 2] = ["_serde", "Deserialize"];
+pub const SERDE_DESERIALIZE: [&str; 3] = ["serde", "de", "Deserialize"];
 pub const SERDE_DE_VISITOR: [&str; 3] = ["serde", "de", "Visitor"];
 pub const SLICE_INTO_VEC: [&str; 4] = ["alloc", "slice", "<impl [T]>", "into_vec"];
 pub const SLICE_ITER: [&str; 4] = ["core", "slice", "iter", "Iter"];
diff --git a/tests/ui/invalid_paths.rs b/tests/ui/invalid_paths.rs
@@ -0,0 +1,23 @@
+#![warn(clippy::internal)]
+
+mod paths {
+    // Good path
+    pub const ANY_TRAIT: [&str; 3] = ["std", "any", "Any"];
+
+    // Path to method on inherent impl of a primitive type
+    pub const F32_EPSILON: [&str; 4] = ["core", "f32", "<impl f32>", "EPSILON"];
+
+    // Path to method on inherent impl
+    pub const ARC_PTR_EQ: [&str; 4] = ["alloc", "sync", "Arc", "ptr_eq"];
+
+    // Path with empty segment
+    pub const TRANSMUTE: [&str; 4] = ["core", "intrinsics", "", "transmute"];
+
+    // Path with bad crate
+    pub const BAD_CRATE_PATH: [&str; 2] = ["bad", "path"];
+
+    // Path with bad module
+    pub const BAD_MOD_PATH: [&str; 2] = ["std", "xxx"];
+}
+
+fn main() {}
diff --git a/tests/ui/invalid_paths.stderr b/tests/ui/invalid_paths.stderr
@@ -0,0 +1,16 @@
+error: invalid path
+  --> $DIR/invalid_paths.rs:17:5
+   |
+LL |     pub const BAD_CRATE_PATH: [&str; 2] = ["bad", "path"];
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::clippy-lints-internal` implied by `-D warnings`
+
+error: invalid path
+  --> $DIR/invalid_paths.rs:20:5
+   |
+LL |     pub const BAD_MOD_PATH: [&str; 2] = ["std", "xxx"];
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+
