Skip to content

Commit da30882

Browse files
TaKO8KiTaKO8Ki
authored
Rollup merge of #118495 - weiznich:more_tests_for_on_unimplemented, r=compiler-errors
Restrict what symbols can be used in `#[diagnostic::on_unimplemented]` format strings This commit restricts what symbols can be used in a format string for any option of the `diagnostic::on_unimplemented` attribute. We previously allowed all the ad-hoc options supported by the internal `#[rustc_on_unimplemented]` attribute. For the stable attribute we only want to support generic parameter names and `{Self}` as parameters. For any other parameter an warning is emitted and the parameter is replaced by the literal parameter string, so for example `{integer}` turns into `{integer}`. This follows the general design of attributes in the `#[diagnostic]` attribute namespace, that any syntax "error" is treated as warning and subsequently ignored. r? `@compiler-errors`
2 parents cf8d812 + 1a1cd6e commit da30882

File tree

6 files changed

+523
-61
lines changed

6 files changed

+523
-61
lines changed

‎compiler/rustc_trait_selection/messages.ftl

+3
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,6 @@ trait_selection_trait_has_no_impls = this trait has no implementations, consider
5555
5656
trait_selection_ty_alias_overflow = in case this is a recursive type alias, consider using a struct, enum, or union instead
5757
trait_selection_unable_to_construct_constant_value = unable to construct a constant value for the unevaluated constant {$unevaluated}
58+
59+
trait_selection_unknown_format_parameter_for_on_unimplemented_attr = there is no parameter `{$argument_name}` on trait `{$trait_name}`
60+
.help = expect either a generic argument name or {"`{Self}`"} as format argument

‎compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs

+95-52
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,11 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
321321
}
322322

323323
#[derive(Clone, Debug)]
324-
pub struct OnUnimplementedFormatString(Symbol, Span);
324+
pub struct OnUnimplementedFormatString {
325+
symbol: Symbol,
326+
span: Span,
327+
is_diagnostic_namespace_variant: bool,
328+
}
325329

326330
#[derive(Debug)]
327331
pub struct OnUnimplementedDirective {
@@ -401,6 +405,14 @@ impl IgnoredDiagnosticOption {
401405
}
402406
}
403407

