Auto merge of #125645 - RalfJung:unclear_local_imports, r=nnethercote

add unqualified_local_imports lint

This lint helps deal with https://github.com/rust-lang/rustfmt/issues/4709 by having the compiler detect imports of local items that are not syntactically distinguishable from imports from other cates. Making them  syntactically distinguishable ensures rustfmt can consistently apply the desired import grouping.

compiler/rustc_const_eval/src/const_eval/mod.rs

@@ -16,12 +16,12 @@ mod fn_queries;
 mod machine;
 mod valtrees;
 
-pub use dummy_machine::*;
-pub use error::*;
-pub use eval_queries::*;
-pub use fn_queries::*;
-pub use machine::*;
-pub(crate) use valtrees::{eval_to_valtree, valtree_to_const_value};
+pub use self::dummy_machine::*;
+pub use self::error::*;
+pub use self::eval_queries::*;
+pub use self::fn_queries::*;
+pub use self::machine::*;
+pub(crate) use self::valtrees::{eval_to_valtree, valtree_to_const_value};
 
 // We forbid type-level constants that contain more than `VALTREE_MAX_NODES` nodes.
 const VALTREE_MAX_NODES: usize = 100000;

compiler/rustc_const_eval/src/interpret/mod.rs

@@ -19,12 +19,12 @@ mod util;
 mod validity;
 mod visitor;
 
-use eval_context::{from_known_layout, mir_assign_valid_types};
 #[doc(no_inline)]
 pub use rustc_middle::mir::interpret::*; // have all the `interpret` symbols in one place: here
 
 pub use self::call::FnArg;
 pub use self::eval_context::{InterpCx, format_interp_error};
+use self::eval_context::{from_known_layout, mir_assign_valid_types};
 pub use self::intern::{
     HasStaticRootDefId, InternKind, InternResult, intern_const_alloc_for_constprop,
     intern_const_alloc_recursive,

compiler/rustc_const_eval/src/lib.rs

@@ -1,6 +1,8 @@
 // tidy-alphabetical-start
 #![allow(internal_features)]
 #![allow(rustc::diagnostic_outside_of_impl)]
+#![cfg_attr(not(bootstrap), feature(unqualified_local_imports))]
+#![cfg_attr(not(bootstrap), warn(unqualified_local_imports))]
 #![doc(rust_logo)]
 #![feature(assert_matches)]
 #![feature(box_patterns)]
@@ -25,10 +27,11 @@ pub mod util;
 
 use std::sync::atomic::AtomicBool;
 
-pub use errors::ReportErrorExt;
 use rustc_middle::ty;
 use rustc_middle::util::Providers;
 
+pub use self::errors::ReportErrorExt;
+
 rustc_fluent_macro::fluent_messages! { "../messages.ftl" }
 
 pub fn provide(providers: &mut Providers) {

compiler/rustc_feature/src/unstable.rs

@@ -227,6 +227,8 @@ declare_features! (
     (internal, staged_api, "1.0.0", None),
     /// Added for testing unstable lints; perma-unstable.
     (internal, test_unstable_lint, "1.60.0", None),
+    /// Helps with formatting for `group_imports = "StdExternalCrate"`.
+    (unstable, unqualified_local_imports, "CURRENT_RUSTC_VERSION", None),
     /// Use for stable + negative coherence and strict coherence depending on trait's
     /// rustc_strict_coherence value.
     (unstable, with_negative_coherence, "1.60.0", None),

compiler/rustc_lint/messages.ftl

@@ -899,6 +899,8 @@ lint_unnameable_test_items = cannot test inner items
 lint_unnecessary_qualification = unnecessary qualification
     .suggestion = remove the unnecessary path segments
 
+lint_unqualified_local_imports = `use` of a local item without leading `self::`, `super::`, or `crate::`
+
 lint_unsafe_attr_outside_unsafe = unsafe attribute used without unsafe
     .label = usage of unsafe attribute
 lint_unsafe_attr_outside_unsafe_suggestion = wrap the attribute in `unsafe(...)`

compiler/rustc_lint/src/lib.rs

@@ -86,6 +86,7 @@ mod tail_expr_drop_order;
 mod traits;
 mod types;
 mod unit_bindings;
+mod unqualified_local_imports;
 mod unused;
 
 use async_closures::AsyncClosureUsage;
@@ -126,6 +127,7 @@ use tail_expr_drop_order::TailExprDropOrder;
 use traits::*;
 use types::*;
 use unit_bindings::*;
+use unqualified_local_imports::*;
 use unused::*;
 
 #[rustfmt::skip]
@@ -249,6 +251,7 @@ late_lint_methods!(
             TailExprDropOrder: TailExprDropOrder,
             IfLetRescope: IfLetRescope::default(),
             StaticMutRefs: StaticMutRefs,
+            UnqualifiedLocalImports: UnqualifiedLocalImports,
         ]
     ]
 );

compiler/rustc_lint/src/lints.rs

@@ -3093,3 +3093,7 @@ pub(crate) enum MutRefSugg {
         span: Span,
     },
 }
