From 0f5d4a2b1c0dc1871c27b37cfe58b1409ed32fc2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 28 May 2024 08:53:08 +0200 Subject: [PATCH] add unqualified_local_imports lint --- compiler/rustc_lint/messages.ftl | 2 + compiler/rustc_lint/src/lib.rs | 3 + compiler/rustc_lint/src/lints.rs | 4 ++ compiler/rustc_lint/src/passes.rs | 4 ++ .../src/unqualified_local_imports.rs | 64 +++++++++++++++++++ tests/ui/lint/unclear_local_imports.rs | 17 +++++ tests/ui/lint/unclear_local_imports.stderr | 14 ++++ 7 files changed, 108 insertions(+) create mode 100644 compiler/rustc_lint/src/unqualified_local_imports.rs create mode 100644 tests/ui/lint/unclear_local_imports.rs create mode 100644 tests/ui/lint/unclear_local_imports.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 7d7b97e2eb1a3..cad4346248f47 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -761,6 +761,8 @@ lint_tykind = usage of `ty::TyKind` lint_tykind_kind = usage of `ty::TyKind::` .suggestion = try using `ty::` directly +lint_unqualified_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 diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index fc073233d9721..298591931a096 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -80,6 +80,7 @@ mod shadowed_into_iter; mod traits; mod types; mod unit_bindings; +mod unqualified_local_imports; mod unused; pub use shadowed_into_iter::{ARRAY_INTO_ITER, BOXED_SLICE_INTO_ITER}; @@ -118,6 +119,7 @@ use shadowed_into_iter::ShadowedIntoIter; use traits::*; use types::*; use unit_bindings::*; +use unqualified_local_imports::*; use unused::*; pub use builtin::{MissingDoc, SoftLints}; @@ -234,6 +236,7 @@ late_lint_methods!( AsyncFnInTrait: AsyncFnInTrait, NonLocalDefinitions: NonLocalDefinitions::default(), ImplTraitOvercaptures: ImplTraitOvercaptures, + UnqualifiedLocalImports: UnqualifiedLocalImports, ] ] ); diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 6c5f366727f95..a1334fb6478b8 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -2953,3 +2953,7 @@ pub struct UnsafeAttrOutsideUnsafeSuggestion { pub struct OutOfScopeMacroCalls { pub path: String, } + +#[derive(LintDiagnostic)] +#[diag(lint_unqualified_local_imports)] +pub struct UnqualifiedLocalImportsDiag {} diff --git a/compiler/rustc_lint/src/passes.rs b/compiler/rustc_lint/src/passes.rs index 2a843977990c2..74915ac0570a2 100644 --- a/compiler/rustc_lint/src/passes.rs +++ b/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); diff --git a/compiler/rustc_lint/src/unqualified_local_imports.rs b/compiler/rustc_lint/src/unqualified_local_imports.rs new file mode 100644 index 0000000000000..69c3866dc1256 --- /dev/null +++ b/compiler/rustc_lint/src/unqualified_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 `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; + /// ``` + /// + /// {{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::`" +} + +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. + // 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( + UNQUALIFIED_LOCAL_IMPORTS, + first_seg.ident.span, + lints::UnqualifiedLocalImportsDiag {}, + ); + } +} diff --git a/tests/ui/lint/unclear_local_imports.rs b/tests/ui/lint/unclear_local_imports.rs new file mode 100644 index 0000000000000..d4b74bc32d767 --- /dev/null +++ b/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() {} diff --git a/tests/ui/lint/unclear_local_imports.stderr b/tests/ui/lint/unclear_local_imports.stderr new file mode 100644 index 0000000000000..6678d48ec139f --- /dev/null +++ b/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 +