408+
#[derive(LintDiagnostic)]
409+
#[diag(trait_selection_unknown_format_parameter_for_on_unimplemented_attr)]
410+
#[help]
411+
pub struct UnknownFormatParameterForOnUnimplementedAttr {
412+
argument_name: Symbol,
413+
trait_name: Symbol,
414+
}
415+
404416
impl<'tcx> OnUnimplementedDirective {
405417
fn parse(
406418
tcx: TyCtxt<'tcx>,
@@ -414,8 +426,14 @@ impl<'tcx> OnUnimplementedDirective {
414426
let mut item_iter = items.iter();
415427

416428
let parse_value = |value_str, value_span| {
417-
OnUnimplementedFormatString::try_parse(tcx, item_def_id, value_str, span, value_span)
418-
.map(Some)
429+
OnUnimplementedFormatString::try_parse(
430+
tcx,
431+
item_def_id,
432+
value_str,
433+
value_span,
434+
is_diagnostic_namespace_variant,
435+
)
436+
.map(Some)
419437
};
420438

421439
let condition = if is_root {
@@ -552,15 +570,15 @@ impl<'tcx> OnUnimplementedDirective {
552570
IgnoredDiagnosticOption::maybe_emit_warning(
553571
tcx,
554572
item_def_id,
555-
directive.message.as_ref().map(|f| f.1),
556-
aggr.message.as_ref().map(|f| f.1),
573+
directive.message.as_ref().map(|f| f.span),
574+
aggr.message.as_ref().map(|f| f.span),
557575
"message",
558576
);
559577
IgnoredDiagnosticOption::maybe_emit_warning(
560578
tcx,
561579
item_def_id,
562-
directive.label.as_ref().map(|f| f.1),
563-
aggr.label.as_ref().map(|f| f.1),
580+
directive.label.as_ref().map(|f| f.span),
581+
aggr.label.as_ref().map(|f| f.span),
564582
"label",
565583
);
566584
IgnoredDiagnosticOption::maybe_emit_warning(
@@ -573,8 +591,8 @@ impl<'tcx> OnUnimplementedDirective {
573591
IgnoredDiagnosticOption::maybe_emit_warning(
574592
tcx,
575593
item_def_id,
576-
directive.parent_label.as_ref().map(|f| f.1),
577-
aggr.parent_label.as_ref().map(|f| f.1),
594+
directive.parent_label.as_ref().map(|f| f.span),
595+
aggr.parent_label.as_ref().map(|f| f.span),
578596
"parent_label",
579597
);
580598
IgnoredDiagnosticOption::maybe_emit_warning(
@@ -634,7 +652,7 @@ impl<'tcx> OnUnimplementedDirective {
634652
item_def_id,
635653
value,
636654
attr.span,
637-
attr.span,
655+
is_diagnostic_namespace_variant,
638656
)?),
639657
notes: Vec::new(),
640658
parent_label: None,
@@ -713,7 +731,12 @@ impl<'tcx> OnUnimplementedDirective {
713731
// `with_no_visible_paths` is also used when generating the options,
714732
// so we need to match it here.
715733
ty::print::with_no_visible_paths!(
716-
OnUnimplementedFormatString(v, cfg.span).format(
734+
OnUnimplementedFormatString {
735+
symbol: v,
736+
span: cfg.span,
737+
is_diagnostic_namespace_variant: false
738+
}
739+
.format(
717740
tcx,
718741
trait_ref,
719742
&options_map
@@ -761,20 +784,19 @@ impl<'tcx> OnUnimplementedFormatString {
761784
tcx: TyCtxt<'tcx>,
762785
item_def_id: DefId,
763786
from: Symbol,
764-
err_sp: Span,
765787
value_span: Span,
788+
is_diagnostic_namespace_variant: bool,
766789
) -> Result<Self, ErrorGuaranteed> {
767-
let result = OnUnimplementedFormatString(from, value_span);
768-
result.verify(tcx, item_def_id, err_sp)?;
790+
let result = OnUnimplementedFormatString {
791+
symbol: from,
792+
span: value_span,
793+
is_diagnostic_namespace_variant,
794+
};
795+
result.verify(tcx, item_def_id)?;
769796
Ok(result)
770797
}
771798

772-
fn verify(
773-
&self,
774-
tcx: TyCtxt<'tcx>,
775-
item_def_id: DefId,
776-
span: Span,
777-
) -> Result<(), ErrorGuaranteed> {
799+
fn verify(&self, tcx: TyCtxt<'tcx>, item_def_id: DefId) -> Result<(), ErrorGuaranteed> {
778800
let trait_def_id = if tcx.is_trait(item_def_id) {
779801
item_def_id
780802
} else {
@@ -783,7 +805,7 @@ impl<'tcx> OnUnimplementedFormatString {
783805
};
784806
let trait_name = tcx.item_name(trait_def_id);
785807
let generics = tcx.generics_of(item_def_id);
786-
let s = self.0.as_str();
808+
let s = self.symbol.as_str();
787809
let parser = Parser::new(s, None, None, false, ParseMode::Format);
788810
let mut result = Ok(());
789811
for token in parser {
@@ -793,32 +815,48 @@ impl<'tcx> OnUnimplementedFormatString {
793815
Position::ArgumentNamed(s) => {
794816
match Symbol::intern(s) {
795817
// `{ThisTraitsName}` is allowed
796-
s if s == trait_name => (),
797-
s if ALLOWED_FORMAT_SYMBOLS.contains(&s) => (),
818+
s if s == trait_name && !self.is_diagnostic_namespace_variant => (),
819+
s if ALLOWED_FORMAT_SYMBOLS.contains(&s)
820+
&& !self.is_diagnostic_namespace_variant =>
821+
{
822+
()
823+
}
798824
// So is `{A}` if A is a type parameter
799825
s if generics.params.iter().any(|param| param.name == s) => (),
800826
s => {
801-
result = Err(struct_span_err!(
802-
tcx.sess,
803-
span,
804-
E0230,
805-
"there is no parameter `{}` on {}",
806-
s,
807-
if trait_def_id == item_def_id {
808-
format!("trait `{trait_name}`")
809-
} else {
810-
"impl".to_string()
811-
}
812-
)
813-
.emit());
827+
if self.is_diagnostic_namespace_variant {
828+
tcx.emit_spanned_lint(
829+
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
830+
tcx.local_def_id_to_hir_id(item_def_id.expect_local()),
831+
self.span,
832+
UnknownFormatParameterForOnUnimplementedAttr {
833+
argument_name: s,
834+
trait_name,
835+
},
836+
);
837+
} else {
838+
result = Err(struct_span_err!(
839+
tcx.sess,
840+
self.span,
841+
E0230,
842+
"there is no parameter `{}` on {}",
843+
s,
844+
if trait_def_id == item_def_id {
845+
format!("trait `{trait_name}`")
846+
} else {
847+
"impl".to_string()
848+
}
849+
)
850+
.emit());
851+
}
814852
}
815853
}
816854
}
817855
// `{:1}` and `{}` are not to be used
818856
Position::ArgumentIs(..) | Position::ArgumentImplicitlyIs(_) => {
819857
let reported = struct_span_err!(
820858
tcx.sess,
821-
span,
859+
self.span,
822860
E0231,
823861
"only named substitution parameters are allowed"
824862
)
@@ -857,45 +895,50 @@ impl<'tcx> OnUnimplementedFormatString {
857895
.collect::<FxHashMap<Symbol, String>>();
858896
let empty_string = String::new();
859897

860-
let s = self.0.as_str();
898+
let s = self.symbol.as_str();
861899
let parser = Parser::new(s, None, None, false, ParseMode::Format);
862900
let item_context = (options.get(&sym::ItemContext)).unwrap_or(&empty_string);
863901
parser
864902
.map(|p| match p {
865-
Piece::String(s) => s,
903+
Piece::String(s) => s.to_owned(),
866904
Piece::NextArgument(a) => match a.position {
867-
Position::ArgumentNamed(s) => {
868-
let s = Symbol::intern(s);
905+
Position::ArgumentNamed(arg) => {
906+
let s = Symbol::intern(arg);
869907
match generic_map.get(&s) {
870-
Some(val) => val,
871-
None if s == name => &trait_str,
908+
Some(val) => val.to_string(),
909+
None if self.is_diagnostic_namespace_variant => {
910+
format!("{{{arg}}}")
911+
}
912+
None if s == name => trait_str.clone(),
872913
None => {
873914
if let Some(val) = options.get(&s) {
874-
val
915+
val.clone()
875916
} else if s == sym::from_desugaring {
876917
// don't break messages using these two arguments incorrectly
877-
&empty_string
878-
} else if s == sym::ItemContext {
879-
item_context
918+
String::new()
919+
} else if s == sym::ItemContext
920+
&& !self.is_diagnostic_namespace_variant
921+
{
922+
item_context.clone()
880923
} else if s == sym::integral {
881-
"{integral}"
924+
String::from("{integral}")
882925
} else if s == sym::integer_ {
883-
"{integer}"
926+
String::from("{integer}")
884927
} else if s == sym::float {
885-
"{float}"
928+
String::from("{float}")
886929
} else {
887930
bug!(
888931
"broken on_unimplemented {:?} for {:?}: \
889932
no argument matching {:?}",
890-
self.0,
933+
self.symbol,
891934
trait_ref,
892935
s
893936
)
894937
}
895938
}
896939
}
897940
}
898-
_ => bug!("broken on_unimplemented {:?} - bad format arg", self.0),
941+
_ => bug!("broken on_unimplemented {:?} - bad format arg", self.symbol),
899942
},
900943
})
901944
.collect()

‎tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.rs

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
#![feature(diagnostic_namespace)]
2+
3+
#[diagnostic::on_unimplemented(
4+
on(_Self = "&str"),
5+
//~^WARN malformed `on_unimplemented` attribute
6+
//~|WARN malformed `on_unimplemented` attribute
7+
message = "trait has `{Self}` and `{T}` as params",
8+
label = "trait has `{Self}` and `{T}` as params",
9+
note = "trait has `{Self}` and `{T}` as params",
10+
parent_label = "in this scope",
11+
//~^WARN malformed `on_unimplemented` attribute
12+
//~|WARN malformed `on_unimplemented` attribute
13+
append_const_msg
14+
//~^WARN malformed `on_unimplemented` attribute
15+
//~|WARN malformed `on_unimplemented` attribute
16+
)]
17+
trait Foo<T> {}
18+
19+
#[diagnostic::on_unimplemented = "Message"]
20+
//~^WARN malformed `on_unimplemented` attribute
21+
//~|WARN malformed `on_unimplemented` attribute
22+
trait Bar {}
23+
24+
#[diagnostic::on_unimplemented(message = "Not allowed to apply it on a impl")]
25+
//~^WARN #[diagnostic::on_unimplemented]` can only be applied to trait definitions
26+
impl Bar for i32 {}
27+
28+
// cannot use special rustc_on_unimplement symbols
29+
// in the format string
30+
#[diagnostic::on_unimplemented(
31+
message = "{from_desugaring}{direct}{cause}{integral}{integer}",
32+
//~^WARN there is no parameter `from_desugaring` on trait `Baz`
33+
//~|WARN there is no parameter `from_desugaring` on trait `Baz`
34+
//~|WARN there is no parameter `direct` on trait `Baz`
35+
//~|WARN there is no parameter `direct` on trait `Baz`
36+
//~|WARN there is no parameter `cause` on trait `Baz`
37+
//~|WARN there is no parameter `cause` on trait `Baz`
38+
//~|WARN there is no parameter `integral` on trait `Baz`
39+
//~|WARN there is no parameter `integral` on trait `Baz`
40+
//~|WARN there is no parameter `integer` on trait `Baz`
41+
//~|WARN there is no parameter `integer` on trait `Baz`
42+
label = "{float}{_Self}{crate_local}{Trait}{ItemContext}"
43+
//~^WARN there is no parameter `float` on trait `Baz`
44+
//~|WARN there is no parameter `float` on trait `Baz`
45+
//~|WARN there is no parameter `_Self` on trait `Baz`
46+
//~|WARN there is no parameter `_Self` on trait `Baz`
47+
//~|WARN there is no parameter `crate_local` on trait `Baz`
48+
//~|WARN there is no parameter `crate_local` on trait `Baz`
49+
//~|WARN there is no parameter `Trait` on trait `Baz`
50+
//~|WARN there is no parameter `Trait` on trait `Baz`
51+
//~|WARN there is no parameter `ItemContext` on trait `Baz`
52+
//~|WARN there is no parameter `ItemContext` on trait `Baz`
53+
)]
54+
trait Baz {}
55+
56+
fn takes_foo(_: impl Foo<i32>) {}
57+
fn takes_bar(_: impl Bar) {}
58+
fn takes_baz(_: impl Baz) {}
59+
60+
fn main() {
61+
takes_foo(());
62+
//~^ERROR trait has `()` and `i32` as params
63+
takes_bar(());
64+
//~^ERROR the trait bound `(): Bar` is not satisfied
65+
takes_baz(());
66+
//~^ERROR {from_desugaring}{direct}{cause}{integral}{integer}
67+
}

0 commit comments

Comments
 (0)