Skip to content

Commit

Permalink
util: treat format arguments equally
Browse files Browse the repository at this point in the history 
Two changes here which bring us closer to the console standard:

- Arguments to `util.format` are no longer formatted differently
  depending on their order, with format strings being an exception.
- Format specifier formatting is now only triggered if the string
  actually contains a format string.

Under the hood, we now use a single shared function to format the given
arguments which will make the code easier to read and modify.

PR-URL: #23162
Fixes: #23137
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
  • Loading branch information
@silverwind
silverwind committed Oct 17, 2018
1 parent c979fad commit c1b9be5
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 102 deletions.
50 changes: 31 additions & 19 deletions doc/api/util.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,17 @@ property take precedence over `--trace-deprecation` and
<!-- YAML
added: v0.5.3
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/23162
description: The `format` argument is now only taken as such if it actually
contains format specifiers.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/23162
description: If the `format` argument is not a format string, the output
string's formatting is no longer dependent on the type of the
first argument. This change removes previously present quotes
from strings that were being output when the first argument
was not a string.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/17907
description: The `%o` specifier's `depth` option will now fall back to the
Expand All @@ -195,11 +206,9 @@ changes:
* `format` {string} A `printf`-like format string.

The `util.format()` method returns a formatted string using the first argument
as a `printf`-like format.

The first argument is a string containing zero or more *placeholder* tokens.
Each placeholder token is replaced with the converted value from the
corresponding argument. Supported placeholders are:
as a `printf`-like format string which can contain zero or more format
specifiers. Each specifier is replaced with the converted value from the
corresponding argument. Supported specifiers are:

* `%s` - `String`.
* `%d` - `Number` (integer or floating point value) or `BigInt`.
Expand All @@ -218,37 +227,40 @@ contains circular references.
* `%%` - single percent sign (`'%'`). This does not consume an argument.
* Returns: {string} The formatted string

If the placeholder does not have a corresponding argument, the placeholder is
not replaced.
If a specifier does not have a corresponding argument, it is not replaced:

```js
util.format('%s:%s', 'foo');
// Returns: 'foo:%s'
```

If there are more arguments passed to the `util.format()` method than the number
of placeholders, the extra arguments are coerced into strings then concatenated
to the returned string, each delimited by a space. Excessive arguments whose
`typeof` is `'object'` or `'symbol'` (except `null`) will be transformed by
`util.inspect()`.
Values that are not part of the format string are formatted using
`util.inspect()` if their type is either `'object'`, `'symbol'`, `'function'`
or `'number'` and using `String()` in all other cases.

If there are more arguments passed to the `util.format()` method than the
number of specifiers, the extra arguments are concatenated to the returned
string, separated by spaces:

```js
util.format('%s:%s', 'foo', 'bar', 'baz'); // 'foo:bar baz'
util.format('%s:%s', 'foo', 'bar', 'baz');
// Returns: 'foo:bar baz'
```

If the first argument is not a string then `util.format()` returns
a string that is the concatenation of all arguments separated by spaces.
Each argument is converted to a string using `util.inspect()`.
If the first argument does not contain a valid format specifier, `util.format()`
returns a string that is the concatenation of all arguments separated by spaces:

```js
util.format(1, 2, 3); // '1 2 3'
util.format(1, 2, 3);
// Returns: '1 2 3'
```

If only one argument is passed to `util.format()`, it is returned as it is
without any formatting.
without any formatting:

```js
util.format('%% %s'); // '%% %s'
util.format('%% %s');
// Returns: '%% %s'
```

Please note that `util.format()` is a synchronous method that is mainly
Expand Down
181 changes: 98 additions & 83 deletions lib/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,98 +72,113 @@ function format(...args) {
return formatWithOptions(emptyOptions, ...args);
}

