-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
Introduce #[diagnostic::on_type_error(message)] #155200
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,61 @@ | ||||||
| use rustc_feature::template; | ||||||
| use rustc_hir::attrs::AttributeKind; | ||||||
| use rustc_span::sym; | ||||||
|
|
||||||
| use crate::attributes::diagnostic::*; | ||||||
| use crate::attributes::prelude::*; | ||||||
| use crate::context::{AcceptContext, Stage}; | ||||||
| use crate::parser::ArgParser; | ||||||
| use crate::target_checking::{ALL_TARGETS, AllowedTargets}; | ||||||
|
|
||||||
| #[derive(Default)] | ||||||
| pub(crate) struct OnTypeErrorParser { | ||||||
| span: Option<Span>, | ||||||
| directive: Option<(Span, Directive)>, | ||||||
| } | ||||||
|
|
||||||
| impl OnTypeErrorParser { | ||||||
| fn parse<'sess, S: Stage>( | ||||||
| &mut self, | ||||||
| cx: &mut AcceptContext<'_, 'sess, S>, | ||||||
| args: &ArgParser, | ||||||
| mode: Mode, | ||||||
| ) { | ||||||
| if !cx.features().diagnostic_on_type_error() { | ||||||
| // `UnknownDiagnosticAttribute` is emitted in rustc_resolve/macros.rs | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| let span = cx.attr_span; | ||||||
| self.span = Some(span); | ||||||
|
|
||||||
| let Some(items) = parse_list(cx, args, mode) else { return }; | ||||||
|
|
||||||
| if let Some(directive) = parse_directive_items(cx, mode, items.mixed(), true) { | ||||||
| merge_directives(cx, &mut self.directive, (span, directive)); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| impl<S: Stage> AttributeParser<S> for OnTypeErrorParser { | ||||||
| const ATTRIBUTES: AcceptMapping<Self, S> = &[( | ||||||
| &[sym::diagnostic, sym::on_type_error], | ||||||
| template!(List: &[r#" note = "...""#]), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| |this, cx, args| { | ||||||
| this.parse(cx, args, Mode::DiagnosticOnTypeError); | ||||||
| }, | ||||||
| )]; | ||||||
|
|
||||||
| const ALLOWED_TARGETS: AllowedTargets = AllowedTargets::AllowList(ALL_TARGETS); | ||||||
|
|
||||||
| fn finalize(self, _cx: &FinalizeContext<'_, '_, S>) -> Option<AttributeKind> { | ||||||
| if let Some(span) = self.span { | ||||||
| Some(AttributeKind::OnTypeError { | ||||||
| span, | ||||||
| directive: self.directive.map(|d| Box::new(d.1)), | ||||||
| }) | ||||||
| } else { | ||||||
| None | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -509,6 +509,7 @@ fn try_extract_error_from_region_constraints<'a, 'tcx>( | |||
| .try_report_from_nll() | ||||
| .or_else(|| { | ||||
| if let SubregionOrigin::Subtype(trace) = cause { | ||||
| tracing::info!("borrow checker"); | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
if it's useful to you then leave it for now, but it should be removed at some point
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is noise, will remove it. |
||||
| Some(infcx.err_ctxt().report_and_explain_type_error( | ||||
| *trace, | ||||
| infcx.tcx.param_env(generic_param_scope), | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,6 +81,7 @@ declare_lint_pass! { | |
| NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE, | ||
| NON_CONTIGUOUS_RANGE_ENDPOINTS, | ||
| NON_EXHAUSTIVE_OMITTED_PATTERNS, | ||
| ON_TYPE_ERROR_MULTIPLE_GENERICS, | ||
| OUT_OF_SCOPE_MACRO_CALLS, | ||
| OVERLAPPING_RANGE_ENDPOINTS, | ||
| PATTERNS_IN_FNS_WITHOUT_BODY, | ||
|
|
@@ -4574,6 +4575,43 @@ declare_lint! { | |
| Warn, | ||
| "detects diagnostic attribute with malformed diagnostic format literals", | ||
| } | ||
|
|
||
| declare_lint! { | ||
| /// The `on_type_error_multiple_generics` lint detects when | ||
| /// `#[diagnostic::on_type_error]` is used on items with more than one generic parameter. | ||
| /// | ||
| /// ### Example | ||
| /// | ||
| /// ```rust,ignore (requires nightly feature) | ||
| /// #![feature(diagnostic_on_type_error)] | ||
| /// #[diagnostic::on_type_error(note = "too many generics")] | ||
| /// struct TooMany<T, U>(T, U); | ||
| /// ``` | ||
| /// | ||
| /// This will produce: | ||
| /// | ||
| /// ```text | ||
| /// warning: `#[diagnostic::on_type_error]` only supports one ADT generic parameter, but found `2` | ||
| /// --> lint_example.rs:3:15 | ||
| /// | | ||
| /// 3 | struct TooMany<T, U>(T, U); | ||
| /// | ^^^^^^ | ||
| /// | | ||
| /// = note: `#[warn(on_type_error_multiple_generics)]` on by default | ||
| /// ``` | ||
| /// | ||
| /// ### Explanation | ||
| /// | ||
| /// The `#[diagnostic::on_type_error]` attribute currently only supports items | ||
| /// with a single generic parameter. Using it on an item with multiple generic | ||
| /// parameters will cause the attribute to be ignored. | ||
| /// | ||
| /// [reference]: https://doc.rust-lang.org/nightly/reference/attributes/diagnostics.html#the-diagnostic-tool-attribute-namespace | ||
| pub ON_TYPE_ERROR_MULTIPLE_GENERICS, | ||
| Warn, | ||
| "detects use of #[diagnostic::on_type_error] with multiple generic parameters", | ||
| } | ||
|
|
||
|
Comment on lines
+4579
to
+4602
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This lint feels like an overly specific case of "you're using the attribute wrong". I guess we don't really have a specific lint for that, maybe It also looks like it isn't going to age well when we start expanding what I propose we drop the lint and just use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that is valid. Will remove. |
||
| 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. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,7 +39,7 @@ use rustc_session::config::CrateType; | |
| use rustc_session::lint; | ||
| use rustc_session::lint::builtin::{ | ||
| CONFLICTING_REPR_HINTS, INVALID_DOC_ATTRIBUTES, MALFORMED_DIAGNOSTIC_FORMAT_LITERALS, | ||
| MISPLACED_DIAGNOSTIC_ATTRIBUTES, UNUSED_ATTRIBUTES, | ||
| MISPLACED_DIAGNOSTIC_ATTRIBUTES, ON_TYPE_ERROR_MULTIPLE_GENERICS, UNUSED_ATTRIBUTES, | ||
| }; | ||
| use rustc_session::parse::feature_err; | ||
| use rustc_span::edition::Edition; | ||
|
|
@@ -79,6 +79,10 @@ struct DiagnosticOnUnknownOnlyForImports { | |
| item_span: Span, | ||
| } | ||
|
|
||
| #[derive(Diagnostic)] | ||
| #[diag("`#[diagnostic::on_type_error]` can only be applied to enums, structs or unions")] | ||
| struct DiagnosticOnTypeErrorOnlyForAdt; | ||
|
|
||
| fn target_from_impl_item<'tcx>(tcx: TyCtxt<'tcx>, impl_item: &hir::ImplItem<'_>) -> Target { | ||
| match impl_item.kind { | ||
| hir::ImplItemKind::Const(..) => Target::AssocConst, | ||
|
|
@@ -228,6 +232,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> { | |
| Attribute::Parsed(AttributeKind::OnMove { span, directive }) => { | ||
| self.check_diagnostic_on_move(*span, hir_id, target, directive.as_deref()) | ||
| }, | ||
| Attribute::Parsed(AttributeKind::OnTypeError{ span, directive }) => { | ||
| self.check_diagnostic_on_type_error(*span, hir_id, target, directive.as_deref()) | ||
| }, | ||
| Attribute::Parsed( | ||
| // tidy-alphabetical-start | ||
| AttributeKind::RustcAllowIncoherentImpl(..) | ||
|
|
@@ -688,6 +695,74 @@ impl<'tcx> CheckAttrVisitor<'tcx> { | |
| } | ||
| } | ||
|
|
||
| /// Checks if `#[diagnostic::on_type_error]` is applied to an ADT definition | ||
| fn check_diagnostic_on_type_error( | ||
| &self, | ||
| attr_span: Span, | ||
| hir_id: HirId, | ||
| target: Target, | ||
| directive: Option<&Directive>, | ||
| ) { | ||
| if !matches!(target, Target::Enum | Target::Struct | Target::Union) { | ||
| self.tcx.emit_node_span_lint( | ||
| MISPLACED_DIAGNOSTIC_ATTRIBUTES, | ||
| hir_id, | ||
| attr_span, | ||
| DiagnosticOnTypeErrorOnlyForAdt, | ||
| ); | ||
| } | ||
|
|
||
| if let Some(directive) = directive { | ||
| if let Node::Item(Item { | ||
| kind: | ||
| ItemKind::Struct(_, generics, _) | ||
| | ItemKind::Enum(_, generics, _) | ||
| | ItemKind::Union(_, generics, _), | ||
| .. | ||
| }) = self.tcx.hir_node(hir_id) | ||
| { | ||
| let generic_count = generics | ||
| .params | ||
| .iter() | ||
| .filter(|p| !matches!(p.kind, GenericParamKind::Lifetime { .. })) | ||
| .count(); | ||
|
|
||
| // Enforce: at most one generic | ||
| if generic_count > 1 { | ||
| self.tcx.emit_node_span_lint( | ||
| ON_TYPE_ERROR_MULTIPLE_GENERICS, | ||
| hir_id, | ||
| generics.span, | ||
| errors::OnTypeErrorMultipleGenerics { count: generic_count }, | ||
| ); | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this be exactly one ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes please :) Also can you rename the error type...to something like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will do. |
||
|
|
||
| directive.visit_params(&mut |argument_name, span| { | ||
| let has_generic = generics.params.iter().any(|p| { | ||
| if !matches!(p.kind, GenericParamKind::Lifetime { .. }) | ||
| && let ParamName::Plain(name) = p.name | ||
| && name.name == argument_name | ||
| { | ||
| true | ||
| } else { | ||
| false | ||
| } | ||
| }); | ||
|
|
||
| let is_allowed = argument_name == sym::Expected || argument_name == sym::Found; | ||
| if !(has_generic | is_allowed) { | ||
| self.tcx.emit_node_span_lint( | ||
| MALFORMED_DIAGNOSTIC_FORMAT_LITERALS, | ||
| hir_id, | ||
| span, | ||
| errors::OnTypeErrorMalformedFormatLiterals { name: argument_name }, | ||
| ) | ||
| } | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Checks if an `#[inline]` is applied to a function or a closure. | ||
| fn check_inline(&self, hir_id: HirId, attr_span: Span, kind: &InlineAttr, target: Target) { | ||
| match target { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep in mind that the complexity of this entire match will grow over time as more attributes and options get added. Can you instead change the existing arms to match on the diagnostic attributes that do allow message and label?
With that, I mean:
etc.
While it is more wordy, it is easier to understand from a control flow perspective. You can also do
use Mode::*;inside this function to use the enum variant names by themselves.