[tool] Guard more write/writeln calls on Process.stdin

[andrewkolos]

Contributes to fixing #137184.

This PR guards write calls in non-test files. This PR excludes

• packages/flutter_tools/lib/src/dart/analysis.dart due to a test timeout I would like to figure out in a separate PR and
• packages/flutter_tools/lib/src/compile.dart due to [tool] Adding an await to DefaultResidentCompiler._recompile fails test(s) in test/general.shard/devfs_test.dart #151255

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• I followed the breaking change policy and added Data Driven Fixes where supported.
• All existing and new tests are passing.

[christopherfujino]

What about instead of having four of these public functions, we just have one, with a boolean parameter (that defaults to true) for whether to append newline, and then a nullable onError callback parameter--if onError is null then we unconditionally call completer.completeError() with caught errors?

My concern with four functions is that contributors will cargo cult the wrong one. There's a lot of cognitive complexity here understanding the differences. In a code review also, it's difficult to parse the differences.

[andrewkolos]

with a boolean parameter (that defaults to true)

If you were to rewrite the IOSink API, would you remove writeln in favor of adding a boolean parameter to write with a default? If not, why should the ProcessUtils counterpart not exist as its own function?

and then a nullable onError callback parameter--if onError is null then we unconditionally call completer.completeError() with caught errors?

is

ProcessUtils.writeToStdinUnsafe(stdin: myProcess.stdin, line: 'foo');

less readable than

ProcessUtils.writeToStdinGuarded(
  stdin: myProcess.stdin,
  line: 'foo',
  onError: null,
);

?

My concern with four functions is that contributors will cargo cult the wrong one.

How are separate named functions more cargo-cultable than fewer ones with boolean parameters (especially when one or more of those boolean parameters are optional)? Looking at the two examples I wrote earlier, is the idea that onError: null is easier to notice and think about than looking at the function name?

[andrewkolos]

In a code review also, it's difficult to parse the differences.

How so? Is this something I can address with doc comments? If the diff is difficult to parse, I'd recommend just reading the code directly.

[christopherfujino]

SGTM

[christopherfujino]

LGTM