From 76fbda3e140f06a2cb1599df6a8e97d32b6d8290 Mon Sep 17 00:00:00 2001 From: Hideki Nakamura Date: Tue, 21 Feb 2023 23:44:51 +0900 Subject: [PATCH 1/4] assert: fix exception message for assert(0) on try catch block Fixes: https://github.com/nodejs/node/issues/30872 --- lib/assert.js | 44 ++++++++++++++++++++---------------- test/parallel/test-assert.js | 43 +++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 20 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index f200507ad60537..e60e670765704a 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -78,6 +78,7 @@ let isDeepEqual; let isDeepStrictEqual; let parseExpressionAt; let findNodeAround; +let tokenizer; let decoder; function lazyLoadComparison() { @@ -242,39 +243,42 @@ function getCode(fd, line, column) { function parseCode(code, offset) { // Lazy load acorn. - if (parseExpressionAt === undefined) { + if (parseExpressionAt === undefined || tokenizer === undefined) { const Parser = require('internal/deps/acorn/acorn/dist/acorn').Parser; ({ findNodeAround } = require('internal/deps/acorn/acorn-walk/dist/walk')); parseExpressionAt = FunctionPrototypeBind(Parser.parseExpressionAt, Parser); + tokenizer = FunctionPrototypeBind(Parser.tokenizer, Parser); } let node; - let start = 0; + let start; // Parse the read code until the correct expression is found. - do { + for (const token of tokenizer(code, { ecmaVersion: 'latest' })) { + start = token.start; + if (start > offset) { + // No matching expression found. This could happen if the assert + // expression is bigger than the provided buffer. + break; + } try { node = parseExpressionAt(code, start, { ecmaVersion: 'latest' }); - 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; + if (node && node.node.end >= offset) { + return [ + node.node.start, + StringPrototypeReplace(StringPrototypeSlice(code, + node.node.start, node.node.end), + escapeSequencesRegExp, escapeFn), + ]; } + // eslint-disable-next-line no-unused-vars + } catch (err) { + continue; } - } while (node === undefined || node.node.end < offset); - - return [ - node.node.start, - StringPrototypeReplace(StringPrototypeSlice(code, - node.node.start, node.node.end), - escapeSequencesRegExp, escapeFn), - ]; + } + // eslint-disable-next-line no-throw-literal + throw null; } function getErrMessage(message, fn) { diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index 0a270d030feea6..45028b7bc00409 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -726,6 +726,49 @@ assert.throws( 'assert.ok(null)\n' } ); +assert.throws( + () => { + try { assert.ok(0); // eslint-disable-line no-useless-catch, brace-style + } catch (err) { + throw err; + } + }, + { + code: 'ERR_ASSERTION', + constructor: assert.AssertionError, + generatedMessage: true, + message: 'The expression evaluated to a falsy value:\n\n ' + + 'assert.ok(0)\n' + } +); +assert.throws( + () => { + try { + throw new Error(); + } catch (err) { assert.ok(0); } // eslint-disable-line no-unused-vars + }, + { + code: 'ERR_ASSERTION', + constructor: assert.AssertionError, + generatedMessage: true, + message: 'The expression evaluated to a falsy value:\n\n ' + + 'assert.ok(0)\n' + } +); +assert.throws( + () => { + function test() { assert.ok(0); // eslint-disable-line brace-style + } + test(); + }, + { + code: 'ERR_ASSERTION', + constructor: assert.AssertionError, + generatedMessage: true, + message: 'The expression evaluated to a falsy value:\n\n ' + + 'assert.ok(0)\n' + } +); assert.throws( () => assert(typeof 123n === 'string'), { From 3e8d7aeace418a7b8ea9222d7b6ce1b0d95ee70c Mon Sep 17 00:00:00 2001 From: Hideki Nakamura Date: Wed, 22 Feb 2023 22:29:11 +0900 Subject: [PATCH 2/4] add comments asking not to reformat --- test/parallel/test-assert.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index 45028b7bc00409..521d28b62a3ad8 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -728,6 +728,9 @@ assert.throws( ); assert.throws( () => { + // This test case checks if `try` left brace without a line break + // before the assertion causes any wrong assertion message. + // Therefore, don't reformat the following code. try { assert.ok(0); // eslint-disable-line no-useless-catch, brace-style } catch (err) { throw err; @@ -745,6 +748,9 @@ assert.throws( () => { try { throw new Error(); + // This test case checks if `catch` left brace without a line break + // before the assertion causes any wrong assertion message. + // Therefore, don't reformat the following code. } catch (err) { assert.ok(0); } // eslint-disable-line no-unused-vars }, { @@ -757,6 +763,9 @@ assert.throws( ); assert.throws( () => { + // This test case checks if `function` left brace without a line break + // before the assertion causes any wrong assertion message. + // Therefore, don't reformat the following code. function test() { assert.ok(0); // eslint-disable-line brace-style } test(); From 1cb6f7c748050cc2d17425e26722a56770b8cac4 Mon Sep 17 00:00:00 2001 From: Hideki Nakamura Date: Wed, 22 Feb 2023 22:36:30 +0900 Subject: [PATCH 3/4] Update lib/assert.js --- lib/assert.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index e60e670765704a..a7d44b91e76e9e 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -243,7 +243,7 @@ function getCode(fd, line, column) { function parseCode(code, offset) { // Lazy load acorn. - if (parseExpressionAt === undefined || tokenizer === undefined) { + if (parseExpressionAt === undefined) { const Parser = require('internal/deps/acorn/acorn/dist/acorn').Parser; ({ findNodeAround } = require('internal/deps/acorn/acorn-walk/dist/walk')); @@ -264,7 +264,7 @@ function parseCode(code, offset) { node = parseExpressionAt(code, start, { ecmaVersion: 'latest' }); // Find the CallExpression in the tree. node = findNodeAround(node, offset, 'CallExpression'); - if (node && node.node.end >= offset) { + if (node?.node.end >= offset) { return [ node.node.start, StringPrototypeReplace(StringPrototypeSlice(code, From b7b8f643e40f9d2795db7a5c58a7db5430a65d56 Mon Sep 17 00:00:00 2001 From: Hideki Nakamura Date: Fri, 24 Feb 2023 18:01:25 +0900 Subject: [PATCH 4/4] Add a link to the issue in the added comments --- test/parallel/test-assert.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index 521d28b62a3ad8..0343e8422885ad 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -731,6 +731,7 @@ assert.throws( // This test case checks if `try` left brace without a line break // before the assertion causes any wrong assertion message. // Therefore, don't reformat the following code. + // Refs: https://github.com/nodejs/node/issues/30872 try { assert.ok(0); // eslint-disable-line no-useless-catch, brace-style } catch (err) { throw err; @@ -751,6 +752,7 @@ assert.throws( // This test case checks if `catch` left brace without a line break // before the assertion causes any wrong assertion message. // Therefore, don't reformat the following code. + // Refs: https://github.com/nodejs/node/issues/30872 } catch (err) { assert.ok(0); } // eslint-disable-line no-unused-vars }, { @@ -766,6 +768,7 @@ assert.throws( // This test case checks if `function` left brace without a line break // before the assertion causes any wrong assertion message. // Therefore, don't reformat the following code. + // Refs: https://github.com/nodejs/node/issues/30872 function test() { assert.ok(0); // eslint-disable-line brace-style } test();