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

Make it crystal clear what lint type_alias_bounds actually signifies #126575

Merged
merged 9 commits into from
Jul 26, 2024
13 changes: 9 additions & 4 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
hir_analysis_ambiguous_assoc_item = ambiguous associated {$assoc_kind} `{$assoc_name}` in bounds of `{$ty_param_name}`
hir_analysis_ambiguous_assoc_item = ambiguous associated {$assoc_kind} `{$assoc_name}` in bounds of `{$qself}`
.label = ambiguous associated {$assoc_kind} `{$assoc_name}`

hir_analysis_ambiguous_lifetime_bound =
Expand All @@ -12,16 +12,21 @@ hir_analysis_assoc_item_is_private = {$kind} `{$name}` is private
.label = private {$kind}
.defined_here_label = the {$kind} is defined here

hir_analysis_assoc_item_not_found = associated {$assoc_kind} `{$assoc_name}` not found for `{$ty_param_name}`
hir_analysis_assoc_item_not_found = associated {$assoc_kind} `{$assoc_name}` not found for `{$qself}`

hir_analysis_assoc_item_not_found_found_in_other_trait_label = there is {$identically_named ->
[true] an
*[false] a similarly named
} associated {$assoc_kind} `{$suggested_name}` in the trait `{$trait_name}`
hir_analysis_assoc_item_not_found_label = associated {$assoc_kind} `{$assoc_name}` not found
hir_analysis_assoc_item_not_found_other_sugg = `{$ty_param_name}` has the following associated {$assoc_kind}
hir_analysis_assoc_item_not_found_other_sugg = `{$qself}` has the following associated {$assoc_kind}
hir_analysis_assoc_item_not_found_similar_in_other_trait_qpath_sugg =
consider fully qualifying{$identically_named ->
[true] {""}
*[false] {" "}and renaming
} the associated {$assoc_kind}
hir_analysis_assoc_item_not_found_similar_in_other_trait_sugg = change the associated {$assoc_kind} name to use `{$suggested_name}` from `{$trait_name}`
hir_analysis_assoc_item_not_found_similar_in_other_trait_with_bound_sugg = and also change the associated {$assoc_kind} name
hir_analysis_assoc_item_not_found_similar_in_other_trait_with_bound_sugg = ...and changing the associated {$assoc_kind} name
hir_analysis_assoc_item_not_found_similar_sugg = there is an associated {$assoc_kind} with a similar name

hir_analysis_assoc_kind_mismatch = expected {$expected}, found {$got}
Expand Down
31 changes: 25 additions & 6 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub struct AmbiguousAssocItem<'a> {
pub span: Span,
pub assoc_kind: &'static str,
pub assoc_name: Ident,
pub ty_param_name: &'a str,
pub qself: &'a str,
}

#[derive(Diagnostic)]
Expand Down Expand Up @@ -75,7 +75,7 @@ pub struct AssocItemNotFound<'a> {
pub span: Span,
pub assoc_name: Ident,
pub assoc_kind: &'static str,
pub ty_param_name: &'a str,
pub qself: &'a str,
#[subdiagnostic]
pub label: Option<AssocItemNotFoundLabel<'a>>,
#[subdiagnostic]
Expand Down Expand Up @@ -126,13 +126,32 @@ pub enum AssocItemNotFoundSugg<'a> {
assoc_kind: &'static str,
suggested_name: Symbol,
},
#[suggestion(hir_analysis_assoc_item_not_found_other_sugg, code = "{suggested_name}")]
#[multipart_suggestion(
hir_analysis_assoc_item_not_found_similar_in_other_trait_qpath_sugg,
style = "verbose"
)]
SimilarInOtherTraitQPath {
#[suggestion_part(code = "<")]
lo: Span,
#[suggestion_part(code = " as {trait_ref}>")]
mi: Span,
#[suggestion_part(code = "{suggested_name}")]
hi: Option<Span>,
trait_ref: String,
suggested_name: Symbol,
identically_named: bool,
#[applicability]
applicability: Applicability,
},
#[suggestion(
hir_analysis_assoc_item_not_found_other_sugg,
code = "{suggested_name}",
applicability = "maybe-incorrect"
)]
Other {
#[primary_span]
span: Span,
#[applicability]
applicability: Applicability,
ty_param_name: &'a str,
qself: &'a str,
assoc_kind: &'static str,
suggested_name: Symbol,
},
Expand Down
9 changes: 3 additions & 6 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_middle::bug;
use rustc_middle::ty::print::PrintTraitRefExt as _;
use rustc_middle::ty::{self as ty, IsSuggestable, Ty, TyCtxt};
use rustc_span::symbol::Ident;
use rustc_span::{ErrorGuaranteed, Span, Symbol};
Expand All @@ -16,9 +15,8 @@ use smallvec::SmallVec;

