GHC's new diagnostic infrastructure: remaining work

NOTE: This ticket is not actionable, but I am using it as a dumping ground to collect everything which needs to be done to consider #18516 morally "finished". Me and @rae will eventually break this down into smaller tasks.

The first round of #18516 is approaching its natural end, but the work in this area in not done. This is an unordered collection of things which are missing and would be nice to do:

Port the remaining TcRn messages to use the new TcRnMessage diagnostic type. Currently, for a lot of GHC typecheck- rename errors and warnings we "cheat", i.e we rewrap the old SDoc-style messages into an TcRnUnknownMessage SDoc type constructor (the type is slightly more complicated than this, but that gives you the idea).
Action: We need to grep for every occurrence of TcRnUnknownMessage and convert that into a proper type constructor.
Ticket(s): #19926 (closed), #19927 (closed), #19930 (closed), #20114 (closed), #20115 (closed), #20116 (closed), #20117 (closed), #20118 (closed), #20119 (closed)
Example: GHC.Tc.Module.tcRnModule currently emits:
```
[...]
    err_msg = mkPlainErrorMsgEnvelope loc $
        TcRnUnknownMessage $ mkPlainError noHints $
          text "Module does not have a RealSrcSpan:" <+> ppr this_mod
[...]
```
We need to add a new type constructor for TcRnMessage called, for example, TcRnModuleHasNoRealSrcSpan !Module, write a proper comment for it, add this constructor to the Diagnostic instance inside GHC.Tc.Errors.Ppr and eventually replace that err_msg with something like:
```
[...]
    err_msg = TcRnModuleHasNoRealSrcSpan this_mod
[...]
```

Port the remaining Ds messages to use the new DsMessage diagnostic type. This is largely what I am already doing as part of !5719 (closed) ~~, but I am currently trying to fix the humongous increase in parser deps~~
Action: Me (Alfredo) needs to make this eventually mergeable, or pass the baton to someone which can make it mergeable.
Example: Same idea as per above.

Port the remaining Driver messages to use the new DriverMessage diagnostic type.
Action: grep for DriverUnknownMessage and follow the same recipe as described for the TcRnMessage case.
Example: Same idea as per the TcRnMessage.

Document all the type constructors for all the diagnostic types and all the hints. This is an heroic (and admittedly a bit tedious) task, but also an important one. Currently we are trying our best to document each type constructor
to give information about what this message is, which flag (if any) controls it, and which are the tests that triggers it.
Action: Pick one or more undocumented type constructors. Choose from the pool of GhcMessage, TcRnMessage, DsMessage, PsMessage and GhcHint. Follow the blueprint for the other (documented) constructors. If a tests is missing, and is not too complicated to add, consider adding it.
Example: Currently the PsWarnUnrecognisedPragma type constructor of GHC.Parser.Errors.Types has a single-line, bland comment. Replace it with something like:
```
{-| PsWarnUnrecognisedPragma is a warning (controlled by the
 -Wunrecognised-pragmas flag) that occurs when trying to use a pragma that GHC doesn't know.

 Example(s):

    module Foo where

    {-# THISISATYPO foo #-}
    foo :: ()
    foo = ()

 Test case(s): parser/should_compile/read064
-}
| PsWarnUnrecognisedPragma
```

Improve existing type constructors. To go back to the previous example, currently PsWarnUnrecognisedPragma doesn't take the offending pragma as a type argument, which means that GHC can only produce a warning message like "Unrecognised pragma", but in principle it would be nice to also report which pragma that is.
Action: Work backward: pick a diagnostic message in GHC you don't like or you find incomplete, trace back which type constructor that correspond, and improve it.
Example: The PsWarnUnrecognisedPragma is a good candidate. Eventually it might not be feasible to improve it if this is emitted in the Lexer.y and it's cumbersome to get the parsed pragma. If that's the case, pick another one.

Improve the diagnostic messages and hints. Currently GHC is not disciplined when it comes to emitting diagnostic messages and hints, in the sense that during #18516 we largely moved the existing SDoc messages into the Ppr module, without changing too much the same and form of these diagnostics to not break too many tests. However, in an ideal world, we should be consistent.
Action: Pick a diagnostic message (or a hint) you are not happy about, and try to make it either more precise, or more fluent, or more consistent with the rest.
Example: There are some hints which are posed as a question, with a question mark at the end, but in this new "brave new world(TM)" they should really be sentences, as they appears in a list of "Suggested fixes".

Move hints in the Parser.Errors.Ppr diagnostics into proper types. Some PsMessage constructors are still emitting hints bundled with diagnostic messages, which means that calling diagnosticHints on these constructors will yield noHints, which is not very useful.
Action: Pick a diagnostic message where the hint is bundled with the diagnostic message (look at the diagnosticMessage implementation) and move it into a separate hint.
Tickets: #20055 (closed)

Review usages of MessageClass, and make sure those are appropriate. During #18516, we disentangled the Severity used for diagnostics with the severity of messages emitted by GHC via putLogMsg, and the MessageClass abstraction was born. There might be diagnostic messages emitted in GHC that are not using the MCDiagnostic constructor, but one of the other MCOutput or MCFatal. To say it differently, everything which is not logged using MCDiagnostic won't enter the proper diagnostic infrastructure, and it will be treated by GHC like any other "general purpose" logging message, which means, for example, that -Werror won't be honoured etc etc.
Action: Do a grep for every type constructor of MessageClass, and make sure that everything we is morally a diagnostic message is actually outputted and treated like so.
Example: GHC.Linker.ExtraObj.mkExtraObjToLinkIntoBinary has the following:
```
mkExtraObjToLinkIntoBinary :: Logger -> TmpFs -> DynFlags -> UnitState -> IO (Maybe FilePath)
mkExtraObjToLinkIntoBinary logger tmpfs dflags unit_state = do
when (gopt Opt_NoHsMain dflags && haveRtsOptsFlags dflags) $
   logInfo logger dflags $ withPprStyle defaultUserStyle
       (text "Warning: -rtsopts and -with-rtsopts have no effect with -no-hs-main." $$
        text "    Call hs_init_ghc() from your main() function to set these options.")
```
I am not entirely sure if this should be a "proper" GHC warning or if it ought to stay a general informational message, but these are the kind of candidates we should review.

Add a JSON interface. This is a big one in itself, but there might be tooling that would be interested in just getting its hands on the JSON output of the diagnostics that GHC emits, without worrying about linking against the GHC API and work with the Haskell values. Action: I am not sure what is the best plan of action, but ideally we should be able to define a JSON instance for our diagnostic types. The crucial thing here would be to make the format of this JSON documented and predictable, so that it doesn't break with every GHC release and users don't have to read the GHC source code to reverse engineer the format
. Ticket(s): #19278

Little general improvements

@shayne-fletcher-da suggested to rename getMessages (either in the Parser API or in the getter of the Messages newtype) to avoid import collisions.
Issue: #19920 (closed)

There are probably many other things I am forgetting right now, and I will add them as they come to mind.

Edited Mar 30, 2024 by Patrick

To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information