fs: flush option ignored when writing/appending strings

[ChALkeR]

#50009 and #49884 conflict and were merged independently

This was exposed in #60539 CI run

node/test/parallel/test-fs-write-file-flush.js

Lines 26 to 37 in dbe45b7

	await t.test('performs flush', (t) => {
	const spy = t.mock.method(fs, 'fsyncSync');
	const file = nextFile();
	fs.writeFileSync(file, data, { flush: true });
	const calls = spy.mock.calls;
	assert.strictEqual(calls.length, 1);
	assert.strictEqual(calls[0].result, undefined);
	assert.strictEqual(calls[0].error, undefined);
	assert.strictEqual(calls[0].arguments.length, 1);
	assert.strictEqual(typeof calls[0].arguments[0], 'number');
	assert.strictEqual(fs.readFileSync(file, 'utf8'), data);
	});

fails if encoding is specified:

diff --git a/test/parallel/test-fs-write-file-flush.js b/test/parallel/test-fs-write-file-flush.js
index 98a8d637c5f..c28e536c528 100644
--- a/test/parallel/test-fs-write-file-flush.js
+++ b/test/parallel/test-fs-write-file-flush.js
@@ -26,7 +26,7 @@ test('synchronous version', async (t) => {
   await t.test('performs flush', (t) => {
     const spy = t.mock.method(fs, 'fsyncSync');
     const file = nextFile();
-    fs.writeFileSync(file, data, { flush: true });
+    fs.writeFileSync(file, data, { flush: true, encoding: 'utf8' });
     const calls = spy.mock.calls;
     assert.strictEqual(calls.length, 1);
     assert.strictEqual(calls[0].result, undefined);

Here, if options are set to e.g. { flush: true } (or any other object), they do not default to those values

node/lib/fs.js

Lines 2322 to 2327 in dbe45b7

	options = getOptions(options, {
	encoding: 'utf8',
	mode: 0o666,
	flag: 'w',
	flush: false,
	});

As getOptions does not merge options with the default ones, unless it's just an encoding string:

node/lib/internal/fs/utils.js

Lines 318 to 339 in dbe45b7

	function getOptions(options, defaultOptions = kEmptyObject) {
	if (options == null || typeof options === 'function') {
	return defaultOptions;
	}
	if (typeof options === 'string') {
	defaultOptions = { ...defaultOptions };
	defaultOptions.encoding = options;
	options = defaultOptions;
	} else if (typeof options !== 'object') {
	throw new ERR_INVALID_ARG_TYPE('options', ['string', 'Object'], options);
	}
	if (options.encoding !== 'buffer')
	assertEncoding(options.encoding);
	if (options.signal !== undefined) {
	validateAbortSignal(options.signal, 'options.signal');
	}
	return options;
	}

And the options.encoding === 'utf8' codepath was not called when options were set but options.encoding was undefined, as in flush testcases, resulting in the slow path

The bug was still triggered if both encoding and flush were set though

#60539 fixes that accidental slow path when options object without encoding is passed
But it exposes the flush bug even when encoding is not set

And fast path doesn't handle options.flush at all

[ChALkeR]

Side note: the whole logic around getOptions in fs is complex and prone to bugs
There is no significant perf hit at merging them into a new copy anymore when we are comparing to fs ops

A second param to getOptions is passed only in a few places, and all of them have some further logic to actually default to those params individually:

1. writeFileSync (the one with the bug)
   

   node/lib/fs.js

   Lines 2371 to 2383 in dbe45b7

   	function writeFileSync(path, data, options) {
   	options = getOptions(options, {
   	encoding: 'utf8',
   	mode: 0o666,
   	flag: 'w',
   	flush: false,
   	});
   	const flush = options.flush ?? false;
   	validateBoolean(flush, 'options.flush');
   	const flag = options.flag || 'w';

2. appendFileSync (also with the bug as calls writeFileSync), appendFile is identical
   

   node/lib/fs.js

   Lines 2464 to 2471 in dbe45b7

   	options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' });
   	// Don't make changes directly on options object
   	options = copyObject(options);
   	// Force append behavior when using a supplied file descriptor
   	if (!options.flag || isFd(path))
   	options.flag = 'a';

3. writeFile
   

   node/lib/fs.js

   Lines 2321 to 2331 in dbe45b7

   	validateFunction(callback, 'cb');
   	options = getOptions(options, {
   	encoding: 'utf8',
   	mode: 0o666,
   	flag: 'w',
   	flush: false,
   	});
   	const flag = options.flag || 'w';
   	const flush = options.flush ?? false;
   	validateBoolean(flush, 'options.flush');

4. readFile / readFileSync are identical and just
   https://github.com/nodejs/node/blob/dbe45b767b0bb9c984206edd4ad0b5658d7c2fe4/lib/fs.js#L359C3-L359C10

   defaults to r behavior inside:
   

   node/lib/fs.js

   Line 377 in dbe45b7

   const flagsNumber = stringToFlags(options.flag, 'options.flag');