diff --git a/compiler/rustc_hir_analysis/locales/en-US.ftl b/compiler/rustc_hir_analysis/locales/en-US.ftl index 50b0816889b..40b5bc2a32e 100644 --- a/compiler/rustc_hir_analysis/locales/en-US.ftl +++ b/compiler/rustc_hir_analysis/locales/en-US.ftl @@ -62,14 +62,6 @@ hir_analysis_manual_implementation = hir_analysis_substs_on_overridden_impl = could not resolve substs on overridden impl -hir_analysis_unused_extern_crate = - unused extern crate - .suggestion = remove it - -hir_analysis_extern_crate_not_idiomatic = - `extern crate` is not idiomatic in the new edition - .suggestion = convert it to a `{$msg_code}` - hir_analysis_trait_object_declared_with_no_traits = at least one trait is required for an object type .alias_span = this alias does not contain a trait diff --git a/compiler/rustc_hir_analysis/src/check_unused.rs b/compiler/rustc_hir_analysis/src/check_unused.rs index 5716be4f1a9..f3f5851d8f9 100644 --- a/compiler/rustc_hir_analysis/src/check_unused.rs +++ b/compiler/rustc_hir_analysis/src/check_unused.rs @@ -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 = Default::default(); @@ -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 = 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, - - /// 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, } diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index a566e73912e..203e0f85cad 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -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}; @@ -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 { diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index aace4974cc9..f5a711315ea 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/compiler/rustc_lint/src/context.rs @@ -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) diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index 6efbf5ce9ee..534aff7fb62 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -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 diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index d4435a54b4a..d37c8d3813b 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1830,9 +1830,6 @@ rustc_queries! { query maybe_unused_trait_imports(_: ()) -> &'tcx FxIndexSet { 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 { desc { |tcx| "finding names imported by glob use for `{}`", tcx.def_path_str(def_id.to_def_id()) } } diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 433c9ab9aa2..0333198c203 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -2487,8 +2487,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()) }; diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 17262a0be24..c3e9dc7bcc1 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -165,12 +165,8 @@ pub struct ResolverGlobalCtxt { pub effective_visibilities: EffectiveVisibilities, pub extern_crate_map: FxHashMap, pub maybe_unused_trait_imports: FxIndexSet, - pub maybe_unused_extern_crates: Vec<(LocalDefId, Span)>, pub reexport_map: FxHashMap>, pub glob_map: FxHashMap>, - /// 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, pub main_def: Option, pub trait_impls: FxIndexMap>, /// A list of proc macro LocalDefIds, written out in the order in which diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs index 0114e116386..b2578e4c4b4 100644 --- a/compiler/rustc_resolve/src/check_unused.rs +++ b/compiler/rustc_resolve/src/check_unused.rs @@ -29,11 +29,12 @@ use crate::Resolver; use rustc_ast as ast; use rustc_ast::visit::{self, Visitor}; -use rustc_data_structures::fx::FxIndexMap; +use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_data_structures::unord::UnordSet; use rustc_errors::{pluralize, MultiSpan}; -use rustc_session::lint::builtin::{MACRO_USE_EXTERN_CRATE, UNUSED_IMPORTS}; +use rustc_session::lint::builtin::{MACRO_USE_EXTERN_CRATE, UNUSED_EXTERN_CRATES, UNUSED_IMPORTS}; use rustc_session::lint::BuiltinLintDiagnostics; +use rustc_span::symbol::Ident; use rustc_span::{Span, DUMMY_SP}; struct UnusedImport<'a> { @@ -53,11 +54,28 @@ struct UnusedImportCheckVisitor<'a, 'b, 'tcx> { r: &'a mut Resolver<'b, 'tcx>, /// All the (so far) unused imports, grouped path list unused_imports: FxIndexMap>, + extern_crate_items: Vec, base_use_tree: Option<&'a ast::UseTree>, base_id: ast::NodeId, item_span: Span, } +struct ExternCrateToLint { + id: ast::NodeId, + /// Span from the item + span: Span, + /// Span to use to suggest complete removal. + span_with_attributes: Span, + /// Span of the visibility, if any. + vis_span: Span, + /// Whether the item has attrs. + has_attrs: bool, + /// Name used to refer to the crate. + ident: Ident, + /// Whether the statement renames the crate `extern crate orig_name as new_name;`. + renames: bool, +} + impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> { // We have information about whether `use` (import) items are actually // used now. If an import is not used at all, we signal a lint error. @@ -96,18 +114,27 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> { impl<'a, 'b, 'tcx> Visitor<'a> for UnusedImportCheckVisitor<'a, 'b, 'tcx> { fn visit_item(&mut self, item: &'a ast::Item) { - self.item_span = item.span_with_attributes(); - - // Ignore is_public import statements because there's no way to be sure - // whether they're used or not. Also ignore imports with a dummy span - // because this means that they were generated in some fashion by the - // compiler and we don't need to consider them. - if let ast::ItemKind::Use(..) = item.kind { - if item.vis.kind.is_pub() || item.span.is_dummy() { - return; + match item.kind { + // Ignore is_public import statements because there's no way to be sure + // whether they're used or not. Also ignore imports with a dummy span + // because this means that they were generated in some fashion by the + // compiler and we don't need to consider them. + ast::ItemKind::Use(..) if item.vis.kind.is_pub() || item.span.is_dummy() => return, + ast::ItemKind::ExternCrate(orig_name) => { + self.extern_crate_items.push(ExternCrateToLint { + id: item.id, + span: item.span, + vis_span: item.vis.span, + span_with_attributes: item.span_with_attributes(), + has_attrs: !item.attrs.is_empty(), + ident: item.ident, + renames: orig_name.is_some(), + }); } + _ => {} } + self.item_span = item.span_with_attributes(); visit::walk_item(self, item); } @@ -224,6 +251,9 @@ fn calc_unused_spans( impl Resolver<'_, '_> { pub(crate) fn check_unused(&mut self, krate: &ast::Crate) { + let tcx = self.tcx; + let mut maybe_unused_extern_crates = FxHashMap::default(); + for import in self.potentially_unused_imports.iter() { match import.kind { _ if import.used.get() @@ -246,7 +276,14 @@ impl Resolver<'_, '_> { } ImportKind::ExternCrate { id, .. } => { let def_id = self.local_def_id(id); - self.maybe_unused_extern_crates.push((def_id, import.span)); + if self.extern_crate_map.get(&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) + }) { + maybe_unused_extern_crates.insert(id, import.span); + } } ImportKind::MacroUse => { let msg = "unused `#[macro_use]` import"; @@ -259,6 +296,7 @@ impl Resolver<'_, '_> { let mut visitor = UnusedImportCheckVisitor { r: self, unused_imports: Default::default(), + extern_crate_items: Default::default(), base_use_tree: None, base_id: ast::DUMMY_NODE_ID, item_span: DUMMY_SP, @@ -290,7 +328,7 @@ impl Resolver<'_, '_> { let ms = MultiSpan::from_spans(spans.clone()); let mut span_snippets = spans .iter() - .filter_map(|s| match visitor.r.tcx.sess.source_map().span_to_snippet(*s) { + .filter_map(|s| match tcx.sess.source_map().span_to_snippet(*s) { Ok(s) => Some(format!("`{}`", s)), _ => None, }) @@ -317,7 +355,7 @@ impl Resolver<'_, '_> { // If we are in the `--test` mode, suppress a help that adds the `#[cfg(test)]` // attribute; however, if not, suggest adding the attribute. There is no way to // retrieve attributes here because we do not have a `TyCtxt` yet. - let test_module_span = if visitor.r.tcx.sess.opts.test { + let test_module_span = if tcx.sess.opts.test { None } else { let parent_module = visitor.r.get_nearest_non_block_module( @@ -346,5 +384,74 @@ impl Resolver<'_, '_> { BuiltinLintDiagnostics::UnusedImports(fix_msg.into(), fixes, test_module_span), ); } + + for extern_crate in visitor.extern_crate_items { + let warn_if_unused = !extern_crate.ident.name.as_str().starts_with('_'); + + // If the crate is fully unused, we suggest removing it altogether. + // We do this in any edition. + if warn_if_unused { + if let Some(&span) = maybe_unused_extern_crates.get(&extern_crate.id) { + visitor.r.lint_buffer.buffer_lint_with_diagnostic( + UNUSED_EXTERN_CRATES, + extern_crate.id, + span, + "unused extern crate", + BuiltinLintDiagnostics::UnusedExternCrate { + removal_span: extern_crate.span_with_attributes, + }, + ); + 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 has any attributes, they may have funky + // semantics we can't faithfully represent using `use` (most + // notably `#[macro_use]`). Ignore it. + if extern_crate.has_attrs { + 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.renames { + continue; + } + + // If the extern crate isn't in the extern prelude, + // there is no way it can be written as a `use`. + if !visitor + .r + .extern_prelude + .get(&extern_crate.ident) + .map_or(false, |entry| !entry.introduced_by_item) + { + continue; + } + + let vis_span = extern_crate + .vis_span + .find_ancestor_inside(extern_crate.span) + .unwrap_or(extern_crate.vis_span); + let ident_span = extern_crate + .ident + .span + .find_ancestor_inside(extern_crate.span) + .unwrap_or(extern_crate.ident.span); + visitor.r.lint_buffer.buffer_lint_with_diagnostic( + UNUSED_EXTERN_CRATES, + extern_crate.id, + extern_crate.span, + "`extern crate` is not idiomatic in the new edition", + BuiltinLintDiagnostics::ExternCrateNotIdiomatic { vis_span, ident_span }, + ); + } } } diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 7a1f14f71f2..1fdfb1a53d4 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -946,7 +946,6 @@ pub struct Resolver<'a, 'tcx> { has_pub_restricted: bool, used_imports: FxHashSet, maybe_unused_trait_imports: FxIndexSet, - maybe_unused_extern_crates: Vec<(LocalDefId, Span)>, /// Privacy errors are delayed until the end in order to deduplicate them. privacy_errors: Vec>, @@ -1284,7 +1283,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { has_pub_restricted: false, used_imports: FxHashSet::default(), maybe_unused_trait_imports: Default::default(), - maybe_unused_extern_crates: Vec::new(), privacy_errors: Vec::new(), ambiguity_errors: Vec::new(), @@ -1400,7 +1398,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { let extern_crate_map = self.extern_crate_map; let reexport_map = self.reexport_map; let maybe_unused_trait_imports = self.maybe_unused_trait_imports; - let maybe_unused_extern_crates = self.maybe_unused_extern_crates; let glob_map = self.glob_map; let main_def = self.main_def; let confused_type_with_std_module = self.confused_type_with_std_module; @@ -1414,12 +1411,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { reexport_map, glob_map, maybe_unused_trait_imports, - maybe_unused_extern_crates, - extern_prelude: self - .extern_prelude - .iter() - .map(|(ident, entry)| (ident.name, entry.introduced_by_item)) - .collect(), main_def, trait_impls: self.trait_impls, proc_macros, diff --git a/tests/ui/rust-2018/remove-extern-crate.fixed b/tests/ui/rust-2018/remove-extern-crate.fixed index 832632268fb..15e0ccc5256 100644 --- a/tests/ui/rust-2018/remove-extern-crate.fixed +++ b/tests/ui/rust-2018/remove-extern-crate.fixed @@ -23,6 +23,7 @@ extern crate alloc; fn main() { another_name::mem::drop(3); another::foo(); + with_visibility::foo(); remove_extern_crate::foo!(); bar!(); alloc::vec![5]; @@ -37,3 +38,12 @@ mod another { remove_extern_crate::foo!(); } } + +mod with_visibility { + pub use core; //~ WARNING `extern crate` is not idiomatic + + pub fn foo() { + core::mem::drop(4); + remove_extern_crate::foo!(); + } +} diff --git a/tests/ui/rust-2018/remove-extern-crate.rs b/tests/ui/rust-2018/remove-extern-crate.rs index bbb84cd462d..aec0bc7c374 100644 --- a/tests/ui/rust-2018/remove-extern-crate.rs +++ b/tests/ui/rust-2018/remove-extern-crate.rs @@ -23,6 +23,7 @@ extern crate alloc; fn main() { another_name::mem::drop(3); another::foo(); + with_visibility::foo(); remove_extern_crate::foo!(); bar!(); alloc::vec![5]; @@ -37,3 +38,12 @@ mod another { remove_extern_crate::foo!(); } } + +mod with_visibility { + pub extern crate core; //~ WARNING `extern crate` is not idiomatic + + pub fn foo() { + core::mem::drop(4); + remove_extern_crate::foo!(); + } +} diff --git a/tests/ui/rust-2018/remove-extern-crate.stderr b/tests/ui/rust-2018/remove-extern-crate.stderr index bde4c180811..d07358e471b 100644 --- a/tests/ui/rust-2018/remove-extern-crate.stderr +++ b/tests/ui/rust-2018/remove-extern-crate.stderr @@ -12,10 +12,26 @@ LL | #![warn(rust_2018_idioms)] = note: `#[warn(unused_extern_crates)]` implied by `#[warn(rust_2018_idioms)]` warning: `extern crate` is not idiomatic in the new edition - --> $DIR/remove-extern-crate.rs:32:5 + --> $DIR/remove-extern-crate.rs:33:5 | LL | extern crate core; - | ^^^^^^^^^^^^^^^^^^ help: convert it to a `use` + | ^^^^^^^^^^^^^^^^^^ + | +help: convert it to a `use` + | +LL | use core; + | ~~~ -warning: 2 warnings emitted +warning: `extern crate` is not idiomatic in the new edition + --> $DIR/remove-extern-crate.rs:43:5 + | +LL | pub extern crate core; + | ^^^^^^^^^^^^^^^^^^^^^^ + | +help: convert it to a `use` + | +LL | pub use core; + | ~~~ + +warning: 3 warnings emitted