1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

assert: improve simple assert

Browse Source 
1) If simple assert is called in the very first line of a file and
it causes an error, it used to report the wrong code. The reason
is that the column that is reported is faulty. This is fixed by
subtracting the offset from now on in such cases.

2) The actual code read is now limited to the part that is actually
required to visualize the call site. All other code in e.g. minified
files will not cause a significant overhead anymore.

3) The number of allocations is now significantly lower than it used
to be. The buffer is reused until the correct line in the code is
found. In general the algorithm tries to safe operations where
possible.

4) The indentation is now corrected depending on where the statement
actually beginns.

5) It is now possible to handle `.call()` and `.apply()` properly.

6) The user defined function name will now always be used instead of
only choosing either `assert.ok()` or `assert()`.

PR-URL: https://github.com/nodejs/node/pull/21626
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ben Coe <bencoe@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Ruben Bridgewater 2018-07-02 20:29:57 +02:00
parent 7eeb7948b3
commit 43ee4d692a
No known key found for this signature in database
GPG Key ID: F07496B3EB3C1762
5 changed files with 222 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

222
lib/assert.js
View File

@ -35,6 +35,10 @@ const { NativeModule } = require('internal/bootstrap/loaders');
let isDeepEqual;
let isDeepStrictEqual;
let parseExpressionAt;
let findNodeAround;
let columnOffset = 0;
let decoder;
function lazyLoadComparison() {
const comparison = require('internal/util/comparisons');
@ -114,52 +118,141 @@ function fail(actual, expected, message, operator, stackStartFn) {
assert.fail = fail;
// The AssertionError is defined in internal/error.
// new assert.AssertionError({ message: message,
// actual: actual,
// expected: expected });
assert.AssertionError = AssertionError;
function getBuffer(fd, assertLine) {
function findColumn(fd, column, code) {
if (code.length > column + 100) {
try {
return parseCode(code, column);
} catch {
// End recursion in case no code could be parsed. The expression should
// have been found after 2500 characters, so stop trying.
if (code.length - column > 2500) {
// eslint-disable-next-line no-throw-literal
throw null;
}
}
}
// Read up to 2500 bytes more than necessary in columns. That way we address
// multi byte characters and read enough data to parse the code.
const bytesToRead = column - code.length + 2500;
const buffer = Buffer.allocUnsafe(bytesToRead);
const bytesRead = readSync(fd, buffer, 0, bytesToRead);
code += decoder.write(buffer.slice(0, bytesRead));
// EOF: fast path.
if (bytesRead < bytesToRead) {
return parseCode(code, column);
}
// Read potentially missing code.
return findColumn(fd, column, code);
}
function getCode(fd, line, column) {
let bytesRead = 0;
if (line === 0) {
// Special handle line number one. This is more efficient and simplifies the
// rest of the algorithm. Read more than the regular column number in bytes
// to prevent multiple reads in case multi byte characters are used.
return findColumn(fd, column, '');
}
let lines = 0;
// Prevent blocking the event loop by limiting the maximum amount of
// data that may be read.
let maxReads = 64; // bytesPerRead * maxReads = 512 kb
let bytesRead = 0;
let startBuffer = 0; // Start reading from that char on
const bytesPerRead = 8192;
const buffers = [];
do {
const buffer = Buffer.allocUnsafe(bytesPerRead);
// Use a single buffer up front that is reused until the call site is found.
let buffer = Buffer.allocUnsafe(bytesPerRead);
while (maxReads-- !== 0) {
// Only allocate a new buffer in case the needed line is found. All data
// before that can be discarded.
buffer = lines < line ? buffer : Buffer.allocUnsafe(bytesPerRead);
bytesRead = readSync(fd, buffer, 0, bytesPerRead);
// Read the buffer until the required code line is found.
for (var i = 0; i < bytesRead; i++) {
if (buffer[i] === 10) {
lines++;
if (lines === assertLine) {
startBuffer = i + 1;
// Read up to 15 more lines to make sure all code gets matched
} else if (lines === assertLine + 16) {
buffers.push(buffer.slice(startBuffer, i));
return buffers;
if (buffer[i] === 10 && ++lines === line) {
// If the end of file is reached, directly parse the code and return.
if (bytesRead < bytesPerRead) {
return parseCode(buffer.toString('utf8', i + 1, bytesRead), column);
}
// Check if the read code is sufficient or read more until the whole
// expression is read. Make sure multi byte characters are preserved
// properly by using the decoder.
const code = decoder.write(buffer.slice(i + 1, bytesRead));
return findColumn(fd, column, code);
}
}
}
if (lines >= assertLine) {
buffers.push(buffer.slice(startBuffer, bytesRead));
// Reset the startBuffer in case we need more than one chunk
startBuffer = 0;
}
} while (--maxReads !== 0 && bytesRead !== 0);
return buffers;
}
function getErrMessage(call) {
function parseCode(code, offset) {
// Lazy load acorn.
if (parseExpressionAt === undefined) {
({ parseExpressionAt } = require('internal/deps/acorn/dist/acorn'));
({ findNodeAround } = require('internal/deps/acorn/dist/walk'));
}
let node;
let start = 0;
// Parse the read code until the correct expression is found.
do {
try {
node = parseExpressionAt(code, start);
start = node.end + 1 || start;
// Find the CallExpression in the tree.
node = findNodeAround(node, offset, 'CallExpression');
} catch (err) {
// Unexpected token error and the like.
start += err.raisedAt || 1;
if (start > offset) {
// No matching expression found. This could happen if the assert
// expression is bigger than the provided buffer.
// eslint-disable-next-line no-throw-literal
throw null;
}
}
} while (node === undefined || node.node.end < offset);
return [
node.node.start,
code.slice(node.node.start, node.node.end)
.replace(escapeSequencesRegExp, escapeFn)
];
}
function getErrMessage(message, fn) {
const tmpLimit = Error.stackTraceLimit;
// Make sure the limit is set to 1. Otherwise it could fail (<= 0) or it
// does to much work.
Error.stackTraceLimit = 1;
// We only need the stack trace. To minimize the overhead use an object
// instead of an error.
const err = {};
Error.captureStackTrace(err, fn);
Error.stackTraceLimit = tmpLimit;
const tmpPrepare = Error.prepareStackTrace;
Error.prepareStackTrace = (_, stack) => stack;
const call = err.stack[0];
Error.prepareStackTrace = tmpPrepare;
const filename = call.getFileName();
if (!filename) {
return;
return message;
}
const line = call.getLineNumber() - 1;
const column = call.getColumnNumber() - 1;
let column = call.getColumnNumber() - 1;
// Line number one reports the wrong column due to being wrapped in a
// function. Remove that offset to get the actual call.
if (line === 0) {
if (columnOffset === 0) {
const { wrapper } = require('internal/modules/cjs/loader');
columnOffset = wrapper[0].length;
}
column -= columnOffset;
}
const identifier = `${filename}${line}${column}`;
if (errorCache.has(identifier)) {
@ -172,57 +265,49 @@ function getErrMessage(call) {
return;
}
let fd, message;
let fd;
try {
// Set the stack trace limit to zero. This makes sure unexpected token
// errors are handled faster.
Error.stackTraceLimit = 0;
if (decoder === undefined) {
const { StringDecoder } = require('string_decoder');
decoder = new StringDecoder('utf8');
}
fd = openSync(filename, 'r', 0o666);
const buffers = getBuffer(fd, line);
const code = Buffer.concat(buffers).toString('utf8');
// Lazy load acorn.
const { parseExpressionAt } = require('internal/deps/acorn/dist/acorn');
const nodes = parseExpressionAt(code, column);
// Node type should be "CallExpression" and some times
// "SequenceExpression".
const node = nodes.type === 'CallExpression' ? nodes : nodes.expressions[0];
const name = node.callee.name;
// Calling `ok` with .apply or .call is uncommon but we use a simple
// safeguard nevertheless.
if (name !== 'apply' && name !== 'call') {
// Only use `assert` and `assert.ok` to reference the "real API" and
// not user defined function names.
const ok = name === 'ok' ? '.ok' : '';
const args = node.arguments;
message = code
.slice(args[0].start, args[args.length - 1].end)
.replace(escapeSequencesRegExp, escapeFn);
// Reset column and message.
[column, message] = getCode(fd, line, column);
// Flush unfinished multi byte characters.
decoder.end();
// Always normalize indentation, otherwise the message could look weird.
if (message.indexOf('\n') !== -1) {
if (EOL === '\r\n') {
message = message.replace(/\r\n/g, '\n');
}
// Always normalize indentation, otherwise the message could look weird.
if (message.indexOf('\n') !== -1) {
const tmp = message.split('\n');
message = tmp[0];
for (var i = 1; i < tmp.length; i++) {
const frames = message.split('\n');
message = frames.shift();
for (const frame of frames) {
let pos = 0;
while (pos < column &&
(tmp[i][pos] === ' ' || tmp[i][pos] === '\t')) {
while (pos < column && (frame[pos] === ' ' || frame[pos] === '\t')) {
pos++;
}
message += `\n ${tmp[i].slice(pos)}`;
message += `\n ${frame.slice(pos)}`;
}
}
message = 'The expression evaluated to a falsy value:' +
`\n\n assert${ok}(${message})\n`;
}
message = `The expression evaluated to a falsy value:\n\n ${message}\n`;
// Make sure to always set the cache! No matter if the message is
// undefined or not
errorCache.set(identifier, message);
return message;
} catch (e) {
// Invalidate cache to prevent trying to read this part again.
errorCache.set(identifier, undefined);
} finally {
// Reset limit.
Error.stackTraceLimit = tmpLimit;
if (fd !== undefined)
closeSync(fd);
}
@ -236,25 +321,8 @@ function innerOk(fn, argLen, value, message) {
generatedMessage = true;
message = 'No value argument passed to `assert.ok()`';
} else if (message == null) {
// Use the call as error message if possible.
// This does not work with e.g. the repl.
// eslint-disable-next-line no-restricted-syntax
const err = new Error();
// Make sure the limit is set to 1. Otherwise it could fail (<= 0) or it
// does to much work.
const tmpLimit = Error.stackTraceLimit;
Error.stackTraceLimit = 1;
Error.captureStackTrace(err, fn);
Error.stackTraceLimit = tmpLimit;
const tmpPrepare = Error.prepareStackTrace;
Error.prepareStackTrace = (_, stack) => stack;
const call = err.stack[0];
Error.prepareStackTrace = tmpPrepare;
// Make sure it would be "null" in case that is used.
message = getErrMessage(call) || message;
generatedMessage = true;
message = getErrMessage(message, fn);
} else if (message instanceof Error) {
throw message;
}

2
test/fixtures/assert-first-line.js vendored Normal file
View File

@ -0,0 +1,2 @@
'use strict'; const ässört = require('assert'); ässört(true); ässört.ok(''); ässört(null);
// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(false);

1
test/fixtures/assert-long-line.js vendored Normal file
View File

File diff suppressed because one or more lines are too long

23
test/parallel/test-assert-first-line.js Normal file
View File

@ -0,0 +1,23 @@
'use strict';
// Verify that asserting in the very first line produces the expected result.
require('../common');
const assert = require('assert');
const { path } = require('../common/fixtures');
assert.throws(
() => require(path('assert-first-line')),
{
name: 'AssertionError [ERR_ASSERTION]',
message: "The expression evaluated to a falsy value:\n\n ässört.ok('')\n"
}
);
assert.throws(
() => require(path('assert-long-line')),
{
name: 'AssertionError [ERR_ASSERTION]',
message: "The expression evaluated to a falsy value:\n\n assert.ok('')\n"
}
);

63
test/parallel/test-assert.js
View File

@ -457,7 +457,7 @@ assert.throws(
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'The expression evaluated to a falsy value:\n\n ' +
"assert.ok(typeof 123 === 'string')\n"
"assert.ok(\n typeof 123 === 'string'\n )\n"
}
);
Error.stackTraceLimit = tmpLimit;
@ -661,7 +661,7 @@ common.expectsError(
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'The expression evaluated to a falsy value:\n\n ' +
"assert(Buffer.from('test') instanceof Error)\n"
"assert(\n (Buffer.from('test') instanceof Error)\n )\n"
}
);
common.expectsError(
@ -670,7 +670,7 @@ common.expectsError(
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'The expression evaluated to a falsy value:\n\n ' +
"assert(Buffer.from('test') instanceof Error)\n"
"assert(\n (Buffer.from('test') instanceof Error)\n )\n"
}
);
fs.close = tmp;
@ -690,11 +690,13 @@ common.expectsError(
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'The expression evaluated to a falsy value:\n\n' +
' assert((() => \'string\')()\n' +
' a(\n' +
' (() => \'string\')()\n' +
' // eslint-disable-next-line\n' +
' ===\n' +
' 123 instanceof\n' +
' Buffer)\n'
' Buffer\n' +
' )\n'
}
);
@ -712,11 +714,13 @@ common.expectsError(
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'The expression evaluated to a falsy value:\n\n' +
' assert((() => \'string\')()\n' +
' a(\n' +
' (() => \'string\')()\n' +
' // eslint-disable-next-line\n' +
' ===\n' +
' 123 instanceof\n' +
' Buffer)\n'
' Buffer\n' +
' )\n'
}
);
@ -731,16 +735,19 @@ Buffer
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'The expression evaluated to a falsy value:\n\n' +
' assert((\n' +
' a((\n' +
' () => \'string\')() ===\n' +
' 123 instanceof\n' +
' Buffer)\n'
' Buffer\n' +
' )\n'
}
);
/* eslint-enable indent */
common.expectsError(
() => assert(null, undefined),
() => {
assert(true); assert(null, undefined);
},
{
code: 'ERR_ASSERTION',
type: assert.AssertionError,
@ -750,11 +757,38 @@ common.expectsError(
);
common.expectsError(
() => assert.ok.apply(null, [0]),
() => {
assert
.ok(null, undefined);
},
{
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: '0 == true'
message: 'The expression evaluated to a falsy value:\n\n ' +
'ok(null, undefined)\n'
}
);
common.expectsError(
// eslint-disable-next-line dot-notation, quotes
() => assert['ok']["apply"](null, [0]),
{
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'The expression evaluated to a falsy value:\n\n ' +
'assert[\'ok\']["apply"](null, [0])\n'
}
);
common.expectsError(
() => {
const wrapper = (fn, value) => fn(value);
wrapper(assert, false);
},
{
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'The expression evaluated to a falsy value:\n\n fn(value)\n'
}
);
@ -763,7 +797,8 @@ common.expectsError(
{
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: '0 == true',
message: 'The expression evaluated to a falsy value:\n\n ' +
'assert.ok.call(null, 0)\n',
generatedMessage: true
}
);
@ -778,7 +813,7 @@ common.expectsError(
}
);
// works in eval
// Works in eval.
common.expectsError(
() => new Function('assert', 'assert(1 === 2);')(assert),
{