mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-24 05:33:27 +00:00
Auto merge of #93368 - eddyb:diagbld-guarantee, r=estebank
rustc_errors: let `DiagnosticBuilder::emit` return a "guarantee of emission". That is, `DiagnosticBuilder` is now generic over the return type of `.emit()`, so we'll now have: * `DiagnosticBuilder<ErrorReported>` for error (incl. fatal/bug) diagnostics * can only be created via a `const L: Level`-generic constructor, that limits allowed variants via a `where` clause, so not even `rustc_errors` can accidentally bypass this limitation * asserts `diagnostic.is_error()` on emission, just in case the construction restriction was bypassed (e.g. by replacing the whole `Diagnostic` inside `DiagnosticBuilder`) * `.emit()` returns `ErrorReported`, as a "proof" token that `.emit()` was called (though note that this isn't a real guarantee until after completing the work on #69426) * `DiagnosticBuilder<()>` for everything else (warnings, notes, etc.) * can also be obtained from other `DiagnosticBuilder`s by calling `.forget_guarantee()` This PR is a companion to other ongoing work, namely: * #69426 and it's ongoing implementation: #93222 the API changes in this PR are needed to get statically-checked "only errors produce `ErrorReported` from `.emit()`", but doesn't itself provide any really strong guarantees without those other `ErrorReported` changes * #93244 would make the choices of API changes (esp. naming) in this PR fit better overall In order to be able to let `.emit()` return anything trustable, several changes had to be made: * `Diagnostic`'s `level` field is now private to `rustc_errors`, to disallow arbitrary "downgrade"s from "some kind of error" to "warning" (or anything else that doesn't cause compilation to fail) * it's still possible to replace the whole `Diagnostic` inside the `DiagnosticBuilder`, sadly, that's harder to fix, but it's unlikely enough that we can paper over it with asserts on `.emit()` * `.cancel()` now consumes `DiagnosticBuilder`, preventing `.emit()` calls on a cancelled diagnostic * it's also now done internally, through `DiagnosticBuilder`-private state, instead of having a `Level::Cancelled` variant that can be read (or worse, written) by the user * this removes a hazard of calling `.cancel()` on an error then continuing to attach details to it, and even expect to be able to `.emit()` it * warnings were switched to *only* `can_emit_warnings` on emission (instead of pre-cancelling early) * `struct_dummy` was removed (as it relied on a pre-`Cancelled` `Diagnostic`) * since `.emit()` doesn't consume the `DiagnosticBuilder` <sub>(I tried and gave up, it's much more work than this PR)</sub>, we have to make `.emit()` idempotent wrt the guarantees it returns * thankfully, `err.emit(); err.emit();` can return `ErrorReported` both times, as the second `.emit()` call has no side-effects *only* because the first one did do the appropriate emission * `&mut Diagnostic` is now used in a lot of function signatures, which used to take `&mut DiagnosticBuilder` (in the interest of not having to make those functions generic) * the APIs were already mostly identical, allowing for low-effort porting to this new setup * only some of the suggestion methods needed some rework, to have the extra `DiagnosticBuilder` functionality on the `Diagnostic` methods themselves (that change is also present in #93259) * `.emit()`/`.cancel()` aren't available, but IMO calling them from an "error decorator/annotator" function isn't a good practice, and can lead to strange behavior (from the caller's perspective) * `.downgrade_to_delayed_bug()` was added, letting you convert any `.is_error()` diagnostic into a `delay_span_bug` one (which works because in both cases the guarantees available are the same) This PR should ideally be reviewed commit-by-commit, since there is a lot of fallout in each. r? `@estebank` cc `@Manishearth` `@nikomatsakis` `@mark-i-m`
This commit is contained in:
commit
87355df6d4
9 changed files with 21 additions and 23 deletions
|
@ -6,7 +6,7 @@ use clippy_utils::{
|
||||||
};
|
};
|
||||||
use if_chain::if_chain;
|
use if_chain::if_chain;
|
||||||
use rustc_data_structures::fx::FxHashSet;
|
use rustc_data_structures::fx::FxHashSet;
|
||||||
use rustc_errors::{Applicability, DiagnosticBuilder};
|
use rustc_errors::{Applicability, Diagnostic};
|
||||||
use rustc_hir::intravisit::{self, Visitor};
|
use rustc_hir::intravisit::{self, Visitor};
|
||||||
use rustc_hir::{Block, Expr, ExprKind, HirId};
|
use rustc_hir::{Block, Expr, ExprKind, HirId};
|
||||||
use rustc_lint::{LateContext, LateLintPass, LintContext};
|
use rustc_lint::{LateContext, LateLintPass, LintContext};
|
||||||
|
@ -489,7 +489,7 @@ fn emit_branches_sharing_code_lint(
|
||||||
add_expr_note = !cx.typeck_results().expr_ty(if_expr).is_unit();
|
add_expr_note = !cx.typeck_results().expr_ty(if_expr).is_unit();
|
||||||
}
|
}
|
||||||
|
|
||||||
let add_optional_msgs = |diag: &mut DiagnosticBuilder<'_>| {
|
let add_optional_msgs = |diag: &mut Diagnostic| {
|
||||||
if add_expr_note {
|
if add_expr_note {
|
||||||
diag.note("The end suggestion probably needs some adjustments to use the expression result correctly");
|
diag.note("The end suggestion probably needs some adjustments to use the expression result correctly");
|
||||||
}
|
}
|
||||||
|
|
|
@ -628,9 +628,7 @@ fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) {
|
||||||
let mut parser = match maybe_new_parser_from_source_str(&sess, filename, code) {
|
let mut parser = match maybe_new_parser_from_source_str(&sess, filename, code) {
|
||||||
Ok(p) => p,
|
Ok(p) => p,
|
||||||
Err(errs) => {
|
Err(errs) => {
|
||||||
for mut err in errs {
|
drop(errs);
|
||||||
err.cancel();
|
|
||||||
}
|
|
||||||
return false;
|
return false;
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
@ -668,7 +666,7 @@ fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) {
|
||||||
_ => {},
|
_ => {},
|
||||||
},
|
},
|
||||||
Ok(None) => break,
|
Ok(None) => break,
|
||||||
Err(mut e) => {
|
Err(e) => {
|
||||||
e.cancel();
|
e.cancel();
|
||||||
return false;
|
return false;
|
||||||
},
|
},
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
use std::borrow::Cow;
|
use std::borrow::Cow;
|
||||||
use std::collections::BTreeMap;
|
use std::collections::BTreeMap;
|
||||||
|
|
||||||
use rustc_errors::DiagnosticBuilder;
|
use rustc_errors::Diagnostic;
|
||||||
use rustc_hir as hir;
|
use rustc_hir as hir;
|
||||||
use rustc_hir::intravisit::{walk_body, walk_expr, walk_inf, walk_ty, Visitor};
|
use rustc_hir::intravisit::{walk_body, walk_expr, walk_inf, walk_ty, Visitor};
|
||||||
use rustc_hir::{Body, Expr, ExprKind, GenericArg, Item, ItemKind, QPath, TyKind};
|
use rustc_hir::{Body, Expr, ExprKind, GenericArg, Item, ItemKind, QPath, TyKind};
|
||||||
|
@ -68,7 +68,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitHasher {
|
||||||
|
|
||||||
fn suggestion<'tcx>(
|
fn suggestion<'tcx>(
|
||||||
cx: &LateContext<'tcx>,
|
cx: &LateContext<'tcx>,
|
||||||
diag: &mut DiagnosticBuilder<'_>,
|
diag: &mut Diagnostic,
|
||||||
generics_span: Span,
|
generics_span: Span,
|
||||||
generics_suggestion_span: Span,
|
generics_suggestion_span: Span,
|
||||||
target: &ImplicitHasherType<'_>,
|
target: &ImplicitHasherType<'_>,
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
//! checks for `#[inline]` on trait methods without bodies
|
//! checks for `#[inline]` on trait methods without bodies
|
||||||
|
|
||||||
use clippy_utils::diagnostics::span_lint_and_then;
|
use clippy_utils::diagnostics::span_lint_and_then;
|
||||||
use clippy_utils::sugg::DiagnosticBuilderExt;
|
use clippy_utils::sugg::DiagnosticExt;
|
||||||
use rustc_ast::ast::Attribute;
|
use rustc_ast::ast::Attribute;
|
||||||
use rustc_errors::Applicability;
|
use rustc_errors::Applicability;
|
||||||
use rustc_hir::{TraitFn, TraitItem, TraitItemKind};
|
use rustc_hir::{TraitFn, TraitItem, TraitItemKind};
|
||||||
|
|
|
@ -6,7 +6,7 @@ use clippy_utils::{get_trait_def_id, is_self, paths};
|
||||||
use if_chain::if_chain;
|
use if_chain::if_chain;
|
||||||
use rustc_ast::ast::Attribute;
|
use rustc_ast::ast::Attribute;
|
||||||
use rustc_data_structures::fx::FxHashSet;
|
use rustc_data_structures::fx::FxHashSet;
|
||||||
use rustc_errors::{Applicability, DiagnosticBuilder};
|
use rustc_errors::{Applicability, Diagnostic};
|
||||||
use rustc_hir::intravisit::FnKind;
|
use rustc_hir::intravisit::FnKind;
|
||||||
use rustc_hir::{BindingAnnotation, Body, FnDecl, GenericArg, HirId, Impl, ItemKind, Node, PatKind, QPath, TyKind};
|
use rustc_hir::{BindingAnnotation, Body, FnDecl, GenericArg, HirId, Impl, ItemKind, Node, PatKind, QPath, TyKind};
|
||||||
use rustc_hir::{HirIdMap, HirIdSet};
|
use rustc_hir::{HirIdMap, HirIdSet};
|
||||||
|
@ -196,7 +196,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue {
|
||||||
}
|
}
|
||||||
|
|
||||||
// Dereference suggestion
|
// Dereference suggestion
|
||||||
let sugg = |diag: &mut DiagnosticBuilder<'_>| {
|
let sugg = |diag: &mut Diagnostic| {
|
||||||
if let ty::Adt(def, ..) = ty.kind() {
|
if let ty::Adt(def, ..) = ty.kind() {
|
||||||
if let Some(span) = cx.tcx.hir().span_if_local(def.did) {
|
if let Some(span) = cx.tcx.hir().span_if_local(def.did) {
|
||||||
if can_type_implement_copy(cx.tcx, cx.param_env, ty, traits::ObligationCause::dummy_with_span(span)).is_ok() {
|
if can_type_implement_copy(cx.tcx, cx.param_env, ty, traits::ObligationCause::dummy_with_span(span)).is_ok() {
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
use clippy_utils::diagnostics::span_lint_hir_and_then;
|
use clippy_utils::diagnostics::span_lint_hir_and_then;
|
||||||
use clippy_utils::return_ty;
|
use clippy_utils::return_ty;
|
||||||
use clippy_utils::source::snippet;
|
use clippy_utils::source::snippet;
|
||||||
use clippy_utils::sugg::DiagnosticBuilderExt;
|
use clippy_utils::sugg::DiagnosticExt;
|
||||||
use if_chain::if_chain;
|
use if_chain::if_chain;
|
||||||
use rustc_errors::Applicability;
|
use rustc_errors::Applicability;
|
||||||
use rustc_hir as hir;
|
use rustc_hir as hir;
|
||||||
|
|
|
@ -534,7 +534,7 @@ impl Write {
|
||||||
match parser
|
match parser
|
||||||
.parse_expr()
|
.parse_expr()
|
||||||
.map(rustc_ast::ptr::P::into_inner)
|
.map(rustc_ast::ptr::P::into_inner)
|
||||||
.map_err(|mut e| e.cancel())
|
.map_err(|e| e.cancel())
|
||||||
{
|
{
|
||||||
// write!(e, ...)
|
// write!(e, ...)
|
||||||
Ok(p) if parser.eat(&token::Comma) => Some(p),
|
Ok(p) if parser.eat(&token::Comma) => Some(p),
|
||||||
|
@ -563,7 +563,7 @@ impl Write {
|
||||||
}
|
}
|
||||||
|
|
||||||
let comma_span = parser.prev_token.span;
|
let comma_span = parser.prev_token.span;
|
||||||
let token_expr = if let Ok(expr) = parser.parse_expr().map_err(|mut err| err.cancel()) {
|
let token_expr = if let Ok(expr) = parser.parse_expr().map_err(|err| err.cancel()) {
|
||||||
expr
|
expr
|
||||||
} else {
|
} else {
|
||||||
return (Some(fmtstr), None);
|
return (Some(fmtstr), None);
|
||||||
|
|
|
@ -8,13 +8,13 @@
|
||||||
//! Thank you!
|
//! Thank you!
|
||||||
//! ~The `INTERNAL_METADATA_COLLECTOR` lint
|
//! ~The `INTERNAL_METADATA_COLLECTOR` lint
|
||||||
|
|
||||||
use rustc_errors::{Applicability, DiagnosticBuilder};
|
use rustc_errors::{Applicability, Diagnostic};
|
||||||
use rustc_hir::HirId;
|
use rustc_hir::HirId;
|
||||||
use rustc_lint::{LateContext, Lint, LintContext};
|
use rustc_lint::{LateContext, Lint, LintContext};
|
||||||
use rustc_span::source_map::{MultiSpan, Span};
|
use rustc_span::source_map::{MultiSpan, Span};
|
||||||
use std::env;
|
use std::env;
|
||||||
|
|
||||||
fn docs_link(diag: &mut DiagnosticBuilder<'_>, lint: &'static Lint) {
|
fn docs_link(diag: &mut Diagnostic, lint: &'static Lint) {
|
||||||
if env::var("CLIPPY_DISABLE_DOCS_LINKS").is_err() {
|
if env::var("CLIPPY_DISABLE_DOCS_LINKS").is_err() {
|
||||||
if let Some(lint) = lint.name_lower().strip_prefix("clippy::") {
|
if let Some(lint) = lint.name_lower().strip_prefix("clippy::") {
|
||||||
diag.help(&format!(
|
diag.help(&format!(
|
||||||
|
@ -145,7 +145,7 @@ pub fn span_lint_and_then<C, S, F>(cx: &C, lint: &'static Lint, sp: S, msg: &str
|
||||||
where
|
where
|
||||||
C: LintContext,
|
C: LintContext,
|
||||||
S: Into<MultiSpan>,
|
S: Into<MultiSpan>,
|
||||||
F: FnOnce(&mut DiagnosticBuilder<'_>),
|
F: FnOnce(&mut Diagnostic),
|
||||||
{
|
{
|
||||||
cx.struct_span_lint(lint, sp, |diag| {
|
cx.struct_span_lint(lint, sp, |diag| {
|
||||||
let mut diag = diag.build(msg);
|
let mut diag = diag.build(msg);
|
||||||
|
@ -169,7 +169,7 @@ pub fn span_lint_hir_and_then(
|
||||||
hir_id: HirId,
|
hir_id: HirId,
|
||||||
sp: impl Into<MultiSpan>,
|
sp: impl Into<MultiSpan>,
|
||||||
msg: &str,
|
msg: &str,
|
||||||
f: impl FnOnce(&mut DiagnosticBuilder<'_>),
|
f: impl FnOnce(&mut Diagnostic),
|
||||||
) {
|
) {
|
||||||
cx.tcx.struct_span_lint_hir(lint, hir_id, sp, |diag| {
|
cx.tcx.struct_span_lint_hir(lint, hir_id, sp, |diag| {
|
||||||
let mut diag = diag.build(msg);
|
let mut diag = diag.build(msg);
|
||||||
|
@ -219,7 +219,7 @@ pub fn span_lint_and_sugg<'a, T: LintContext>(
|
||||||
/// appear once per
|
/// appear once per
|
||||||
/// replacement. In human-readable format though, it only appears once before
|
/// replacement. In human-readable format though, it only appears once before
|
||||||
/// the whole suggestion.
|
/// the whole suggestion.
|
||||||
pub fn multispan_sugg<I>(diag: &mut DiagnosticBuilder<'_>, help_msg: &str, sugg: I)
|
pub fn multispan_sugg<I>(diag: &mut Diagnostic, help_msg: &str, sugg: I)
|
||||||
where
|
where
|
||||||
I: IntoIterator<Item = (Span, String)>,
|
I: IntoIterator<Item = (Span, String)>,
|
||||||
{
|
{
|
||||||
|
@ -232,7 +232,7 @@ where
|
||||||
/// multiple spans. This is tracked in issue [rustfix#141](https://github.com/rust-lang/rustfix/issues/141).
|
/// multiple spans. This is tracked in issue [rustfix#141](https://github.com/rust-lang/rustfix/issues/141).
|
||||||
/// Suggestions with multiple spans will be silently ignored.
|
/// Suggestions with multiple spans will be silently ignored.
|
||||||
pub fn multispan_sugg_with_applicability<I>(
|
pub fn multispan_sugg_with_applicability<I>(
|
||||||
diag: &mut DiagnosticBuilder<'_>,
|
diag: &mut Diagnostic,
|
||||||
help_msg: &str,
|
help_msg: &str,
|
||||||
applicability: Applicability,
|
applicability: Applicability,
|
||||||
sugg: I,
|
sugg: I,
|
||||||
|
|
|
@ -673,8 +673,8 @@ fn indentation<T: LintContext>(cx: &T, span: Span) -> Option<String> {
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Convenience extension trait for `DiagnosticBuilder`.
|
/// Convenience extension trait for `Diagnostic`.
|
||||||
pub trait DiagnosticBuilderExt<T: LintContext> {
|
pub trait DiagnosticExt<T: LintContext> {
|
||||||
/// Suggests to add an attribute to an item.
|
/// Suggests to add an attribute to an item.
|
||||||
///
|
///
|
||||||
/// Correctly handles indentation of the attribute and item.
|
/// Correctly handles indentation of the attribute and item.
|
||||||
|
@ -721,7 +721,7 @@ pub trait DiagnosticBuilderExt<T: LintContext> {
|
||||||
fn suggest_remove_item(&mut self, cx: &T, item: Span, msg: &str, applicability: Applicability);
|
fn suggest_remove_item(&mut self, cx: &T, item: Span, msg: &str, applicability: Applicability);
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<T: LintContext> DiagnosticBuilderExt<T> for rustc_errors::DiagnosticBuilder<'_> {
|
impl<T: LintContext> DiagnosticExt<T> for rustc_errors::Diagnostic {
|
||||||
fn suggest_item_with_attr<D: Display + ?Sized>(
|
fn suggest_item_with_attr<D: Display + ?Sized>(
|
||||||
&mut self,
|
&mut self,
|
||||||
cx: &T,
|
cx: &T,
|
||||||
|
|
Loading…
Reference in a new issue