From 096c064d4374daf6292cb54f7cf4fac4a8eda718 Mon Sep 17 00:00:00 2001 From: Florian Hartwig Date: Thu, 19 Nov 2015 20:13:36 +0100 Subject: [PATCH] Simplify has_debug_impl --- src/lib.rs | 2 +- src/methods.rs | 87 +++++++++-------------------------- tests/compile-fail/methods.rs | 4 +- 3 files changed, 27 insertions(+), 66 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 35e303b749b7..d977ed07bfd2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -85,7 +85,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box unicode::Unicode); reg.register_late_lint_pass(box strings::StringAdd); reg.register_early_lint_pass(box returns::ReturnPass); - reg.register_late_lint_pass(box methods::MethodsPass::new()); + reg.register_late_lint_pass(box methods::MethodsPass); reg.register_late_lint_pass(box shadow::ShadowPass); reg.register_late_lint_pass(box types::LetPass); reg.register_late_lint_pass(box types::UnitCmp); diff --git a/src/methods.rs b/src/methods.rs index dbc18fdfe33c..858a5a3dca08 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -4,7 +4,6 @@ use rustc::middle::ty; use rustc::middle::subst::{Subst, TypeSpace}; use std::iter; use std::borrow::Cow; -use std::collections::HashSet; use utils::{snippet, span_lint, match_path, match_type, walk_ptrs_ty_depth, walk_ptrs_ty}; @@ -13,69 +12,8 @@ use utils::{OPTION_PATH, RESULT_PATH, STRING_PATH}; use self::SelfKind::*; use self::OutType::*; -use rustc::middle::def_id::DefId; - -use rustc::middle::ty::TypeFlags; - #[derive(Clone)] -pub struct MethodsPass { types_implementing_debug: Option> } - -impl MethodsPass { - pub fn new() -> MethodsPass { - MethodsPass { types_implementing_debug: None } - } - - fn get_debug_impls(&mut self, cx: &LateContext) -> Option<&HashSet> { - if self.types_implementing_debug.is_none() { - let debug = match cx.tcx.lang_items.debug_trait() { - Some(debug) => debug, - None => return None - }; - let debug_def = cx.tcx.lookup_trait_def(debug); - let mut impls = HashSet::new(); - debug_def.for_each_impl(cx.tcx, |d| { - let o_self_ty = &cx.tcx.impl_trait_ref(d) - .map(|x| x.substs) - .and_then(|x| x.self_ty()); - let self_ty = match *o_self_ty { - Some(self_type) => self_type, - None => return - }; - let self_ty_def_id = self_ty.ty_to_def_id(); - if let Some(self_ty_def_id) = self_ty_def_id { - let has_params = self_ty.flags.get().contains(TypeFlags::HAS_PARAMS); - if !has_params { - impls.insert(self_ty_def_id); - } - } - }); - self.types_implementing_debug = Some(impls); - } - self.types_implementing_debug.as_ref() - } - - // This checks whether a given type is known to implement Debug. It's - // conservative, i.e. it should not return false positives, but will return - // false negatives. - fn has_debug_impl(&mut self, ty: ty::Ty, cx: &LateContext) -> bool { - let debug_impls = match self.get_debug_impls(cx) { - Some(debug_impls) => debug_impls, - None => return false - }; - match walk_ptrs_ty(ty).sty { - ty::TyBool | ty::TyChar | ty::TyInt(..) | ty::TyUint(..) - | ty::TyFloat(..) | ty::TyStr => true, - ty::TyTuple(ref v) if v.is_empty() => true, - ty::TyStruct(..) | ty::TyEnum(..) => { - match ty.ty_to_def_id() { - Some(ref ty_def_id) => debug_impls.contains(ty_def_id), - None => false - } - }, - _ => false - } - } -} +pub struct MethodsPass; declare_lint!(pub OPTION_UNWRAP_USED, Allow, "using `Option.unwrap()`, which should at least get a better message using `expect()`"); @@ -144,7 +82,7 @@ impl LateLintPass for MethodsPass { && match_type(cx, cx.tcx.expr_ty(&inner_args[0]), &RESULT_PATH) { let result_type = cx.tcx.expr_ty(&inner_args[0]); if let Some(error_type) = get_error_type(cx, result_type) { - if self.has_debug_impl(error_type, cx) { + if has_debug_impl(error_type, cx) { span_lint(cx, OK_EXPECT, expr.span, "called `ok().expect()` on a Result \ value. You can call `expect` directly @@ -212,6 +150,27 @@ fn get_error_type<'a>(cx: &LateContext, ty: ty::Ty<'a>) -> Option> { None } +// This checks whether a given type is known to implement Debug. It's +// conservative, i.e. it should not return false positives, but will return +// false negatives. +fn has_debug_impl<'a, 'b>(ty: ty::Ty<'a>, cx: &LateContext<'b, 'a>) -> bool { + let ty = walk_ptrs_ty(ty); + let debug = match cx.tcx.lang_items.debug_trait() { + Some(debug) => debug, + None => return false + }; + let debug_def = cx.tcx.lookup_trait_def(debug); + let mut debug_impl_exists = false; + debug_def.for_each_relevant_impl(cx.tcx, ty, |d| { + let self_ty = &cx.tcx.impl_trait_ref(d).and_then(|im| im.substs.self_ty()); + if let Some(self_ty) = *self_ty { + if !self_ty.flags.get().contains(ty::TypeFlags::HAS_PARAMS) { + debug_impl_exists = true; + } + } + }); + debug_impl_exists +} const CONVENTIONS: [(&'static str, &'static [SelfKind]); 5] = [ ("into_", &[ValueSelf]), diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs index aeb79503504b..6d543596cf5c 100644 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -54,7 +54,7 @@ fn main() { // the error type implements `Debug` let res2: Result = Ok(0); res2.ok().expect("oh noes!"); - // we're currently don't warn if the error type has a type parameter + // we currently don't warn if the error type has a type parameter // (but it would be nice if we did) let res3: Result>= Ok(0); res3.ok().expect("whoof"); @@ -62,6 +62,8 @@ fn main() { res4.ok().expect("argh"); //~ERROR called `ok().expect()` let res5: io::Result = Ok(0); res5.ok().expect("oops"); //~ERROR called `ok().expect()` + let res6: Result = Ok(0); + res6.ok().expect("meh"); //~ERROR called `ok().expect()` } struct MyError(()); // doesn't implement Debug