Auto merge of rust-lang#9243 - Jarcho:std_core, r=Manishearth

Don't lint `std_instead_of_core` on `std::env`

fixes rust-lang#9239

This also reorders the execution of the lint to do as little as possible in the case where the path doesn't start with `std` or `alloc`.

changelog: [`std_instead_of_core`](https://rust-lang.github.io/rust-clippy/master/#std_instead_of_core): Don't lint on `use std::env`
changelog: [`std_instead_of_alloc`](https://rust-lang.github.io/rust-clippy/master/#std_instead_of_alloc): Don't lint `use std::vec` twice

clippy_lints/src/lib.rs

@@ -918,7 +918,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     let verbose_bit_mask_threshold = conf.verbose_bit_mask_threshold;
     store.register_late_pass(move || Box::new(operators::Operators::new(verbose_bit_mask_threshold)));
     store.register_late_pass(|| Box::new(invalid_utf8_in_unchecked::InvalidUtf8InUnchecked));
-    store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports));
+    store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports::default()));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/std_instead_of_core.rs

@@ -1,8 +1,8 @@
 use clippy_utils::diagnostics::span_lint_and_help;
 use rustc_hir::{def::Res, HirId, Path, PathSegment};
-use rustc_lint::{LateContext, LateLintPass, Lint};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::{sym, symbol::kw, Symbol};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::{sym, symbol::kw, Span};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -81,39 +81,55 @@ declare_clippy_lint! {
     "type is imported from alloc when available in core"
 }
 