use crate::bounds::Bounds;
use crate::errors;
use crate::hir_ty_lowering::{HirTyLowerer, OnlySelfBounds, PredicateFilter};

use super::RegionInferReason;
use crate::hir_ty_lowering::HirTyLowerer;
use crate::hir_ty_lowering::{AssocItemQSelf, OnlySelfBounds, PredicateFilter, RegionInferReason};

impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
/// Add a `Sized` bound to the `bounds` if appropriate.
Expand Down Expand Up @@ -288,8 +286,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// one that does define it.
self.probe_single_bound_for_assoc_item(
|| traits::supertraits(tcx, trait_ref),
trait_ref.skip_binder().print_only_trait_name(),
None,
AssocItemQSelf::Trait(trait_ref.def_id()),
assoc_kind,
constraint.ident,
path_span,
Expand Down
127 changes: 78 additions & 49 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,17 @@ use crate::errors::{
ParenthesizedFnTraitExpansion, TraitObjectDeclaredWithNoTraits,
};
use crate::fluent_generated as fluent;
use crate::hir_ty_lowering::HirTyLowerer;
use crate::hir_ty_lowering::{AssocItemQSelf, HirTyLowerer};
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
use rustc_data_structures::sorted_map::SortedMap;
use rustc_data_structures::unord::UnordMap;
use rustc_errors::MultiSpan;
use rustc_errors::{
codes::*, pluralize, struct_span_code_err, Applicability, Diag, ErrorGuaranteed,
};
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::{self as hir, Node};
use rustc_hir::def_id::DefId;
use rustc_middle::bug;
use rustc_middle::query::Key;
use rustc_middle::ty::print::{PrintPolyTraitRefExt as _, PrintTraitRefExt as _};
Expand Down Expand Up @@ -116,8 +116,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
pub(super) fn complain_about_assoc_item_not_found<I>(
&self,
all_candidates: impl Fn() -> I,
ty_param_name: &str,
ty_param_def_id: Option<LocalDefId>,
qself: AssocItemQSelf,
assoc_kind: ty::AssocKind,
assoc_name: Ident,
span: Span,
Expand All @@ -139,7 +138,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
);
}

let assoc_kind_str = super::assoc_kind_str(assoc_kind);
let assoc_kind_str = assoc_kind_str(assoc_kind);
let qself_str = qself.to_string(tcx);

