Backwards compatibility for tool/clippy lints

src/librustc/lint/context.rs

@@ -67,9 +67,10 @@ pub struct LintStore {
     /// Lints indexed by name.
     by_name: FxHashMap<String, TargetLint>,
 
-    /// 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<LintId>, 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<LintId>, 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::<Vec<_>>();
             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<LintId>) {
-        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<LintId>,
+    ) {
+        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,34 +353,79 @@ 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.
                 _ => {}
             }
         }
         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.

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 => {}

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;

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)),*]);
             )
     }
 

src/librustc_plugin/registry.rs

@@ -53,7 +53,7 @@ pub struct Registry<'a> {
     pub late_lint_passes: Vec<LateLintPassObject>,
 
     #[doc(hidden)]
-    pub lint_groups: FxHashMap<&'static str, Vec<LintId>>,
+    pub lint_groups: FxHashMap<&'static str, (Vec<LintId>, Option<&'static str>)>,
 
     #[doc(hidden)]
     pub llvm_passes: Vec<String>,
@@ -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.

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]);
 }

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]);
 }

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]);
 }

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() { }
 }

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