(Nearly) All status, debug, and progress output is now sent to STDERR instead of STDOUT

[jrfnl]

Description

✨ New Util\StatusWriter for writing output to STDERR

This is a different implementation of the principle for writing to stdErr as originally committed in 2020.

In the original implementation:

• The methods for writing to stdErr were added to the Common utility method class.
  As this is a group of methods which belong closely together, moving them to a separate class seems more appropriate.
• The [force]write() method(s) had a (bool) $suppressNewline parameter with a default value of false.
  This has been changed to a (int) $newlines parameter with a default value of 1, which provides more flexibility.
• Writing to STDERR was hard-coded in the forcePrint() method.
  As PHPUnit has no facility for capturing output send to stdErr, this made testing output send to stdErr impossible.
  The stream to write to has now been set up as a (private) class property and a test helper trait and an abstract base class for testing output send to stdErr have been added to the testsuite.
• The original implementation allowed for pausing/resuming the StatusWriter, but did not allow for "nested" pauses. Support for this has been added.

This commit introduces the "wiring" for all this, i.e. the new StatusWriter class, fully covered by tests and the test helpers.

Other notes:

• Just like in the original implementation, this is still a "static" class as we need to be able to globally pause/unpause the StatusWriter when the PHPCS fixer is running, so we can't have separate instances of the StatusWriter class everywhere it is used.
  I did consider making it a singleton, but I see no upside to that compared to a static class as testing is awkward in both cases.
• I did consider adding an interface to allow for additional Writer classes - I have one in mind for a new feature for PHPCS 4.1 -, but when looking at that new feature, I realized that the method signatures would need to diverge between those classes, so introducing an interface would make live harder not more straight-forward.

Props to @edorian for inspiration for the test setup.

(Nearly) All status, debug, and progress output is now sent to STDERR instead of STDOUT

What with the StatusWriter class now being available, this commit starts using it in (nearly) all appropriate places.

Notes:

• The original implementation removed the output buffering from the Fixer class.
  While I would absolutely LOVE to get rid of that, my own manual tests of these changes showed that this led to undesired side-effects in the debug output, so I have not included that part of the original changeset for now, though I may re-attempt this at a later point in time.
• As the fast majority of the output is "hidden" behind conditions involving the global PHP_CODESNIFFER_VERBOSITY constant, which is set to 0 in the test bootstrap, it is neigh impossible (at this time) to add tests to cover the changes in this commit.
  Then again, testing the debug output isn't something which should get the highest priority anyway and could be considered redundant.

And while the changes in this commit have been made comprehensive based on the current codebase, there are still some situations where error output goes to STDOUT instead of STDERR. This is related to a) output buffering and b) the use of the DeepExitException to pass both error as well as requested output.
I'm still investigating a solution to safely and reliably change this, but don't deem that solution ready for commit yet, though I'm hopeful that solution may still make it into 4.0.

Includes updating the related tests to ensure that the output from the Timing class and the MessageCollector is send to stdErr and not to stdOut.

I have "manually" tested the results of this change and have found it working, except for the edge case as annotated above.
I'll happily share the test setup I've used for this so others can verify my findings.

Requirements check: write error messages to STDERR

As the requirements file needs to stay PHP cross-version compatible, hard-code this, instead of using the StatusWriter.

Runner::registerOutOfMemoryShutdownMessage(): write error messages to STDERR

As we may not have access to any of our classes when the process runs out of memory and even if we did, we don't know whether the StatusWriter is paused, so to ensure that this out of memory helper info is send to STDERR, use a direct call to fwrite(STDERR, ...).

Pause StatusWriter when using the PHP tokenizer in sniffs

As things were, debug information coming from the tokenization of the patterns/comments would be injected into the debug information about the actual scan.

This confuses things as these tokenization debug snippets are basically about nonsense code, so let's silence the StatusWriter when new PHP() is used for small code snippets within sniffs.

Suggested changelog entry

Changed:

• All status, debug, and progress output is now sent to STDERR instead of STDOUT.
  • Only report output now goes through STDOUT.
  • Piping output to a file will now only include report output.
    • Pipe both STDERR and STDOUT to the same file to capture the entire output of the run.
  • The --report-file functionality remains untouched.

Related issues/external references

Fixes squizlabs/PHP_CodeSniffer#1612
Supersedes squizlabs/PHP_CodeSniffer#3078

[jrfnl]

The phpcs-4.0/pr-1010-explainer branch contains some test code to demonstrate the behavioural change of this PR. We'll be talking this through in the livestream now.

[rodrigoprimo]

Thanks for working on this PR, @jrfnl. Having status, debug, and progress output sent to STDERR is a great improvement.

I left a few comments with some suggestions.

[rodrigoprimo]

Maybe it is just me, but I'm unsure about the name of this property, $pauseCount, and the pause() method. When I see a call to StatusWriter::pause(), it is not immediately clear that the output will be suppressed/ignored. The description of this property reinforces this impression. It reads "ignored" and not "paused".

StatusWriter::pause() doesn't just pause output. It creates a state where regular output is suppressed/ignored.

With that in mind, here are a few alternatives that occurred to me to replace the pause() method (and then the properties, and potentially resume() could be renamed following the same pattern):

• suppressOutput()
• startQuietMode()

What do you think?

[petitphp]

The pause/resume naming make sense to me, but I can see how one could think at first that the messages are buffered when the writer is paused and not discarded.

Maybe using stop() would be more explicit here ?

[jrfnl]

I'm going to have a think about this for a moment.

[jrfnl]

As this PR is blocking everything after, I'm going to merge it without changing for the moment.

If we come up with better names, either for the methods or the class, I'll happily have a look at a follow-up PR to this one, either before or after the 4.0 release (as this class is marked as @internal, so there is no BC-guarantee and we can still change it if we deem it necessary).

[jrfnl]

@rodrigoprimo and me discussed the changes I made based on his review and I've squashed the changes into the appropriate commits now they've been discussed. Merging once the build passes.

Any name changes can still be addressed later if anyone can come up with a good set of better names.