Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move the unused extern crate check back to the resolver. #108363

Merged
merged 4 commits into from
Feb 27, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 1 addition & 132 deletions compiler/rustc_hir_analysis/src/check_unused.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
use crate::errors::{ExternCrateNotIdiomatic, UnusedExternCrate};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::unord::UnordSet;
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::def_id::LocalDefId;
use rustc_middle::ty::TyCtxt;
use rustc_session::lint;
use rustc_span::{Span, Symbol};

pub fn check_crate(tcx: TyCtxt<'_>) {
let mut used_trait_imports: UnordSet<LocalDefId> = Default::default();
Expand Down Expand Up @@ -43,131 +39,4 @@ pub fn check_crate(tcx: TyCtxt<'_>) {
|lint| lint,
);
}

unused_crates_lint(tcx);
}

fn unused_crates_lint(tcx: TyCtxt<'_>) {
let lint = lint::builtin::UNUSED_EXTERN_CRATES;

// Collect first the crates that are completely unused. These we
// can always suggest removing (no matter which edition we are
// in).
let unused_extern_crates: FxHashMap<LocalDefId, Span> = tcx
.maybe_unused_extern_crates(())
.iter()
.filter(|&&(def_id, _)| {
tcx.extern_mod_stmt_cnum(def_id).map_or(true, |cnum| {
!tcx.is_compiler_builtins(cnum)
&& !tcx.is_panic_runtime(cnum)
&& !tcx.has_global_allocator(cnum)
&& !tcx.has_panic_handler(cnum)
})
})
.cloned()
.collect();

// Collect all the extern crates (in a reliable order).
let mut crates_to_lint = vec![];

for id in tcx.hir().items() {
if matches!(tcx.def_kind(id.owner_id), DefKind::ExternCrate) {
let item = tcx.hir().item(id);
if let hir::ItemKind::ExternCrate(orig_name) = item.kind {
crates_to_lint.push(ExternCrateToLint {
def_id: item.owner_id.to_def_id(),
span: item.span,
orig_name,
warn_if_unused: !item.ident.as_str().starts_with('_'),
});
}
}
}

let extern_prelude = &tcx.resolutions(()).extern_prelude;

for extern_crate in &crates_to_lint {
let def_id = extern_crate.def_id.expect_local();
let item = tcx.hir().expect_item(def_id);

// If the crate is fully unused, we suggest removing it altogether.
// We do this in any edition.
if extern_crate.warn_if_unused {
if let Some(&span) = unused_extern_crates.get(&def_id) {
// Removal suggestion span needs to include attributes (Issue #54400)
let id = tcx.hir().local_def_id_to_hir_id(def_id);
let span_with_attrs = tcx
.hir()
.attrs(id)
.iter()
.map(|attr| attr.span)
.fold(span, |acc, attr_span| acc.to(attr_span));

tcx.emit_spanned_lint(lint, id, span, UnusedExternCrate { span: span_with_attrs });
continue;
}
}

// If we are not in Rust 2018 edition, then we don't make any further
// suggestions.
if !tcx.sess.rust_2018() {
continue;
}

// If the extern crate isn't in the extern prelude,
// there is no way it can be written as a `use`.
let orig_name = extern_crate.orig_name.unwrap_or(item.ident.name);
if !extern_prelude.get(&orig_name).map_or(false, |from_item| !from_item) {
continue;
}

// If the extern crate is renamed, then we cannot suggest replacing it with a use as this
// would not insert the new name into the prelude, where other imports in the crate may be
// expecting it.
if extern_crate.orig_name.is_some() {
continue;
}

let id = tcx.hir().local_def_id_to_hir_id(def_id);
// If the extern crate has any attributes, they may have funky
// semantics we can't faithfully represent using `use` (most
// notably `#[macro_use]`). Ignore it.
if !tcx.hir().attrs(id).is_empty() {
continue;
}

let base_replacement = match extern_crate.orig_name {
Some(orig_name) => format!("use {} as {};", orig_name, item.ident.name),
None => format!("use {};", item.ident.name),
};
let vis = tcx.sess.source_map().span_to_snippet(item.vis_span).unwrap_or_default();
let add_vis = |to| if vis.is_empty() { to } else { format!("{} {}", vis, to) };
tcx.emit_spanned_lint(
lint,
id,
extern_crate.span,
ExternCrateNotIdiomatic {
span: extern_crate.span,
msg_code: add_vis("use".to_string()),
suggestion_code: add_vis(base_replacement),
},
);
}
}

