From 630bb76f960c49d559efde55030b9a3ccb8b5704 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Fri, 21 Aug 2015 22:53:47 +0200 Subject: [PATCH] new lint: type complexity (fixes #93) Still very naive, but it's a start. --- README.md | 1 + src/lib.rs | 2 + src/types.rs | 125 ++++++++++++++++++++++++++++ tests/compile-fail/complex_types.rs | 44 ++++++++++ 4 files changed, 172 insertions(+) create mode 100755 tests/compile-fail/complex_types.rs diff --git a/README.md b/README.md index fbb16fcb170..55976104058 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,7 @@ string_add | allow | using `x + ..` where x is a `String`; sugge string_add_assign | allow | using `x = x + ..` where x is a `String`; suggests using `push_str()` instead string_to_string | warn | calling `String.to_string()` which is a no-op toplevel_ref_arg | warn | a function argument is declared `ref` (i.e. `fn foo(ref x: u8)`, but not `fn foo((ref x, ref y): (u8, u8))`) +type_complexity | warn | usage of very complex types; recommends factoring out parts into `type` definitions unit_cmp | warn | comparing unit values (which is always `true` or `false`, respectively) zero_width_space | deny | using a zero-width space in a string literal, which is confusing diff --git a/src/lib.rs b/src/lib.rs index 9ea1efeed5f..b0f46ae45ab 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -70,6 +70,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_lint_pass(box lifetimes::LifetimePass as LintPassObject); reg.register_lint_pass(box ranges::StepByZero as LintPassObject); reg.register_lint_pass(box types::CastPass as LintPassObject); + reg.register_lint_pass(box types::TypeComplexityPass as LintPassObject); reg.register_lint_group("clippy", vec![ approx_const::APPROX_CONSTANT, @@ -111,6 +112,7 @@ pub fn plugin_registrar(reg: &mut Registry) { types::CAST_SIGN_LOSS, types::LET_UNIT_VALUE, types::LINKEDLIST, + types::TYPE_COMPLEXITY, types::UNIT_CMP, unicode::NON_ASCII_LITERAL, unicode::ZERO_WIDTH_SPACE, diff --git a/src/types.rs b/src/types.rs index 622f733f812..54c36535286 100644 --- a/src/types.rs +++ b/src/types.rs @@ -2,6 +2,8 @@ use rustc::lint::*; use syntax::ast; use syntax::ast::*; use syntax::ast_util::{is_comparison_binop, binop_to_string}; +use syntax::codemap::Span; +use syntax::visit::{FnKind, Visitor, walk_ty}; use rustc::middle::ty; use syntax::codemap::ExpnInfo; @@ -183,3 +185,126 @@ impl LintPass for CastPass { } } } + +declare_lint!(pub TYPE_COMPLEXITY, Warn, + "usage of very complex types; recommends factoring out parts into `type` definitions"); + +#[allow(missing_copy_implementations)] +pub struct TypeComplexityPass; + +impl LintPass for TypeComplexityPass { + fn get_lints(&self) -> LintArray { + lint_array!(TYPE_COMPLEXITY) + } + + fn check_fn(&mut self, cx: &Context, _: FnKind, decl: &FnDecl, _: &Block, _: Span, _: NodeId) { + check_fndecl(cx, decl); + } + + fn check_struct_field(&mut self, cx: &Context, field: &StructField) { + check_type(cx, &*field.node.ty); + } + + fn check_variant(&mut self, cx: &Context, var: &Variant, _: &Generics) { + // StructVariant is covered by check_struct_field + if let TupleVariantKind(ref args) = var.node.kind { + for arg in args { + check_type(cx, &*arg.ty); + } + } + } + + fn check_item(&mut self, cx: &Context, item: &Item) { + match item.node { + ItemStatic(ref ty, _, _) | + ItemConst(ref ty, _) => check_type(cx, ty), + // functions, enums, structs, impls and traits are covered + _ => () + } + } + + fn check_trait_item(&mut self, cx: &Context, item: &TraitItem) { + match item.node { + ConstTraitItem(ref ty, _) | + TypeTraitItem(_, Some(ref ty)) => check_type(cx, ty), + MethodTraitItem(MethodSig { ref decl, .. }, None) => check_fndecl(cx, decl), + // methods with default impl are covered by check_fn + _ => () + } + } + + fn check_impl_item(&mut self, cx: &Context, item: &ImplItem) { + match item.node { + ConstImplItem(ref ty, _) | + TypeImplItem(ref ty) => check_type(cx, ty), + // methods are covered by check_fn + _ => () + } + } + + fn check_local(&mut self, cx: &Context, local: &Local) { + if let Some(ref ty) = local.ty { + check_type(cx, ty); + } + } +} + +fn check_fndecl(cx: &Context, decl: &FnDecl) { + for arg in &decl.inputs { + check_type(cx, &*arg.ty); + } + if let Return(ref ty) = decl.output { + check_type(cx, ty); + } +} + +fn check_type(cx: &Context, ty: &ast::Ty) { + let score = { + let mut visitor = TypeComplexityVisitor { score: 0, nest: 1 }; + visitor.visit_ty(ty); + visitor.score + }; + // println!("{:?} --> {}", ty, score); + if score > 250 { + span_lint(cx, TYPE_COMPLEXITY, ty.span, &format!( + "very complex type used. Consider factoring parts into `type` definitions")); + } +} + +/// Walks a type and assigns a complexity score to it. +struct TypeComplexityVisitor { + /// total complexity score of the type + score: u32, + /// current nesting level + nest: u32, +} + +impl<'v> Visitor<'v> for TypeComplexityVisitor { + fn visit_ty(&mut self, ty: &'v ast::Ty) { + let (add_score, sub_nest) = match ty.node { + // _, &x and *x have only small overhead; don't mess with nesting level + TyInfer | + TyPtr(..) | + TyRptr(..) => (1, 0), + + // the "normal" components of a type: named types, arrays/tuples + TyPath(..) | + TyVec(..) | + TyTup(..) | + TyFixedLengthVec(..) => (10 * self.nest, 1), + + // "Sum" of trait bounds + TyObjectSum(..) => (20 * self.nest, 0), + + // function types and "for<...>" bring a lot of overhead + TyBareFn(..) | + TyPolyTraitRef(..) => (50 * self.nest, 1), + + _ => (0, 0) + }; + self.score += add_score; + self.nest += sub_nest; + walk_ty(self, ty); + self.nest -= sub_nest; + } +} diff --git a/tests/compile-fail/complex_types.rs b/tests/compile-fail/complex_types.rs new file mode 100755 index 00000000000..995132ba88c --- /dev/null +++ b/tests/compile-fail/complex_types.rs @@ -0,0 +1,44 @@ +#![feature(plugin)] +#![plugin(clippy)] +#![deny(clippy)] +#![allow(unused)] +#![feature(associated_consts, associated_type_defaults)] + +type Alias = Vec>>; // no warning here + +const CST: (u32, (u32, (u32, (u32, u32)))) = (0, (0, (0, (0, 0)))); //~ERROR very complex type +static ST: (u32, (u32, (u32, (u32, u32)))) = (0, (0, (0, (0, 0)))); //~ERROR very complex type + +struct S { + f: Vec>>, //~ERROR very complex type +} + +struct TS(Vec>>); //~ERROR very complex type + +enum E { + V1(Vec>>), //~ERROR very complex type + V2 { f: Vec>> }, //~ERROR very complex type +} + +impl S { + const A: (u32, (u32, (u32, (u32, u32)))) = (0, (0, (0, (0, 0)))); //~ERROR very complex type + fn impl_method(&self, p: Vec>>) { } //~ERROR very complex type +} + +trait T { + const A: Vec>>; //~ERROR very complex type + type B = Vec>>; //~ERROR very complex type + fn method(&self, p: Vec>>); //~ERROR very complex type + fn def_method(&self, p: Vec>>) { } //~ERROR very complex type +} + +fn test1() -> Vec>> { vec![] } //~ERROR very complex type + +fn test2(_x: Vec>>) { } //~ERROR very complex type + +fn test3() { + let _y: Vec>> = vec![]; //~ERROR very complex type +} + +fn main() { +}