Skip to content

resolve: Turn deprecation lints pub_use_of_private_extern_crate and private_macro_use into hard errors #155945

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.

Sign up for GitHub

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

Open
petrochenkov wants to merge 1 commit into
base: main
Choose a base branch
from
Open

resolve: Turn deprecation lints pub_use_of_private_extern_crate and private_macro_use into hard errors #155945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions compiler/rustc_lint/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,16 @@ fn register_builtins(store: &mut LintStore) {
);
store.register_removed("wasm_c_abi", "the wasm C ABI has been fixed");
store.register_removed("soft_unstable", "the general soft-unstable mechanism has been removed");
store.register_removed(
"private_macro_use",
"converted into hard error, \
see <https://github.com/rust-lang/rust/issues/120192> for more information",
);
store.register_removed(
"pub_use_of_private_extern_crate",
"converted into hard error, \
see <https://github.com/rust-lang/rust/issues/127909> for more information",
);
}

fn register_internals(store: &mut LintStore) {
Expand Down
82 changes: 0 additions & 82 deletions compiler/rustc_lint_defs/src/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ declare_lint_pass! {
PRIVATE_BOUNDS,
PRIVATE_INTERFACES,
PROC_MACRO_DERIVE_RESOLUTION_FALLBACK,
PUB_USE_OF_PRIVATE_EXTERN_CRATE,
REDUNDANT_IMPORTS,
REDUNDANT_LIFETIMES,
REFINING_IMPL_TRAIT_INTERNAL,
Expand Down Expand Up @@ -1291,40 +1290,6 @@ declare_lint! {
"public interface leaks type from a private dependency"
}

declare_lint! {
/// The `pub_use_of_private_extern_crate` lint detects a specific
/// situation of re-exporting a private `extern crate`.
///
/// ### Example
///
/// ```rust,compile_fail
/// extern crate core;
/// pub use core as reexported_core;
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// A public `use` declaration should not be used to publically re-export a
/// private `extern crate`. `pub extern crate` should be used instead.
///
/// This was historically allowed, but is not the intended behavior
/// according to the visibility rules. This is a [future-incompatible]
/// lint to transition this to a hard error in the future. See [issue
/// #127909] for more details.
///
/// [issue #127909]: https://github.com/rust-lang/rust/issues/127909
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub PUB_USE_OF_PRIVATE_EXTERN_CRATE,
Deny,
"detect public re-exports of private extern crates",
@future_incompatible = FutureIncompatibleInfo {
reason: fcw!(FutureReleaseError #127909),
report_in_deps: true,
};
}

declare_lint! {
/// The `invalid_type_param_default` lint detects type parameter defaults
/// erroneously allowed in an invalid location.
Expand Down Expand Up @@ -4910,53 +4875,6 @@ declare_lint! {
};
}

declare_lint! {
/// The `private_macro_use` lint detects private macros that are imported
/// with `#[macro_use]`.
///
/// ### Example
///
/// ```rust,ignore (needs extern crate)
/// // extern_macro.rs
/// macro_rules! foo_ { () => {}; }
/// use foo_ as foo;
///
/// // code.rs
///
/// #![deny(private_macro_use)]
///
/// #[macro_use]
/// extern crate extern_macro;
///
/// fn main() {
/// foo!();
/// }
/// ```
///
/// This will produce:
///
/// ```text
/// error: cannot find macro `foo` in this scope
/// ```
///
/// ### Explanation
///
/// This lint arises from overlooking visibility checks for macros
/// in an external crate.
///
/// This is a [future-incompatible] lint to transition this to a
/// hard error in the future.
///
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub PRIVATE_MACRO_USE,
Deny,
"detects certain macro bindings that should not be re-exported",
@future_incompatible = FutureIncompatibleInfo {
reason: fcw!(FutureReleaseError #120192),
report_in_deps: true,
};
}

declare_lint! {
/// The `uncovered_param_in_projection` lint detects a violation of one of Rust's orphan rules for
/// foreign trait implementations that concerns the use of type parameters inside trait associated
Expand Down
24 changes: 7 additions & 17 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1122,9 +1122,9 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
}
}

let macro_use_import = |this: &Self, span, warn_private| {
let macro_use_import = |this: &Self, span| {
this.r.arenas.alloc_import(ImportData {
kind: ImportKind::MacroUse { warn_private },
kind: ImportKind::MacroUse,
root_id: item.id,
parent_scope: this.parent_scope,
imported_module: CmCell::new(Some(ModuleOrUniformRoot::Module(module))),
Expand All @@ -1142,22 +1142,12 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {

let allow_shadowing = self.parent_scope.expansion == LocalExpnId::ROOT;
if let Some(span) = import_all {
let import = macro_use_import(self, span, false);
let import = macro_use_import(self, span);
self.r.potentially_unused_imports.push(import);
module.for_each_child_mut(self, |this, ident, _, ns, binding| {
if ns == MacroNS {
let import =
if this.r.is_accessible_from(binding.vis(), this.parent_scope.module) {
import
} else {
// FIXME: This branch is used for reporting the `private_macro_use` lint
// and should eventually be removed.
if this.r.macro_use_prelude.contains_key(&ident.name) {
// Do not override already existing entries with compatibility entries.
return;
}
macro_use_import(this, span, true)
};
if ns == MacroNS
&& this.r.is_accessible_from(binding.vis(), this.parent_scope.module)
{
let import_decl = this.r.new_import_decl(binding, import);
this.add_macro_use_decl(ident.name, import_decl, span, allow_shadowing);
}
Expand All @@ -1172,7 +1162,7 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
None,
);
if let Ok(binding) = result {
let import = macro_use_import(self, ident.span, false);
let import = macro_use_import(self, ident.span);
self.r.potentially_unused_imports.push(import);
let import_decl = self.r.new_import_decl(binding, import);
self.add_macro_use_decl(ident.name, import_decl, ident.span, allow_shadowing);
Expand Down
20 changes: 10 additions & 10 deletions compiler/rustc_resolve/src/check_unused.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,15 +420,15 @@ impl Resolver<'_, '_> {
|| import.span.is_dummy()
|| self.import_use_map.contains_key(import) =>
{
if let ImportKind::MacroUse { .. } = import.kind {
if !import.span.is_dummy() {
self.lint_buffer.buffer_lint(
MACRO_USE_EXTERN_CRATE,
import.root_id,
import.span,
crate::errors::MacroUseDeprecated,
);
}
if let ImportKind::MacroUse = import.kind
&& !import.span.is_dummy()
{
self.lint_buffer.buffer_lint(
MACRO_USE_EXTERN_CRATE,
import.root_id,
import.span,
crate::errors::MacroUseDeprecated,
);
}
}
ImportKind::ExternCrate { id, .. } => {
Expand All @@ -443,7 +443,7 @@ impl Resolver<'_, '_> {
maybe_unused_extern_crates.insert(id, import.span);
}
}
ImportKind::MacroUse { .. } => {
ImportKind::MacroUse => {
self.lint_buffer.buffer_lint(
UNUSED_IMPORTS,
import.root_id,
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_resolve/src/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
use DeclKind::Import;
let can_suggest = |binding: Decl<'_>, import: self::Import<'_>| {
!binding.span.is_dummy()
&& !matches!(import.kind, ImportKind::MacroUse { .. } | ImportKind::MacroExport)
&& !matches!(import.kind, ImportKind::MacroUse | ImportKind::MacroExport)
};
let import = match (&new_binding.kind, &old_binding.kind) {
// If there are two imports where one or both have attributes then prefer removing the
Expand Down Expand Up @@ -2286,9 +2286,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
next_ident = source;
Some(source_decl)
}
ImportKind::Glob { .. }
| ImportKind::MacroUse { .. }
| ImportKind::MacroExport => Some(source_decl),
ImportKind::Glob { .. } | ImportKind::MacroUse | ImportKind::MacroExport => {
Some(source_decl)
}
ImportKind::ExternCrate { .. } => None,
},
_ => None,
Expand Down
19 changes: 0 additions & 19 deletions compiler/rustc_resolve/src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,19 +794,6 @@ pub(crate) struct CannotBeReexportedCratePublicNS {
pub(crate) ident: Ident,
}

#[derive(Diagnostic)]
#[diag("extern crate `{$ident}` is private and cannot be re-exported", code = E0365)]
pub(crate) struct PrivateExternCrateReexport {
pub ident: Ident,
#[suggestion(
"consider making the `extern crate` item publicly accessible",
code = "pub ",
style = "verbose",
applicability = "maybe-incorrect"
)]
pub sugg: Span,
}

#[derive(Subdiagnostic)]
#[help("consider adding a `#[macro_export]` to the macro in the imported module")]
pub(crate) struct ConsiderAddingMacroExport {
Expand Down Expand Up @@ -1428,12 +1415,6 @@ pub(crate) struct UnusedMacroUse;
#[help("remove it and import macros at use sites with a `use` item instead")]
pub(crate) struct MacroUseDeprecated;

#[derive(Diagnostic)]
#[diag("macro `{$ident}` is private")]
pub(crate) struct MacroIsPrivate {
pub ident: Ident,
}

#[derive(Diagnostic)]
#[diag("unused macro definition: `{$name}`")]
pub(crate) struct UnusedMacroDefinition {
Expand Down
52 changes: 8 additions & 44 deletions compiler/rustc_resolve/src/imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use rustc_middle::span_bug;
use rustc_middle::ty::{TyCtxt, Visibility};
use rustc_session::lint::builtin::{
AMBIGUOUS_GLOB_REEXPORTS, EXPORTED_PRIVATE_DEPENDENCIES, HIDDEN_GLOB_REEXPORTS,
PUB_USE_OF_PRIVATE_EXTERN_CRATE, REDUNDANT_IMPORTS, UNUSED_IMPORTS,
REDUNDANT_IMPORTS, UNUSED_IMPORTS,
};
use rustc_session::parse::feature_err;
use rustc_span::edit_distance::find_best_match_for_name;
Expand Down Expand Up @@ -99,11 +99,7 @@ pub(crate) enum ImportKind<'ra> {
target: Ident,
id: NodeId,
},
MacroUse {
/// A field has been added indicating whether it should be reported as a lint,
/// addressing issue#119301.
warn_private: bool,
},
MacroUse,
MacroExport,
}

Expand Down Expand Up @@ -134,9 +130,7 @@ impl<'ra> std::fmt::Debug for ImportKind<'ra> {
.field("target", target)
.field("id", id)
.finish(),
MacroUse { warn_private } => {
f.debug_struct("MacroUse").field("warn_private", warn_private).finish()
}
MacroUse => f.debug_struct("MacroUse").finish(),
MacroExport => f.debug_struct("MacroExport").finish(),
}
}
Expand Down Expand Up @@ -253,7 +247,7 @@ impl<'ra> ImportData<'ra> {
ImportKind::Single { id, .. }
| ImportKind::Glob { id, .. }
| ImportKind::ExternCrate { id, .. } => Some(id),
ImportKind::MacroUse { .. } | ImportKind::MacroExport => None,
ImportKind::MacroUse | ImportKind::MacroExport => None,
}
}

Expand All @@ -263,7 +257,7 @@ impl<'ra> ImportData<'ra> {
ImportKind::Single { id, .. } => Reexport::Single(to_def_id(id)),
ImportKind::Glob { id, .. } => Reexport::Glob(to_def_id(id)),
ImportKind::ExternCrate { id, .. } => Reexport::ExternCrate(to_def_id(id)),
ImportKind::MacroUse { .. } => Reexport::MacroUse,
ImportKind::MacroUse => Reexport::MacroUse,
ImportKind::MacroExport => Reexport::MacroExport,
}
}
Expand All @@ -272,7 +266,6 @@ impl<'ra> ImportData<'ra> {
ImportSummary {
vis: self.vis,
nearest_parent_mod: self.parent_scope.module.nearest_parent_mod().expect_local(),
is_single: matches!(self.kind, ImportKind::Single { .. }),
}
}
}
Expand Down Expand Up @@ -333,20 +326,6 @@ struct UnresolvedImportError {
on_unknown_attr: Option<OnUnknownData>,
}

// Reexports of the form `pub use foo as bar;` where `foo` is `extern crate foo;`
// are permitted for backward-compatibility under a deprecation lint.
fn pub_use_of_private_extern_crate_hack(import: ImportSummary, decl: Decl<'_>) -> Option<NodeId> {
match (import.is_single, decl.kind) {
(true, DeclKind::Import { import: decl_import, .. })
if let ImportKind::ExternCrate { id, .. } = decl_import.kind
&& import.vis.is_public() =>
{
Some(id)
}
_ => None,
}
}

/// Removes identical import layers from two declarations.
fn remove_same_import<'ra>(d1: Decl<'ra>, d2: Decl<'ra>) -> (Decl<'ra>, Decl<'ra>) {
if let DeclKind::Import { import: import1, source_decl: d1_next } = d1.kind
Expand Down Expand Up @@ -374,7 +353,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
let decl_vis = decl.vis();
if decl_vis.partial_cmp(import.vis, self.tcx) == Some(Ordering::Less)
&& decl_vis.is_accessible_from(import.nearest_parent_mod, self.tcx)
&& pub_use_of_private_extern_crate_hack(import, decl).is_none()
{
// Imported declaration is less visible than the import, but is still visible
// from the current module, use the declaration's visibility.
Expand All @@ -383,8 +361,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
// Good case - imported declaration is more visible than the import, or the same,
// use the import's visibility.
// Bad case - imported declaration is too private for the current module.
// It doesn't matter what visibility we choose here (except in the `PRIVATE_MACRO_USE`
// and `PUB_USE_OF_PRIVATE_EXTERN_CRATE` cases), because either some error will be
// It doesn't matter what visibility we choose here, because either some error will be
// reported, or the import declaration will be thrown away (unfortunately cannot use
// delayed bug here for this reason).
// Use import visibility to keep the all declaration visibilities in a module ordered.
Expand Down Expand Up @@ -1495,20 +1472,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
// All namespaces must be re-exported with extra visibility for an error to occur.
if !any_successful_reexport {
let (ns, binding) = reexport_error.unwrap();
if let Some(extern_crate_id) =
pub_use_of_private_extern_crate_hack(import.summary(), binding)
{
let extern_crate_sp = self.tcx.source_span(self.local_def_id(extern_crate_id));
self.lint_buffer.buffer_lint(
PUB_USE_OF_PRIVATE_EXTERN_CRATE,
import_id,
import.span,
crate::errors::PrivateExternCrateReexport {
ident,
sugg: extern_crate_sp.shrink_to_lo(),
},
);
} else if ns == TypeNS {
if ns == TypeNS {
let err = if crate_private_reexport {
self.dcx()
.create_err(CannotBeReexportedCratePublicNS { span: import.span, ident })
Expand Down Expand Up @@ -1802,7 +1766,7 @@ fn import_kind_to_string(import_kind: &ImportKind<'_>) -> String {
ImportKind::Single { source, .. } => source.to_string(),
ImportKind::Glob { .. } => "*".to_string(),
ImportKind::ExternCrate { .. } => "<extern crate>".to_string(),
ImportKind::MacroUse { .. } => "#[macro_use]".to_string(),
ImportKind::MacroUse => "#[macro_use]".to_string(),
ImportKind::MacroExport => "#[macro_export]".to_string(),
}
}
Loading
Loading