Auto merge of #4162 - krk:static-static, r=flip1995

Add lint for statics with explicit static lifetime.

changelog: Add lint for statics with explicit static lifetime, fixes #4138.

Author: bors

CHANGELOG.md

@@ -881,7 +881,6 @@ All notable changes to this project will be documented in this file.
 [`cmp_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_owned
 [`cognitive_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity
 [`collapsible_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
-[`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime
 [`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator
 [`crosspointer_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#crosspointer_transmute
 [`dbg_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#dbg_macro
@@ -1068,6 +1067,7 @@ All notable changes to this project will be documented in this file.
 [`redundant_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
 [`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern
 [`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching
+[`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
 [`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
 [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro
 [`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts

clippy_lints/src/lib.rs

@@ -157,7 +157,6 @@ pub mod cargo_common_metadata;
 pub mod checked_conversions;
 pub mod cognitive_complexity;
 pub mod collapsible_if;
-pub mod const_static_lifetime;
 pub mod copies;
 pub mod copy_iterator;
 pub mod dbg_macro;
@@ -249,6 +248,7 @@ pub mod ranges;
 pub mod redundant_clone;
 pub mod redundant_field_names;
 pub mod redundant_pattern_matching;
+pub mod redundant_static_lifetimes;
 pub mod reference;
 pub mod regex;
 pub mod replace_consts;
@@ -553,7 +553,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     reg.register_late_lint_pass(box invalid_ref::InvalidRef);
     reg.register_late_lint_pass(box identity_conversion::IdentityConversion::default());
     reg.register_late_lint_pass(box types::ImplicitHasher);
-    reg.register_early_lint_pass(box const_static_lifetime::StaticConst);
+    reg.register_early_lint_pass(box redundant_static_lifetimes::RedundantStaticLifetimes);
     reg.register_late_lint_pass(box fallible_impl_from::FallibleImplFrom);
     reg.register_late_lint_pass(box replace_consts::ReplaceConsts);
     reg.register_late_lint_pass(box types::UnitArg);
@@ -686,7 +686,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         bytecount::NAIVE_BYTECOUNT,
         cognitive_complexity::COGNITIVE_COMPLEXITY,
         collapsible_if::COLLAPSIBLE_IF,
-        const_static_lifetime::CONST_STATIC_LIFETIME,
         copies::IFS_SAME_COND,
         copies::IF_SAME_THEN_ELSE,
         derive::DERIVE_HASH_XOR_EQ,
@@ -834,6 +833,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         ranges::RANGE_ZIP_WITH_LEN,
         redundant_field_names::REDUNDANT_FIELD_NAMES,
         redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING,
+        redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES,
         reference::DEREF_ADDROF,
         reference::REF_IN_DEREF,
         regex::INVALID_REGEX,
@@ -901,7 +901,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR,
         block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT,
         collapsible_if::COLLAPSIBLE_IF,
-        const_static_lifetime::CONST_STATIC_LIFETIME,
         enum_variants::ENUM_VARIANT_NAMES,
         enum_variants::MODULE_INCEPTION,
         eq_op::OP_REF,
@@ -957,6 +956,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         question_mark::QUESTION_MARK,
         redundant_field_names::REDUNDANT_FIELD_NAMES,
         redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING,
+        redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES,
         regex::REGEX_MACRO,
         regex::TRIVIAL_REGEX,
         returns::LET_AND_RETURN,
@@ -1153,6 +1153,7 @@ pub fn register_renamed(ls: &mut rustc::lint::LintStore) {
     ls.register_renamed("clippy::stutter", "clippy::module_name_repetitions");
     ls.register_renamed("clippy::new_without_default_derive", "clippy::new_without_default");
     ls.register_renamed("clippy::cyclomatic_complexity", "clippy::cognitive_complexity");
+    ls.register_renamed("clippy::const_static_lifetime", "clippy::redundant_static_lifetimes");
 }
 
 // only exists to let the dogfood integration test works.

clippy_lints/src/const_static_lifetime.rs renamed to clippy_lints/src/redundant_static_lifetimes.rs

@@ -5,7 +5,7 @@ use rustc_errors::Applicability;
 use syntax::ast::*;
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for constants with an explicit `'static` lifetime.
+    /// **What it does:** Checks for constants and statics with an explicit `'static` lifetime.
     ///
     /// **Why is this bad?** Adding `'static` to every reference can create very
     /// complicated types.
@@ -16,29 +16,32 @@ declare_clippy_lint! {
     /// ```ignore
     /// const FOO: &'static [(&'static str, &'static str, fn(&Bar) -> bool)] =
     /// &[...]
+    /// static FOO: &'static [(&'static str, &'static str, fn(&Bar) -> bool)] =
+    /// &[...]
     /// ```
     /// This code can be rewritten as
     /// ```ignore
     ///  const FOO: &[(&str, &str, fn(&Bar) -> bool)] = &[...]
+    ///  static FOO: &[(&str, &str, fn(&Bar) -> bool)] = &[...]
     /// ```
-    pub CONST_STATIC_LIFETIME,
+    pub REDUNDANT_STATIC_LIFETIMES,
     style,
-    "Using explicit `'static` lifetime for constants when elision rules would allow omitting them."
+    "Using explicit `'static` lifetime for constants or statics when elision rules would allow omitting them."
 }
 
-declare_lint_pass!(StaticConst => [CONST_STATIC_LIFETIME]);
+declare_lint_pass!(RedundantStaticLifetimes => [REDUNDANT_STATIC_LIFETIMES]);
 
-impl StaticConst {
+impl RedundantStaticLifetimes {
     // Recursively visit types
-    fn visit_type(&mut self, ty: &Ty, cx: &EarlyContext<'_>) {
+    fn visit_type(&mut self, ty: &Ty, cx: &EarlyContext<'_>, reason: &str) {
         match ty.node {
             // Be careful of nested structures (arrays and tuples)
             TyKind::Array(ref ty, _) => {
-                self.visit_type(&*ty, cx);
+                self.visit_type(&*ty, cx, reason);
             },
             TyKind::Tup(ref tup) => {
                 for tup_ty in tup {
-                    self.visit_type(&*tup_ty, cx);
+                    self.visit_type(&*tup_ty, cx, reason);
                 }
             },
             // This is what we are looking for !
@@ -50,44 +53,40 @@ impl StaticConst {
                             if lifetime.ident.name == syntax::symbol::kw::StaticLifetime {
                                 let snip = snippet(cx, borrow_type.ty.span, "<type>");
                                 let sugg = format!("&{}", snip);
-                                span_lint_and_then(
-                                    cx,
-                                    CONST_STATIC_LIFETIME,
-                                    lifetime.ident.span,
-                                    "Constants have by default a `'static` lifetime",
-                                    |db| {
-                                        db.span_suggestion(
-                                            ty.span,
-                                            "consider removing `'static`",
-                                            sugg,
-                                            Applicability::MachineApplicable, //snippet
-                                        );
-                                    },
-                                );
+                                span_lint_and_then(cx, REDUNDANT_STATIC_LIFETIMES, lifetime.ident.span, reason, |db| {
+                                    db.span_suggestion(
+                                        ty.span,
+                                        "consider removing `'static`",
+                                        sugg,
+                                        Applicability::MachineApplicable, //snippet
+                                    );
+                                });
                             }
                         },
                         _ => {},
                     }
                 }
-                self.visit_type(&*borrow_type.ty, cx);
+                self.visit_type(&*borrow_type.ty, cx, reason);
             },
             TyKind::Slice(ref ty) => {
-                self.visit_type(ty, cx);
+                self.visit_type(ty, cx, reason);
             },
             _ => {},
         }
     }
 }
 
-impl EarlyLintPass for StaticConst {
+impl EarlyLintPass for RedundantStaticLifetimes {
     fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
         if !in_macro_or_desugar(item.span) {
-            // Match only constants...
             if let ItemKind::Const(ref var_type, _) = item.node {
-                self.visit_type(var_type, cx);
+                self.visit_type(var_type, cx, "Constants have by default a `'static` lifetime");
+                // Don't check associated consts because `'static` cannot be elided on those (issue #2438)
+            }
+
+            if let ItemKind::Static(ref var_type, _, _) = item.node {
+                self.visit_type(var_type, cx, "Statics have by default a `'static` lifetime");
             }
         }
     }
-
-    // Don't check associated consts because `'static` cannot be elided on those (issue #2438)
 }

tests/ui/const_static_lifetime.stderr

@@ -23,6 +23,28 @@ const VAR_TUPLE: &'static (u8, u8) = &(1, 2); // ERROR Consider removing 'static
 
 const VAR_ARRAY: &'static [u8; 1] = b"T"; // ERROR Consider removing 'static.
 
+static STATIC_VAR_ONE: &'static str = "Test static #1"; // ERROR Consider removing 'static.
+
+static STATIC_VAR_TWO: &str = "Test static #2"; // This line should not raise a warning.
+
+static STATIC_VAR_THREE: &[&'static str] = &["one", "two"]; // ERROR Consider removing 'static
+
+static STATIC_VAR_FOUR: (&str, (&str, &'static str), &'static str) = ("on", ("th", "th"), "on"); // ERROR Consider removing 'static
+
+static STATIC_VAR_FIVE: &'static [&[&'static str]] = &[&["test"], &["other one"]]; // ERROR Consider removing 'static
+
+static STATIC_VAR_SIX: &'static u8 = &5;
+
+static STATIC_VAR_SEVEN: &[&(&str, &'static [&'static str])] = &[&("one", &["other one"])];
+
+static STATIC_VAR_HEIGHT: &'static Foo = &Foo {};
+
+static STATIC_VAR_SLICE: &'static [u8] = b"Test static #3"; // ERROR Consider removing 'static.
+
+static STATIC_VAR_TUPLE: &'static (u8, u8) = &(1, 2); // ERROR Consider removing 'static.
+
+static STATIC_VAR_ARRAY: &'static [u8; 1] = b"T"; // ERROR Consider removing 'static.
+
 fn main() {
     let false_positive: &'static str = "test";
     println!("{}", VAR_ONE);