From 322effe4158cd3f2c48eba83af86383cd8d5612f Mon Sep 17 00:00:00 2001 From: Paul Florence Date: Fri, 20 Oct 2017 09:51:35 -0400 Subject: [PATCH] Implementation of the `const_static_lifetime` lint. --- clippy_lints/src/const_static_lifetime.rs | 82 +++++++++++++++++++++++ clippy_lints/src/lib.rs | 3 + tests/ui/const_static_lifetime.rs | 33 +++++++++ tests/ui/regex.rs | 4 +- 4 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 clippy_lints/src/const_static_lifetime.rs create mode 100644 tests/ui/const_static_lifetime.rs diff --git a/clippy_lints/src/const_static_lifetime.rs b/clippy_lints/src/const_static_lifetime.rs new file mode 100644 index 00000000000..3e426d9cebb --- /dev/null +++ b/clippy_lints/src/const_static_lifetime.rs @@ -0,0 +1,82 @@ +use syntax::ast::{Item, ItemKind, TyKind, Ty}; +use rustc::lint::{LintPass, EarlyLintPass, LintArray, EarlyContext}; +use utils::{span_help_and_lint, in_macro}; + +/// **What it does:** Checks for constants with an explicit `'static` lifetime. +/// +/// **Why is this bad?** Adding `'static` to every reference can create very complicated types. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// const FOO: &'static [(&'static str, &'static str, fn(&Bar) -> bool)] = &[..] +/// ``` +/// This code can be rewritten as +/// ```rust +/// const FOO: &[(&str, &str, fn(&Bar) -> bool)] = &[...] +/// ``` + +declare_lint! { + pub CONST_STATIC_LIFETIME, + Warn, + "Using explicit `'static` lifetime for constants when elision rules would allow omitting them." +} + +pub struct StaticConst; + +impl LintPass for StaticConst { + fn get_lints(&self) -> LintArray { + lint_array!(CONST_STATIC_LIFETIME) + } +} + +impl StaticConst { + // Recursively visit types + fn visit_type(&mut self, ty: &Ty, cx: &EarlyContext) { + match ty.node { + // Be carefull of nested structures (arrays and tuples) + TyKind::Array(ref ty, _) => { + println!("array"); + self.visit_type(&*ty, cx); + }, + TyKind::Tup(ref tup) => { + for tup_ty in tup { + self.visit_type(&*tup_ty, cx); + } + }, + // This is what we are looking for ! + TyKind::Rptr(ref optional_lifetime, ref borrow_type) => { + // Match the 'static lifetime + if let Some(lifetime) = *optional_lifetime { + if let TyKind::Path(_, _) = borrow_type.ty.node { + // Verify that the path is a str + if lifetime.ident.name == "'static" { + span_help_and_lint(cx, + CONST_STATIC_LIFETIME, + lifetime.span, + "Constants have by default a `'static` lifetime", + "consider removing `'static`"); + } + } + } + self.visit_type(&*borrow_type.ty, cx); + }, + TyKind::Slice(ref ty) => { + self.visit_type(&ty, cx); + }, + _ => {}, + } + } +} + +impl EarlyLintPass for StaticConst { + fn check_item(&mut self, cx: &EarlyContext, item: &Item) { + if !in_macro(item.span) { + // Match only constants... + if let ItemKind::Const(ref var_type, _) = item.node { + self.visit_type(var_type, cx); + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 198a876c352..9b78e435645 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -76,6 +76,7 @@ pub mod block_in_if_condition; pub mod booleans; pub mod bytecount; pub mod collapsible_if; +pub mod const_static_lifetime; pub mod copies; pub mod cyclomatic_complexity; pub mod derive; @@ -339,6 +340,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { 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_lint_group("clippy_restrictions", vec![ arithmetic::FLOAT_ARITHMETIC, @@ -349,6 +351,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_lint_group("clippy_pedantic", vec![ booleans::NONMINIMAL_BOOL, + const_static_lifetime::CONST_STATIC_LIFETIME, empty_enum::EMPTY_ENUM, enum_glob_use::ENUM_GLOB_USE, enum_variants::PUB_ENUM_VARIANT_NAMES, diff --git a/tests/ui/const_static_lifetime.rs b/tests/ui/const_static_lifetime.rs new file mode 100644 index 00000000000..05b7a3e0117 --- /dev/null +++ b/tests/ui/const_static_lifetime.rs @@ -0,0 +1,33 @@ +#![feature(plugin)] + +#[derive(Debug)] +struct Foo {} + +const VAR_ONE: &'static str = "Test constant #1"; // ERROR Consider removing 'static. + +const VAR_TWO: &str = "Test constant #2"; // This line should not raise a warning. + +const VAR_THREE: &[&'static str] = &["one", "two"]; // ERROR Consider removing 'static + +const VAR_FOUR: (&str, (&str, &'static str), &'static str) = ("on", ("th", "th"), "on"); // ERROR Consider removing 'static + +const VAR_FIVE: &'static [&[&'static str]] = &[&["test"], &["other one"]]; // ERROR Consider removing 'static + +const VAR_SIX: &'static u8 = &5; + +const VAR_SEVEN: &[&(&str, &'static [&'static str])] = &[&("one", &["other one"])]; + +const VAR_HEIGHT: &'static Foo = &Foo {}; + +fn main() { + let false_positive: &'static str = "test"; + println!("{}", VAR_ONE); + println!("{}", VAR_TWO); + println!("{:?}", VAR_THREE); + println!("{:?}", VAR_FOUR); + println!("{:?}", VAR_FIVE); + println!("{:?}", VAR_SIX); + println!("{:?}", VAR_SEVEN); + println!("{:?}", VAR_HEIGHT); + println!("{}", false_positive); +} diff --git a/tests/ui/regex.rs b/tests/ui/regex.rs index 3dd1f64202c..2007f1fad55 100644 --- a/tests/ui/regex.rs +++ b/tests/ui/regex.rs @@ -9,8 +9,8 @@ extern crate regex; use regex::{Regex, RegexSet, RegexBuilder}; use regex::bytes::{Regex as BRegex, RegexSet as BRegexSet, RegexBuilder as BRegexBuilder}; -const OPENING_PAREN : &'static str = "("; -const NOT_A_REAL_REGEX : &'static str = "foobar"; +const OPENING_PAREN: &str = "("; +const NOT_A_REAL_REGEX: &str = "foobar"; fn syntax_error() { let pipe_in_wrong_position = Regex::new("|");