X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Ffallible_impl_from.rs;h=7e4d1b3ef9f0d78365391ada01d0c3e81492024b;hb=21da42ce29c33e4a33b215a93b19bf8b5f06228d;hp=71d0aba8acac2d1c118def6e8865ae8bc96f932e;hpb=5b255883f5bb92772a04e60d57fb2bffd037b890;p=rust.git diff --git a/clippy_lints/src/fallible_impl_from.rs b/clippy_lints/src/fallible_impl_from.rs index 71d0aba8aca..7e4d1b3ef9f 100644 --- a/clippy_lints/src/fallible_impl_from.rs +++ b/clippy_lints/src/fallible_impl_from.rs @@ -1,29 +1,49 @@ -use crate::utils::paths::{BEGIN_PANIC, BEGIN_PANIC_FMT, FROM_TRAIT, OPTION, RESULT}; -use crate::utils::{is_expn_of, match_def_path, method_chain_args, span_lint_and_then, walk_ptrs_ty}; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{is_expn_of, match_panic_def_id, method_chain_args}; use if_chain::if_chain; -use rustc::declare_lint_pass; -use rustc::hir; -use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; -use rustc::ty::{self, Ty}; -use rustc_session::declare_tool_lint; -use syntax_pos::Span; +use rustc_hir as hir; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::map::Map; +use rustc_middle::ty; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::{sym, Span}; declare_clippy_lint! { - /// **What it does:** Checks for impls of `From<..>` that contain `panic!()` or `unwrap()` + /// ### What it does + /// Checks for impls of `From<..>` that contain `panic!()` or `unwrap()` /// - /// **Why is this bad?** `TryFrom` should be used if there's a possibility of failure. + /// ### Why is this bad? + /// `TryFrom` should be used if there's a possibility of failure. /// - /// **Known problems:** None. - /// - /// **Example:** + /// ### Example /// ```rust /// struct Foo(i32); + /// + /// // Bad /// impl From for Foo { /// fn from(s: String) -> Self { /// Foo(s.parse().unwrap()) /// } /// } /// ``` + /// + /// ```rust + /// // Good + /// struct Foo(i32); + /// + /// use std::convert::TryFrom; + /// impl TryFrom for Foo { + /// type Error = (); + /// fn try_from(s: String) -> Result { + /// if let Ok(parsed) = s.parse() { + /// Ok(Foo(parsed)) + /// } else { + /// Err(()) + /// } + /// } + /// } + /// ``` pub FALLIBLE_IMPL_FROM, nursery, "Warn on impls of `From<..>` that contain `panic!()` or `unwrap()`" @@ -31,40 +51,40 @@ declare_lint_pass!(FallibleImplFrom => [FALLIBLE_IMPL_FROM]); -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FallibleImplFrom { - fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item<'_>) { +impl<'tcx> LateLintPass<'tcx> for FallibleImplFrom { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) { // check for `impl From for ..` - let impl_def_id = cx.tcx.hir().local_def_id(item.hir_id); if_chain! { - if let hir::ItemKind::Impl(.., impl_items) = item.kind; - if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(impl_def_id); - if match_def_path(cx, impl_trait_ref.def_id, &FROM_TRAIT); + if let hir::ItemKind::Impl(impl_) = &item.kind; + if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(item.def_id); + if cx.tcx.is_diagnostic_item(sym::from_trait, impl_trait_ref.def_id); then { - lint_impl_body(cx, item.span, impl_items); + lint_impl_body(cx, item.span, impl_.items); } } } } -fn lint_impl_body<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, impl_span: Span, impl_items: &[hir::ImplItemRef<'_>]) { - use rustc::hir::intravisit::{self, NestedVisitorMap, Visitor}; - use rustc::hir::*; +fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_items: &[hir::ImplItemRef<'_>]) { + use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; + use rustc_hir::{Expr, ExprKind, ImplItemKind, QPath}; struct FindPanicUnwrap<'a, 'tcx> { - lcx: &'a LateContext<'a, 'tcx>, - tables: &'tcx ty::TypeckTables<'tcx>, + lcx: &'a LateContext<'tcx>, + typeck_results: &'tcx ty::TypeckResults<'tcx>, result: Vec, } impl<'a, 'tcx> Visitor<'tcx> for FindPanicUnwrap<'a, 'tcx> { + type Map = Map<'tcx>; + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { // check for `begin_panic` if_chain! { - if let ExprKind::Call(ref func_expr, _) = expr.kind; - if let ExprKind::Path(QPath::Resolved(_, ref path)) = func_expr.kind; + if let ExprKind::Call(func_expr, _) = expr.kind; + if let ExprKind::Path(QPath::Resolved(_, path)) = func_expr.kind; if let Some(path_def_id) = path.res.opt_def_id(); - if match_def_path(self.lcx, path_def_id, &BEGIN_PANIC) || - match_def_path(self.lcx, path_def_id, &BEGIN_PANIC_FMT); + if match_panic_def_id(self.lcx, path_def_id); if is_expn_of(expr.span, "unreachable").is_none(); then { self.result.push(expr.span); @@ -73,8 +93,10 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { // check for `unwrap` if let Some(arglists) = method_chain_args(expr, &["unwrap"]) { - let reciever_ty = walk_ptrs_ty(self.tables.expr_ty(&arglists[0][0])); - if match_type(self.lcx, reciever_ty, &OPTION) || match_type(self.lcx, reciever_ty, &RESULT) { + let reciever_ty = self.typeck_results.expr_ty(&arglists[0][0]).peel_refs(); + if is_type_diagnostic_item(self.lcx, reciever_ty, sym::option_type) + || is_type_diagnostic_item(self.lcx, reciever_ty, sym::result_type) + { self.result.push(expr.span); } } @@ -83,23 +105,22 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { intravisit::walk_expr(self, expr); } - fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { + fn nested_visit_map(&mut self) -> NestedVisitorMap { NestedVisitorMap::None } } for impl_item in impl_items { if_chain! { - if impl_item.ident.name == sym!(from); - if let ImplItemKind::Method(_, body_id) = + if impl_item.ident.name == sym::from; + if let ImplItemKind::Fn(_, body_id) = cx.tcx.hir().impl_item(impl_item.id).kind; then { // check the body for `begin_panic` or `unwrap` let body = cx.tcx.hir().body(body_id); - let impl_item_def_id = cx.tcx.hir().local_def_id(impl_item.id.hir_id); let mut fpu = FindPanicUnwrap { lcx: cx, - tables: cx.tcx.typeck_tables_of(impl_item_def_id), + typeck_results: cx.tcx.typeck(impl_item.id.def_id), result: Vec::new(), }; fpu.visit_expr(&body.value); @@ -111,21 +132,14 @@ fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { FALLIBLE_IMPL_FROM, impl_span, "consider implementing `TryFrom` instead", - move |db| { - db.help( + move |diag| { + diag.help( "`From` is intended for infallible conversions only. \ - Use `TryFrom` if there's a possibility for the conversion to fail."); - db.span_note(fpu.result, "potential failure(s)"); + Use `TryFrom` if there's a possibility for the conversion to fail"); + diag.span_note(fpu.result, "potential failure(s)"); }); } } } } } - -fn match_type(cx: &LateContext<'_, '_>, ty: Ty<'_>, path: &[&str]) -> bool { - match ty.kind { - ty::Adt(adt, _) => match_def_path(cx, adt.did, path), - _ => false, - } -}