From c508bfc66bb6d7cf0867fccf0a42bc9f9ae97241 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 6 Nov 2020 15:20:06 +0100 Subject: [PATCH] assert: refactor to use more primordials PR-URL: https://github.com/nodejs/node/pull/35998 Reviewed-By: Rich Trott --- lib/internal/assert/assertion_error.js | 52 +++++++++++++++----------- lib/internal/assert/calltracker.js | 9 +++-- 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/lib/internal/assert/assertion_error.js b/lib/internal/assert/assertion_error.js index ff2d2dbc617557..c13a9d1ca1c3d8 100644 --- a/lib/internal/assert/assertion_error.js +++ b/lib/internal/assert/assertion_error.js @@ -1,6 +1,8 @@ 'use strict'; const { + ArrayPrototypeJoin, + ArrayPrototypePop, Error, ErrorCaptureStackTrace, MathMax, @@ -9,6 +11,10 @@ const { ObjectGetPrototypeOf, ObjectKeys, String, + StringPrototypeEndsWith, + StringPrototypeRepeat, + StringPrototypeSlice, + StringPrototypeSplit, } = primordials; const { inspect } = require('internal/util/inspect'); @@ -79,8 +85,8 @@ function createErrDiff(actual, expected, operator) { let end = ''; let skipped = false; const actualInspected = inspectValue(actual); - const actualLines = actualInspected.split('\n'); - const expectedLines = inspectValue(expected).split('\n'); + const actualLines = StringPrototypeSplit(actualInspected, '\n'); + const expectedLines = StringPrototypeSplit(inspectValue(expected), '\n'); let i = 0; let indicator = ''; @@ -127,7 +133,7 @@ function createErrDiff(actual, expected, operator) { if (i > 2) { // Add position indicator for the first mismatch in case it is a // single line and the input length is less than the column length. - indicator = `\n ${' '.repeat(i)}^`; + indicator = `\n ${StringPrototypeRepeat(' ', i)}^`; i = 0; } } @@ -144,8 +150,8 @@ function createErrDiff(actual, expected, operator) { } else { other = a; } - actualLines.pop(); - expectedLines.pop(); + ArrayPrototypePop(actualLines); + ArrayPrototypePop(expectedLines); if (actualLines.length === 0 || expectedLines.length === 0) break; a = actualLines[actualLines.length - 1]; @@ -157,18 +163,19 @@ function createErrDiff(actual, expected, operator) { // E.g., assert.deepStrictEqual({ a: Symbol() }, { a: Symbol() }) if (maxLines === 0) { // We have to get the result again. The lines were all removed before. - const actualLines = actualInspected.split('\n'); + const actualLines = StringPrototypeSplit(actualInspected, '\n'); // Only remove lines in case it makes sense to collapse those. // TODO: Accept env to always show the full error. if (actualLines.length > 50) { actualLines[46] = `${blue}...${white}`; while (actualLines.length > 47) { - actualLines.pop(); + ArrayPrototypePop(actualLines); } } - return `${kReadableOperator.notIdentical}\n\n${actualLines.join('\n')}\n`; + return `${kReadableOperator.notIdentical}\n\n` + + `${ArrayPrototypeJoin(actualLines, '\n')}\n`; } // There were at least five identical lines at the end. Mark a couple of @@ -235,9 +242,10 @@ function createErrDiff(actual, expected, operator) { // If the lines diverge, specifically check for lines that only diverge by // a trailing comma. In that case it is actually identical and we should // mark it as such. - let divergingLines = actualLine !== expectedLine && - (!actualLine.endsWith(',') || - actualLine.slice(0, -1) !== expectedLine); + let divergingLines = + actualLine !== expectedLine && + (!StringPrototypeEndsWith(actualLine, ',') || + StringPrototypeSlice(actualLine, 0, -1) !== expectedLine); // If the expected line has a trailing comma but is otherwise identical, // add a comma at the end of the actual line. Otherwise the output could // look weird as in: @@ -248,8 +256,8 @@ function createErrDiff(actual, expected, operator) { // ] // if (divergingLines && - expectedLine.endsWith(',') && - expectedLine.slice(0, -1) === actualLine) { + StringPrototypeEndsWith(expectedLine, ',') && + StringPrototypeSlice(expectedLine, 0, -1) === actualLine) { divergingLines = false; actualLine += ','; } @@ -362,7 +370,7 @@ class AssertionError extends Error { // In case the objects are equal but the operator requires unequal, show // the first object and say A equals B let base = kReadableOperator[operator]; - const res = inspectValue(actual).split('\n'); + const res = StringPrototypeSplit(inspectValue(actual), '\n'); // In case "actual" is an object or a function, it should not be // reference equal. @@ -377,7 +385,7 @@ class AssertionError extends Error { if (res.length > 50) { res[46] = `${blue}...${white}`; while (res.length > 47) { - res.pop(); + ArrayPrototypePop(res); } } @@ -385,7 +393,7 @@ class AssertionError extends Error { if (res.length === 1) { super(`${base}${res[0].length > 5 ? '\n\n' : ' '}${res[0]}`); } else { - super(`${base}\n\n${res.join('\n')}\n`); + super(`${base}\n\n${ArrayPrototypeJoin(res, '\n')}\n`); } } else { let res = inspectValue(actual); @@ -394,15 +402,15 @@ class AssertionError extends Error { if (operator === 'notDeepEqual' && res === other) { res = `${knownOperator}\n\n${res}`; if (res.length > 1024) { - res = `${res.slice(0, 1021)}...`; + res = `${StringPrototypeSlice(res, 0, 1021)}...`; } super(res); } else { if (res.length > 512) { - res = `${res.slice(0, 509)}...`; + res = `${StringPrototypeSlice(res, 0, 509)}...`; } if (other.length > 512) { - other = `${other.slice(0, 509)}...`; + other = `${StringPrototypeSlice(other, 0, 509)}...`; } if (operator === 'deepEqual') { res = `${knownOperator}\n\n${res}\n\nshould loosely deep-equal\n\n`; @@ -463,12 +471,12 @@ class AssertionError extends Error { for (const name of ['actual', 'expected']) { if (typeof this[name] === 'string') { - const lines = this[name].split('\n'); + const lines = StringPrototypeSplit(this[name], '\n'); if (lines.length > 10) { lines.length = 10; - this[name] = `${lines.join('\n')}\n...`; + this[name] = `${ArrayPrototypeJoin(lines, '\n')}\n...`; } else if (this[name].length > 512) { - this[name] = `${this[name].slice(512)}...`; + this[name] = `${StringPrototypeSlice(this[name], 512)}...`; } } } diff --git a/lib/internal/assert/calltracker.js b/lib/internal/assert/calltracker.js index 74f517f3f9e99b..d45fb67d611e8b 100644 --- a/lib/internal/assert/calltracker.js +++ b/lib/internal/assert/calltracker.js @@ -1,7 +1,10 @@ 'use strict'; const { + ArrayPrototypePush, Error, + FunctionPrototype, + ReflectApply, SafeSet, } = primordials; @@ -15,7 +18,7 @@ const { validateUint32, } = require('internal/validators'); -const noop = () => {}; +const noop = FunctionPrototype; class CallTracker { @@ -55,7 +58,7 @@ class CallTracker { if (context.actual === context.exact + 1) { callChecks.add(context); } - return fn.apply(this, arguments); + return ReflectApply(fn, this, arguments); }; } @@ -67,7 +70,7 @@ class CallTracker { const message = `Expected the ${context.name} function to be ` + `executed ${context.exact} time(s) but was ` + `executed ${context.actual} time(s).`; - errors.push({ + ArrayPrototypePush(errors, { message, actual: context.actual, expected: context.exact,