Merge pull request #219 from birkenfeld/type_complexity

[RFC] new lint: type complexity (fixes #93)

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
 

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,

src/ptr_arg.rs

@@ -45,20 +45,18 @@ impl LintPass for PtrArg {
 
 fn check_fn(cx: &Context, decl: &FnDecl) {
     for arg in &decl.inputs {
-        if arg.ty.node == TyInfer {  // "self" arguments
-            continue;
-        }
-        let ref sty = cx.tcx.pat_ty(&*arg.pat).sty;
-        if let &ty::TyRef(_, ty::TypeAndMut { ty, mutbl: MutImmutable }) = sty {
-            if match_type(cx, ty, &VEC_PATH) {
-                span_lint(cx, PTR_ARG, arg.ty.span,
-                          "writing `&Vec<_>` instead of `&[_]` involves one more reference \
-                           and cannot be used with non-Vec-based slices. Consider changing \
-                           the type to `&[...]`");
-            } else if match_type(cx, ty, &STRING_PATH) {
-                span_lint(cx, PTR_ARG, arg.ty.span,
-                          "writing `&String` instead of `&str` involves a new object \
-                           where a slice will do. Consider changing the type to `&str`");
+        if let Some(pat_ty) = cx.tcx.pat_ty_opt(&*arg.pat) {
+            if let ty::TyRef(_, ty::TypeAndMut { ty, mutbl: MutImmutable }) = pat_ty.sty {
+                if match_type(cx, ty, &VEC_PATH) {
+                    span_lint(cx, PTR_ARG, arg.ty.span,
+                              "writing `&Vec<_>` instead of `&[_]` involves one more reference \
+                               and cannot be used with non-Vec-based slices. Consider changing \
+                               the type to `&[...]`");
+                } else if match_type(cx, ty, &STRING_PATH) {
+                    span_lint(cx, PTR_ARG, arg.ty.span,
+                              "writing `&String` instead of `&str` involves a new object \
+                               where a slice will do. Consider changing the type to `&str`");
+                }
             }
         }
     }

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;
+    }
+}

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<Vec<Box<(u32, u32, u32, u32)>>>; // 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<Vec<Box<(u32, u32, u32, u32)>>>, //~ERROR very complex type
+}
+
+struct TS(Vec<Vec<Box<(u32, u32, u32, u32)>>>); //~ERROR very complex type
+
+enum E {
+    V1(Vec<Vec<Box<(u32, u32, u32, u32)>>>), //~ERROR very complex type
+    V2 { f: Vec<Vec<Box<(u32, u32, u32, u32)>>> }, //~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<Vec<Box<(u32, u32, u32, u32)>>>) { } //~ERROR very complex type
+}
+
+trait T {
+    const A: Vec<Vec<Box<(u32, u32, u32, u32)>>>; //~ERROR very complex type
+    type B = Vec<Vec<Box<(u32, u32, u32, u32)>>>; //~ERROR very complex type
+    fn method(&self, p: Vec<Vec<Box<(u32, u32, u32, u32)>>>); //~ERROR very complex type
+    fn def_method(&self, p: Vec<Vec<Box<(u32, u32, u32, u32)>>>) { } //~ERROR very complex type
+}
+
+fn test1() -> Vec<Vec<Box<(u32, u32, u32, u32)>>> { vec![] } //~ERROR very complex type
+
+fn test2(_x: Vec<Vec<Box<(u32, u32, u32, u32)>>>) { } //~ERROR very complex type
+
+fn test3() {
+    let _y: Vec<Vec<Box<(u32, u32, u32, u32)>>> = vec![]; //~ERROR very complex type
+}
+
+fn main() {
+}