Improve diagnostic formatting infrastructure

[arthurcro]

Bug/issue #, if applicable: swiftlang#663

Summary

Provide a description of what your PR addresses, explaining the expected user experience.
Also, provide an overview of your implementation.

Testing

Describe how a reviewer can test the functionality of your PR. Provide test content to test with if
applicable.

Steps:

1. Provide setup instructions.
2. Explain in detail how the functionality can be tested.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

• Added tests
• Ran the ./bin/test script and it succeeded
• Updated documentation if necessary

[arthurcro]

Hi @d-ronnqvist 👋 I'm opening this PR on my fork to disucss implementation details and not overwelm the issue or the actuall PR with comments, I hope that's okay! If you have time, I'd like to share some thoughts with you.

I finally had the time to take a look at revamping our diagnsotic formatting infrastructure.
After your suggestions, instead of relying on the DiagnosticConsoleWriter to decide which to use, I introduced a DiagnosticFormattingOutputStreamOptions protocol and separated different formatting options in two instances:

• ToolsFormattingOptions
• HumanReadableFormattingOptions

I also opted for introducing two internal output stream writers:

• BufferedDiagnosticOutputStreamWriter:
• StreamingDiagnosticOutputStreamWriter:

Both of them have an internal initializer that takes an instance of DiagnosticConsoleFormatter and a TextOutputStream.

I believe it helps with separating concerns, testability and composing formatters and writers together. It gives us more flexibility to introduce more instances of those protocols in the future.

Using this new infrastructure looks like this:

if formatConsoleOutputForTools || diagnosticFilePath != nil {
  formattingOptions = ToolsFormattingOptions()
} else {
   formattingOptions = HumanReadableFormattingOptions(
      baseUrl: documentationBundleURL ?? URL(fileURLWithPath: fileManager.currentDirectoryPath),
       dataProvider: dataProvider
      )
}

...

engine.add(formattingOptions.makeFormatter(LogHandle.standardError)

I am overwhole quite happy with this approach, as I believe it solves our intitial concerns.
However, I have a couple of concerns I would like to disucss with the help of this PR.

1. Currently DiagnosticFormattingOutputStreamOptions.makeFormatter(_:) returns a DiagnosticConsumer. I'm wondering if this is the correct return type or if we would need to introduce a DiagnosticFormattingOutputStreamConsumer protocol similar to DiagnosticFormattingConsumer?

2. There is a set of public static function helpers on DiagnosticConsoleWriter that are use extensively in the code base to format diagnostics and problems. They make use of the now deprecated DiagnosticFormattingOptions. It's unclear to me how to deprecate those and provide ergonomic alternatives. I was thinking to introduce those static functions on the instances of DiagnosticFormattingOutputStreamOptions but it's a lot of boilerplate. What do you think?

3. Regarding the deprecation strategy, I have completely deprecated several types since I believe they do not fit into the new infrastructure anymore. Is that the approach you had in mind?

4. One goal was also to avoid callers inspecting formattings options to decide which formatter should be used. This approach does not achieve this goal although I believe we could with static factory methods on the DiagnosticFormattingOutputStreamOptions protocol. What do you think?

Do you see other concerns looking at the pull request?

As always, thank you so much for all the help and guidance! It's greately appreciated, I hope I can get a proper pull request out soon! 😊