From 5f09020e90b485481fde984cc2e31c0112ebf1f1 Mon Sep 17 00:00:00 2001 From: kennytm Date: Thu, 1 Sep 2016 15:07:37 +0800 Subject: [PATCH] Added a lint_without_lint_pass lint. Four lints were missing from LintPass, making them unavailable unless the `clippy` lint group is explicitly enabled: * `for_loop_over_result` * `for_loop_over_option` * `match_overlapping_arm` * `filter_next` --- clippy_lints/src/lib.rs | 6 ++ clippy_lints/src/loops.rs | 2 + clippy_lints/src/matches.rs | 2 +- clippy_lints/src/methods.rs | 1 + clippy_lints/src/utils/internal_lints.rs | 125 ++++++++++++++++++++++- clippy_lints/src/utils/paths.rs | 2 + tests/compile-fail/lint_pass.rs | 27 +++++ tests/dogfood.rs | 2 +- 8 files changed, 162 insertions(+), 5 deletions(-) create mode 100644 tests/compile-fail/lint_pass.rs diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 12370fb2630..36d466ed9c9 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -172,6 +172,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box serde::Serde); reg.register_early_lint_pass(box utils::internal_lints::Clippy); + reg.register_late_lint_pass(box utils::internal_lints::LintWithoutLintPass::default()); reg.register_late_lint_pass(box types::TypePass); reg.register_late_lint_pass(box booleans::NonminimalBool); reg.register_early_lint_pass(box module_inception::Pass); @@ -297,6 +298,11 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { unicode::UNICODE_NOT_NFC, ]); + reg.register_lint_group("clippy_internal", vec![ + utils::internal_lints::CLIPPY_LINTS_INTERNAL, + utils::internal_lints::LINT_WITHOUT_LINT_PASS, + ]); + reg.register_lint_group("clippy", vec![ approx_const::APPROX_CONSTANT, array_indexing::OUT_OF_BOUNDS_INDEXING, diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 612133a1ddf..8707d3be153 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -276,6 +276,8 @@ impl LintPass for Pass { lint_array!(NEEDLESS_RANGE_LOOP, EXPLICIT_ITER_LOOP, ITER_NEXT_LOOP, + FOR_LOOP_OVER_RESULT, + FOR_LOOP_OVER_OPTION, WHILE_LET_LOOP, UNUSED_COLLECT, REVERSE_RANGE_LOOP, diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index d96854cc38a..55f9e7ec06d 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -125,7 +125,7 @@ pub struct MatchPass; impl LintPass for MatchPass { fn get_lints(&self) -> LintArray { - lint_array!(SINGLE_MATCH, MATCH_REF_PATS, MATCH_BOOL, SINGLE_MATCH_ELSE) + lint_array!(SINGLE_MATCH, MATCH_REF_PATS, MATCH_BOOL, SINGLE_MATCH_ELSE, MATCH_OVERLAPPING_ARM) } } diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index 02366c4a52f..ecb19495c20 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -461,6 +461,7 @@ impl LintPass for Pass { SINGLE_CHAR_PATTERN, SEARCH_IS_SOME, TEMPORARY_CSTRING_AS_PTR, + FILTER_NEXT, FILTER_MAP, ITER_NTH) } diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 443d433e1d3..964774562e2 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -1,7 +1,12 @@ use rustc::lint::*; -use utils::span_lint; +use rustc::hir::*; +use rustc::hir::intravisit::{Visitor, walk_expr}; +use utils::{paths, match_path, span_lint}; use syntax::parse::token::InternedString; -use syntax::ast::*; +use syntax::ast::{Name, NodeId, ItemKind, Crate as AstCrate}; +use syntax::codemap::Span; +use std::collections::{HashSet, HashMap}; + /// **What it does:** Checks for various things we like to keep tidy in clippy. /// @@ -17,6 +22,36 @@ declare_lint! { } +/// **What it does:** Ensures every lint is associated to a `LintPass`. +/// +/// **Why is this bad?** The compiler only knows lints via a `LintPass`. Without +/// putting a lint to a `LintPass::get_lints()`'s return, the compiler will not +/// know the name of the lint. +/// +/// **Known problems:** Only checks for lints associated using the `lint_array!` +/// macro. +/// +/// **Example:** +/// ```rust +/// declare_lint! { pub LINT_1, ... } +/// declare_lint! { pub LINT_2, ... } +/// declare_lint! { pub FORGOTTEN_LINT, ... } +/// // ... +/// pub struct Pass; +/// impl LintPass for Pass { +/// fn get_lints(&self) -> LintArray { +/// lint_array![LINT_1, LINT_2] +/// // missing FORGOTTEN_LINT +/// } +/// } +/// ``` +declare_lint! { + pub LINT_WITHOUT_LINT_PASS, + Warn, + "declaring a lint without associating it in a LintPass" +} + + #[derive(Copy, Clone)] pub struct Clippy; @@ -27,7 +62,7 @@ impl LintPass for Clippy { } impl EarlyLintPass for Clippy { - fn check_crate(&mut self, cx: &EarlyContext, krate: &Crate) { + fn check_crate(&mut self, cx: &EarlyContext, krate: &AstCrate) { if let Some(utils) = krate.module.items.iter().find(|item| item.ident.name.as_str() == "utils") { if let ItemKind::Mod(ref utils_mod) = utils.node { if let Some(paths) = utils_mod.items.iter().find(|item| item.ident.name.as_str() == "paths") { @@ -52,3 +87,87 @@ impl EarlyLintPass for Clippy { } } } + + + +#[derive(Clone, Debug, Default)] +pub struct LintWithoutLintPass { + declared_lints: HashMap, + registered_lints: HashSet, +} + + +impl LintPass for LintWithoutLintPass { + fn get_lints(&self) -> LintArray { + lint_array!(LINT_WITHOUT_LINT_PASS) + } +} + + +impl LateLintPass for LintWithoutLintPass { + fn check_item(&mut self, _: &LateContext, item: &Item) { + if let ItemStatic(ref ty, MutImmutable, ref expr) = item.node { + if is_lint_ref_type(ty) { + self.declared_lints.insert(item.name, item.span); + } else if is_lint_array_type(ty) && item.vis == Visibility::Inherited && item.name.as_str() == "ARRAY" { + let mut collector = LintCollector { output: &mut self.registered_lints }; + collector.visit_expr(expr); + } + } + } + + fn check_crate_post(&mut self, cx: &LateContext, _: &Crate) { + for (lint_name, &lint_span) in &self.declared_lints { + // When using the `declare_lint!` macro, the original `lint_span`'s + // file points to "". + // `compiletest-rs` thinks that's an error in a different file and + // just ignores it. This causes the test in compile-fail/lint_pass + // not able to capture the error. + // Therefore, we need to climb the macro expansion tree and find the + // actual span that invoked `declare_lint!`: + let lint_span = cx.sess().codemap().source_callsite(lint_span); + + if !self.registered_lints.contains(lint_name) { + span_lint(cx, + LINT_WITHOUT_LINT_PASS, + lint_span, + &format!("the lint `{}` is not added to any `LintPass`", lint_name)); + } + } + } +} + + +fn is_lint_ref_type(ty: &Ty) -> bool { + if let TyRptr(Some(_), MutTy { ty: ref inner, mutbl: MutImmutable }) = ty.node { + if let TyPath(None, ref path) = inner.node { + return match_path(path, &paths::LINT); + } + } + false +} + + +fn is_lint_array_type(ty: &Ty) -> bool { + if let TyPath(None, ref path) = ty.node { + match_path(path, &paths::LINT_ARRAY) + } else { + false + } +} + +struct LintCollector<'a> { + output: &'a mut HashSet, +} + +impl<'v, 'a: 'v> Visitor<'v> for LintCollector<'a> { + fn visit_expr(&mut self, expr: &'v Expr) { + walk_expr(self, expr); + } + + fn visit_path(&mut self, path: &'v Path, _: NodeId) { + if path.segments.len() == 1 { + self.output.insert(path.segments[0].name); + } + } +} diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index a652d0f42f8..cd9b7c83eda 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -26,6 +26,8 @@ pub const HASHSET: [&'static str; 5] = ["std", "collections", "hash", "set", "Ha pub const IO_PRINT: [&'static str; 3] = ["std", "io", "_print"]; pub const ITERATOR: [&'static str; 4] = ["core", "iter", "iterator", "Iterator"]; pub const LINKED_LIST: [&'static str; 3] = ["collections", "linked_list", "LinkedList"]; +pub const LINT: [&'static str; 3] = ["rustc", "lint", "Lint"]; +pub const LINT_ARRAY: [&'static str; 3] = ["rustc", "lint", "LintArray"]; pub const MEM_FORGET: [&'static str; 3] = ["core", "mem", "forget"]; pub const MUTEX: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"]; pub const OPEN_OPTIONS: [&'static str; 3] = ["std", "fs", "OpenOptions"]; diff --git a/tests/compile-fail/lint_pass.rs b/tests/compile-fail/lint_pass.rs new file mode 100644 index 00000000000..980ae5cb7a1 --- /dev/null +++ b/tests/compile-fail/lint_pass.rs @@ -0,0 +1,27 @@ +#![feature(plugin)] +#![feature(rustc_private)] +#![plugin(clippy)] + +#![deny(lint_without_lint_pass)] + +#[macro_use] extern crate rustc; + +use rustc::lint::{LintPass, LintArray}; + +declare_lint! { GOOD_LINT, Warn, "good lint" } +declare_lint! { MISSING_LINT, Warn, "missing lint" } +//~^ ERROR: the lint `MISSING_LINT` is not added to any `LintPass` + +pub struct Pass; + +impl LintPass for Pass { + fn get_lints(&self) -> LintArray { + lint_array![GOOD_LINT] + } +} + +fn main() { + let _ = MISSING_LINT; +} + + diff --git a/tests/dogfood.rs b/tests/dogfood.rs index 3340e4986cd..540040a5ed5 100644 --- a/tests/dogfood.rs +++ b/tests/dogfood.rs @@ -24,7 +24,7 @@ fn dogfood() { let mut s = String::new(); s.push_str(" -L target/debug/"); s.push_str(" -L target/debug/deps"); - s.push_str(" -Zextra-plugins=clippy -Ltarget_recur/debug -Dclippy_pedantic -Dclippy -Dclippy_lints_internal"); + s.push_str(" -Zextra-plugins=clippy -Ltarget_recur/debug -Dclippy_pedantic -Dclippy -Dclippy_internal"); config.target_rustcflags = Some(s); if let Ok(name) = var("TESTNAME") { config.filter = Some(name.to_owned())