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.

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

Jump to bottom

Restrict what symbols can be used in #[diagnostic::on_unimplemented] format strings #118495

Merged
merged 1 commit into from
Dec 4, 2023
Merged

Restrict what symbols can be used in #[diagnostic::on_unimplemented] format strings #118495

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions compiler/rustc_trait_selection/messages.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,6 @@ trait_selection_trait_has_no_impls = this trait has no implementations, consider

trait_selection_ty_alias_overflow = in case this is a recursive type alias, consider using a struct, enum, or union instead
trait_selection_unable_to_construct_constant_value = unable to construct a constant value for the unevaluated constant {$unevaluated}

trait_selection_unknown_format_parameter_for_on_unimplemented_attr = there is no parameter `{$argument_name}` on trait `{$trait_name}`
.help = expect either a generic argument name or {"`{Self}`"} as format argument
147 changes: 95 additions & 52 deletions compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,11 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
}

#[derive(Clone, Debug)]
pub struct OnUnimplementedFormatString(Symbol, Span);
pub struct OnUnimplementedFormatString {
symbol: Symbol,
span: Span,
is_diagnostic_namespace_variant: bool,
}

#[derive(Debug)]
pub struct OnUnimplementedDirective {
Expand Down Expand Up @@ -401,6 +405,14 @@ impl IgnoredDiagnosticOption {
}
}

#[derive(LintDiagnostic)]
#[diag(trait_selection_unknown_format_parameter_for_on_unimplemented_attr)]
#[help]
pub struct UnknownFormatParameterForOnUnimplementedAttr {
argument_name: Symbol,
trait_name: Symbol,
}

