Auto merge of #4650 - Mark-Simulacrum:clippy-up-lintstore-lockless, r=phansch

Update clippy for latest rustc changes

Specifically, this revises the clippy integration to utilize a new
callback to register its lints, as the prior editing of lint store in
Session is no longer possible.

---

changelog: none

CONTRIBUTING.md

@@ -147,7 +147,7 @@ You can use [rustup-toolchain-install-master][rtim] to do that:
 
 ```bash
 cargo install rustup-toolchain-install-master
-rustup-toolchain-install-master --force -n master
+rustup-toolchain-install-master --force -n master -c rustc-dev
 rustup override set master
 cargo test
 ```

appveyor.yml

@@ -25,7 +25,7 @@ install:
     - set PATH=%PATH%;C:\Users\appveyor\.cargo\bin
     - del rust-toolchain
     - cargo install -Z install-upgrade rustup-toolchain-install-master
-    - rustup-toolchain-install-master -f -n master
+    - rustup-toolchain-install-master -f -n master -c rustc-dev
     - rustup component add rustfmt --toolchain nightly & exit 0 # Format test handles missing rustfmt
     - rustup default master
     - set PATH=%PATH%;C:\Users\appveyor\.rustup\toolchains\master\bin

clippy_dev/src/lib.rs

@@ -82,7 +82,7 @@ pub fn gen_lint_group_list(lints: Vec<Lint>) -> Vec<String> {
             if l.is_internal() || l.deprecation.is_some() {
                 None
             } else {
-                Some(format!("        {}::{},", l.module, l.name.to_uppercase()))
+                Some(format!("        LintId::of(&{}::{}),", l.module, l.name.to_uppercase()))
             }
         })
         .sorted()
@@ -143,6 +143,26 @@ pub fn gen_deprecated(lints: &[Lint]) -> Vec<String> {
         .collect::<Vec<String>>()
 }
 
+#[must_use]
+pub fn gen_register_lint_list(lints: &[Lint]) -> Vec<String> {
+    let pre = "    store.register_lints(&[".to_string();
+    let post = "    ]);".to_string();
+    let mut inner = lints
+        .iter()
+        .filter_map(|l| {
+            if !l.is_internal() && l.deprecation.is_none() {
+                Some(format!("        &{}::{},", l.module, l.name.to_uppercase()))
+            } else {
+                None
+            }
+        })
+        .sorted()
+        .collect::<Vec<String>>();
+    inner.insert(0, pre);
+    inner.push(post);
+    inner
+}
+
 /// Gathers all files in `src/clippy_lints` and gathers all lints inside
 pub fn gather_all() -> impl Iterator<Item = Lint> {
     lint_files().flat_map(|f| gather_from_file(&f))
@@ -487,8 +507,8 @@ fn test_gen_lint_group_list() {
         Lint::new("incorrect_internal", "internal_style", "abc", None, "module_name"),
     ];
     let expected = vec![
-        "        module_name::ABC,".to_string(),
-        "        module_name::SHOULD_ASSERT_EQ,".to_string(),
+        "        LintId::of(&module_name::ABC),".to_string(),
+        "        LintId::of(&module_name::SHOULD_ASSERT_EQ),".to_string(),
     ];
     assert_eq!(expected, gen_lint_group_list(lints));
 }

clippy_dev/src/main.rs

@@ -109,6 +109,7 @@ fn print_lints() {
     println!("there are {} lints", lint_count);
 }
 
