Merge pull request #2895 from gnzlbg/lint_no_inline

Add missing_inline lint

CHANGELOG.md

@@ -744,6 +744,7 @@ All notable changes to this project will be documented in this file.
 [`misaligned_transmute`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#misaligned_transmute
 [`misrefactored_assign_op`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#misrefactored_assign_op
 [`missing_docs_in_private_items`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#missing_docs_in_private_items
+[`missing_inline_in_public_items`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#missing_inline_in_public_items
 [`mixed_case_hex_literals`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#mixed_case_hex_literals
 [`module_inception`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#module_inception
 [`modulo_one`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#modulo_one

README.md

@@ -9,7 +9,7 @@ We are currently in the process of discussing Clippy 1.0 via the RFC process in
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 272 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
+[There are 273 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much clippy is supposed to ~~annoy~~ help you:
 

clippy_lints/src/lib.rs

@@ -134,6 +134,7 @@ pub mod minmax;
 pub mod misc;
 pub mod misc_early;
 pub mod missing_doc;
+pub mod missing_inline;
 pub mod multiple_crate_versions;
 pub mod mut_mut;
 pub mod mut_reference;
@@ -364,6 +365,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_late_lint_pass(box let_if_seq::LetIfSeq);
     reg.register_late_lint_pass(box eval_order_dependence::EvalOrderDependence);
     reg.register_late_lint_pass(box missing_doc::MissingDoc::new());
+    reg.register_late_lint_pass(box missing_inline::MissingInline);
     reg.register_late_lint_pass(box ok_if_let::Pass);
     reg.register_late_lint_pass(box if_let_redundant_pattern_matching::Pass);
     reg.register_late_lint_pass(box partialeq_ne_impl::Pass);
@@ -422,6 +424,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         methods::WRONG_PUB_SELF_CONVENTION,
         misc::FLOAT_CMP_CONST,
         missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS,
+        missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS,
         panic_unimplemented::UNIMPLEMENTED,
         shadow::SHADOW_REUSE,
         shadow::SHADOW_SAME,

clippy_lints/src/missing_inline.rs

@@ -0,0 +1,198 @@
+//   Copyright 2012-2015 The Rust Project Developers. See the COPYRIGHT
+//   file at the top-level directory of this distribution and at
+//   http://rust-lang.org/COPYRIGHT.
+//
+//   Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+//   http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+//   <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+//   option. This file may not be copied, modified, or distributed
+//   except according to those terms.
+//
+
+use rustc::hir;
+use rustc::lint::*;
+use syntax::ast;
+use syntax::codemap::Span;
+
+/// **What it does:** it lints if an exported function, method, trait method with default impl,
+/// or trait method impl is not `#[inline]`.
+///
+/// **Why is this bad?** In general, it is not. Functions can be inlined across
+/// crates when that's profitable as long as any form of LTO is used. When LTO is disabled,
+/// functions that are not `#[inline]` cannot be inlined across crates. Certain types of crates
+/// might intend for most of the methods in their public API to be able to be inlined across
+/// crates even when LTO is disabled. For these types of crates, enabling this lint might make sense.
+/// It allows the crate to require all exported methods to be `#[inline]` by default, and then opt
+/// out for specific methods where this might not make sense.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// pub fn foo() {} // missing #[inline]
+/// fn ok() {} // ok
+/// #[inline] pub fn bar() {} // ok
+/// #[inline(always)] pub fn baz() {} // ok
+///
+/// pub trait Bar {
+///   fn bar(); // ok
+///   fn def_bar() {} // missing #[inline]
+/// }
+///
+/// struct Baz;
+/// impl Baz {
+///    fn priv() {} // ok
+/// }
+///
+/// impl Bar for Baz {
+///   fn bar() {} // ok - Baz is not exported
+/// }
+///
+/// pub struct PubBaz;
+/// impl PubBaz {
+///    fn priv() {} // ok
+///    pub not_ptriv() {} // missing #[inline]
+/// }
+///
+/// impl Bar for PubBaz {
+///    fn bar() {} // missing #[inline]
+///    fn def_bar() {} // missing #[inline]
+/// }
+/// ```
+declare_clippy_lint! {
+    pub MISSING_INLINE_IN_PUBLIC_ITEMS,
+    restriction,
+    "detects missing #[inline] attribute for public callables (functions, trait methods, methods...)"
+}
+
+pub struct MissingInline;
+
+fn check_missing_inline_attrs(cx: &LateContext,
+                              attrs: &[ast::Attribute], sp: Span, desc: &'static str) {
+    let has_inline = attrs
+        .iter()
+        .any(|a| a.name() == "inline" );
+    if !has_inline {
+        cx.span_lint(
+            MISSING_INLINE_IN_PUBLIC_ITEMS,
+            sp,
+            &format!("missing `#[inline]` for {}", desc),
+        );
+    }
+}
+
+fn is_executable<'a, 'tcx>(cx: &LateContext<'a, 'tcx>) -> bool {
+    use rustc::session::config::CrateType;
+
+    cx.tcx.sess.crate_types.get().iter().any(|t: &CrateType| {
+        match t {
+            CrateType::CrateTypeExecutable => true,
+            _ => false,
+        }
+    })
+}
+
+impl LintPass for MissingInline {
+    fn get_lints(&self) -> LintArray {
+        lint_array![MISSING_INLINE_IN_PUBLIC_ITEMS]
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingInline {
+    fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, it: &'tcx hir::Item) {
+        if is_executable(cx) {
+            return;
+        }
+
+        if !cx.access_levels.is_exported(it.id) {
+            return;
+        }
+        match it.node {
+            hir::ItemFn(..) => {
+                let desc = "a function";
+                check_missing_inline_attrs(cx, &it.attrs, it.span, desc);
+            },
+            hir::ItemTrait(ref _is_auto, ref _unsafe, ref _generics,
+                           ref _bounds, ref trait_items)  => {
+                // note: we need to check if the trait is exported so we can't use
+                // `LateLintPass::check_trait_item` here.
+                for tit in trait_items {
+                    let tit_ = cx.tcx.hir.trait_item(tit.id);
+                    match tit_.node {
+                        hir::TraitItemKind::Const(..) |
+                        hir::TraitItemKind::Type(..) => {},
+                        hir::TraitItemKind::Method(..) => {
+                            if tit.defaultness.has_value() {
+                                // trait method with default body needs inline in case
+                                // an impl is not provided
+                                let desc = "a default trait method";
+                                let item = cx.tcx.hir.expect_trait_item(tit.id.node_id);
+                                check_missing_inline_attrs(cx, &item.attrs,
+                                                                item.span, desc);
+                            }
+                        },
+                    }
+                }
+            }
+            hir::ItemConst(..) |
+            hir::ItemEnum(..) |
+            hir::ItemMod(..) |
+            hir::ItemStatic(..) |
+            hir::ItemStruct(..) |
+            hir::ItemTraitAlias(..) |
+            hir::ItemGlobalAsm(..) |
+            hir::ItemTy(..) |
+            hir::ItemUnion(..) |
+            hir::ItemExistential(..) |
+            hir::ItemExternCrate(..) |
+            hir::ItemForeignMod(..) |
+            hir::ItemImpl(..) |
+            hir::ItemUse(..) => {},
+        };
+    }
+
+    fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, impl_item: &'tcx hir::ImplItem) {
+        use rustc::ty::{TraitContainer, ImplContainer};
+        if is_executable(cx) {
+            return;
+        }
+
+        // If the item being implemented is not exported, then we don't need #[inline]
+        if !cx.access_levels.is_exported(impl_item.id) {
+            return;
+        }
+
+        let desc = match impl_item.node {
+            hir::ImplItemKind::Method(..) => "a method",
+            hir::ImplItemKind::Const(..) |
+            hir::ImplItemKind::Type(_) => return,
+        };
+
+        let def_id = cx.tcx.hir.local_def_id(impl_item.id);
+        match cx.tcx.associated_item(def_id).container {
+            TraitContainer(cid) => {
+                if let Some(n) = cx.tcx.hir.as_local_node_id(cid) {
+                    if !cx.access_levels.is_exported(n) {
+                        // If a trait is being implemented for an item, and the
+                        // trait is not exported, we don't need #[inline]
+                        return;
+                    }
+                }
+            },
+            ImplContainer(cid) => {
+                if cx.tcx.impl_trait_ref(cid).is_some() {
+                    let trait_ref = cx.tcx.impl_trait_ref(cid).unwrap();
+                    if let Some(n) = cx.tcx.hir.as_local_node_id(trait_ref.def_id) {
+                        if !cx.access_levels.is_exported(n) {
+                            // If a trait is being implemented for an item, and the
+                            // trait is not exported, we don't need #[inline]
+                            return;
+                        }
+                    }
+                }
+            },
+        }
+
+        check_missing_inline_attrs(cx, &impl_item.attrs, impl_item.span, desc);
+    }
+}

tests/ui/missing_inline.rs

@@ -0,0 +1,72 @@
+/* This file incorporates work covered by the following copyright and
+ * permission notice:
+ *   Copyright 2013 The Rust Project Developers. See the COPYRIGHT
+ *   file at the top-level directory of this distribution and at
+ *   http://rust-lang.org/COPYRIGHT.
+ *
+ *   Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+ *   http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+ *   <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+ *   option. This file may not be copied, modified, or distributed
+ *   except according to those terms.
+ */
+#![warn(missing_inline_in_public_items)]
+#![crate_type = "dylib"]
+// When denying at the crate level, be sure to not get random warnings from the
+// injected intrinsics by the compiler.
+#![allow(dead_code, non_snake_case)]
+
+type Typedef = String;
+pub type PubTypedef = String;
+
+struct Foo {} // ok
+pub struct PubFoo { } // ok
+enum FooE {} // ok
+pub enum PubFooE {} // ok
+
+mod module {} // ok
+pub mod pub_module {} // ok
+
+fn foo() {}
+pub fn pub_foo() {} // missing #[inline]
+#[inline] pub fn pub_foo_inline() {} // ok
+#[inline(always)] pub fn pub_foo_inline_always() {} // ok
+
+#[allow(missing_inline_in_public_items)]
+pub fn pub_foo_no_inline() {}
+
+trait Bar {
+    fn Bar_a(); // ok
+    fn Bar_b() {} // ok
+}
+
+
+pub trait PubBar {
+    fn PubBar_a(); // ok
+    fn PubBar_b() {} // missing #[inline]
+    #[inline] fn PubBar_c() {} // ok
+}
+
+// none of these need inline because Foo is not exported
+impl PubBar for Foo {
+    fn PubBar_a() {} // ok
+    fn PubBar_b() {} // ok
+    fn PubBar_c() {} // ok
+}
+
+// all of these need inline because PubFoo is exported
+impl PubBar for PubFoo {
+    fn PubBar_a() {} // missing #[inline]
+    fn PubBar_b() {} // missing #[inline]
+    fn PubBar_c() {} // missing #[inline]
+}
+
+// do not need inline because Foo is not exported
+impl Foo {
+    fn FooImpl() {} // ok
+}
+
+// need inline because PubFoo is exported
+impl PubFoo {
+    pub fn PubFooImpl() {} // missing #[inline]
+}

tests/ui/missing_inline.stderr

@@ -0,0 +1,40 @@
+error: missing `#[inline]` for a function
+  --> $DIR/missing_inline.rs:31:1
+   |
+31 | pub fn pub_foo() {} // missing #[inline]
+   | ^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D missing-inline-in-public-items` implied by `-D warnings`
+
+error: missing `#[inline]` for a default trait method
+  --> $DIR/missing_inline.rs:46:5
+   |
+46 |     fn PubBar_b() {} // missing #[inline]
+   |     ^^^^^^^^^^^^^^^^
+
+error: missing `#[inline]` for a method
+  --> $DIR/missing_inline.rs:59:5
+   |
+59 |     fn PubBar_a() {} // missing #[inline]
+   |     ^^^^^^^^^^^^^^^^
+
+error: missing `#[inline]` for a method
+  --> $DIR/missing_inline.rs:60:5
+   |
+60 |     fn PubBar_b() {} // missing #[inline]
+   |     ^^^^^^^^^^^^^^^^
+
+error: missing `#[inline]` for a method
+  --> $DIR/missing_inline.rs:61:5
+   |
+61 |     fn PubBar_c() {} // missing #[inline]
+   |     ^^^^^^^^^^^^^^^^
+
+error: missing `#[inline]` for a method
+  --> $DIR/missing_inline.rs:71:5
+   |
+71 |     pub fn PubFooImpl() {} // missing #[inline]
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 6 previous errors
+