Skip to content

Commit

Permalink
fix(resolve): update the ambiguity glob binding as warning recursively
Browse files Browse the repository at this point in the history
  • Loading branch information
bvanjoi committed Jul 28, 2023
1 parent 317ec04 commit cac0bd0
Show file tree
Hide file tree
Showing 65 changed files with 1,533 additions and 57 deletions.
16 changes: 16 additions & 0 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1871,6 +1871,22 @@ pub fn add_elided_lifetime_in_path_suggestion(
});
}

pub fn report_ambiguity_error<'a, G: EmissionGuarantee>(
db: &mut DiagnosticBuilder<'a, G>,
ambiguity: rustc_lint_defs::AmbiguityErrorDiag,
) {
db.span_label(ambiguity.label_span, ambiguity.label_msg);
db.note(ambiguity.note_msg);
db.span_note(ambiguity.b1_span, ambiguity.b1_note_msg);
for help_msg in ambiguity.b1_help_msgs {
db.help(help_msg);
}
db.span_note(ambiguity.b2_span, ambiguity.b2_note_msg);
for help_msg in ambiguity.b2_help_msgs {
db.help(help_msg);
}
}

#[derive(Clone, Copy, PartialEq, Hash, Debug)]
pub enum TerminalUrl {
No,
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -946,6 +946,9 @@ pub trait LintContext: Sized {
Applicability::MachineApplicable,
);
}
BuiltinLintDiagnostics::AmbiguousGlobImports { diag } => {
rustc_errors::report_ambiguity_error(db, diag);
}
BuiltinLintDiagnostics::AmbiguousGlobReexports { name, namespace, first_reexport_span, duplicate_reexport_span } => {
db.span_label(first_reexport_span, format!("the name `{name}` in the {namespace} namespace is first re-exported here"));
db.span_label(duplicate_reexport_span, format!("but the name `{name}` in the {namespace} namespace is also re-exported here"));
Expand Down
44 changes: 44 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3316,6 +3316,7 @@ declare_lint_pass! {
// tidy-alphabetical-start
ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE,
AMBIGUOUS_ASSOCIATED_ITEMS,
AMBIGUOUS_GLOB_IMPORTS,
AMBIGUOUS_GLOB_REEXPORTS,
ARITHMETIC_OVERFLOW,
ASM_SUB_REGISTER,
Expand Down Expand Up @@ -4405,3 +4406,46 @@ declare_lint! {
Warn,
"unrecognized diagnostic attribute"
}

declare_lint! {
/// The `ambiguous_glob_imports` lint detects glob imports that should report ambiguity
/// errors, but previously didn't do that due to rustc bugs.
///
/// ### Example
///
/// ```rust,compile_fail
///
/// #![deny(ambiguous_glob_imports)]
/// pub fn foo() -> u32 {
/// use sub::*;
/// C
/// }
///
/// mod sub {
/// mod mod1 { pub const C: u32 = 1; }
/// mod mod2 { pub const C: u32 = 2; }
///
/// pub use mod1::*;
/// pub use mod2::*;
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Previous versions of Rust compile it successfully because it
/// had lost the ambiguity error when resolve `use sub::mod2::*`.
///
/// This is a [future-incompatible] lint to transition this to a
/// hard error in the future.
///
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub AMBIGUOUS_GLOB_IMPORTS,
Warn,
"detects certain glob imports that require reporting an ambiguity error",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseError,
reference: "issue #114095 <https://github.com/rust-lang/rust/issues/114095>",
};
}
18 changes: 18 additions & 0 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,21 @@ impl<HCX> ToStableHashKey<HCX> for LintId {
}
}

#[derive(Debug)]
pub struct AmbiguityErrorDiag {
pub msg: String,
pub span: Span,
pub label_span: Span,
pub label_msg: String,
pub note_msg: String,
pub b1_span: Span,
pub b1_note_msg: String,
pub b1_help_msgs: Vec<String>,
pub b2_span: Span,
pub b2_note_msg: String,
pub b2_help_msgs: Vec<String>,
}

