Auto merge of rust-lang#13294 - WeiTheShinobi:new_lint_used_underscore_items, r=llogiq

Add new lint: `used_underscore_items`

Closes rust-lang#13260

---

changelog: new [`used_underscore_items`] lint against using items with a single leading underscore

Author: bors

CHANGELOG.md

@@ -6013,6 +6013,7 @@ Released 2018-09-13
 [`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug
 [`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self
 [`used_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding
+[`used_underscore_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_items
 [`useless_asref`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_asref
 [`useless_attribute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_attribute
 [`useless_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion

clippy_lints/src/declared_lints.rs

@@ -486,6 +486,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::misc::SHORT_CIRCUIT_STATEMENT_INFO,
     crate::misc::TOPLEVEL_REF_ARG_INFO,
     crate::misc::USED_UNDERSCORE_BINDING_INFO,
+    crate::misc::USED_UNDERSCORE_ITEMS_INFO,
     crate::misc_early::BUILTIN_TYPE_SHADOW_INFO,
     crate::misc_early::DOUBLE_NEG_INFO,
     crate::misc_early::DUPLICATE_UNDERSCORE_ARGUMENT_INFO,

clippy_lints/src/misc.rs

@@ -7,6 +7,7 @@ use clippy_utils::{
 };
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
+use rustc_hir::def_id::LOCAL_CRATE;
 use rustc_hir::intravisit::FnKind;
 use rustc_hir::{
     BinOpKind, BindingMode, Body, ByRef, Expr, ExprKind, FnDecl, Mutability, PatKind, QPath, Stmt, StmtKind,
@@ -80,6 +81,45 @@ declare_clippy_lint! {
     "using a binding which is prefixed with an underscore"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for the use of item with a single leading
+    /// underscore.
+    ///
+    /// ### Why is this bad?
+    /// A single leading underscore is usually used to indicate
+    /// that a item will not be used. Using such a item breaks this
+    /// expectation.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// fn _foo() {}
+    ///
+    /// struct _FooStruct {}
+    ///
+    /// fn main() {
+    ///     _foo();
+    ///     let _ = _FooStruct{};
+    /// }
+    /// ```
+    ///
+    /// Use instead:
+    /// ```no_run
+    /// fn foo() {}
+    ///
+    /// struct FooStruct {}
+    ///
+    /// fn main() {
+    ///     foo();
+    ///     let _ = FooStruct{};
+    /// }
+    /// ```
+    #[clippy::version = "pre 1.29.0"]
+    pub USED_UNDERSCORE_ITEMS,
+    pedantic,
+    "using a item which is prefixed with an underscore"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for the use of short circuit boolean conditions as
@@ -104,6 +144,7 @@ declare_clippy_lint! {
 declare_lint_pass!(LintPass => [
     TOPLEVEL_REF_ARG,
     USED_UNDERSCORE_BINDING,
+    USED_UNDERSCORE_ITEMS,
     SHORT_CIRCUIT_STATEMENT,
 ]);
 
@@ -205,51 +246,104 @@ impl<'tcx> LateLintPass<'tcx> for LintPass {
         {
             return;
         }
-        let (definition_hir_id, ident) = match expr.kind {
-            ExprKind::Path(ref qpath) => {
-                if let QPath::Resolved(None, path) = qpath
-                    && let Res::Local(id) = path.res
-                    && is_used(cx, expr)
-                {
-                    (id, last_path_segment(qpath).ident)
-                } else {
-                    return;
-                }
-            },
-            ExprKind::Field(recv, ident) => {
-                if let Some(adt_def) = cx.typeck_results().expr_ty_adjusted(recv).ty_adt_def()
-                    && let Some(field) = adt_def.all_fields().find(|field| field.name == ident.name)
-                    && let Some(local_did) = field.did.as_local()
-                    && !cx.tcx.type_of(field.did).skip_binder().is_phantom_data()
-                {
-                    (cx.tcx.local_def_id_to_hir_id(local_did), ident)
-                } else {
-                    return;
-                }
+
+        used_underscore_binding(cx, expr);
+        used_underscore_items(cx, expr);
+    }
+}
+
+fn used_underscore_items<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+    let (def_id, ident) = match expr.kind {
+        ExprKind::Call(func, ..) => {
+            if let ExprKind::Path(QPath::Resolved(.., path)) = func.kind
+                && let Some(last_segment) = path.segments.last()
+                && let Res::Def(_, def_id) = last_segment.res
+            {
+                (def_id, last_segment.ident)
+            } else {
+                return;
+            }
+        },
+        ExprKind::MethodCall(path, ..) => {
+            if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) {
+                (def_id, path.ident)
+            } else {
+                return;
+            }
+        },
+        ExprKind::Struct(QPath::Resolved(_, path), ..) => {
+            if let Some(last_segment) = path.segments.last()
+                && let Res::Def(_, def_id) = last_segment.res
+            {
+                (def_id, last_segment.ident)
+            } else {
+                return;
+            }
+        },
+        _ => return,
+    };
+
+    let name = ident.name.as_str();
+    let definition_span = cx.tcx.def_span(def_id);
+    if name.starts_with('_')
+        && !name.starts_with("__")
+        && !definition_span.from_expansion()
+        && def_id.krate == LOCAL_CRATE
+    {
+        span_lint_and_then(
+            cx,
+            USED_UNDERSCORE_ITEMS,
+            expr.span,
+            "used underscore-prefixed item".to_string(),
+            |diag| {
+                diag.span_note(definition_span, "item is defined here".to_string());
             },
-            _ => return,
-        };
+        );
+    }
+}
 
-        let name = ident.name.as_str();
-        if name.starts_with('_')
-            && !name.starts_with("__")
-            && let definition_span = cx.tcx.hir().span(definition_hir_id)
-            && !definition_span.from_expansion()
-            && !fulfill_or_allowed(cx, USED_UNDERSCORE_BINDING, [expr.hir_id, definition_hir_id])
-        {
-            span_lint_and_then(
-                cx,
-                USED_UNDERSCORE_BINDING,
-                expr.span,
-                format!(
-                    "used binding `{name}` which is prefixed with an underscore. A leading \
-                     underscore signals that a binding will not be used"
-                ),
-                |diag| {
-                    diag.span_note(definition_span, format!("`{name}` is defined here"));
-                },
-            );
-        }
+fn used_underscore_binding<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+    let (definition_hir_id, ident) = match expr.kind {
+        ExprKind::Path(ref qpath) => {
+            if let QPath::Resolved(None, path) = qpath
+                && let Res::Local(id) = path.res
+                && is_used(cx, expr)
+            {
+                (id, last_path_segment(qpath).ident)
+            } else {
+                return;
+            }
+        },
+        ExprKind::Field(recv, ident) => {
+            if let Some(adt_def) = cx.typeck_results().expr_ty_adjusted(recv).ty_adt_def()
+                && let Some(field) = adt_def.all_fields().find(|field| field.name == ident.name)
+                && let Some(local_did) = field.did.as_local()
+                && !cx.tcx.type_of(field.did).skip_binder().is_phantom_data()
+            {
+                (cx.tcx.local_def_id_to_hir_id(local_did), ident)
+            } else {
+                return;
+            }
+        },
+        _ => return,
+    };
+
+    let name = ident.name.as_str();
+    if name.starts_with('_')
+        && !name.starts_with("__")
+        && let definition_span = cx.tcx.hir().span(definition_hir_id)
+        && !definition_span.from_expansion()
+        && !fulfill_or_allowed(cx, USED_UNDERSCORE_BINDING, [expr.hir_id, definition_hir_id])
+    {
+        span_lint_and_then(
+            cx,
+            USED_UNDERSCORE_BINDING,
+            expr.span,
+            "used underscore-prefixed binding".to_string(),
+            |diag| {
+                diag.span_note(definition_span, "binding is defined here".to_string());
+            },
+        );
     }
 }
 

tests/ui/auxiliary/external_item.rs

@@ -0,0 +1,7 @@
+pub struct _ExternalStruct {}
+
+impl _ExternalStruct {
+    pub fn _foo(self) {}
+}
+
+pub fn _exernal_foo() {}

tests/ui/used_underscore_binding.stderr

@@ -1,72 +1,72 @@
-error: used binding `_foo` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
+error: used underscore-prefixed binding
   --> tests/ui/used_underscore_binding.rs:23:5
    |
 LL |     _foo + 1
    |     ^^^^
    |
-note: `_foo` is defined here
+note: binding is defined here
   --> tests/ui/used_underscore_binding.rs:22:22
    |
 LL | fn prefix_underscore(_foo: u32) -> u32 {
    |                      ^^^^
    = note: `-D clippy::used-underscore-binding` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::used_underscore_binding)]`
 
-error: used binding `_foo` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
+error: used underscore-prefixed binding
   --> tests/ui/used_underscore_binding.rs:28:20
    |
 LL |     println!("{}", _foo);
    |                    ^^^^
    |
-note: `_foo` is defined here
+note: binding is defined here
   --> tests/ui/used_underscore_binding.rs:27:24
    |
 LL | fn in_macro_or_desugar(_foo: u32) {
    |                        ^^^^
 
-error: used binding `_foo` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
+error: used underscore-prefixed binding
   --> tests/ui/used_underscore_binding.rs:29:16
    |
 LL |     assert_eq!(_foo, _foo);
    |                ^^^^
    |
-note: `_foo` is defined here
+note: binding is defined here
   --> tests/ui/used_underscore_binding.rs:27:24
    |
 LL | fn in_macro_or_desugar(_foo: u32) {
    |                        ^^^^
 
-error: used binding `_foo` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
+error: used underscore-prefixed binding
   --> tests/ui/used_underscore_binding.rs:29:22
    |
 LL |     assert_eq!(_foo, _foo);
    |                      ^^^^
    |
-note: `_foo` is defined here
+note: binding is defined here
   --> tests/ui/used_underscore_binding.rs:27:24
    |
 LL | fn in_macro_or_desugar(_foo: u32) {
    |                        ^^^^
 
-error: used binding `_underscore_field` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
+error: used underscore-prefixed binding
   --> tests/ui/used_underscore_binding.rs:42:5
    |
 LL |     s._underscore_field += 1;
    |     ^^^^^^^^^^^^^^^^^^^
    |
-note: `_underscore_field` is defined here
+note: binding is defined here
   --> tests/ui/used_underscore_binding.rs:36:5
    |
 LL |     _underscore_field: u32,
    |     ^^^^^^^^^^^^^^^^^^^^^^
 
-error: used binding `_i` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
+error: used underscore-prefixed binding
   --> tests/ui/used_underscore_binding.rs:103:16
    |
 LL |         uses_i(_i);
    |                ^^
    |
-note: `_i` is defined here
+note: binding is defined here
   --> tests/ui/used_underscore_binding.rs:102:13
    |
 LL |         let _i = 5;

tests/ui/used_underscore_items.rs

@@ -0,0 +1,63 @@
+//@aux-build:external_item.rs
+#![allow(unused)]
+#![warn(clippy::used_underscore_items)]
+
+extern crate external_item;
+
+// should not lint macro
+macro_rules! macro_wrap_func {
+    () => {
+        fn _marco_foo() {}
+    };
+}
+
+macro_wrap_func!();
+
+struct _FooStruct {}
+
+impl _FooStruct {
+    fn _method_call(self) {}
+}
+
+fn _foo1() {}
+
+fn _foo2() -> i32 {
+    0
+}
+
+mod a {
+    pub mod b {
+        pub mod c {
+            pub fn _foo3() {}
+
+            pub struct _FooStruct2 {}
+
+            impl _FooStruct2 {
+                pub fn _method_call(self) {}
+            }
+        }
+    }
+}
+
+fn main() {
+    _foo1();
+    let _ = _foo2();
+    a::b::c::_foo3();
+    let _ = &_FooStruct {};
+    let _ = _FooStruct {};
+
+    let foo_struct = _FooStruct {};
+    foo_struct._method_call();
+
+    let foo_struct2 = a::b::c::_FooStruct2 {};
+    foo_struct2._method_call();
+}
+
+// should not lint exteranl crate.
+// user cannot control how others name their items
+fn external_item_call() {
+    let foo_struct3 = external_item::_ExternalStruct {};
+    foo_struct3._foo();
+
+    external_item::_exernal_foo();
+}