impl<'tcx> OnUnimplementedDirective {
fn parse(
tcx: TyCtxt<'tcx>,
Expand All @@ -414,8 +426,14 @@ impl<'tcx> OnUnimplementedDirective {
let mut item_iter = items.iter();

let parse_value = |value_str, value_span| {
OnUnimplementedFormatString::try_parse(tcx, item_def_id, value_str, span, value_span)
.map(Some)
OnUnimplementedFormatString::try_parse(
tcx,
item_def_id,
value_str,
value_span,
is_diagnostic_namespace_variant,
)
.map(Some)
};

let condition = if is_root {
Expand Down Expand Up @@ -552,15 +570,15 @@ impl<'tcx> OnUnimplementedDirective {
IgnoredDiagnosticOption::maybe_emit_warning(
tcx,
item_def_id,
directive.message.as_ref().map(|f| f.1),
aggr.message.as_ref().map(|f| f.1),
directive.message.as_ref().map(|f| f.span),
aggr.message.as_ref().map(|f| f.span),
"message",
);
IgnoredDiagnosticOption::maybe_emit_warning(
tcx,
item_def_id,
directive.label.as_ref().map(|f| f.1),
aggr.label.as_ref().map(|f| f.1),
directive.label.as_ref().map(|f| f.span),
aggr.label.as_ref().map(|f| f.span),
"label",
);
IgnoredDiagnosticOption::maybe_emit_warning(
Expand All @@ -573,8 +591,8 @@ impl<'tcx> OnUnimplementedDirective {
IgnoredDiagnosticOption::maybe_emit_warning(
tcx,
item_def_id,
directive.parent_label.as_ref().map(|f| f.1),
aggr.parent_label.as_ref().map(|f| f.1),
directive.parent_label.as_ref().map(|f| f.span),
aggr.parent_label.as_ref().map(|f| f.span),
"parent_label",
);
IgnoredDiagnosticOption::maybe_emit_warning(
Expand Down Expand Up @@ -634,7 +652,7 @@ impl<'tcx> OnUnimplementedDirective {
item_def_id,
value,
attr.span,
attr.span,
is_diagnostic_namespace_variant,
)?),
notes: Vec::new(),
parent_label: None,
Expand Down Expand Up @@ -712,7 +730,12 @@ impl<'tcx> OnUnimplementedDirective {
// `with_no_visible_paths` is also used when generating the options,
// so we need to match it here.
ty::print::with_no_visible_paths!(
OnUnimplementedFormatString(v, cfg.span).format(
OnUnimplementedFormatString {
symbol: v,
span: cfg.span,
is_diagnostic_namespace_variant: false
}
.format(
tcx,
trait_ref,
&options_map
Expand Down Expand Up @@ -760,20 +783,19 @@ impl<'tcx> OnUnimplementedFormatString {
tcx: TyCtxt<'tcx>,
item_def_id: DefId,
from: Symbol,
err_sp: Span,
value_span: Span,
is_diagnostic_namespace_variant: bool,
) -> Result<Self, ErrorGuaranteed> {
let result = OnUnimplementedFormatString(from, value_span);
result.verify(tcx, item_def_id, err_sp)?;
let result = OnUnimplementedFormatString {
symbol: from,
span: value_span,
is_diagnostic_namespace_variant,
};
result.verify(tcx, item_def_id)?;
Ok(result)
}

fn verify(
&self,
tcx: TyCtxt<'tcx>,
item_def_id: DefId,
span: Span,
) -> Result<(), ErrorGuaranteed> {
fn verify(&self, tcx: TyCtxt<'tcx>, item_def_id: DefId) -> Result<(), ErrorGuaranteed> {
let trait_def_id = if tcx.is_trait(item_def_id) {
item_def_id
} else {
Expand All @@ -782,7 +804,7 @@ impl<'tcx> OnUnimplementedFormatString {
};
let trait_name = tcx.item_name(trait_def_id);
let generics = tcx.generics_of(item_def_id);
let s = self.0.as_str();
let s = self.symbol.as_str();
let parser = Parser::new(s, None, None, false, ParseMode::Format);
let mut result = Ok(());
for token in parser {
Expand All @@ -792,32 +814,48 @@ impl<'tcx> OnUnimplementedFormatString {
Position::ArgumentNamed(s) => {
match Symbol::intern(s) {
// `{ThisTraitsName}` is allowed
s if s == trait_name => (),
s if ALLOWED_FORMAT_SYMBOLS.contains(&s) => (),
s if s == trait_name && !self.is_diagnostic_namespace_variant => (),
s if ALLOWED_FORMAT_SYMBOLS.contains(&s)
&& !self.is_diagnostic_namespace_variant =>
{
()
}
// So is `{A}` if A is a type parameter
s if generics.params.iter().any(|param| param.name == s) => (),
s => {
result = Err(struct_span_err!(
tcx.sess,
span,
E0230,
"there is no parameter `{}` on {}",
s,
if trait_def_id == item_def_id {
format!("trait `{trait_name}`")
} else {
"impl".to_string()
}
)
.emit());
if self.is_diagnostic_namespace_variant {
tcx.emit_spanned_lint(
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
tcx.local_def_id_to_hir_id(item_def_id.expect_local()),
self.span,
UnknownFormatParameterForOnUnimplementedAttr {
argument_name: s,
trait_name,
},
);
} else {
result = Err(struct_span_err!(
tcx.sess,
self.span,
E0230,
"there is no parameter `{}` on {}",
s,
if trait_def_id == item_def_id {
format!("trait `{trait_name}`")
} else {
"impl".to_string()
}
)
.emit());
}
}
}
}
// `{:1}` and `{}` are not to be used
Position::ArgumentIs(..) | Position::ArgumentImplicitlyIs(_) => {
let reported = struct_span_err!(
tcx.sess,
span,
self.span,
E0231,
"only named substitution parameters are allowed"
)
Expand Down Expand Up @@ -856,45 +894,50 @@ impl<'tcx> OnUnimplementedFormatString {
.collect::<FxHashMap<Symbol, String>>();
let empty_string = String::new();

let s = self.0.as_str();
let s = self.symbol.as_str();
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
let parser = Parser::new(s, None, None, false, ParseMode::Format);
let item_context = (options.get(&sym::ItemContext)).unwrap_or(&empty_string);
parser
.map(|p| match p {
Piece::String(s) => s,
Piece::String(s) => s.to_owned(),
Piece::NextArgument(a) => match a.position {
Position::ArgumentNamed(s) => {
let s = Symbol::intern(s);
Position::ArgumentNamed(arg) => {
let s = Symbol::intern(arg);
match generic_map.get(&s) {
Some(val) => val,
None if s == name => &trait_str,
Some(val) => val.to_string(),
None if self.is_diagnostic_namespace_variant => {
format!("{{{arg}}}")
}
None if s == name => trait_str.clone(),
None => {
if let Some(val) = options.get(&s) {
val
val.clone()
} else if s == sym::from_desugaring {
// don't break messages using these two arguments incorrectly
&empty_string
} else if s == sym::ItemContext {
item_context
String::new()
} else if s == sym::ItemContext
&& !self.is_diagnostic_namespace_variant
{
item_context.clone()
} else if s == sym::integral {
"{integral}"
String::from("{integral}")
} else if s == sym::integer_ {
"{integer}"
String::from("{integer}")
} else if s == sym::float {
"{float}"
String::from("{float}")
} else {
bug!(
"broken on_unimplemented {:?} for {:?}: \
no argument matching {:?}",
self.0,
self.symbol,
trait_ref,
s
)
}
}
}
}
_ => bug!("broken on_unimplemented {:?} - bad format arg", self.0),
_ => bug!("broken on_unimplemented {:?} - bad format arg", self.symbol),
},
})
.collect()
Expand Down
67 changes: 67 additions & 0 deletions ...ostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
#![feature(diagnostic_namespace)]

#[diagnostic::on_unimplemented(
on(_Self = "&str"),
//~^WARN malformed `on_unimplemented` attribute
//~|WARN malformed `on_unimplemented` attribute
message = "trait has `{Self}` and `{T}` as params",
label = "trait has `{Self}` and `{T}` as params",
note = "trait has `{Self}` and `{T}` as params",
parent_label = "in this scope",
//~^WARN malformed `on_unimplemented` attribute
//~|WARN malformed `on_unimplemented` attribute
append_const_msg
//~^WARN malformed `on_unimplemented` attribute
//~|WARN malformed `on_unimplemented` attribute
)]
trait Foo<T> {}

#[diagnostic::on_unimplemented = "Message"]
//~^WARN malformed `on_unimplemented` attribute
//~|WARN malformed `on_unimplemented` attribute
trait Bar {}

#[diagnostic::on_unimplemented(message = "Not allowed to apply it on a impl")]
//~^WARN #[diagnostic::on_unimplemented]` can only be applied to trait definitions
impl Bar for i32 {}

// cannot use special rustc_on_unimplement symbols
// in the format string
#[diagnostic::on_unimplemented(
message = "{from_desugaring}{direct}{cause}{integral}{integer}",
//~^WARN there is no parameter `from_desugaring` on trait `Baz`
//~|WARN there is no parameter `from_desugaring` on trait `Baz`
//~|WARN there is no parameter `direct` on trait `Baz`
//~|WARN there is no parameter `direct` on trait `Baz`
//~|WARN there is no parameter `cause` on trait `Baz`
//~|WARN there is no parameter `cause` on trait `Baz`
//~|WARN there is no parameter `integral` on trait `Baz`
//~|WARN there is no parameter `integral` on trait `Baz`
//~|WARN there is no parameter `integer` on trait `Baz`
//~|WARN there is no parameter `integer` on trait `Baz`
label = "{float}{_Self}{crate_local}{Trait}{ItemContext}"
//~^WARN there is no parameter `float` on trait `Baz`
//~|WARN there is no parameter `float` on trait `Baz`
//~|WARN there is no parameter `_Self` on trait `Baz`
//~|WARN there is no parameter `_Self` on trait `Baz`
//~|WARN there is no parameter `crate_local` on trait `Baz`
//~|WARN there is no parameter `crate_local` on trait `Baz`
//~|WARN there is no parameter `Trait` on trait `Baz`
//~|WARN there is no parameter `Trait` on trait `Baz`
//~|WARN there is no parameter `ItemContext` on trait `Baz`
//~|WARN there is no parameter `ItemContext` on trait `Baz`
)]
trait Baz {}

fn takes_foo(_: impl Foo<i32>) {}
fn takes_bar(_: impl Bar) {}
fn takes_baz(_: impl Baz) {}

fn main() {
takes_foo(());
//~^ERROR trait has `()` and `i32` as params
takes_bar(());
//~^ERROR the trait bound `(): Bar` is not satisfied
takes_baz(());
//~^ERROR {from_desugaring}{direct}{cause}{integral}{integer}
}