Improve handling of dart format stderr output

[natebosch]

Previously the stdout and stderr from the format execution were
merged and there was special handling for 1 specific warning message
followed by a standard 2 lines of boilerplate.

Now stdout an stderr are read separately. stderr is ignored unless
there is no output on stdout. This filters any warnings that didn't
prevent formatting, including the old hardcoded warning as well as a new
one around resolving analysis_options.yaml and any future warnings
that may come up.

When there is no stdout content check the stderr for the diagnostic
format. If any lines match, ignore any non-matching lines. If no lines
match, show the output anyway. I'm not currently aware of any output
here that we wouldn't want to hide, but if error output becomes
important for the user to see we may need to revisit this design.

• Migrate from systemlist to job_start which allows separating the
  output streams.
• Pull the result handling out into a separate function so it can be the
  callback for when the output has been read.

[natebosch]

I started getting bad formatting results because we only handle 1 hardcoded output on stderr before the formatting results and I now commonly see

Warning: Package resolution error when reading "analysis_options.yaml" file:
Failed to resolve package URI "package:dart_flutter_team_lints/analysis_options.yaml" in include

cc @munificent - I think the formatter will always fail to resolve out-of-package includes right? Should we be suppressing this in the formatter output?

This change is very broad in what it ignores on stderr - should we consider still only ignoring hardcoded messages we expect to be safe to hide, and expose the rest as error messages? WDYT @sigmundch

[munificent]

cc @munificent - I think the formatter will always fail to resolve out-of-package includes right? Should we be suppressing this in the formatter output?

I'm not sure what you mean by "out-of-package" includes. If the nearest analysis_options.yaml file has an include that has a package: URI, and the analysis_options.yaml file is not in a directory (immediately or transitively) that is a pub package with a .dart_tool/package_config.json, then, yes, you'll get that warning.

If it's annoying, we could add a flag to suppress it.

[sigmundch]

I like your approach of ignoring stderr if there is a valid output in stdout. Do we have a guarantee that the output will be empty if any issue arises? Or do we need a secondary signal, like dartfmt's exit code?

For the error modality, I worry we may hide an actionable warning/error, but I don't want to spam users with useless messages. Rather than always showing potentially non-actionable warning messages that don't match the diagnostic template, what if we only show extra warnings if we couldn't match any stderr line with the diagnostic template format? That would ensure that if there is no stdout, we always have something to share with users:

• non-empty stdout ==> don't show any issues
• empty stdout + some stderr line matches diagnostic template ==> show diagnostic
• empty stdout + no stderr line matches diagnostic template ==> show the full stderr

[natebosch]

If it's annoying, we could add a flag to suppress it.

It's not actionable so I think its confusing to print it as a "Warning". Is there any reason not to suppress it always? Or to rewrite it as something like "Note: dart format does not read analysis file includes".

[natebosch]

what if we only show extra warnings if we couldn't match any stderr line with the diagnostic template format? That would ensure that if there is no stdout, we always have something to share with users:

This sounds like a good approach to me, I'll try it out.