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

add unqualified_local_imports lint

This lint helps deal with rust-lang/rustfmt#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.

Author: bors

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,