function formatWithOptions(inspectOptions, f) {
let i, tempStr;
if (typeof f !== 'string') {
if (arguments.length === 1) return '';
let res = '';
for (i = 1; i < arguments.length - 1; i++) {
res += inspect(arguments[i], inspectOptions);
res += ' ';
}
res += inspect(arguments[i], inspectOptions);
return res;
function formatValue(val, inspectOptions) {
const inspectTypes = ['object', 'symbol', 'function', 'number'];

if (inspectTypes.includes(typeof val)) {
return inspect(val, inspectOptions);
} else {
return String(val);
}
}

function formatWithOptions(inspectOptions, ...args) {
const first = args[0];
const parts = [];

const firstIsString = typeof first === 'string';

if (firstIsString && args.length === 1) {
return first;
}

if (arguments.length === 2) return f;

let str = '';
let a = 2;
let lastPos = 0;
for (i = 0; i < f.length - 1; i++) {
if (f.charCodeAt(i) === 37) { // '%'
const nextChar = f.charCodeAt(++i);
if (a !== arguments.length) {
switch (nextChar) {
case 115: // 's'
tempStr = String(arguments[a++]);
break;
case 106: // 'j'
tempStr = tryStringify(arguments[a++]);
break;
case 100: // 'd'
const tempNum = arguments[a++];
// eslint-disable-next-line valid-typeof
if (typeof tempNum === 'bigint') {
tempStr = `${tempNum}n`;
} else {
tempStr = `${Number(tempNum)}`;
if (firstIsString && /%[sjdOoif%]/.test(first)) {
let i, tempStr;
let str = '';
let a = 1;
let lastPos = 0;

for (i = 0; i < first.length - 1; i++) {
if (first.charCodeAt(i) === 37) { // '%'
const nextChar = first.charCodeAt(++i);
if (a !== args.length) {
switch (nextChar) {
case 115: // 's'
tempStr = String(args[a++]);
break;
case 106: // 'j'
tempStr = tryStringify(args[a++]);
break;
case 100: // 'd'
const tempNum = args[a++];
// eslint-disable-next-line valid-typeof
if (typeof tempNum === 'bigint') {
tempStr = `${tempNum}n`;
} else {
tempStr = `${Number(tempNum)}`;
}
break;
case 79: // 'O'
tempStr = inspect(args[a++], inspectOptions);
break;
case 111: // 'o'
{
const opts = Object.assign({}, inspectOptions, {
showHidden: true,
showProxy: true,
depth: 4
});
tempStr = inspect(args[a++], opts);
break;
}
break;
case 79: // 'O'
tempStr = inspect(arguments[a++], inspectOptions);
break;
case 111: // 'o'
{
const opts = Object.assign({}, inspectOptions, {
showHidden: true,
showProxy: true
});
tempStr = inspect(arguments[a++], opts);
break;
case 105: // 'i'
const tempInteger = args[a++];
// eslint-disable-next-line valid-typeof
if (typeof tempInteger === 'bigint') {
tempStr = `${tempInteger}n`;
} else {
tempStr = `${parseInt(tempInteger)}`;
}
break;
case 102: // 'f'
tempStr = `${parseFloat(args[a++])}`;
break;
case 37: // '%'
str += first.slice(lastPos, i);
lastPos = i + 1;
continue;
default: // any other character is not a correct placeholder
continue;
}
case 105: // 'i'
const tempInteger = arguments[a++];
// eslint-disable-next-line valid-typeof
if (typeof tempInteger === 'bigint') {
tempStr = `${tempInteger}n`;
} else {
tempStr = `${parseInt(tempInteger)}`;
}
break;
case 102: // 'f'
tempStr = `${parseFloat(arguments[a++])}`;
break;
case 37: // '%'
str += f.slice(lastPos, i);
lastPos = i + 1;
continue;
default: // any other character is not a correct placeholder
continue;
if (lastPos !== i - 1) {
str += first.slice(lastPos, i - 1);
}
str += tempStr;
lastPos = i + 1;
} else if (nextChar === 37) {
str += first.slice(lastPos, i);
lastPos = i + 1;
}
if (lastPos !== i - 1)
str += f.slice(lastPos, i - 1);
str += tempStr;
lastPos = i + 1;
} else if (nextChar === 37) {
str += f.slice(lastPos, i);
lastPos = i + 1;
}
}
}
if (lastPos === 0)
str = f;
else if (lastPos < f.length)
str += f.slice(lastPos);
while (a < arguments.length) {
const x = arguments[a++];
if ((typeof x !== 'object' && typeof x !== 'symbol') || x === null) {
str += ` ${x}`;
} else {
str += ` ${inspect(x, inspectOptions)}`;
if (lastPos === 0) {
str = first;
} else if (lastPos < first.length) {
str += first.slice(lastPos);
}

parts.push(str);
while (a < args.length) {
parts.push(formatValue(args[a], inspectOptions));
a++;
}
} else {
for (const arg of args) {
parts.push(formatValue(arg, inspectOptions));
}
}
return str;

return parts.join(' ');
}

const debugs = {};
Expand Down
16 changes: 16 additions & 0 deletions test/parallel/test-util-format.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,10 @@ assert.strictEqual(util.format('percent: %d%, fraction: %d', 10, 0.1),
'percent: 10%, fraction: 0.1');
assert.strictEqual(util.format('abc%', 1), 'abc% 1');

// Additional arguments after format specifiers
assert.strictEqual(util.format('%i', 1, 'number'), '1 number');
assert.strictEqual(util.format('%i', 1, () => {}), '1 [Function]');

{
const o = {};
o.o = o;
Expand Down Expand Up @@ -315,3 +319,15 @@ function BadCustomError(msg) {
util.inherits(BadCustomError, Error);
assert.strictEqual(util.format(new BadCustomError('foo')),
'[BadCustomError: foo]');

// The format of arguments should not depend on type of the first argument
assert.strictEqual(util.format('1', '1'), '1 1');
assert.strictEqual(util.format(1, '1'), '1 1');
assert.strictEqual(util.format('1', 1), '1 1');
assert.strictEqual(util.format(1, 1), '1 1');
assert.strictEqual(util.format('1', () => {}), '1 [Function]');
assert.strictEqual(util.format(1, () => {}), '1 [Function]');
assert.strictEqual(util.format('1', "'"), "1 '");
assert.strictEqual(util.format(1, "'"), "1 '");
assert.strictEqual(util.format('1', 'number'), '1 number');
assert.strictEqual(util.format(1, 'number'), '1 number');

0 comments on commit c1b9be5

Please sign in to comment.