diff options
| author | bors <bors@rust-lang.org> | 2022-10-01 10:44:25 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-10-01 10:44:25 +0000 |
| commit | 744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1 (patch) | |
| tree | 1721987352b5f0a8548fc46984821d974b661934 /compiler/rustc_lint/src/internal.rs | |
| parent | 277bb6653b55475b5fbce6309e9852fa2100dabe (diff) | |
| parent | b5b3ffe3fc9cfb524a6432ec60a0fc95c514d2e1 (diff) | |
| download | rust-744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1.tar.gz rust-744e397d8855f7da87d70aa8d0bd9e0f5f0b51a1.zip | |
Auto merge of #101986 - WaffleLapkin:move_lint_note_to_the_bottom, r=estebank
Move lint level source explanation to the bottom So, uhhhhh r? `@estebank` ## User-facing change "note: `#[warn(...)]` on by default" and such are moved to the bottom of the diagnostic: ```diff - = note: `#[warn(unsupported_calling_conventions)]` on by default = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #87678 <https://github.com/rust-lang/rust/issues/87678> + = note: `#[warn(unsupported_calling_conventions)]` on by default ``` Why warning is enabled is the least important thing, so it shouldn't be the first note the user reads, IMO. ## Developer-facing change `struct_span_lint` and similar methods have a different signature. Before: `..., impl for<'a> FnOnce(LintDiagnosticBuilder<'a, ()>)` After: `..., impl Into<DiagnosticMessage>, impl for<'a, 'b> FnOnce(&'b mut DiagnosticBuilder<'a, ()>) -> &'b mut DiagnosticBuilder<'a, ()>` The reason for this is that `struct_span_lint` needs to edit the diagnostic _after_ `decorate` closure is called. This also makes lint code a little bit nicer in my opinion. Another option is to use `impl for<'a> FnOnce(LintDiagnosticBuilder<'a, ()>) -> DiagnosticBuilder<'a, ()>` altough I don't _really_ see reasons to do `let lint = lint.build(message)` everywhere. ## Subtle problem By moving the message outside of the closure (that may not be called if the lint is disabled) `format!(...)` is executed earlier, possibly formatting `Ty` which may call a query that trims paths that crashes the compiler if there were no warnings... I don't think it's that big of a deal, considering that we move from `format!(...)` to `fluent` (which is lazy by-default) anyway, however this required adding a workaround which is unfortunate. ## P.S. I'm sorry, I do not how to make this PR smaller/easier to review. Changes to the lint API affect SO MUCH 😢
Diffstat (limited to 'compiler/rustc_lint/src/internal.rs')
| -rw-r--r-- | compiler/rustc_lint/src/internal.rs | 205 |
1 files changed, 105 insertions, 100 deletions
diff --git a/compiler/rustc_lint/src/internal.rs b/compiler/rustc_lint/src/internal.rs index 9c4f9efde5a..8f5e38fdbcc 100644 --- a/compiler/rustc_lint/src/internal.rs +++ b/compiler/rustc_lint/src/internal.rs @@ -34,13 +34,16 @@ impl LateLintPass<'_> for DefaultHashTypes { Some(sym::HashSet) => "FxHashSet", _ => return, }; - cx.struct_span_lint(DEFAULT_HASH_TYPES, path.span, |lint| { - lint.build(fluent::lint::default_hash_types) - .set_arg("preferred", replace) - .set_arg("used", cx.tcx.item_name(def_id)) - .note(fluent::lint::note) - .emit(); - }); + cx.struct_span_lint( + DEFAULT_HASH_TYPES, + path.span, + fluent::lint::default_hash_types, + |lint| { + lint.set_arg("preferred", replace) + .set_arg("used", cx.tcx.item_name(def_id)) + .note(fluent::lint::note) + }, + ); } } @@ -80,12 +83,12 @@ impl LateLintPass<'_> for QueryStability { if let Ok(Some(instance)) = ty::Instance::resolve(cx.tcx, cx.param_env, def_id, substs) { let def_id = instance.def_id(); if cx.tcx.has_attr(def_id, sym::rustc_lint_query_instability) { - cx.struct_span_lint(POTENTIAL_QUERY_INSTABILITY, span, |lint| { - lint.build(fluent::lint::query_instability) - .set_arg("query", cx.tcx.item_name(def_id)) - .note(fluent::lint::note) - .emit(); - }) + cx.struct_span_lint( + POTENTIAL_QUERY_INSTABILITY, + span, + fluent::lint::query_instability, + |lint| lint.set_arg("query", cx.tcx.item_name(def_id)).note(fluent::lint::note), + ) } } } @@ -123,15 +126,14 @@ impl<'tcx> LateLintPass<'tcx> for TyTyKind { let span = path.span.with_hi( segment.args.map_or(segment.ident.span, |a| a.span_ext).hi() ); - cx.struct_span_lint(USAGE_OF_TY_TYKIND, path.span, |lint| { - lint.build(fluent::lint::tykind_kind) + cx.struct_span_lint(USAGE_OF_TY_TYKIND, path.span, fluent::lint::tykind_kind, |lint| { + lint .span_suggestion( span, fluent::lint::suggestion, "ty", Applicability::MaybeIncorrect, // ty maybe needs an import ) - .emit(); }); } } @@ -140,76 +142,77 @@ impl<'tcx> LateLintPass<'tcx> for TyTyKind { match &ty.kind { TyKind::Path(QPath::Resolved(_, path)) => { if lint_ty_kind_usage(cx, &path.res) { - cx.struct_span_lint(USAGE_OF_TY_TYKIND, path.span, |lint| { - let hir = cx.tcx.hir(); - match hir.find(hir.get_parent_node(ty.hir_id)) { - Some(Node::Pat(Pat { - kind: - PatKind::Path(qpath) - | PatKind::TupleStruct(qpath, ..) - | PatKind::Struct(qpath, ..), - .. - })) => { - if let QPath::TypeRelative(qpath_ty, ..) = qpath - && qpath_ty.hir_id == ty.hir_id - { - lint.build(fluent::lint::tykind_kind) - .span_suggestion( - path.span, - fluent::lint::suggestion, - "ty", - Applicability::MaybeIncorrect, // ty maybe needs an import - ) - .emit(); - return; - } + let hir = cx.tcx.hir(); + let span = match hir.find(hir.get_parent_node(ty.hir_id)) { + Some(Node::Pat(Pat { + kind: + PatKind::Path(qpath) + | PatKind::TupleStruct(qpath, ..) + | PatKind::Struct(qpath, ..), + .. + })) => { + if let QPath::TypeRelative(qpath_ty, ..) = qpath + && qpath_ty.hir_id == ty.hir_id + { + Some(path.span) + } else { + None } - Some(Node::Expr(Expr { - kind: ExprKind::Path(qpath), - .. - })) => { - if let QPath::TypeRelative(qpath_ty, ..) = qpath - && qpath_ty.hir_id == ty.hir_id - { - lint.build(fluent::lint::tykind_kind) - .span_suggestion( - path.span, - fluent::lint::suggestion, - "ty", - Applicability::MaybeIncorrect, // ty maybe needs an import - ) - .emit(); - return; - } + } + Some(Node::Expr(Expr { + kind: ExprKind::Path(qpath), + .. + })) => { + if let QPath::TypeRelative(qpath_ty, ..) = qpath + && qpath_ty.hir_id == ty.hir_id + { + Some(path.span) + } else { + None } - // Can't unify these two branches because qpath below is `&&` and above is `&` - // and `A | B` paths don't play well together with adjustments, apparently. - Some(Node::Expr(Expr { - kind: ExprKind::Struct(qpath, ..), - .. - })) => { - if let QPath::TypeRelative(qpath_ty, ..) = qpath - && qpath_ty.hir_id == ty.hir_id - { - lint.build(fluent::lint::tykind_kind) - .span_suggestion( - path.span, - fluent::lint::suggestion, - "ty", - Applicability::MaybeIncorrect, // ty maybe needs an import - ) - .emit(); - return; - } + } + // Can't unify these two branches because qpath below is `&&` and above is `&` + // and `A | B` paths don't play well together with adjustments, apparently. + Some(Node::Expr(Expr { + kind: ExprKind::Struct(qpath, ..), + .. + })) => { + if let QPath::TypeRelative(qpath_ty, ..) = qpath + && qpath_ty.hir_id == ty.hir_id + { + Some(path.span) + } else { + None } - _ => {} } - lint.build(fluent::lint::tykind).help(fluent::lint::help).emit(); - }) + _ => None + }; + + match span { + Some(span) => { + cx.struct_span_lint( + USAGE_OF_TY_TYKIND, + path.span, + fluent::lint::tykind_kind, + |lint| lint.span_suggestion( + span, + fluent::lint::suggestion, + "ty", + Applicability::MaybeIncorrect, // ty maybe needs an import + ) + ) + }, + None => cx.struct_span_lint( + USAGE_OF_TY_TYKIND, + path.span, + fluent::lint::tykind, + |lint| lint.help(fluent::lint::help) + ) + } } else if !ty.span.from_expansion() && let Some(t) = is_ty_or_ty_ctxt(cx, &path) { if path.segments.len() > 1 { - cx.struct_span_lint(USAGE_OF_QUALIFIED_TY, path.span, |lint| { - lint.build(fluent::lint::ty_qualified) + cx.struct_span_lint(USAGE_OF_QUALIFIED_TY, path.span, fluent::lint::ty_qualified, |lint| { + lint .set_arg("ty", t.clone()) .span_suggestion( path.span, @@ -218,7 +221,6 @@ impl<'tcx> LateLintPass<'tcx> for TyTyKind { // The import probably needs to be changed Applicability::MaybeIncorrect, ) - .emit(); }) } } @@ -308,11 +310,8 @@ impl EarlyLintPass for LintPassImpl { cx.struct_span_lint( LINT_PASS_IMPL_WITHOUT_MACRO, lint_pass.path.span, - |lint| { - lint.build(fluent::lint::lintpass_by_hand) - .help(fluent::lint::help) - .emit(); - }, + fluent::lint::lintpass_by_hand, + |lint| lint.help(fluent::lint::help), ) } } @@ -349,12 +348,12 @@ impl<'tcx> LateLintPass<'tcx> for ExistingDocKeyword { if is_doc_keyword(v) { return; } - cx.struct_span_lint(EXISTING_DOC_KEYWORD, attr.span, |lint| { - lint.build(fluent::lint::non_existant_doc_keyword) - .set_arg("keyword", v) - .help(fluent::lint::help) - .emit(); - }); + cx.struct_span_lint( + EXISTING_DOC_KEYWORD, + attr.span, + fluent::lint::non_existant_doc_keyword, + |lint| lint.set_arg("keyword", v).help(fluent::lint::help), + ); } } } @@ -412,9 +411,12 @@ impl LateLintPass<'_> for Diagnostics { } debug!(?found_impl); if !found_parent_with_attr && !found_impl { - cx.struct_span_lint(DIAGNOSTIC_OUTSIDE_OF_IMPL, span, |lint| { - lint.build(fluent::lint::diag_out_of_impl).emit(); - }) + cx.struct_span_lint( + DIAGNOSTIC_OUTSIDE_OF_IMPL, + span, + fluent::lint::diag_out_of_impl, + |lint| lint, + ) } let mut found_diagnostic_message = false; @@ -430,9 +432,12 @@ impl LateLintPass<'_> for Diagnostics { } debug!(?found_diagnostic_message); if !found_parent_with_attr && !found_diagnostic_message { - cx.struct_span_lint(UNTRANSLATABLE_DIAGNOSTIC, span, |lint| { - lint.build(fluent::lint::untranslatable_diag).emit(); - }) + cx.struct_span_lint( + UNTRANSLATABLE_DIAGNOSTIC, + span, + fluent::lint::untranslatable_diag, + |lint| lint, + ) } } } @@ -464,8 +469,8 @@ impl LateLintPass<'_> for BadOptAccess { let Some(literal) = item.literal() && let ast::LitKind::Str(val, _) = literal.kind { - cx.struct_span_lint(BAD_OPT_ACCESS, expr.span, |lint| { - lint.build(val.as_str()).emit(); } + cx.struct_span_lint(BAD_OPT_ACCESS, expr.span, val.as_str(), |lint| + lint ); } } |
