add unclear_local_imports lint

Author: RalfJung

compiler/rustc_lint/messages.ftl

@@ -728,6 +728,8 @@ lint_tykind = usage of `ty::TyKind`
 lint_tykind_kind = usage of `ty::TyKind::<kind>`
     .suggestion = try using `ty::<kind>` directly
 
+lint_unclear_local_imports = `use` of a local item without leading `self::`, `super::`, or `crate::`
+
 lint_undropped_manually_drops = calls to `std::mem::drop` with `std::mem::ManuallyDrop` instead of the inner value does nothing
     .label = argument has type `{$arg_ty}`
     .suggestion = use `std::mem::ManuallyDrop::into_inner` to get the inner value

compiler/rustc_lint/src/lib.rs

@@ -75,6 +75,7 @@ mod reference_casting;
 mod shadowed_into_iter;
 mod traits;
 mod types;
+mod unclear_local_imports;
 mod unit_bindings;
 mod unused;
 
@@ -111,6 +112,7 @@ use reference_casting::*;
 use shadowed_into_iter::ShadowedIntoIter;
 use traits::*;
 use types::*;
+use unclear_local_imports::*;
 use unit_bindings::*;
 use unused::*;
 
@@ -228,6 +230,7 @@ late_lint_methods!(
             AsyncFnInTrait: AsyncFnInTrait,
             NonLocalDefinitions: NonLocalDefinitions::default(),
             ImplTraitOvercaptures: ImplTraitOvercaptures,
+            UnclearLocalImports: UnclearLocalImports,
         ]
     ]
 );

compiler/rustc_lint/src/lints.rs

@@ -2768,3 +2768,7 @@ pub struct RedundantImportVisibility {
     pub import_vis: String,
     pub max_vis: String,
 }
+
+#[derive(LintDiagnostic)]
+#[diag(lint_unclear_local_imports)]
+pub struct UnclearLocalImportsDiag {}

compiler/rustc_lint/src/passes.rs

@@ -14,6 +14,8 @@ macro_rules! late_lint_methods {
             fn check_mod(a: &'tcx rustc_hir::Mod<'tcx>, b: rustc_hir::HirId);
             fn check_foreign_item(a: &'tcx rustc_hir::ForeignItem<'tcx>);
             fn check_item(a: &'tcx rustc_hir::Item<'tcx>);
+            /// This is called *after* recursing into the item
+            /// (in contrast to `check_item`, which is checked before).
             fn check_item_post(a: &'tcx rustc_hir::Item<'tcx>);
             fn check_local(a: &'tcx rustc_hir::LetStmt<'tcx>);
             fn check_block(a: &'tcx rustc_hir::Block<'tcx>);
@@ -135,6 +137,8 @@ macro_rules! early_lint_methods {
             fn check_crate(a: &rustc_ast::Crate);
             fn check_crate_post(a: &rustc_ast::Crate);
             fn check_item(a: &rustc_ast::Item);
+            /// This is called *after* recursing into the item
+            /// (in contrast to `check_item`, which is checked before).
             fn check_item_post(a: &rustc_ast::Item);
             fn check_local(a: &rustc_ast::Local);
             fn check_block(a: &rustc_ast::Block);

compiler/rustc_lint/src/unclear_local_imports.rs

@@ -0,0 +1,64 @@
+use rustc_hir as hir;
+use rustc_session::{declare_lint, declare_lint_pass};
+use rustc_span::symbol::kw;
+
+use crate::{lints, LateContext, LateLintPass, LintContext};
+
+declare_lint! {
+    /// The `unclear_local_imports` lint checks for `use` items that import a local item using a
+    /// path that does not start with `self::`, `super::`, or `crate::`.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,edition2018
+    /// #![warn(unclear_local_imports)]
+    ///
+    /// mod localmod {
+    ///     pub struct S;
+    /// }
+    ///
+    /// use localmod::S;
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// This lint is meant to be used with the (unstable) rustfmt setting `group_imports = "StdExternalCrate"`.
+    /// That setting makes rustfmt group `self::`, `super::`, and `crate::` imports separately from those
+    /// refering to other crates. However, rustfmt cannot know whether `use c::S;` refers to a local module `c`
+    /// or an external crate `c`, so it always gets categorized as an import from another crate.
+    /// To ensure consistent grouping of imports from the local crate, all local imports must
+    /// start with `self::`, `super::`, or `crate::`. This lint can be used to enforce that style.
+    pub UNCLEAR_LOCAL_IMPORTS,
+    Allow,
+    "`use` of a local item without leading `self::`, `super::`, or `crate::`"
+}
+
+declare_lint_pass!(UnclearLocalImports => [UNCLEAR_LOCAL_IMPORTS]);
+
+impl<'tcx> LateLintPass<'tcx> for UnclearLocalImports {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
+        let hir::ItemKind::Use(path, _kind) = item.kind else { return };
+        // `path` has three resolutions for the type, module, value namespaces.
+        // However, it shouldn't be possible for those to be in different crates so we only check the first.
+        let Some(hir::def::Res::Def(_def_kind, def_id)) = path.res.first() else { return };
+        if !def_id.is_local() {
+            return;
+        }
+        // So this does refer to something local. Let's check whether it starts with `self`,
+        // `super`, or `crate`. If the path is empty, that means we have a `use *`, which is
+        // equivalent to `use crate::*` so we don't fire the lint in that case.
+        let Some(first_seg) = path.segments.first() else { return };
+        if matches!(first_seg.ident.name, kw::SelfLower | kw::Super | kw::Crate) {
+            return;
+        }
+
+        // This `use` qualifies for our lint!
+        cx.emit_span_lint(
+            UNCLEAR_LOCAL_IMPORTS,
+            first_seg.ident.span,
+            lints::UnclearLocalImportsDiag {},
+        );
+    }
+}

tests/ui/lint/unclear_local_imports.rs

@@ -0,0 +1,17 @@
+#![deny(unclear_local_imports)]
+
+mod localmod {
+    pub struct S;
+    pub struct T;
+}
+
+// Not a local import, so no lint.
+use std::cell::Cell;
+
+// Implicitly local import, gets lint.
+use localmod::S; //~ERROR: unclear
+
+// Explicitly local import, no lint.
+use self::localmod::T;
+
+fn main() {}

tests/ui/lint/unclear_local_imports.stderr

@@ -0,0 +1,14 @@
+error: `use` of a local item without leading `self::`, `super::`, or `crate::`
+  --> $DIR/unclear_local_imports.rs:12:5
+   |
+LL | use localmod::S;
+   |     ^^^^^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/unclear_local_imports.rs:1:9
+   |
+LL | #![deny(unclear_local_imports)]
+   |         ^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+