diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index 5cc862a58d35a..d5247b4ce9de8 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -67,9 +67,10 @@ pub struct LintStore { /// Lints indexed by name. by_name: FxHashMap, - /// Map of registered lint groups to what lints they expand to. The bool - /// is true if the lint group was added by a plugin. - lint_groups: FxHashMap<&'static str, (Vec, bool)>, + /// Map of registered lint groups to what lints they expand to. The first + /// bool is true if the lint group was added by a plugin. The optional string + /// is used to store the new names of deprecated lint group names. + lint_groups: FxHashMap<&'static str, (Vec, bool, Option<&'static str>)>, /// Extra info for future incompatibility lints, describing the /// issue or RFC that caused the incompatibility. @@ -138,7 +139,7 @@ pub enum CheckLintNameResult<'a> { /// compiled with the tool and therefore the lint was never /// added to the `LintStore`. Otherwise the `LintId` will be /// returned as if it where a rustc lint. - Tool(Option<&'a [LintId]>), + Tool(Result<&'a [LintId], (Option<&'a [LintId]>, String)>), } impl LintStore { @@ -221,7 +222,7 @@ impl LintStore { let lints = lints.iter().filter(|f| f.edition == Some(*edition)).map(|f| f.id) .collect::>(); if !lints.is_empty() { - self.register_group(sess, false, edition.lint_name(), lints) + self.register_group(sess, false, edition.lint_name(), None, lints) } } @@ -231,19 +232,35 @@ impl LintStore { self.future_incompatible.insert(lint.id, lint); } - self.register_group(sess, false, "future_incompatible", future_incompatible); - - + self.register_group( + sess, + false, + "future_incompatible", + None, + future_incompatible, + ); } pub fn future_incompatible(&self, id: LintId) -> Option<&FutureIncompatibleInfo> { self.future_incompatible.get(&id) } - pub fn register_group(&mut self, sess: Option<&Session>, - from_plugin: bool, name: &'static str, - to: Vec) { - let new = self.lint_groups.insert(name, (to, from_plugin)).is_none(); + pub fn register_group( + &mut self, + sess: Option<&Session>, + from_plugin: bool, + name: &'static str, + deprecated_name: Option<&'static str>, + to: Vec, + ) { + let new = self + .lint_groups + .insert(name, (to, from_plugin, None)) + .is_none(); + if let Some(deprecated) = deprecated_name { + self.lint_groups + .insert(deprecated, (vec![], from_plugin, Some(name))); + } if !new { let msg = format!("duplicate specification of lint group {}", name); @@ -336,13 +353,14 @@ impl LintStore { } else { lint_name.to_string() }; + // If the lint was scoped with `tool::` check if the tool lint exists if let Some(_) = tool_name { match self.by_name.get(&complete_name) { None => match self.lint_groups.get(&*complete_name) { - None => return CheckLintNameResult::Tool(None), - Some(ids) => return CheckLintNameResult::Tool(Some(&ids.0)), + None => return CheckLintNameResult::Tool(Err((None, String::new()))), + Some(ids) => return CheckLintNameResult::Tool(Ok(&ids.0)), }, - Some(&Id(ref id)) => return CheckLintNameResult::Tool(Some(slice::from_ref(id))), + Some(&Id(ref id)) => return CheckLintNameResult::Tool(Ok(slice::from_ref(id))), // If the lint was registered as removed or renamed by the lint tool, we don't need // to treat tool_lints and rustc lints different and can use the code below. _ => {} @@ -350,20 +368,64 @@ impl LintStore { } match self.by_name.get(&complete_name) { Some(&Renamed(ref new_name, _)) => CheckLintNameResult::Warning( - format!("lint `{}` has been renamed to `{}`", lint_name, new_name), + format!( + "lint `{}` has been renamed to `{}`", + complete_name, new_name + ), Some(new_name.to_owned()), ), Some(&Removed(ref reason)) => CheckLintNameResult::Warning( - format!("lint `{}` has been removed: `{}`", lint_name, reason), + format!("lint `{}` has been removed: `{}`", complete_name, reason), None, ), None => match self.lint_groups.get(&*complete_name) { - None => CheckLintNameResult::NoLint, - Some(ids) => CheckLintNameResult::Ok(&ids.0), + // If neither the lint, nor the lint group exists check if there is a `clippy::` + // variant of this lint + None => self.check_tool_name_for_backwards_compat(&complete_name, "clippy"), + Some(ids) => { + // Check if the lint group name is deprecated + if let Some(new_name) = ids.2 { + let lint_ids = self.lint_groups.get(new_name).unwrap(); + return CheckLintNameResult::Tool(Err(( + Some(&lint_ids.0), + new_name.to_string(), + ))); + } + CheckLintNameResult::Ok(&ids.0) + } }, Some(&Id(ref id)) => CheckLintNameResult::Ok(slice::from_ref(id)), } } + + fn check_tool_name_for_backwards_compat( + &self, + lint_name: &str, + tool_name: &str, + ) -> CheckLintNameResult { + let complete_name = format!("{}::{}", tool_name, lint_name); + match self.by_name.get(&complete_name) { + None => match self.lint_groups.get(&*complete_name) { + // Now we are sure, that this lint exists nowhere + None => CheckLintNameResult::NoLint, + Some(ids) => { + // Reaching this would be weird, but lets cover this case anyway + if let Some(new_name) = ids.2 { + let lint_ids = self.lint_groups.get(new_name).unwrap(); + return CheckLintNameResult::Tool(Err(( + Some(&lint_ids.0), + new_name.to_string(), + ))); + } + CheckLintNameResult::Tool(Err((Some(&ids.0), complete_name))) + } + }, + Some(&Id(ref id)) => { + CheckLintNameResult::Tool(Err((Some(slice::from_ref(id)), complete_name))) + } + _ => CheckLintNameResult::NoLint, + } + } } /// Context for lint checking after type checking. diff --git a/src/librustc/lint/levels.rs b/src/librustc/lint/levels.rs index 5b9ddabf21c1b..336ebe79d33ab 100644 --- a/src/librustc/lint/levels.rs +++ b/src/librustc/lint/levels.rs @@ -231,12 +231,13 @@ impl<'a> LintLevelsBuilder<'a> { let gate_feature = !self.sess.features_untracked().tool_lints; let known_tool = attr::is_known_lint_tool(lint_tool); if gate_feature { - feature_gate::emit_feature_err(&sess.parse_sess, - "tool_lints", - word.span, - feature_gate::GateIssue::Language, - &format!("scoped lint `{}` is experimental", - word.ident)); + feature_gate::emit_feature_err( + &sess.parse_sess, + "tool_lints", + word.span, + feature_gate::GateIssue::Language, + &format!("scoped lint `{}` is experimental", word.ident), + ); } if !known_tool { span_err!( @@ -249,7 +250,7 @@ impl<'a> LintLevelsBuilder<'a> { } if gate_feature || !known_tool { - continue + continue; } Some(lint_tool.as_str()) @@ -266,17 +267,52 @@ impl<'a> LintLevelsBuilder<'a> { } CheckLintNameResult::Tool(result) => { - if let Some(ids) = result { - let complete_name = &format!("{}::{}", tool_name.unwrap(), name); - let src = LintSource::Node(Symbol::intern(complete_name), li.span); - for id in ids { - specs.insert(*id, (level, src)); + match result { + Ok(ids) => { + let complete_name = &format!("{}::{}", tool_name.unwrap(), name); + let src = LintSource::Node(Symbol::intern(complete_name), li.span); + for id in ids { + specs.insert(*id, (level, src)); + } + } + Err((Some(ids), new_lint_name)) => { + let lint = builtin::RENAMED_AND_REMOVED_LINTS; + let (lvl, src) = + self.sets + .get_lint_level(lint, self.cur, Some(&specs), &sess); + let msg = format!( + "lint name `{}` is deprecated \ + and may not have an effect in the future. \ + Also `cfg_attr(cargo-clippy)` won't be necessary anymore", + name + ); + let mut err = lint::struct_lint_level( + self.sess, + lint, + lvl, + src, + Some(li.span.into()), + &msg, + ); + err.span_suggestion_with_applicability( + li.span, + "change it to", + new_lint_name.to_string(), + Applicability::MachineApplicable, + ).emit(); + + let src = LintSource::Node(Symbol::intern(&new_lint_name), li.span); + for id in ids { + specs.insert(*id, (level, src)); + } + } + Err((None, _)) => { + // If Tool(Err(None, _)) is returned, then either the lint does not + // exist in the tool or the code was not compiled with the tool and + // therefore the lint was never added to the `LintStore`. To detect + // this is the responsibility of the lint tool. } } - // If Tool(None) is returned, then either the lint does not exist in the - // tool or the code was not compiled with the tool and therefore the lint - // was never added to the `LintStore`. To detect this is the responsibility - // of the lint tool. } _ if !self.warn_about_weird_lints => {} diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 1c2c0ad73a897..c6344cb921044 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -924,8 +924,8 @@ where ls.register_late_pass(Some(sess), true, pass); } - for (name, to) in lint_groups { - ls.register_group(Some(sess), true, name, to); + for (name, (to, deprecated_name)) in lint_groups { + ls.register_group(Some(sess), true, name, deprecated_name, to); } *sess.plugin_llvm_passes.borrow_mut() = llvm_passes; diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 47e9aef6b0030..46c5b0092a271 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -105,7 +105,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { macro_rules! add_lint_group { ($sess:ident, $name:expr, $($lint:ident),*) => ( - store.register_group($sess, false, $name, vec![$(LintId::of($lint)),*]); + store.register_group($sess, false, $name, None, vec![$(LintId::of($lint)),*]); ) } diff --git a/src/librustc_plugin/registry.rs b/src/librustc_plugin/registry.rs index 5ef05cb1d6b6e..6c10ac7ea5cea 100644 --- a/src/librustc_plugin/registry.rs +++ b/src/librustc_plugin/registry.rs @@ -53,7 +53,7 @@ pub struct Registry<'a> { pub late_lint_passes: Vec, #[doc(hidden)] - pub lint_groups: FxHashMap<&'static str, Vec>, + pub lint_groups: FxHashMap<&'static str, (Vec, Option<&'static str>)>, #[doc(hidden)] pub llvm_passes: Vec, @@ -170,8 +170,15 @@ impl<'a> Registry<'a> { self.late_lint_passes.push(lint_pass); } /// Register a lint group. - pub fn register_lint_group(&mut self, name: &'static str, to: Vec<&'static Lint>) { - self.lint_groups.insert(name, to.into_iter().map(|x| LintId::of(x)).collect()); + pub fn register_lint_group( + &mut self, + name: &'static str, + deprecated_name: Option<&'static str>, + to: Vec<&'static Lint> + ) { + self.lint_groups.insert(name, + (to.into_iter().map(|x| LintId::of(x)).collect(), + deprecated_name)); } /// Register an LLVM pass. diff --git a/src/test/compile-fail-fulldeps/auxiliary/lint_group_plugin_test.rs b/src/test/compile-fail-fulldeps/auxiliary/lint_group_plugin_test.rs index 8ccb5878c4045..082f15a39dd95 100644 --- a/src/test/compile-fail-fulldeps/auxiliary/lint_group_plugin_test.rs +++ b/src/test/compile-fail-fulldeps/auxiliary/lint_group_plugin_test.rs @@ -49,5 +49,5 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { #[plugin_registrar] pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box Pass); - reg.register_lint_group("lint_me", vec![TEST_LINT, PLEASE_LINT]); + reg.register_lint_group("lint_me", None, vec![TEST_LINT, PLEASE_LINT]); } diff --git a/src/test/ui-fulldeps/auxiliary/lint_group_plugin_test.rs b/src/test/ui-fulldeps/auxiliary/lint_group_plugin_test.rs index 8ccb5878c4045..082f15a39dd95 100644 --- a/src/test/ui-fulldeps/auxiliary/lint_group_plugin_test.rs +++ b/src/test/ui-fulldeps/auxiliary/lint_group_plugin_test.rs @@ -49,5 +49,5 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { #[plugin_registrar] pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box Pass); - reg.register_lint_group("lint_me", vec![TEST_LINT, PLEASE_LINT]); + reg.register_lint_group("lint_me", None, vec![TEST_LINT, PLEASE_LINT]); } diff --git a/src/test/ui-fulldeps/auxiliary/lint_tool_test.rs b/src/test/ui-fulldeps/auxiliary/lint_tool_test.rs index d7895bd8781ba..e184c0919d0ec 100644 --- a/src/test/ui-fulldeps/auxiliary/lint_tool_test.rs +++ b/src/test/ui-fulldeps/auxiliary/lint_tool_test.rs @@ -25,12 +25,13 @@ use rustc::lint::{EarlyContext, LintContext, LintPass, EarlyLintPass, use rustc_plugin::Registry; use syntax::ast; declare_tool_lint!(pub clippy::TEST_LINT, Warn, "Warn about stuff"); +declare_tool_lint!(pub clippy::TEST_GROUP, Warn, "Warn about other stuff"); struct Pass; impl LintPass for Pass { fn get_lints(&self) -> LintArray { - lint_array!(TEST_LINT) + lint_array!(TEST_LINT, TEST_GROUP) } } @@ -39,10 +40,14 @@ impl EarlyLintPass for Pass { if it.ident.name == "lintme" { cx.span_lint(TEST_LINT, it.span, "item is named 'lintme'"); } + if it.ident.name == "lintmetoo" { + cx.span_lint(TEST_GROUP, it.span, "item is named 'lintmetoo'"); + } } } #[plugin_registrar] pub fn plugin_registrar(reg: &mut Registry) { reg.register_early_lint_pass(box Pass); + reg.register_lint_group("clippy::group", Some("clippy_group"), vec![TEST_LINT, TEST_GROUP]); } diff --git a/src/test/ui-fulldeps/lint_tool_test.rs b/src/test/ui-fulldeps/lint_tool_test.rs index ccdcd2df31b4f..ebe10b3714f20 100644 --- a/src/test/ui-fulldeps/lint_tool_test.rs +++ b/src/test/ui-fulldeps/lint_tool_test.rs @@ -8,17 +8,33 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// run-pass // aux-build:lint_tool_test.rs // ignore-stage1 +// compile-flags: --cfg foo #![feature(plugin)] #![feature(tool_lints)] #![plugin(lint_tool_test)] #![allow(dead_code)] +#![cfg_attr(foo, warn(test_lint))] +//~^ WARNING lint name `test_lint` is deprecated and may not have an effect in the future +//~^^ WARNING lint name `test_lint` is deprecated and may not have an effect in the future +#![deny(clippy_group)] +//~^ WARNING lint name `clippy_group` is deprecated and may not have an effect in the future -fn lintme() { } //~ WARNING item is named 'lintme' +fn lintme() { } //~ ERROR item is named 'lintme' + +#[allow(clippy::group)] +fn lintmetoo() {} #[allow(clippy::test_lint)] pub fn main() { fn lintme() { } + fn lintmetoo() { } //~ ERROR item is named 'lintmetoo' +} + +#[allow(test_group)] +//~^ WARNING lint name `test_group` is deprecated and may not have an effect in the future +#[deny(this_lint_does_not_exist)] //~ WARNING unknown lint: `this_lint_does_not_exist` +fn hello() { + fn lintmetoo() { } } diff --git a/src/test/ui-fulldeps/lint_tool_test.stderr b/src/test/ui-fulldeps/lint_tool_test.stderr index 22d0f458e7d7b..ab0c317e1cd0f 100644 --- a/src/test/ui-fulldeps/lint_tool_test.stderr +++ b/src/test/ui-fulldeps/lint_tool_test.stderr @@ -1,8 +1,62 @@ -warning: item is named 'lintme' - --> $DIR/lint_tool_test.rs:19:1 +warning: lint name `test_lint` is deprecated and may not have an effect in the future. Also `cfg_attr(cargo-clippy)` won't be necessary anymore + --> $DIR/lint_tool_test.rs:18:23 | -LL | fn lintme() { } //~ WARNING item is named 'lintme' +LL | #![cfg_attr(foo, warn(test_lint))] + | ^^^^^^^^^ help: change it to: `clippy::test_lint` + | + = note: #[warn(renamed_and_removed_lints)] on by default + +warning: lint name `clippy_group` is deprecated and may not have an effect in the future. Also `cfg_attr(cargo-clippy)` won't be necessary anymore + --> $DIR/lint_tool_test.rs:21:9 + | +LL | #![deny(clippy_group)] + | ^^^^^^^^^^^^ help: change it to: `clippy::group` + +warning: lint name `test_group` is deprecated and may not have an effect in the future. Also `cfg_attr(cargo-clippy)` won't be necessary anymore + --> $DIR/lint_tool_test.rs:35:9 + | +LL | #[allow(test_group)] + | ^^^^^^^^^^ help: change it to: `clippy::test_group` + +warning: unknown lint: `this_lint_does_not_exist` + --> $DIR/lint_tool_test.rs:37:8 + | +LL | #[deny(this_lint_does_not_exist)] //~ WARNING unknown lint: `this_lint_does_not_exist` + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: #[warn(unknown_lints)] on by default + +warning: lint name `test_lint` is deprecated and may not have an effect in the future. Also `cfg_attr(cargo-clippy)` won't be necessary anymore + --> $DIR/lint_tool_test.rs:18:23 + | +LL | #![cfg_attr(foo, warn(test_lint))] + | ^^^^^^^^^ help: change it to: `clippy::test_lint` + +error: item is named 'lintme' + --> $DIR/lint_tool_test.rs:24:1 + | +LL | fn lintme() { } //~ ERROR item is named 'lintme' | ^^^^^^^^^^^^^^^ | - = note: #[warn(clippy::test_lint)] on by default +note: lint level defined here + --> $DIR/lint_tool_test.rs:21:9 + | +LL | #![deny(clippy_group)] + | ^^^^^^^^^^^^ + = note: #[deny(clippy::test_lint)] implied by #[deny(clippy::group)] + +error: item is named 'lintmetoo' + --> $DIR/lint_tool_test.rs:32:5 + | +LL | fn lintmetoo() { } //~ ERROR item is named 'lintmetoo' + | ^^^^^^^^^^^^^^^^^^ + | +note: lint level defined here + --> $DIR/lint_tool_test.rs:21:9 + | +LL | #![deny(clippy_group)] + | ^^^^^^^^^^^^ + = note: #[deny(clippy::test_group)] implied by #[deny(clippy::group)] + +error: aborting due to 2 previous errors diff --git a/src/tools/clippy b/src/tools/clippy index d99cea0f16633..9abf6fca9c728 160000 --- a/src/tools/clippy +++ b/src/tools/clippy @@ -1 +1 @@ -Subproject commit d99cea0f16633556871a59500c610782b07233b9 +Subproject commit 9abf6fca9c7288cb3bb99c0f7627f94b7930ee98