diff --git a/compiler/rustc_trait_selection/messages.ftl b/compiler/rustc_trait_selection/messages.ftl index 0dcba0e05f76f..f96bd98523774 100644 --- a/compiler/rustc_trait_selection/messages.ftl +++ b/compiler/rustc_trait_selection/messages.ftl @@ -19,6 +19,9 @@ trait_selection_closure_kind_mismatch = expected a closure that implements the ` trait_selection_closure_kind_requirement = the requirement to implement `{$trait_prefix}{$expected}` derives from here +trait_selection_disallowed_positional_argument = positional format arguments are not allowed here + .help = only named format arguments with the name of one of the generic types are allowed in this context + trait_selection_dump_vtable_entries = vtable entries for `{$trait_ref}`: {$entries} trait_selection_empty_on_clause_in_rustc_on_unimplemented = empty `on`-clause in `#[rustc_on_unimplemented]` @@ -30,6 +33,9 @@ trait_selection_ignored_diagnostic_option = `{$option_name}` is ignored due to p trait_selection_inherent_projection_normalization_overflow = overflow evaluating associated type `{$ty}` +trait_selection_invalid_format_specifier = invalid format specifier + .help = no format specifier are supported in this position + trait_selection_invalid_on_clause_in_rustc_on_unimplemented = invalid `on`-clause in `#[rustc_on_unimplemented]` .label = invalid on-clause here @@ -60,3 +66,6 @@ trait_selection_unable_to_construct_constant_value = unable to construct a const 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 + +trait_selection_wrapped_parser_error = {$description} + .label = {$label} diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs index 126bc0c9ec0fc..9e7a53df31994 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs @@ -367,6 +367,23 @@ pub struct UnknownFormatParameterForOnUnimplementedAttr { trait_name: Symbol, } +#[derive(LintDiagnostic)] +#[diag(trait_selection_disallowed_positional_argument)] +#[help] +pub struct DisallowedPositionalArgument; + +#[derive(LintDiagnostic)] +#[diag(trait_selection_invalid_format_specifier)] +#[help] +pub struct InvalidFormatSpecifier; + +#[derive(LintDiagnostic)] +#[diag(trait_selection_wrapped_parser_error)] +pub struct WrappedParserError { + description: String, + label: String, +} + impl<'tcx> OnUnimplementedDirective { fn parse( tcx: TyCtxt<'tcx>, @@ -758,64 +775,108 @@ impl<'tcx> OnUnimplementedFormatString { let trait_name = tcx.item_name(trait_def_id); let generics = tcx.generics_of(item_def_id); let s = self.symbol.as_str(); - let parser = Parser::new(s, None, None, false, ParseMode::Format); + let mut parser = Parser::new(s, None, None, false, ParseMode::Format); let mut result = Ok(()); - for token in parser { + for token in &mut parser { match token { Piece::String(_) => (), // Normal string, no need to check it - Piece::NextArgument(a) => match a.position { - Position::ArgumentNamed(s) => { - match Symbol::intern(s) { - // `{ThisTraitsName}` is allowed - 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 => { - if self.is_diagnostic_namespace_variant { - tcx.emit_node_span_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_code_err!( - tcx.dcx(), - 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()); + Piece::NextArgument(a) => { + let format_spec = a.format; + if self.is_diagnostic_namespace_variant + && (format_spec.ty_span.is_some() + || format_spec.width_span.is_some() + || format_spec.precision_span.is_some() + || format_spec.fill_span.is_some()) + { + tcx.emit_node_span_lint( + UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, + tcx.local_def_id_to_hir_id(item_def_id.expect_local()), + self.span, + InvalidFormatSpecifier, + ); + } + match a.position { + Position::ArgumentNamed(s) => { + match Symbol::intern(s) { + // `{ThisTraitsName}` is allowed + 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 => { + if self.is_diagnostic_namespace_variant { + tcx.emit_node_span_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_code_err!( + tcx.dcx(), + 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(_) => { + if self.is_diagnostic_namespace_variant { + tcx.emit_node_span_lint( + UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, + tcx.local_def_id_to_hir_id(item_def_id.expect_local()), + self.span, + DisallowedPositionalArgument, + ); + } else { + let reported = struct_span_code_err!( + tcx.dcx(), + self.span, + E0231, + "only named generic parameters are allowed" + ) + .emit(); + result = Err(reported); + } + } } - // `{:1}` and `{}` are not to be used - Position::ArgumentIs(..) | Position::ArgumentImplicitlyIs(_) => { - let reported = struct_span_code_err!( - tcx.dcx(), - self.span, - E0231, - "only named generic parameters are allowed" - ) - .emit(); - result = Err(reported); - } - }, + } + } + } + // we cannot return errors from processing the format string as hard error here + // as the diagnostic namespace gurantees that malformed input cannot cause an error + // + // if we encounter any error while processing we nevertheless want to show it as warning + // so that users are aware that something is not correct + for e in parser.errors { + if self.is_diagnostic_namespace_variant { + tcx.emit_node_span_lint( + UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, + tcx.local_def_id_to_hir_id(item_def_id.expect_local()), + self.span, + WrappedParserError { description: e.description, label: e.label }, + ); + } else { + let reported = + struct_span_code_err!(tcx.dcx(), self.span, E0231, "{}", e.description,).emit(); + result = Err(reported); } } @@ -853,9 +914,9 @@ impl<'tcx> OnUnimplementedFormatString { let empty_string = String::new(); let s = self.symbol.as_str(); - let parser = Parser::new(s, None, None, false, ParseMode::Format); + let mut parser = Parser::new(s, None, None, false, ParseMode::Format); let item_context = (options.get(&sym::ItemContext)).unwrap_or(&empty_string); - parser + let constructed_message = (&mut parser) .map(|p| match p { Piece::String(s) => s.to_owned(), Piece::NextArgument(a) => match a.position { @@ -895,9 +956,29 @@ impl<'tcx> OnUnimplementedFormatString { } } } + Position::ArgumentImplicitlyIs(_) if self.is_diagnostic_namespace_variant => { + String::from("{}") + } + Position::ArgumentIs(idx) if self.is_diagnostic_namespace_variant => { + format!("{{{idx}}}") + } _ => bug!("broken on_unimplemented {:?} - bad format arg", self.symbol), }, }) - .collect() + .collect(); + // we cannot return errors from processing the format string as hard error here + // as the diagnostic namespace gurantees that malformed input cannot cause an error + // + // if we encounter any error while processing the format string + // we don't want to show the potentially half assembled formated string, + // therefore we fall back to just showing the input string in this case + // + // The actual parser errors are emitted earlier + // as lint warnings in OnUnimplementedFormatString::verify + if self.is_diagnostic_namespace_variant && !parser.errors.is_empty() { + String::from(s) + } else { + constructed_message + } } } diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/broken_format.rs b/tests/ui/diagnostic_namespace/on_unimplemented/broken_format.rs new file mode 100644 index 0000000000000..8d8917fd319cb --- /dev/null +++ b/tests/ui/diagnostic_namespace/on_unimplemented/broken_format.rs @@ -0,0 +1,45 @@ +#[diagnostic::on_unimplemented(message = "{{Test } thing")] +//~^WARN unmatched `}` found +//~|WARN unmatched `}` found +trait ImportantTrait1 {} + +#[diagnostic::on_unimplemented(message = "Test {}")] +//~^WARN positional format arguments are not allowed here +//~|WARN positional format arguments are not allowed here +trait ImportantTrait2 {} + +#[diagnostic::on_unimplemented(message = "Test {1:}")] +//~^WARN positional format arguments are not allowed here +//~|WARN positional format arguments are not allowed here +trait ImportantTrait3 {} + +#[diagnostic::on_unimplemented(message = "Test {Self:123}")] +//~^WARN invalid format specifier +//~|WARN invalid format specifier +trait ImportantTrait4 {} + +#[diagnostic::on_unimplemented(message = "Test {Self:!}")] +//~^WARN expected `'}'`, found `'!'` +//~|WARN expected `'}'`, found `'!'` +//~|WARN unmatched `}` found +//~|WARN unmatched `}` found +trait ImportantTrait5 {} + +fn check_1(_: impl ImportantTrait1) {} +fn check_2(_: impl ImportantTrait2) {} +fn check_3(_: impl ImportantTrait3) {} +fn check_4(_: impl ImportantTrait4) {} +fn check_5(_: impl ImportantTrait5) {} + +fn main() { + check_1(()); + //~^ERROR {{Test } thing + check_2(()); + //~^ERROR Test {} + check_3(()); + //~^ERROR Test {1} + check_4(()); + //~^ERROR Test () + check_5(()); + //~^ERROR Test {Self:!} +} diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/broken_format.stderr b/tests/ui/diagnostic_namespace/on_unimplemented/broken_format.stderr new file mode 100644 index 0000000000000..932e81ca48e49 --- /dev/null +++ b/tests/ui/diagnostic_namespace/on_unimplemented/broken_format.stderr @@ -0,0 +1,193 @@ +warning: unmatched `}` found + --> $DIR/broken_format.rs:1:32 + | +LL | #[diagnostic::on_unimplemented(message = "{{Test } thing")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(unknown_or_malformed_diagnostic_attributes)]` on by default + +warning: positional format arguments are not allowed here + --> $DIR/broken_format.rs:6:32 + | +LL | #[diagnostic::on_unimplemented(message = "Test {}")] + | ^^^^^^^^^^^^^^^^^^^ + | + = help: only named format arguments with the name of one of the generic types are allowed in this context + +warning: positional format arguments are not allowed here + --> $DIR/broken_format.rs:11:32 + | +LL | #[diagnostic::on_unimplemented(message = "Test {1:}")] + | ^^^^^^^^^^^^^^^^^^^^^ + | + = help: only named format arguments with the name of one of the generic types are allowed in this context + +warning: invalid format specifier + --> $DIR/broken_format.rs:16:32 + | +LL | #[diagnostic::on_unimplemented(message = "Test {Self:123}")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: no format specifier are supported in this position + +warning: expected `'}'`, found `'!'` + --> $DIR/broken_format.rs:21:32 + | +LL | #[diagnostic::on_unimplemented(message = "Test {Self:!}")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: unmatched `}` found + --> $DIR/broken_format.rs:21:32 + | +LL | #[diagnostic::on_unimplemented(message = "Test {Self:!}")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: unmatched `}` found + --> $DIR/broken_format.rs:1:32 + | +LL | #[diagnostic::on_unimplemented(message = "{{Test } thing")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error[E0277]: {{Test } thing + --> $DIR/broken_format.rs:35:13 + | +LL | check_1(()); + | ------- ^^ the trait `ImportantTrait1` is not implemented for `()` + | | + | required by a bound introduced by this call + | +help: this trait has no implementations, consider adding one + --> $DIR/broken_format.rs:4:1 + | +LL | trait ImportantTrait1 {} + | ^^^^^^^^^^^^^^^^^^^^^ +note: required by a bound in `check_1` + --> $DIR/broken_format.rs:28:20 + | +LL | fn check_1(_: impl ImportantTrait1) {} + | ^^^^^^^^^^^^^^^ required by this bound in `check_1` + +warning: positional format arguments are not allowed here + --> $DIR/broken_format.rs:6:32 + | +LL | #[diagnostic::on_unimplemented(message = "Test {}")] + | ^^^^^^^^^^^^^^^^^^^ + | + = help: only named format arguments with the name of one of the generic types are allowed in this context + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error[E0277]: Test {} + --> $DIR/broken_format.rs:37:13 + | +LL | check_2(()); + | ------- ^^ the trait `ImportantTrait2` is not implemented for `()` + | | + | required by a bound introduced by this call + | +help: this trait has no implementations, consider adding one + --> $DIR/broken_format.rs:9:1 + | +LL | trait ImportantTrait2 {} + | ^^^^^^^^^^^^^^^^^^^^^ +note: required by a bound in `check_2` + --> $DIR/broken_format.rs:29:20 + | +LL | fn check_2(_: impl ImportantTrait2) {} + | ^^^^^^^^^^^^^^^ required by this bound in `check_2` + +warning: positional format arguments are not allowed here + --> $DIR/broken_format.rs:11:32 + | +LL | #[diagnostic::on_unimplemented(message = "Test {1:}")] + | ^^^^^^^^^^^^^^^^^^^^^ + | + = help: only named format arguments with the name of one of the generic types are allowed in this context + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error[E0277]: Test {1} + --> $DIR/broken_format.rs:39:13 + | +LL | check_3(()); + | ------- ^^ the trait `ImportantTrait3` is not implemented for `()` + | | + | required by a bound introduced by this call + | +help: this trait has no implementations, consider adding one + --> $DIR/broken_format.rs:14:1 + | +LL | trait ImportantTrait3 {} + | ^^^^^^^^^^^^^^^^^^^^^ +note: required by a bound in `check_3` + --> $DIR/broken_format.rs:30:20 + | +LL | fn check_3(_: impl ImportantTrait3) {} + | ^^^^^^^^^^^^^^^ required by this bound in `check_3` + +warning: invalid format specifier + --> $DIR/broken_format.rs:16:32 + | +LL | #[diagnostic::on_unimplemented(message = "Test {Self:123}")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: no format specifier are supported in this position + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error[E0277]: Test () + --> $DIR/broken_format.rs:41:13 + | +LL | check_4(()); + | ------- ^^ the trait `ImportantTrait4` is not implemented for `()` + | | + | required by a bound introduced by this call + | +help: this trait has no implementations, consider adding one + --> $DIR/broken_format.rs:19:1 + | +LL | trait ImportantTrait4 {} + | ^^^^^^^^^^^^^^^^^^^^^ +note: required by a bound in `check_4` + --> $DIR/broken_format.rs:31:20 + | +LL | fn check_4(_: impl ImportantTrait4) {} + | ^^^^^^^^^^^^^^^ required by this bound in `check_4` + +warning: expected `'}'`, found `'!'` + --> $DIR/broken_format.rs:21:32 + | +LL | #[diagnostic::on_unimplemented(message = "Test {Self:!}")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +warning: unmatched `}` found + --> $DIR/broken_format.rs:21:32 + | +LL | #[diagnostic::on_unimplemented(message = "Test {Self:!}")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error[E0277]: Test {Self:!} + --> $DIR/broken_format.rs:43:13 + | +LL | check_5(()); + | ------- ^^ the trait `ImportantTrait5` is not implemented for `()` + | | + | required by a bound introduced by this call + | +help: this trait has no implementations, consider adding one + --> $DIR/broken_format.rs:26:1 + | +LL | trait ImportantTrait5 {} + | ^^^^^^^^^^^^^^^^^^^^^ +note: required by a bound in `check_5` + --> $DIR/broken_format.rs:32:20 + | +LL | fn check_5(_: impl ImportantTrait5) {} + | ^^^^^^^^^^^^^^^ required by this bound in `check_5` + +error: aborting due to 5 previous errors; 12 warnings emitted + +For more information about this error, try `rustc --explain E0277`.