-declare_lint_pass!(StdReexports => [STD_INSTEAD_OF_CORE, STD_INSTEAD_OF_ALLOC, ALLOC_INSTEAD_OF_CORE]);
+#[derive(Default)]
+pub struct StdReexports {
+    // Paths which can be either a module or a macro (e.g. `std::env`) will cause this check to happen
+    // twice. First for the mod, second for the macro. This is used to avoid the lint reporting for the macro
+    // when the path could be also be used to access the module.
+    prev_span: Span,
+}
+impl_lint_pass!(StdReexports => [STD_INSTEAD_OF_CORE, STD_INSTEAD_OF_ALLOC, ALLOC_INSTEAD_OF_CORE]);
 
 impl<'tcx> LateLintPass<'tcx> for StdReexports {
     fn check_path(&mut self, cx: &LateContext<'tcx>, path: &Path<'tcx>, _: HirId) {
-        // std_instead_of_core
-        check_path(cx, path, sym::std, sym::core, STD_INSTEAD_OF_CORE);
-        // std_instead_of_alloc
-        check_path(cx, path, sym::std, sym::alloc, STD_INSTEAD_OF_ALLOC);
-        // alloc_instead_of_core
-        check_path(cx, path, sym::alloc, sym::core, ALLOC_INSTEAD_OF_CORE);
-    }
-}
-
-fn check_path(cx: &LateContext<'_>, path: &Path<'_>, krate: Symbol, suggested_crate: Symbol, lint: &'static Lint) {
-    if_chain! {
-        // check if path resolves to the suggested crate.
-        if let Res::Def(_, def_id) = path.res;
-        if suggested_crate == cx.tcx.crate_name(def_id.krate);
-
-        // check if the first segment of the path is the crate we want to identify
-        if let Some(path_root_segment) = get_first_segment(path);
-
-        // check if the path matches the crate we want to suggest the other path for.
-        if krate == path_root_segment.ident.name;
-        then {
-            span_lint_and_help(
-                cx,
-                lint,
-                path.span,
-                &format!("used import from `{}` instead of `{}`", krate, suggested_crate),
-                None,
-                &format!("consider importing the item from `{}`", suggested_crate),
-            );
+        if let Res::Def(_, def_id) = path.res
+            && let Some(first_segment) = get_first_segment(path)
+        {
+            let (lint, msg, help) = match first_segment.ident.name {
+                sym::std => match cx.tcx.crate_name(def_id.krate) {
+                    sym::core => (
+                        STD_INSTEAD_OF_CORE,
+                        "used import from `std` instead of `core`",
+                        "consider importing the item from `core`",
+                    ),
+                    sym::alloc => (
+                        STD_INSTEAD_OF_ALLOC,
+                        "used import from `std` instead of `alloc`",
+                        "consider importing the item from `alloc`",
+                    ),
+                    _ => {
+                        self.prev_span = path.span;
+                        return;
+                    },
+                },
+                sym::alloc => {
+                    if cx.tcx.crate_name(def_id.krate) == sym::core {
+                        (
+                            ALLOC_INSTEAD_OF_CORE,
+                            "used import from `alloc` instead of `core`",
+                            "consider importing the item from `core`",
+                        )
+                    } else {
+                        self.prev_span = path.span;
+                        return;
+                    }
+                },
+                _ => return,
+            };
+            if path.span != self.prev_span {
+                span_lint_and_help(cx, lint, path.span, msg, None, help);
+                self.prev_span = path.span;
+            }
         }
     }
 }
@@ -123,12 +139,10 @@ fn check_path(cx: &LateContext<'_>, path: &Path<'_>, krate: Symbol, suggested_cr
 /// If this is a global path (such as `::std::fmt::Debug`), then the segment after [`kw::PathRoot`]
 /// is returned.
 fn get_first_segment<'tcx>(path: &Path<'tcx>) -> Option<&'tcx PathSegment<'tcx>> {
-    let segment = path.segments.first()?;
-
-    // A global path will have PathRoot as the first segment. In this case, return the segment after.
-    if segment.ident.name == kw::PathRoot {
-        path.segments.get(1)
-    } else {
-        Some(segment)
+    match path.segments {
+        // A global path will have PathRoot as the first segment. In this case, return the segment after.
+        [x, y, ..] if x.ident.name == kw::PathRoot => Some(y),
+        [x, ..] => Some(x),
+        _ => None,
     }
 }

tests/ui/std_instead_of_core.rs

@@ -9,6 +9,8 @@ fn std_instead_of_core() {
     use std::hash::Hasher;
     // Absolute path
     use ::std::hash::Hash;
+    // Don't lint on `env` macro
+    use std::env;
 
     // Multiple imports
     use std::fmt::{Debug, Result};
@@ -20,10 +22,14 @@ fn std_instead_of_core() {
     // Types
     let cell = std::cell::Cell::new(8u32);
     let cell_absolute = ::std::cell::Cell::new(8u32);
+
+    let _ = std::env!("PATH");
 }
 
 #[warn(clippy::std_instead_of_alloc)]
 fn std_instead_of_alloc() {
+    // Only lint once.
+    use std::vec;
     use std::vec::Vec;
 }
 

tests/ui/std_instead_of_core.stderr

@@ -16,70 +16,78 @@ LL |     use ::std::hash::Hash;
    = help: consider importing the item from `core`
 
 error: used import from `std` instead of `core`
-  --> $DIR/std_instead_of_core.rs:14:20
+  --> $DIR/std_instead_of_core.rs:16:20
    |
 LL |     use std::fmt::{Debug, Result};
    |                    ^^^^^
    |
    = help: consider importing the item from `core`
 
 error: used import from `std` instead of `core`
-  --> $DIR/std_instead_of_core.rs:14:27
+  --> $DIR/std_instead_of_core.rs:16:27
    |
 LL |     use std::fmt::{Debug, Result};
    |                           ^^^^^^
    |
    = help: consider importing the item from `core`
 
 error: used import from `std` instead of `core`
-  --> $DIR/std_instead_of_core.rs:17:15
+  --> $DIR/std_instead_of_core.rs:19:15
    |
 LL |     let ptr = std::ptr::null::<u32>();
    |               ^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider importing the item from `core`
 
 error: used import from `std` instead of `core`
-  --> $DIR/std_instead_of_core.rs:18:19
+  --> $DIR/std_instead_of_core.rs:20:19
    |
 LL |     let ptr_mut = ::std::ptr::null_mut::<usize>();
    |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider importing the item from `core`
 
 error: used import from `std` instead of `core`
-  --> $DIR/std_instead_of_core.rs:21:16
+  --> $DIR/std_instead_of_core.rs:23:16
    |
 LL |     let cell = std::cell::Cell::new(8u32);
    |                ^^^^^^^^^^^^^^^
    |
    = help: consider importing the item from `core`
 
 error: used import from `std` instead of `core`
-  --> $DIR/std_instead_of_core.rs:22:25
+  --> $DIR/std_instead_of_core.rs:24:25
    |
 LL |     let cell_absolute = ::std::cell::Cell::new(8u32);
    |                         ^^^^^^^^^^^^^^^^^
    |
    = help: consider importing the item from `core`
 
 error: used import from `std` instead of `alloc`
-  --> $DIR/std_instead_of_core.rs:27:9
+  --> $DIR/std_instead_of_core.rs:32:9
+   |
+LL |     use std::vec;
+   |         ^^^^^^^^
+   |
+   = note: `-D clippy::std-instead-of-alloc` implied by `-D warnings`
+   = help: consider importing the item from `alloc`
+
+error: used import from `std` instead of `alloc`
+  --> $DIR/std_instead_of_core.rs:33:9
    |
 LL |     use std::vec::Vec;
    |         ^^^^^^^^^^^^^
    |
-   = note: `-D clippy::std-instead-of-alloc` implied by `-D warnings`
    = help: consider importing the item from `alloc`
 
 error: used import from `alloc` instead of `core`
-  --> $DIR/std_instead_of_core.rs:32:9
+  --> $DIR/std_instead_of_core.rs:38:9
    |
 LL |     use alloc::slice::from_ref;
    |         ^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::alloc-instead-of-core` implied by `-D warnings`
    = help: consider importing the item from `core`
 
-error: aborting due to 10 previous errors
+error: aborting due to 11 previous errors