// The fallback span is needed because `assoc_name` might be an `Fn()`'s `Output` without a
// valid span, so we point at the whole path segment instead.
Expand All @@ -149,7 +149,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
span: if is_dummy { span } else { assoc_name.span },
assoc_name,
assoc_kind: assoc_kind_str,
ty_param_name,
qself: &qself_str,
label: None,
sugg: None,
};
Expand Down Expand Up @@ -219,19 +219,28 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
suggested_name,
identically_named: suggested_name == assoc_name.name,
});
let hir = tcx.hir();
if let Some(def_id) = ty_param_def_id
&& let parent = hir.get_parent_item(tcx.local_def_id_to_hir_id(def_id))
&& let Some(generics) = hir.get_generics(parent.def_id)
if let AssocItemQSelf::TyParam(ty_param_def_id, ty_param_span) = qself
// Not using `self.item_def_id()` here as that would yield the opaque type itself if we're
// inside an opaque type while we're interested in the overarching type alias (TAIT).
// FIXME: However, for trait aliases, this incorrectly returns the enclosing module...
&& let item_def_id =
tcx.hir().get_parent_item(tcx.local_def_id_to_hir_id(ty_param_def_id))
// FIXME: ...which obviously won't have any generics.
&& let Some(generics) = tcx.hir().get_generics(item_def_id.def_id)
{
if generics.bounds_for_param(def_id).flat_map(|pred| pred.bounds.iter()).any(
|b| match b {
// FIXME: Suggest adding supertrait bounds if we have a `Self` type param.
// FIXME(trait_alias): Suggest adding `Self: Trait` to
// `trait Alias = where Self::Proj:;` with `trait Trait { type Proj; }`.
if generics
.bounds_for_param(ty_param_def_id)
.flat_map(|pred| pred.bounds.iter())
.any(|b| match b {
hir::GenericBound::Trait(t, ..) => {
t.trait_ref.trait_def_id() == Some(best_trait)
}
_ => false,
},
) {
})
{
// The type param already has a bound for `trait_name`, we just need to
// change the associated item.
err.sugg = Some(errors::AssocItemNotFoundSugg::SimilarInOtherTrait {
Expand All @@ -242,48 +251,60 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
return self.dcx().emit_err(err);
}

let mut err = self.dcx().create_err(err);
if suggest_constraining_type_param(
tcx,
generics,
&mut err,
&ty_param_name,
&trait_name,
None,
None,
) && suggested_name != assoc_name.name
let trait_args = &ty::GenericArgs::identity_for_item(tcx, best_trait)[1..];
let mut trait_ref = trait_name.clone();
let applicability = if let [arg, args @ ..] = trait_args {
use std::fmt::Write;
write!(trait_ref, "</* {arg}").unwrap();
args.iter().try_for_each(|arg| write!(trait_ref, ", {arg}")).unwrap();
trait_ref += " */>";
Applicability::HasPlaceholders
} else {
Applicability::MaybeIncorrect
};

let identically_named = suggested_name == assoc_name.name;

if let DefKind::TyAlias = tcx.def_kind(item_def_id)
&& !tcx.type_alias_is_lazy(item_def_id)
{
// We suggested constraining a type parameter, but the associated item on it
// was also not an exact match, so we also suggest changing it.
err.span_suggestion_verbose(
assoc_name.span,
fluent::hir_analysis_assoc_item_not_found_similar_in_other_trait_with_bound_sugg,
err.sugg = Some(errors::AssocItemNotFoundSugg::SimilarInOtherTraitQPath {
lo: ty_param_span.shrink_to_lo(),
mi: ty_param_span.shrink_to_hi(),
hi: (!identically_named).then_some(assoc_name.span),
trait_ref,
identically_named,
suggested_name,
Applicability::MaybeIncorrect,
);
applicability,
});
} else {
let mut err = self.dcx().create_err(err);
if suggest_constraining_type_param(
tcx, generics, &mut err, &qself_str, &trait_ref, None, None,
) && !identically_named
{
// We suggested constraining a type parameter, but the associated item on it
// was also not an exact match, so we also suggest changing it.
err.span_suggestion_verbose(
assoc_name.span,
fluent::hir_analysis_assoc_item_not_found_similar_in_other_trait_with_bound_sugg,
suggested_name,
Applicability::MaybeIncorrect,
);
}
return err.emit();
}
return err.emit();
}
return self.dcx().emit_err(err);
}
}

// If we still couldn't find any associated item, and only one associated item exists,
// suggests using it.
// suggest using it.
if let [candidate_name] = all_candidate_names.as_slice() {
// This should still compile, except on `#![feature(associated_type_defaults)]`
// where it could suggests `type A = Self::A`, thus recursing infinitely.
let applicability =
if assoc_kind == ty::AssocKind::Type && tcx.features().associated_type_defaults {
Applicability::Unspecified
} else {
Applicability::MaybeIncorrect
};

err.sugg = Some(errors::AssocItemNotFoundSugg::Other {
span: assoc_name.span,
applicability,
ty_param_name,
qself: &qself_str,
assoc_kind: assoc_kind_str,
suggested_name: *candidate_name,
});
Expand Down Expand Up @@ -349,10 +370,10 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {

self.dcx().emit_err(errors::AssocKindMismatch {
span,
expected: super::assoc_kind_str(expected),
got: super::assoc_kind_str(got),
expected: assoc_kind_str(expected),
got: assoc_kind_str(got),
expected_because_label,
assoc_kind: super::assoc_kind_str(assoc_item.kind),
assoc_kind: assoc_kind_str(assoc_item.kind),
def_span: tcx.def_span(assoc_item.def_id),
bound_on_assoc_const_label,
wrap_in_braces_sugg,
Expand Down Expand Up @@ -746,7 +767,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
if let ([], [bound]) = (&potential_assoc_types[..], &trait_bounds) {
let grandparent = tcx.parent_hir_node(tcx.parent_hir_id(bound.trait_ref.hir_ref_id));
in_expr_or_pat = match grandparent {
Node::Expr(_) | Node::Pat(_) => true,
hir::Node::Expr(_) | hir::Node::Pat(_) => true,
_ => false,
};
match bound.trait_ref.path.segments {
Expand Down Expand Up @@ -1612,3 +1633,11 @@ fn generics_args_err_extend<'a>(
_ => {}
}
}

pub(super) fn assoc_kind_str(kind: ty::AssocKind) -> &'static str {
match kind {
ty::AssocKind::Fn => "function",
ty::AssocKind::Const => "constant",
ty::AssocKind::Type => "type",
}
}
Loading
Loading