+#[allow(clippy::too_many_lines)]
 fn update_lints(update_mode: &UpdateMode) {
     let lint_list: Vec<Lint> = gather_all().collect();
 
@@ -170,6 +171,16 @@ fn update_lints(update_mode: &UpdateMode) {
     )
     .changed;
 
+    file_change |= replace_region_in_file(
+        "../clippy_lints/src/lib.rs",
+        "begin register lints",
+        "end register lints",
+        false,
+        update_mode == &UpdateMode::Change,
+        || gen_register_lint_list(&lint_list),
+    )
+    .changed;
+
     file_change |= replace_region_in_file(
         "../clippy_lints/src/lib.rs",
         "begin lints modules",
@@ -183,7 +194,7 @@ fn update_lints(update_mode: &UpdateMode) {
     // Generate lists of lints in the clippy::all lint group
     file_change |= replace_region_in_file(
         "../clippy_lints/src/lib.rs",
-        r#"reg.register_lint_group\("clippy::all""#,
+        r#"store.register_group\(true, "clippy::all""#,
         r#"\]\);"#,
         false,
         update_mode == &UpdateMode::Change,
@@ -206,7 +217,7 @@ fn update_lints(update_mode: &UpdateMode) {
     for (lint_group, lints) in Lint::by_lint_group(&usable_lints) {
         file_change |= replace_region_in_file(
             "../clippy_lints/src/lib.rs",
-            &format!("reg.register_lint_group\\(\"clippy::{}\"", lint_group),
+            &format!("store.register_group\\(true, \"clippy::{}\"", lint_group),
             r#"\]\);"#,
             false,
             update_mode == &UpdateMode::Change,

clippy_lints/src/trivially_copy_pass_by_ref.rs

@@ -54,6 +54,7 @@ declare_clippy_lint! {
     "functions taking small copyable arguments by reference"
 }
 
+#[derive(Copy, Clone)]
 pub struct TriviallyCopyPassByRef {
     limit: u64,
 }

clippy_lints/src/utils/higher.rs

@@ -64,13 +64,13 @@ pub fn range<'a, 'b, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'b hir::Expr) -> O
     if def_path.data.len() != 3 {
         return None;
     }
-    if def_path.data.get(0)?.data.as_interned_str().as_symbol() != sym!(ops) {
+    if def_path.data.get(0)?.data.as_symbol() != sym!(ops) {
         return None;
     }
-    if def_path.data.get(1)?.data.as_interned_str().as_symbol() != sym!(range) {
+    if def_path.data.get(1)?.data.as_symbol() != sym!(range) {
         return None;
     }
-    let type_name = def_path.data.get(2)?.data.as_interned_str();
+    let type_name = def_path.data.get(2)?.data.as_symbol();
     let range_types = [
         "RangeFrom",
         "RangeFull",

clippy_lints/src/utils/internal_lints.rs

@@ -1,5 +1,6 @@
 use crate::utils::{
-    match_def_path, match_type, method_calls, paths, span_help_and_lint, span_lint, span_lint_and_sugg, walk_ptrs_ty,
+    is_expn_of, match_def_path, match_type, method_calls, paths, span_help_and_lint, span_lint, span_lint_and_sugg,
+    walk_ptrs_ty,
 };
 use if_chain::if_chain;
 use rustc::hir;
@@ -148,25 +149,23 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LintWithoutLintPass {
             if is_lint_ref_type(cx, ty) {
                 self.declared_lints.insert(item.ident.name, item.span);
             }
-        } else if let hir::ItemKind::Impl(.., Some(ref trait_ref), _, ref impl_item_refs) = item.kind {
-            if_chain! {
-                if let hir::TraitRef{path, ..} = trait_ref;
-                if let Res::Def(DefKind::Trait, def_id) = path.res;
-                if match_def_path(cx, def_id, &paths::LINT_PASS);
-                then {
-                    let mut collector = LintCollector {
-                        output: &mut self.registered_lints,
-                        cx,
-                    };
-                    let body_id = cx.tcx.hir().body_owned_by(
-                        impl_item_refs
-                            .iter()
-                            .find(|iiref| iiref.ident.as_str() == "get_lints")
-                            .expect("LintPass needs to implement get_lints")
-                            .id.hir_id
-                    );
-                    collector.visit_expr(&cx.tcx.hir().body(body_id).value);
-                }
+        } else if is_expn_of(item.span, "impl_lint_pass").is_some()
+            || is_expn_of(item.span, "declare_lint_pass").is_some()
+        {
+            if let hir::ItemKind::Impl(.., None, _, ref impl_item_refs) = item.kind {
+                let mut collector = LintCollector {
+                    output: &mut self.registered_lints,
+                    cx,
+                };
+                let body_id = cx.tcx.hir().body_owned_by(
+                    impl_item_refs
+                        .iter()
+                        .find(|iiref| iiref.ident.as_str() == "get_lints")
+                        .expect("LintPass needs to implement get_lints")
+                        .id
+                        .hir_id,
+                );
+                collector.visit_expr(&cx.tcx.hir().body(body_id).value);
             }
         }
     }
@@ -275,9 +274,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CompilerLintFunctions {
     }
 }
 
-pub struct OuterExpnDataPass;
-
-impl_lint_pass!(OuterExpnDataPass => [OUTER_EXPN_EXPN_DATA]);
+declare_lint_pass!(OuterExpnDataPass => [OUTER_EXPN_EXPN_DATA]);
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OuterExpnDataPass {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) {

clippy_lints/src/utils/paths.rs

@@ -46,7 +46,6 @@ pub const ITERATOR: [&str; 5] = ["core", "iter", "traits", "iterator", "Iterator
 pub const LATE_CONTEXT: [&str; 4] = ["rustc", "lint", "context", "LateContext"];
 pub const LINKED_LIST: [&str; 4] = ["alloc", "collections", "linked_list", "LinkedList"];
 pub const LINT: [&str; 3] = ["rustc", "lint", "Lint"];
-pub const LINT_PASS: [&str; 3] = ["rustc", "lint", "LintPass"];
 pub const MEM_DISCRIMINANT: [&str; 3] = ["core", "mem", "discriminant"];
 pub const MEM_FORGET: [&str; 3] = ["core", "mem", "forget"];
 pub const MEM_MAYBEUNINIT: [&str; 4] = ["core", "mem", "maybe_uninit", "MaybeUninit"];

setup-toolchain.sh

@@ -9,5 +9,5 @@ if [[ "$CI" == true ]] || ! command -v rustup-toolchain-install-master > /dev/nu
   cargo install -Z install-upgrade rustup-toolchain-install-master --bin rustup-toolchain-install-master
 fi
 
-rustup-toolchain-install-master -f -n master
+rustup-toolchain-install-master -f -n master -c rustc-dev
 rustup override set master

src/driver.rs

@@ -61,51 +61,20 @@ fn test_arg_value() {
 struct ClippyCallbacks;
 
 impl rustc_driver::Callbacks for ClippyCallbacks {
-    fn after_parsing(&mut self, compiler: &interface::Compiler) -> rustc_driver::Compilation {
-        let sess = compiler.session();
-        let mut registry = rustc_driver::plugin::registry::Registry::new(
-            sess,
-            compiler
-                .parse()
-                .expect(
-                    "at this compilation stage \
-                     the crate must be parsed",
-                )
-                .peek()
-                .span,
-        );
-        registry.args_hidden = Some(Vec::new());
+    fn config(&mut self, config: &mut interface::Config) {
+        let previous = config.register_lints.take();
+        config.register_lints = Some(Box::new(move |sess, mut lint_store| {
+            // technically we're ~guaranteed that this is none but might as well call anything that
+            // is there already. Certainly it can't hurt.
+            if let Some(previous) = &previous {
+                (previous)(sess, lint_store);
+            }
 
-        let conf = clippy_lints::read_conf(&registry);
-        clippy_lints::register_plugins(&mut registry, &conf);
-
-        let rustc_driver::plugin::registry::Registry {
-            early_lint_passes,
-            late_lint_passes,
-            lint_groups,
-            llvm_passes,
-            attributes,
-            ..
-        } = registry;
-        let mut ls = sess.lint_store.borrow_mut();
-        for pass in early_lint_passes {
-            ls.register_early_pass(Some(sess), true, false, pass);
-        }
-        for pass in late_lint_passes {
-            ls.register_late_pass(Some(sess), true, false, false, pass);
-        }
-
-        for (name, (to, deprecated_name)) in lint_groups {
-            ls.register_group(Some(sess), true, name, deprecated_name, to);
-        }
-        clippy_lints::register_pre_expansion_lints(sess, &mut ls, &conf);
-        clippy_lints::register_renamed(&mut ls);
-
-        sess.plugin_llvm_passes.borrow_mut().extend(llvm_passes);
-        sess.plugin_attributes.borrow_mut().extend(attributes);
-
-        // Continue execution
-        rustc_driver::Compilation::Continue
+            let conf = clippy_lints::read_conf(&[], &sess);
+            clippy_lints::register_plugins(&mut lint_store, &sess, &conf);
+            clippy_lints::register_pre_expansion_lints(&mut lint_store, &conf);
+            clippy_lints::register_renamed(&mut lint_store);
+        }));
     }
 }
 

src/lib.rs

@@ -11,22 +11,20 @@ use self::rustc_driver::plugin::Registry;
 
 #[plugin_registrar]
 pub fn plugin_registrar(reg: &mut Registry<'_>) {
-    reg.sess.lint_store.with_read_lock(|lint_store| {
-        for (lint, _, _) in lint_store.get_lint_groups() {
-            reg.sess
-                .struct_warn(
-                    "the clippy plugin is being deprecated, please use cargo clippy or rls with the clippy feature",
-                )
-                .emit();
-            if lint == "clippy" {
-                // cargo clippy run on a crate that also uses the plugin
-                return;
-            }
+    for (lint, _, _) in reg.lint_store.get_lint_groups() {
+        reg.sess
+            .struct_warn(
+                "the clippy plugin is being deprecated, please use cargo clippy or rls with the clippy feature",
+            )
+            .emit();
+        if lint == "clippy" {
+            // cargo clippy run on a crate that also uses the plugin
+            return;
         }
-    });
+    }
 
-    let conf = clippy_lints::read_conf(reg);
-    clippy_lints::register_plugins(reg, &conf);
+    let conf = clippy_lints::read_conf(reg.args(), &reg.sess);
+    clippy_lints::register_plugins(&mut reg.lint_store, &reg.sess, &conf);
 }
 
 // only exists to let the dogfood integration test works.

tests/ui/lint_without_lint_pass.rs

@@ -20,12 +20,14 @@ declare_clippy_lint! {
     ""
 }
 
+declare_clippy_lint! {
+    pub TEST_LINT_REGISTERED_ONLY_IMPL,
+    correctness,
+    ""
+}
+
 pub struct Pass;
 impl LintPass for Pass {
-    fn get_lints(&self) -> LintArray {
-        lint_array!(TEST_LINT_REGISTERED)
-    }
-
     fn name(&self) -> &'static str {
         "TEST_LINT"
     }
@@ -34,6 +36,6 @@ impl LintPass for Pass {
 declare_lint_pass!(Pass2 => [TEST_LINT_REGISTERED]);
 
 pub struct Pass3;
-impl_lint_pass!(Pass3 => [TEST_LINT_REGISTERED]);
+impl_lint_pass!(Pass3 => [TEST_LINT_REGISTERED_ONLY_IMPL]);
 
 fn main() {}