+
+#[derive(LintDiagnostic)]
+#[diag(lint_unqualified_local_imports)]
+pub(crate) struct UnqualifiedLocalImportsDiag {}

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/unqualified_local_imports.rs

@@ -0,0 +1,85 @@
+use rustc_hir::def::{DefKind, Res};
+use rustc_hir::{self as hir};
+use rustc_session::{declare_lint, declare_lint_pass};
+use rustc_span::symbol::kw;
+
+use crate::{LateContext, LateLintPass, LintContext, lints};
+
+declare_lint! {
+    /// The `unqualified_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(unqualified_local_imports)]
+    ///
+    /// mod localmod {
+    ///     pub struct S;
+    /// }
+    ///
+    /// use localmod::S;
+    /// # // We have to actually use `S`, or else the `unused` warnings suppress the lint we care about.
+    /// # pub fn main() {
+    /// #     let _x = 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 UNQUALIFIED_LOCAL_IMPORTS,
+    Allow,
+    "`use` of a local item without leading `self::`, `super::`, or `crate::`",
+    @feature_gate = unqualified_local_imports;
+}
+
+declare_lint_pass!(UnqualifiedLocalImports => [UNQUALIFIED_LOCAL_IMPORTS]);
+
+impl<'tcx> LateLintPass<'tcx> for UnqualifiedLocalImports {
+    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.
+        // Check if any of them qualifies: local crate, and not a macro.
+        // (Macros can't be imported any other way so we don't complain about them.)
+        let is_local_import = |res: &Res| {
+            matches!(
+                res,
+                hir::def::Res::Def(def_kind, def_id)
+                    if def_id.is_local() && !matches!(def_kind, DefKind::Macro(_)),
+            )
+        };
+        if !path.res.iter().any(is_local_import) {
+            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;
+        }
+
+        let encl_item_id = cx.tcx.hir().get_parent_item(item.hir_id());
+        let encl_item = cx.tcx.hir_node_by_def_id(encl_item_id.def_id);
+        if encl_item.fn_kind().is_some() {
+            // `use` in a method -- don't lint, that leads to too many undesirable lints
+            // when a function imports all variants of an enum.
+            return;
+        }
+
+        // This `use` qualifies for our lint!
+        cx.emit_span_lint(
+            UNQUALIFIED_LOCAL_IMPORTS,
+            first_seg.ident.span,
+            lints::UnqualifiedLocalImportsDiag {},
+        );
+    }
+}

compiler/rustc_span/src/symbol.rs