struct ExternCrateToLint {
/// `DefId` of the extern crate
def_id: DefId,

/// span from the item
span: Span,

/// if `Some`, then this is renamed (`extern crate orig_name as
/// crate_name`), and -- perhaps surprisingly -- this stores the
/// *original* name (`item.name` will contain the new name)
orig_name: Option<Symbol>,

/// if `false`, the original name started with `_`, so we shouldn't lint
/// about it going unused (but we should still emit idiom lints).
warn_if_unused: bool,
}
22 changes: 1 addition & 21 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_errors::{
error_code, Applicability, DiagnosticBuilder, ErrorGuaranteed, Handler, IntoDiagnostic,
MultiSpan,
};
use rustc_macros::{Diagnostic, LintDiagnostic};
use rustc_macros::Diagnostic;
use rustc_middle::ty::Ty;
use rustc_span::{symbol::Ident, Span, Symbol};

Expand Down Expand Up @@ -247,26 +247,6 @@ pub struct SubstsOnOverriddenImpl {
pub span: Span,
}

#[derive(LintDiagnostic)]
#[diag(hir_analysis_unused_extern_crate)]
pub struct UnusedExternCrate {
#[suggestion(applicability = "machine-applicable", code = "")]
pub span: Span,
}

#[derive(LintDiagnostic)]
#[diag(hir_analysis_extern_crate_not_idiomatic)]
pub struct ExternCrateNotIdiomatic {
#[suggestion(
style = "short",
applicability = "machine-applicable",
code = "{suggestion_code}"
)]
pub span: Span,
pub msg_code: String,
pub suggestion_code: String,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_const_impl_for_non_const_trait)]
pub struct ConstImplForNonConstTrait {
Expand Down
17 changes: 17 additions & 0 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,23 @@ pub trait LintContext: Sized {
BuiltinLintDiagnostics::ByteSliceInPackedStructWithDerive => {
db.help("consider implementing the trait by hand, or remove the `packed` attribute");
}
BuiltinLintDiagnostics::UnusedExternCrate { removal_span }=> {
db.span_suggestion(
removal_span,
"remove it",
"",
Applicability::MachineApplicable,
);
}
BuiltinLintDiagnostics::ExternCrateNotIdiomatic { vis_span, ident_span }=> {
let suggestion_span = vis_span.between(ident_span);
db.span_suggestion_verbose(
suggestion_span,
"convert it to a `use`",
if vis_span.is_empty() { "use " } else { " use " },
Applicability::MachineApplicable,
);
}
}
// Rewrap `db`, and pass control to the user.
decorate(db)
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,13 @@ pub enum BuiltinLintDiagnostics {
is_formatting_arg: bool,
},
ByteSliceInPackedStructWithDerive,
UnusedExternCrate {
removal_span: Span,
},
ExternCrateNotIdiomatic {
vis_span: Span,
ident_span: Span,
},
}

/// Lints that are buffered up early on in the `Session` before the
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1830,9 +1830,6 @@ rustc_queries! {
query maybe_unused_trait_imports(_: ()) -> &'tcx FxIndexSet<LocalDefId> {
desc { "fetching potentially unused trait imports" }
}
query maybe_unused_extern_crates(_: ()) -> &'tcx [(LocalDefId, Span)] {
desc { "looking up all possibly unused extern crates" }
}
query names_imported_by_glob_use(def_id: LocalDefId) -> &'tcx FxHashSet<Symbol> {
desc { |tcx| "finding names imported by glob use for `{}`", tcx.def_path_str(def_id.to_def_id()) }
}
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2504,8 +2504,6 @@ pub fn provide(providers: &mut ty::query::Providers) {
|tcx, id| tcx.resolutions(()).reexport_map.get(&id).map(|v| &v[..]);
providers.maybe_unused_trait_imports =
|tcx, ()| &tcx.resolutions(()).maybe_unused_trait_imports;
providers.maybe_unused_extern_crates =
|tcx, ()| &tcx.resolutions(()).maybe_unused_extern_crates[..];
providers.names_imported_by_glob_use = |tcx, id| {
tcx.arena.alloc(tcx.resolutions(()).glob_map.get(&id).cloned().unwrap_or_default())
};
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,8 @@ pub struct ResolverGlobalCtxt {
pub effective_visibilities: EffectiveVisibilities,
pub extern_crate_map: FxHashMap<LocalDefId, CrateNum>,
pub maybe_unused_trait_imports: FxIndexSet<LocalDefId>,
pub maybe_unused_extern_crates: Vec<(LocalDefId, Span)>,
pub reexport_map: FxHashMap<LocalDefId, Vec<ModChild>>,
pub glob_map: FxHashMap<LocalDefId, FxHashSet<Symbol>>,
/// Extern prelude entries. The value is `true` if the entry was introduced
/// via `extern crate` item and not `--extern` option or compiler built-in.
pub extern_prelude: FxHashMap<Symbol, bool>,
pub main_def: Option<MainDefinition>,
pub trait_impls: FxIndexMap<DefId, Vec<LocalDefId>>,
/// A list of proc macro LocalDefIds, written out in the order in which
Expand Down
Loading