// This could be a closure, but then implementing derive trait
// becomes hacky (and it gets allocated).
#[derive(Debug)]
Expand Down Expand Up @@ -530,6 +545,9 @@ pub enum BuiltinLintDiagnostics {
vis_span: Span,
ident_span: Span,
},
AmbiguousGlobImports {
diag: AmbiguityErrorDiag,
},
AmbiguousGlobReexports {
/// The name for which collision(s) have occurred.
name: String,
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ impl<'a, Id: Into<DefId>> ToNameBinding<'a>
arenas.alloc_name_binding(NameBindingData {
kind: NameBindingKind::Module(self.0),
ambiguity: None,
warn_ambiguity: false,
vis: self.1.to_def_id(),
span: self.2,
expansion: self.3,
Expand All @@ -53,6 +54,7 @@ impl<'a, Id: Into<DefId>> ToNameBinding<'a> for (Res, ty::Visibility<Id>, Span,
arenas.alloc_name_binding(NameBindingData {
kind: NameBindingKind::Res(self.0),
ambiguity: None,
warn_ambiguity: false,
vis: self.1.to_def_id(),
span: self.2,
expansion: self.3,
Expand All @@ -69,7 +71,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
{
let binding = def.to_name_binding(self.arenas);
let key = self.new_disambiguated_key(ident, ns);
if let Err(old_binding) = self.try_define(parent, key, binding) {
if let Err(old_binding) = self.try_define(parent, key, binding, false) {
self.report_conflict(parent, ident, ns, old_binding, binding);
}
}
Expand Down
75 changes: 52 additions & 23 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@ use rustc_ast::{self as ast, Crate, ItemKind, ModKind, NodeId, Path, CRATE_NODE_
use rustc_ast::{MetaItemKind, NestedMetaItem};
use rustc_ast_pretty::pprust;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{
pluralize, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan,
};
use rustc_errors::{struct_span_err, SuggestionStyle};
use rustc_errors::{pluralize, report_ambiguity_error, struct_span_err, SuggestionStyle};
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan};
use rustc_feature::BUILTIN_ATTRIBUTES;
use rustc_hir::def::Namespace::{self, *};
use rustc_hir::def::{self, CtorKind, CtorOf, DefKind, NonMacroAttrKind, PerNS};
Expand All @@ -17,8 +15,9 @@ use rustc_hir::PrimTy;
use rustc_middle::bug;
use rustc_middle::ty::TyCtxt;
use rustc_session::lint::builtin::ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE;
use rustc_session::lint::builtin::AMBIGUOUS_GLOB_IMPORTS;
use rustc_session::lint::builtin::MACRO_EXPANDED_MACRO_EXPORTS_ACCESSED_BY_ABSOLUTE_PATHS;
use rustc_session::lint::BuiltinLintDiagnostics;
use rustc_session::lint::{AmbiguityErrorDiag, BuiltinLintDiagnostics};
use rustc_session::Session;
use rustc_span::edit_distance::find_best_match_for_name;
use rustc_span::edition::Edition;
Expand Down Expand Up @@ -135,7 +134,23 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}

for ambiguity_error in &self.ambiguity_errors {
self.report_ambiguity_error(ambiguity_error);
let diag = self.ambiguity_diagnostics(ambiguity_error);
if ambiguity_error.warning {
let NameBindingKind::Import { import, .. } = ambiguity_error.b1.0.kind else {
unreachable!()
};
self.lint_buffer.buffer_lint_with_diagnostic(
AMBIGUOUS_GLOB_IMPORTS,
import.root_id,
ambiguity_error.ident.span,
diag.msg.to_string(),
BuiltinLintDiagnostics::AmbiguousGlobImports { diag },
);
} else {
let mut err = struct_span_err!(self.tcx.sess, diag.span, E0659, "{}", &diag.msg);
report_ambiguity_error(&mut err, diag);
err.emit();
}
}

let mut reported_spans = FxHashSet::default();
Expand Down Expand Up @@ -1540,20 +1555,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}

fn report_ambiguity_error(&self, ambiguity_error: &AmbiguityError<'_>) {
let AmbiguityError { kind, ident, b1, b2, misc1, misc2 } = *ambiguity_error;
fn ambiguity_diagnostics(&self, ambiguity_error: &AmbiguityError<'_>) -> AmbiguityErrorDiag {
let AmbiguityError { kind, ident, b1, b2, misc1, misc2, .. } = *ambiguity_error;
let (b1, b2, misc1, misc2, swapped) = if b2.span.is_dummy() && !b1.span.is_dummy() {
// We have to print the span-less alternative first, otherwise formatting looks bad.
(b2, b1, misc2, misc1, true)
} else {
(b1, b2, misc1, misc2, false)
};

let mut err = struct_span_err!(self.tcx.sess, ident.span, E0659, "`{ident}` is ambiguous");
err.span_label(ident.span, "ambiguous name");
err.note(format!("ambiguous because of {}", kind.descr()));

let mut could_refer_to = |b: NameBinding<'_>, misc: AmbiguityErrorMisc, also: &str| {
let could_refer_to = |b: NameBinding<'_>, misc: AmbiguityErrorMisc, also: &str| {
let what = self.binding_description(b, ident, misc == AmbiguityErrorMisc::FromPrelude);
let note_msg = format!("`{ident}` could{also} refer to {what}");

Expand All @@ -1579,16 +1589,35 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
AmbiguityErrorMisc::FromPrelude | AmbiguityErrorMisc::None => {}
}

err.span_note(b.span, note_msg);
for (i, help_msg) in help_msgs.iter().enumerate() {
let or = if i == 0 { "" } else { "or " };
err.help(format!("{}{}", or, help_msg));
}
(
b.span,
note_msg,
help_msgs
.iter()
.enumerate()
.map(|(i, help_msg)| {
let or = if i == 0 { "" } else { "or " };
format!("{}{}", or, help_msg)
})
.collect::<Vec<_>>(),
)
};

could_refer_to(b1, misc1, "");
could_refer_to(b2, misc2, " also");
err.emit();
let (b1_span, b1_note_msg, b1_help_msgs) = could_refer_to(b1, misc1, "");
let (b2_span, b2_note_msg, b2_help_msgs) = could_refer_to(b2, misc2, " also");

AmbiguityErrorDiag {
msg: format!("`{ident}` is ambiguous"),
span: ident.span,
label_span: ident.span,
label_msg: "ambiguous name".to_string(),
note_msg: format!("ambiguous because of {}", kind.descr()),
b1_span,
b1_note_msg,
b1_help_msgs,
b2_span,
b2_note_msg,
b2_help_msgs,
}
}

/// If the binding refers to a tuple struct constructor with fields,
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_resolve/src/ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
ident: orig_ident,
b1: innermost_binding,
b2: binding,
warning: false,
misc1: misc(innermost_flags),
misc2: misc(flags),
});
Expand Down Expand Up @@ -905,6 +906,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
ident,
b1: binding,
b2: shadowed_glob,
warning: false,
misc1: AmbiguityErrorMisc::None,
misc2: AmbiguityErrorMisc::None,
});
Expand Down
Loading

0 comments on commit cac0bd0

Please sign in to comment.