@@ -2058,6 +2058,7 @@ symbols! {
         unmarked_api,
         unnamed_fields,
         unpin,
+        unqualified_local_imports,
         unreachable,
         unreachable_2015,
         unreachable_2015_macro,

src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs

@@ -22,9 +22,9 @@ use crate::borrow_tracker::{
 use crate::concurrency::data_race::{NaReadType, NaWriteType};
 use crate::*;
 
-use diagnostics::{RetagCause, RetagInfo};
-pub use item::{Item, Permission};
-pub use stack::Stack;
+use self::diagnostics::{RetagCause, RetagInfo};
+pub use self::item::{Item, Permission};
+pub use self::stack::Stack;
 
 pub type AllocState = Stacks;
 

src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs

@@ -19,8 +19,8 @@ mod unimap;
 #[cfg(test)]
 mod exhaustive;
 
-use perms::Permission;
-pub use tree::Tree;
+use self::perms::Permission;
+pub use self::tree::Tree;
 
 pub type AllocState = Tree;
 

src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs

@@ -47,7 +47,7 @@ enum PermissionPriv {
     /// rejects: all child accesses (UB).
     Disabled,
 }
-use PermissionPriv::*;
+use self::PermissionPriv::*;
 
 impl PartialOrd for PermissionPriv {
     /// PermissionPriv is ordered by the reflexive transitive closure of

src/tools/miri/src/concurrency/mod.rs

@@ -7,4 +7,4 @@ pub mod thread;
 mod vector_clock;
 pub mod weak_memory;
 
-pub use vector_clock::VClock;
+pub use self::vector_clock::VClock;

src/tools/miri/src/intrinsics/atomic.rs

@@ -1,7 +1,7 @@
 use rustc_middle::{mir, mir::BinOp, ty};
 
 use crate::*;
-use helpers::check_arg_count;
+use self::helpers::check_arg_count;
 
 pub enum AtomicOp {
     /// The `bool` indicates whether the result of the operation should be negated (`UnOp::Not`,

src/tools/miri/src/intrinsics/mod.rs

@@ -13,9 +13,9 @@ use rustc_span::{Symbol, sym};
 use rustc_target::abi::Size;
 
 use crate::*;
-use atomic::EvalContextExt as _;
-use helpers::{ToHost, ToSoft, check_arg_count};
-use simd::EvalContextExt as _;
+use self::atomic::EvalContextExt as _;
+use self::helpers::{ToHost, ToSoft, check_arg_count};
+use self::simd::EvalContextExt as _;
 
 impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
 pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {

src/tools/miri/src/lib.rs

@@ -51,6 +51,8 @@
     clippy::cast_lossless,
     clippy::cast_possible_truncation,
 )]
+#![cfg_attr(not(bootstrap), feature(unqualified_local_imports))]
+#![cfg_attr(not(bootstrap), warn(unqualified_local_imports))]
 // Needed for rustdoc from bootstrap (with `-Znormalize-docs`).
 #![recursion_limit = "256"]
 

src/tools/miri/src/shims/env.rs

@@ -3,7 +3,7 @@ use std::ffi::{OsStr, OsString};
 use rustc_data_structures::fx::FxHashMap;
 
 use crate::*;
-use shims::{unix::UnixEnvVars, windows::WindowsEnvVars};
+use self::shims::{unix::UnixEnvVars, windows::WindowsEnvVars};
 
 #[derive(Default)]
 pub enum EnvVars<'tcx> {

src/tools/miri/src/shims/foreign_items.rs

@@ -15,7 +15,7 @@ use rustc_target::{
 use super::alloc::EvalContextExt as _;
 use super::backtrace::EvalContextExt as _;
 use crate::*;
-use helpers::{ToHost, ToSoft};
+use self::helpers::{ToHost, ToSoft};
 
 /// Type of dynamic symbols (for `dlsym` et al)
 #[derive(Debug, Copy, Clone)]

src/tools/miri/src/shims/mod.rs

@@ -17,7 +17,7 @@ pub mod panic;
 pub mod time;
 pub mod tls;
 
-pub use unix::{DirTable, EpollInterestTable, FdTable};
+pub use self::unix::{DirTable, EpollInterestTable, FdTable};
 
 /// What needs to be done after emulating an item (a shim or an intrinsic) is done.
 pub enum EmulateItemResult {

src/tools/miri/src/shims/panic.rs

@@ -17,7 +17,7 @@ use rustc_target::spec::PanicStrategy;
 use rustc_target::spec::abi::Abi;
 
 use crate::*;
-use helpers::check_arg_count;
+use self::helpers::check_arg_count;
 
 /// Holds all of the relevant data for when unwinding hits a `try` frame.
 #[derive(Debug)]

src/tools/miri/src/shims/unix/foreign_items.rs

@@ -11,11 +11,11 @@ use crate::shims::alloc::EvalContextExt as _;
 use crate::shims::unix::*;
 use crate::*;
 
-use shims::unix::android::foreign_items as android;
-use shims::unix::freebsd::foreign_items as freebsd;
-use shims::unix::linux::foreign_items as linux;
-use shims::unix::macos::foreign_items as macos;
-use shims::unix::solarish::foreign_items as solarish;
+use self::shims::unix::android::foreign_items as android;
+use self::shims::unix::freebsd::foreign_items as freebsd;
+use self::shims::unix::linux::foreign_items as linux;
+use self::shims::unix::macos::foreign_items as macos;
+use self::shims::unix::solarish::foreign_items as solarish;
 
 pub fn is_dyn_sym(name: &str, target_os: &str) -> bool {
     match name {

src/tools/miri/src/shims/unix/fs.rs

@@ -15,7 +15,7 @@ use crate::shims::os_str::bytes_to_os_str;
 use crate::shims::unix::fd::FileDescriptionRef;
 use crate::shims::unix::*;
 use crate::*;
-use shims::time::system_time_to_duration;
+use self::shims::time::system_time_to_duration;
 
 use self::fd::FlockOp;
 

src/tools/miri/src/shims/unix/linux/foreign_items.rs

@@ -5,10 +5,10 @@ use crate::machine::SIGRTMAX;
 use crate::machine::SIGRTMIN;
 use crate::shims::unix::*;
 use crate::*;
-use shims::unix::linux::epoll::EvalContextExt as _;
-use shims::unix::linux::eventfd::EvalContextExt as _;
-use shims::unix::linux::mem::EvalContextExt as _;
-use shims::unix::linux::sync::futex;
+use self::shims::unix::linux::epoll::EvalContextExt as _;
+use self::shims::unix::linux::eventfd::EvalContextExt as _;
+use self::shims::unix::linux::mem::EvalContextExt as _;
+use self::shims::unix::linux::sync::futex;
 
 pub fn is_dyn_sym(name: &str) -> bool {
     matches!(name, "statx")

src/tools/miri/src/shims/unix/mod.rs

@@ -14,18 +14,18 @@ mod linux;
 mod macos;
 mod solarish;
 
-pub use env::UnixEnvVars;
-pub use fd::{FdTable, FileDescription};
-pub use fs::DirTable;
-pub use linux::epoll::EpollInterestTable;
+pub use self::env::UnixEnvVars;
+pub use self::fd::{FdTable, FileDescription};
+pub use self::fs::DirTable;
+pub use self::linux::epoll::EpollInterestTable;
 // All the Unix-specific extension traits
-pub use env::EvalContextExt as _;
-pub use fd::EvalContextExt as _;
-pub use fs::EvalContextExt as _;
-pub use mem::EvalContextExt as _;
-pub use sync::EvalContextExt as _;
-pub use thread::EvalContextExt as _;
-pub use unnamed_socket::EvalContextExt as _;
+pub use self::env::EvalContextExt as _;
+pub use self::fd::EvalContextExt as _;
+pub use self::fs::EvalContextExt as _;
+pub use self::mem::EvalContextExt as _;
+pub use self::sync::EvalContextExt as _;
+pub use self::thread::EvalContextExt as _;
+pub use self::unnamed_socket::EvalContextExt as _;
 
 // Make up some constants.
 const UID: u32 = 1000;

src/tools/miri/src/shims/windows/env.rs

@@ -5,7 +5,7 @@ use std::io::ErrorKind;
 use rustc_data_structures::fx::FxHashMap;
 
 use crate::*;
-use helpers::windows_check_buffer_size;
+use self::helpers::windows_check_buffer_size;
 
 #[derive(Default)]
 pub struct WindowsEnvVars {

src/tools/miri/src/shims/windows/foreign_items.rs

@@ -11,7 +11,7 @@ use rustc_target::spec::abi::Abi;
 use crate::shims::os_str::bytes_to_os_str;
 use crate::shims::windows::*;
 use crate::*;
-use shims::windows::handle::{Handle, PseudoHandle};
+use self::shims::windows::handle::{Handle, PseudoHandle};
 
 pub fn is_dyn_sym(name: &str) -> bool {
     // std does dynamic detection for these symbols

src/tools/miri/src/shims/windows/mod.rs

@@ -5,9 +5,9 @@ mod handle;
 mod sync;
 mod thread;
 
-pub use env::WindowsEnvVars;
+pub use self::env::WindowsEnvVars;
 // All the Windows-specific extension traits
-pub use env::EvalContextExt as _;
-pub use handle::EvalContextExt as _;
-pub use sync::EvalContextExt as _;
-pub use thread::EvalContextExt as _;
+pub use self::env::EvalContextExt as _;
+pub use self::handle::EvalContextExt as _;
+pub use self::sync::EvalContextExt as _;
+pub use self::thread::EvalContextExt as _;

src/tools/miri/src/shims/windows/thread.rs

@@ -2,7 +2,7 @@ use rustc_middle::ty::layout::LayoutOf;
 use rustc_target::spec::abi::Abi;
 
 use crate::*;
-use shims::windows::handle::{EvalContextExt as _, Handle, PseudoHandle};
+use self::shims::windows::handle::{EvalContextExt as _, Handle, PseudoHandle};
 
 impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
 

src/tools/miri/src/shims/x86/mod.rs

@@ -9,7 +9,7 @@ use rustc_target::abi::Size;
 use rustc_target::spec::abi::Abi;
 
 use crate::*;
-use helpers::bool_to_simd_element;
+use self::helpers::bool_to_simd_element;
 
 mod aesni;
 mod avx;

tests/ui/feature-gates/feature-gate-unqualified-local-imports.rs

@@ -0,0 +1,6 @@
+//@ check-pass
+
+#![allow(unqualified_local_imports)]
+//~^ WARNING unknown lint: `unqualified_local_imports`
+
+fn main() {}

tests/ui/feature-gates/feature-gate-unqualified-local-imports.stderr

@@ -0,0 +1,13 @@
+warning: unknown lint: `unqualified_local_imports`
+  --> $DIR/feature-gate-unqualified-local-imports.rs:3:1
+   |
+LL | #![allow(unqualified_local_imports)]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the `unqualified_local_imports` lint is unstable
+   = help: add `#![feature(unqualified_local_imports)]` to the crate attributes to enable
+   = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
+   = note: `#[warn(unknown_lints)]` on by default
+
+warning: 1 warning emitted
+

tests/ui/lint/unqualified_local_imports.rs

@@ -0,0 +1,38 @@
+//@compile-flags: --edition 2018
+#![feature(unqualified_local_imports)]
+#![deny(unqualified_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: unqualified
+
+// Explicitly local import, no lint.
+use self::localmod::T;
+
+macro_rules! mymacro {
+    ($cond:expr) => {
+        if !$cond {
+            continue;
+        }
+    };
+}
+// Macro import: no lint, as there is no other way to write it.
+pub(crate) use mymacro;
+
+#[allow(unused)]
+enum LocalEnum {
+    VarA,
+    VarB,
+}
+
+fn main() {
+    // Import in a function, no lint.
+    use LocalEnum::*;
+}

tests/ui/lint/unqualified_local_imports.stderr

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