From 66d56fefc5b7ce22d2db65ee9dc1a5f9f6bf2f09 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Tue, 27 Oct 2020 07:42:13 +0200 Subject: [PATCH 1/2] Add `invalid_paths` internal lint --- clippy_lints/src/lib.rs | 3 + clippy_lints/src/utils/internal_lints.rs | 79 ++++++++++++++++++++++++ clippy_lints/src/utils/mod.rs | 25 ++++++++ tests/ui/invalid_paths.rs | 23 +++++++ tests/ui/invalid_paths.stderr | 16 +++++ 5 files changed, 146 insertions(+) create mode 100644 tests/ui/invalid_paths.rs create mode 100644 tests/ui/invalid_paths.stderr diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 3be8bc0e36d..7c8cb90fe1c 100644 --- 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 index bfe426a25eb..6ca72d895c8 100644 --- 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, 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 index 8297b9d128d..a1ecca0961a 100644 --- 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 { 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,6 +278,12 @@ pub fn path_to_res(cx: &LateContext<'_>, path: &[&str]) -> Option { 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 { @@ -284,10 +291,28 @@ pub fn path_to_res(cx: &LateContext<'_>, path: &[&str]) -> Option { 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/tests/ui/invalid_paths.rs b/tests/ui/invalid_paths.rs new file mode 100644 index 00000000000..01e28ae5e9d --- /dev/null +++ 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", "", "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 new file mode 100644 index 00000000000..bd69d661b71 --- /dev/null +++ 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 + From f79c4afd3a5c408bd9253311b224773702a912df Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Tue, 27 Oct 2020 07:43:38 +0200 Subject: [PATCH 2/2] Fix invalid paths --- clippy_lints/src/derive.rs | 10 +++++----- clippy_lints/src/float_equality_without_abs.rs | 6 ++++-- clippy_lints/src/utils/paths.rs | 16 ++++++++-------- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index bf8e030cc29..c75efc6e99f 100644 --- 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 index 69818b4d3c6..c1c08597ee6 100644 --- 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/utils/paths.rs b/clippy_lints/src/utils/paths.rs index cd9b92efe58..d5a0e0d1f29 100644 --- 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", "", "EPSILON"]; +pub const F64_EPSILON: [&str; 4] = ["core", "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", "", "into_vec"]; pub const SLICE_ITER: [&str; 4] = ["core", "slice", "iter", "Iter"];