We have quite different APIs for emitting a lint vs a hard error

[RalfJung]

Switching between a lint and a hard error used to be fairly simple, one basically just swapped out the function that is called to emit the diagnostic. However, with translatable diagnostic, this has become more work:

• the error type needs to be changed from derive(LintDiagnostic) to derive(Diagnostic)
• a span field needs to be added and decorated with #[primary_span]
• and only then the diagnostic-emitting code can be adjusted to call emit_err instead of emit_node_span_lint

It can be hard to find out about this recipe. It is also a (small) roadblock whenever one wants to do a crater run with a lint turned into a hard error, just to see how much it happens in the ecosystem.

So maybe something could be done to unify these two APIs? I don't know whether "span-in-error-type" vs "separate-span" is the better API. I was told that sometimes having the span in the type is very useful because then extra attributes can be easily applied to it. OTOH, for parts of the compiler (e.g. the interpreter), the logic that determines which error to emit and the logic that determines the span are quite far apart from each other, leading kind of a mess with the current error APIs. Maybe there is a way to make both styles possible for both lints and hard errors?

Cc @davidtwco (not sure who else is pushing translatable diagnostics?)

[RalfJung]

In addition to that (not sure if that warrants a separate issue), writing a lint has become significantly more work since translatable diagnostics were introduced.

It used to be basically

                self.tcx().emit_spanned_lint(
                    lint::builtin::CONST_PATTERNS_WITHOUT_PARTIAL_EQ,
                    self.id,
                    self.span,
                    format!("to use a constant of type `{}` in a pattern, the type must implement `PartialEq`", cv.ty()),
                );

Now instead one has to write

                self.tcx().emit_spanned_lint(
                    lint::builtin::CONST_PATTERNS_WITHOUT_PARTIAL_EQ,
                    self.id,
                    self.span,
                    NonPartialEqMatch { non_peq_ty: cv.ty() },
                );

and then in a different file

#[derive(LintDiagnostic)]
#[diag(mir_build_non_partial_eq_match)]
pub struct NonPartialEqMatch<'tcx> {
    pub non_peq_ty: Ty<'tcx>,
}

and then in yet another file

mir_build_non_partial_eq_match =
    to use a constant of type `{$non_peq_ty}` in a pattern, the type must implement `PartialEq`

and ofc in the first file one has to add at the top of the file

use crate::errors::NonPartialEqMatch;

That takes about ~4 times as much time, since one has to edit in 4 places instead of 1. If there's any way to reduce this, that would be great. :) As things stand, when I work on diagnostics, the amount of time I spend worrying about how to output the diagnostic has significantly increased compared to pre-translatable-diagnostics time -- it used to be something I didn't have to think about, now it's a significant fraction of the work. (And frustratingly, an actually translated rustc doesn't seem anywhere close. So it fells like all this work has no gain. I understand a lot of upfront investment is needed to eventually get a translatable compiler, but that doesn't change the fact that right now we're working for a reward that is years in the future.)

I guess I should start by not importing these lint structs, and just refer to them by absolute path. Then it's only 3 places to change, which is an improvement. That's not the structure one learns by reading the existing code though. And then maybe instead of having the English text in a separate file, we could have it in an attribute at the NonPartialEqMatch? That would be a big change obviously but it would help quite a bit. And then maybe I can define that struct NonPartialEqMatch locally at the only place where the diagnostic is emitted, and then we're down to a single code location again.