diff options
Diffstat (limited to 'clippy_lints/src/utils')
| -rw-r--r-- | clippy_lints/src/utils/diagnostics.rs | 9 | ||||
| -rw-r--r-- | clippy_lints/src/utils/eager_or_lazy.rs | 128 | ||||
| -rw-r--r-- | clippy_lints/src/utils/internal_lints.rs | 119 | ||||
| -rw-r--r-- | clippy_lints/src/utils/mod.rs | 14 | ||||
| -rw-r--r-- | clippy_lints/src/utils/paths.rs | 4 | ||||
| -rw-r--r-- | clippy_lints/src/utils/sugg.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/utils/usage.rs | 66 |
7 files changed, 328 insertions, 14 deletions
diff --git a/clippy_lints/src/utils/diagnostics.rs b/clippy_lints/src/utils/diagnostics.rs index e4e65b5f4d4..0a58231558e 100644 --- a/clippy_lints/src/utils/diagnostics.rs +++ b/clippy_lints/src/utils/diagnostics.rs @@ -51,6 +51,8 @@ pub fn span_lint<T: LintContext>(cx: &T, lint: &'static Lint, sp: impl Into<Mult /// /// The `help` message can be optionally attached to a `Span`. /// +/// If you change the signature, remember to update the internal lint `CollapsibleCalls` +/// /// # Example /// /// ```ignore @@ -87,6 +89,8 @@ pub fn span_lint_and_help<'a, T: LintContext>( /// The `note` message is presented separately from the main lint message /// and is attached to a specific span: /// +/// If you change the signature, remember to update the internal lint `CollapsibleCalls` +/// /// # Example /// /// ```ignore @@ -126,6 +130,7 @@ pub fn span_lint_and_note<'a, T: LintContext>( /// Like `span_lint` but allows to add notes, help and suggestions using a closure. /// /// If you need to customize your lint output a lot, use this function. +/// If you change the signature, remember to update the internal lint `CollapsibleCalls` pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint, sp: Span, msg: &str, f: F) where F: for<'b> FnOnce(&mut DiagnosticBuilder<'b>), @@ -168,6 +173,10 @@ pub fn span_lint_hir_and_then( /// In the example below, `help` is `"try"` and `sugg` is the suggested replacement `".any(|x| x > /// 2)"`. /// +/// If you change the signature, remember to update the internal lint `CollapsibleCalls` +/// +/// # Example +/// /// ```ignore /// error: This `.fold` can be more succinctly expressed as `.any` /// --> $DIR/methods.rs:390:13 diff --git a/clippy_lints/src/utils/eager_or_lazy.rs b/clippy_lints/src/utils/eager_or_lazy.rs new file mode 100644 index 00000000000..6938d9971d9 --- /dev/null +++ b/clippy_lints/src/utils/eager_or_lazy.rs @@ -0,0 +1,128 @@ +//! Utilities for evaluating whether eagerly evaluated expressions can be made lazy and vice versa. +//! +//! Things to consider: +//! - has the expression side-effects? +//! - is the expression computationally expensive? +//! +//! See lints: +//! - unnecessary-lazy-evaluations +//! - or-fun-call +//! - option-if-let-else + +use crate::utils::is_ctor_or_promotable_const_function; +use rustc_hir::def::{DefKind, Res}; + +use rustc_hir::intravisit; +use rustc_hir::intravisit::{NestedVisitorMap, Visitor}; + +use rustc_hir::{Block, Expr, ExprKind, Path, QPath}; +use rustc_lint::LateContext; +use rustc_middle::hir::map::Map; + +/// Is the expr pure (is it free from side-effects)? +/// This function is named so to stress that it isn't exhaustive and returns FNs. +fn identify_some_pure_patterns(expr: &Expr<'_>) -> bool { + match expr.kind { + ExprKind::Lit(..) | ExprKind::Path(..) | ExprKind::Field(..) => true, + ExprKind::AddrOf(_, _, addr_of_expr) => identify_some_pure_patterns(addr_of_expr), + ExprKind::Tup(tup_exprs) => tup_exprs.iter().all(|expr| identify_some_pure_patterns(expr)), + ExprKind::Struct(_, fields, expr) => { + fields.iter().all(|f| identify_some_pure_patterns(f.expr)) + && expr.map_or(true, |e| identify_some_pure_patterns(e)) + }, + ExprKind::Call( + &Expr { + kind: + ExprKind::Path(QPath::Resolved( + _, + Path { + res: Res::Def(DefKind::Ctor(..) | DefKind::Variant, ..), + .. + }, + )), + .. + }, + args, + ) => args.iter().all(|expr| identify_some_pure_patterns(expr)), + ExprKind::Block( + &Block { + stmts, + expr: Some(expr), + .. + }, + _, + ) => stmts.is_empty() && identify_some_pure_patterns(expr), + ExprKind::Box(..) + | ExprKind::Array(..) + | ExprKind::Call(..) + | ExprKind::MethodCall(..) + | ExprKind::Binary(..) + | ExprKind::Unary(..) + | ExprKind::Cast(..) + | ExprKind::Type(..) + | ExprKind::DropTemps(..) + | ExprKind::Loop(..) + | ExprKind::Match(..) + | ExprKind::Closure(..) + | ExprKind::Block(..) + | ExprKind::Assign(..) + | ExprKind::AssignOp(..) + | ExprKind::Index(..) + | ExprKind::Break(..) + | ExprKind::Continue(..) + | ExprKind::Ret(..) + | ExprKind::InlineAsm(..) + | ExprKind::LlvmInlineAsm(..) + | ExprKind::Repeat(..) + | ExprKind::Yield(..) + | ExprKind::Err => false, + } +} + +/// Identify some potentially computationally expensive patterns. +/// This function is named so to stress that its implementation is non-exhaustive. +/// It returns FNs and FPs. +fn identify_some_potentially_expensive_patterns<'a, 'tcx>(cx: &'a LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { + // Searches an expression for method calls or function calls that aren't ctors + struct FunCallFinder<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + found: bool, + } + + impl<'a, 'tcx> intravisit::Visitor<'tcx> for FunCallFinder<'a, 'tcx> { + type Map = Map<'tcx>; + + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { + let call_found = match &expr.kind { + // ignore enum and struct constructors + ExprKind::Call(..) => !is_ctor_or_promotable_const_function(self.cx, expr), + ExprKind::MethodCall(..) => true, + _ => false, + }; + + if call_found { + self.found |= true; + } + + if !self.found { + intravisit::walk_expr(self, expr); + } + } + + fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { + NestedVisitorMap::None + } + } + + let mut finder = FunCallFinder { cx, found: false }; + finder.visit_expr(expr); + finder.found +} + +pub fn is_eagerness_candidate<'a, 'tcx>(cx: &'a LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { + !identify_some_potentially_expensive_patterns(cx, expr) && identify_some_pure_patterns(expr) +} + +pub fn is_lazyness_candidate<'a, 'tcx>(cx: &'a LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { + identify_some_potentially_expensive_patterns(cx, expr) +} diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 8fa5d22210a..bfe426a25eb 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -1,6 +1,6 @@ use crate::utils::{ - is_expn_of, match_def_path, match_qpath, match_type, method_calls, paths, run_lints, snippet, span_lint, - span_lint_and_help, span_lint_and_sugg, walk_ptrs_ty, SpanlessEq, + 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, }; use if_chain::if_chain; use rustc_ast::ast::{Crate as AstCrate, ItemKind, LitKind, NodeId}; @@ -11,7 +11,7 @@ use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::hir_id::CRATE_HIR_ID; use rustc_hir::intravisit::{NestedVisitorMap, Visitor}; -use rustc_hir::{Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Path, StmtKind, Ty, TyKind}; +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_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; @@ -206,6 +206,29 @@ declare_clippy_lint! { "found collapsible `span_lint_and_then` calls" } +declare_clippy_lint! { + /// **What it does:** Checks for calls to `utils::match_type()` on a type diagnostic item + /// and suggests to use `utils::is_type_diagnostic_item()` instead. + /// + /// **Why is this bad?** `utils::is_type_diagnostic_item()` does not require hardcoded paths. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// Bad: + /// ```rust,ignore + /// utils::match_type(cx, ty, &paths::VEC) + /// ``` + /// + /// Good: + /// ```rust,ignore + /// utils::is_type_diagnostic_item(cx, ty, sym!(vec_type)) + /// ``` + pub MATCH_TYPE_ON_DIAGNOSTIC_ITEM, + internal, + "using `utils::match_type()` instead of `utils::is_type_diagnostic_item()`" +} + declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]); impl EarlyLintPass for ClippyLintsInternal { @@ -404,7 +427,7 @@ impl<'tcx> LateLintPass<'tcx> for CompilerLintFunctions { if let ExprKind::MethodCall(ref path, _, ref args, _) = expr.kind; let fn_name = path.ident; if let Some(sugg) = self.map.get(&*fn_name.as_str()); - let ty = walk_ptrs_ty(cx.typeck_results().expr_ty(&args[0])); + let ty = cx.typeck_results().expr_ty(&args[0]).peel_refs(); if match_type(cx, ty, &paths::EARLY_CONTEXT) || match_type(cx, ty, &paths::LATE_CONTEXT); then { @@ -437,7 +460,7 @@ impl<'tcx> LateLintPass<'tcx> for OuterExpnDataPass { let args = arg_lists[1]; if args.len() == 1; let self_arg = &args[0]; - let self_ty = walk_ptrs_ty(cx.typeck_results().expr_ty(self_arg)); + let self_ty = cx.typeck_results().expr_ty(self_arg).peel_refs(); if match_type(cx, self_ty, &paths::SYNTAX_CONTEXT); then { span_lint_and_sugg( @@ -652,3 +675,89 @@ fn suggest_note( Applicability::MachineApplicable, ); } + +declare_lint_pass!(MatchTypeOnDiagItem => [MATCH_TYPE_ON_DIAGNOSTIC_ITEM]); + +impl<'tcx> LateLintPass<'tcx> for MatchTypeOnDiagItem { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { + if !run_lints(cx, &[MATCH_TYPE_ON_DIAGNOSTIC_ITEM], expr.hir_id) { + return; + } + + if_chain! { + // Check if this is a call to utils::match_type() + if let ExprKind::Call(fn_path, [context, ty, ty_path]) = expr.kind; + if let ExprKind::Path(fn_qpath) = &fn_path.kind; + if match_qpath(&fn_qpath, &["utils", "match_type"]); + // Extract the path to the matched type + if let Some(segments) = path_to_matched_type(cx, ty_path); + let segments: Vec<&str> = segments.iter().map(|sym| &**sym).collect(); + if let Some(ty_did) = path_to_res(cx, &segments[..]).and_then(|res| res.opt_def_id()); + // Check if the matched type is a diagnostic item + let diag_items = cx.tcx.diagnostic_items(ty_did.krate); + if let Some(item_name) = diag_items.iter().find_map(|(k, v)| if *v == ty_did { Some(k) } else { None }); + then { + let cx_snippet = snippet(cx, context.span, "_"); + let ty_snippet = snippet(cx, ty.span, "_"); + + span_lint_and_sugg( + cx, + MATCH_TYPE_ON_DIAGNOSTIC_ITEM, + expr.span, + "usage of `utils::match_type()` on a type diagnostic item", + "try", + format!("utils::is_type_diagnostic_item({}, {}, sym!({}))", cx_snippet, ty_snippet, item_name), + Applicability::MaybeIncorrect, + ); + } + } + } +} + +fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Vec<SymbolStr>> { + use rustc_hir::ItemKind; + + match &expr.kind { + ExprKind::AddrOf(.., expr) => return path_to_matched_type(cx, expr), + ExprKind::Path(qpath) => match qpath_res(cx, qpath, expr.hir_id) { + Res::Local(hir_id) => { + let parent_id = cx.tcx.hir().get_parent_node(hir_id); + if let Some(Node::Local(local)) = cx.tcx.hir().find(parent_id) { + if let Some(init) = local.init { + return path_to_matched_type(cx, init); + } + } + }, + Res::Def(DefKind::Const | DefKind::Static, def_id) => { + if let Some(Node::Item(item)) = cx.tcx.hir().get_if_local(def_id) { + if let ItemKind::Const(.., body_id) | ItemKind::Static(.., body_id) = item.kind { + let body = cx.tcx.hir().body(body_id); + return path_to_matched_type(cx, &body.value); + } + } + }, + _ => {}, + }, + ExprKind::Array(exprs) => { + let segments: Vec<SymbolStr> = exprs + .iter() + .filter_map(|expr| { + if let ExprKind::Lit(lit) = &expr.kind { + if let LitKind::Str(sym, _) = lit.node { + return Some(sym.as_str()); + } + } + + None + }) + .collect(); + + if segments.len() == exprs.len() { + return Some(segments); + } + }, + _ => {}, + } + + None +} diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 3ebbfed6456..ea52741b7cc 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -10,6 +10,7 @@ pub mod comparisons; pub mod conf; pub mod constants; mod diagnostics; +pub mod eager_or_lazy; pub mod higher; mod hir_utils; pub mod inspector; @@ -130,6 +131,9 @@ pub fn is_wild<'tcx>(pat: &impl std::ops::Deref<Target = Pat<'tcx>>) -> bool { } /// Checks if type is struct, enum or union type with the given def path. +/// +/// If the type is a diagnostic item, use `is_type_diagnostic_item` instead. +/// If you change the signature, remember to update the internal lint `MatchTypeOnDiagItem` pub fn match_type(cx: &LateContext<'_>, ty: Ty<'_>, path: &[&str]) -> bool { match ty.kind() { ty::Adt(adt, _) => match_def_path(cx, adt.did, path), @@ -138,6 +142,8 @@ pub fn match_type(cx: &LateContext<'_>, ty: Ty<'_>, path: &[&str]) -> bool { } /// Checks if the type is equal to a diagnostic item +/// +/// If you change the signature, remember to update the internal lint `MatchTypeOnDiagItem` pub fn is_type_diagnostic_item(cx: &LateContext<'_>, ty: Ty<'_>, diag_item: Symbol) -> bool { match ty.kind() { ty::Adt(adt, _) => cx.tcx.is_diagnostic_item(diag_item, adt.did), @@ -748,14 +754,6 @@ pub fn walk_ptrs_hir_ty<'tcx>(ty: &'tcx hir::Ty<'tcx>) -> &'tcx hir::Ty<'tcx> { } } -/// Returns the base type for references and raw pointers. -pub fn walk_ptrs_ty(ty: Ty<'_>) -> Ty<'_> { - match ty.kind() { - ty::Ref(_, ty, _) => walk_ptrs_ty(ty), - _ => ty, - } -} - /// Returns the base type for references and raw pointers, and count reference /// depth. pub fn walk_ptrs_ty_depth(ty: Ty<'_>) -> (Ty<'_>, usize) { diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 3b031a552e5..1583afad208 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -113,8 +113,12 @@ pub const STD_CONVERT_IDENTITY: [&str; 3] = ["std", "convert", "identity"]; pub const STD_FS_CREATE_DIR: [&str; 3] = ["std", "fs", "create_dir"]; pub const STD_MEM_TRANSMUTE: [&str; 3] = ["std", "mem", "transmute"]; pub const STD_PTR_NULL: [&str; 3] = ["std", "ptr", "null"]; +pub const STRING: [&str; 3] = ["alloc", "string", "String"]; pub const STRING_AS_MUT_STR: [&str; 4] = ["alloc", "string", "String", "as_mut_str"]; pub const STRING_AS_STR: [&str; 4] = ["alloc", "string", "String", "as_str"]; +pub const STR_ENDS_WITH: [&str; 4] = ["core", "str", "<impl str>", "ends_with"]; +pub const STR_LEN: [&str; 4] = ["core", "str", "<impl str>", "len"]; +pub const STR_STARTS_WITH: [&str; 4] = ["core", "str", "<impl str>", "starts_with"]; pub const SYNTAX_CONTEXT: [&str; 3] = ["rustc_span", "hygiene", "SyntaxContext"]; pub const TO_OWNED: [&str; 3] = ["alloc", "borrow", "ToOwned"]; pub const TO_OWNED_METHOD: [&str; 4] = ["alloc", "borrow", "ToOwned", "to_owned"]; diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index 811fde388d1..ec8b7e59b59 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -49,7 +49,7 @@ impl<'a> Sugg<'a> { /// Convenience function around `hir_opt` for suggestions with a default /// text. pub fn hir(cx: &LateContext<'_>, expr: &hir::Expr<'_>, default: &'a str) -> Self { - Self::hir_opt(cx, expr).unwrap_or_else(|| Sugg::NonParen(Cow::Borrowed(default))) + Self::hir_opt(cx, expr).unwrap_or(Sugg::NonParen(Cow::Borrowed(default))) } /// Same as `hir`, but it adapts the applicability level by following rules: diff --git a/clippy_lints/src/utils/usage.rs b/clippy_lints/src/utils/usage.rs index 4a64b935ac9..ea1dc3be29b 100644 --- a/clippy_lints/src/utils/usage.rs +++ b/clippy_lints/src/utils/usage.rs @@ -1,6 +1,8 @@ use crate::utils::match_var; use rustc_data_structures::fx::FxHashSet; +use rustc_hir as hir; use rustc_hir::def::Res; +use rustc_hir::intravisit; use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; use rustc_hir::{Expr, HirId, Path}; use rustc_infer::infer::TyCtxtInferExt; @@ -108,3 +110,67 @@ pub fn is_unused<'tcx>(ident: &'tcx Ident, body: &'tcx Expr<'_>) -> bool { walk_expr(&mut visitor, body); !visitor.used } + +pub struct ParamBindingIdCollector { + binding_hir_ids: Vec<hir::HirId>, +} +impl<'tcx> ParamBindingIdCollector { + fn collect_binding_hir_ids(body: &'tcx hir::Body<'tcx>) -> Vec<hir::HirId> { + let mut finder = ParamBindingIdCollector { + binding_hir_ids: Vec::new(), + }; + finder.visit_body(body); + finder.binding_hir_ids + } +} +impl<'tcx> intravisit::Visitor<'tcx> for ParamBindingIdCollector { + type Map = Map<'tcx>; + + fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) { + if let hir::PatKind::Binding(_, hir_id, ..) = param.pat.kind { + self.binding_hir_ids.push(hir_id); + } + } + + fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> { + intravisit::NestedVisitorMap::None + } +} + +pub struct BindingUsageFinder<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + binding_ids: Vec<hir::HirId>, + usage_found: bool, +} +impl<'a, 'tcx> BindingUsageFinder<'a, 'tcx> { + pub fn are_params_used(cx: &'a LateContext<'tcx>, body: &'tcx hir::Body<'tcx>) -> bool { + let mut finder = BindingUsageFinder { + cx, + binding_ids: ParamBindingIdCollector::collect_binding_hir_ids(body), + usage_found: false, + }; + finder.visit_body(body); + finder.usage_found + } +} +impl<'a, 'tcx> intravisit::Visitor<'tcx> for BindingUsageFinder<'a, 'tcx> { + type Map = Map<'tcx>; + + fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) { + if !self.usage_found { + intravisit::walk_expr(self, expr); + } + } + + fn visit_path(&mut self, path: &'tcx hir::Path<'tcx>, _: hir::HirId) { + if let hir::def::Res::Local(id) = path.res { + if self.binding_ids.contains(&id) { + self.usage_found = true; + } + } + } + + fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> { + intravisit::NestedVisitorMap::OnlyBodies(self.cx.tcx.hir()) + } +} |