From 9b74eca3f4020153daf1f47c2873f73c79ce5778 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 9 Mar 2020 21:53:03 +0100 Subject: [PATCH 1/3] tools: add eslint rule to only pass through 'test' to debuglog This makes sure all usages of `util.debuglog()` must contain the string 'test' as argument. --- test/.eslintrc.yaml | 36 ++++++++++++++++++++++++++++++ test/sequential/test-util-debug.js | 1 + 2 files changed, 37 insertions(+) diff --git a/test/.eslintrc.yaml b/test/.eslintrc.yaml index 86d4b595e7dc9a..0d42b922c72ea3 100644 --- a/test/.eslintrc.yaml +++ b/test/.eslintrc.yaml @@ -11,6 +11,42 @@ rules: prefer-const: error symbol-description: off + no-restricted-syntax: + # Config copied from .eslintrc.js + - error + - selector: "CallExpression:matches([callee.name='deepStrictEqual'], [callee.property.name='deepStrictEqual'])[arguments.2.type='Literal']" + message: "Do not use a literal for the third argument of assert.deepStrictEqual()" + - selector: "CallExpression:matches([callee.name='doesNotThrow'], [callee.property.name='doesNotThrow'])" + message: "Do not use `assert.doesNotThrow()`. Write the code without the wrapper and add a comment instead." + - selector: "CallExpression:matches([callee.name='doesNotReject'], [callee.property.name='doesNotReject'])" + message: "Do not use `assert.doesNotReject()`. Write the code without the wrapper and add a comment instead." + - selector: "CallExpression:matches([callee.name='rejects'], [callee.property.name='rejects'])[arguments.length<2]" + message: "`assert.rejects()` must be invoked with at least two arguments." + - selector: "CallExpression[callee.property.name='strictEqual'][arguments.2.type='Literal']" + message: "Do not use a literal for the third argument of assert.strictEqual()" + - selector: "CallExpression:matches([callee.name='throws'], [callee.property.name='throws'])[arguments.1.type='Literal']:not([arguments.1.regex])" + message: "Use an object as second argument of `assert.throws()`." + - selector: "CallExpression:matches([callee.name='throws'], [callee.property.name='throws'])[arguments.length<2]" + message: "`assert.throws()` must be invoked with at least two arguments." + - selector: "CallExpression[callee.name='setTimeout'][arguments.length<2]" + message: "`setTimeout()` must be invoked with at least two arguments." + - selector: "CallExpression[callee.name='setInterval'][arguments.length<2]" + message: "`setInterval()` must be invoked with at least two arguments." + - selector: 'ThrowStatement > CallExpression[callee.name=/Error$/]' + message: "Use `new` keyword when throwing an `Error`." + - selector: "CallExpression:matches([callee.name='notDeepStrictEqual'], [callee.property.name='notDeepStrictEqual'])[arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression']):not([arguments.1.type='UnaryExpression'])" + message: "The first argument should be the `actual`, not the `expected` value." + - selector: "CallExpression:matches([callee.name='notStrictEqual'], [callee.property.name='notStrictEqual'])[arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression']):not([arguments.1.type='UnaryExpression'])" + message: "The first argument should be the `actual`, not the `expected` value." + - selector: "CallExpression:matches([callee.name='deepStrictEqual'], [callee.property.name='deepStrictEqual'])[arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression']):not([arguments.1.type='UnaryExpression'])" + message: "The first argument should be the `actual`, not the `expected` value." + - selector: "CallExpression:matches([callee.name='strictEqual'], [callee.property.name='strictEqual'])[arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression']):not([arguments.1.type='UnaryExpression'])" + message: "The first argument should be the `actual`, not the `expected` value." + - selector: "CallExpression[callee.name='isNaN']" + message: "Use Number.isNaN() instead of the global isNaN() function." + - selector: "CallExpression:matches([callee.name='debuglog'], [callee.property.name='debuglog']):not([arguments.0.value='test'])" + message: "Only use 'test' as debuglog value inside of the tests folder." + # Custom rules in tools/eslint-rules node-core/prefer-assert-iferror: error node-core/prefer-assert-methods: error diff --git a/test/sequential/test-util-debug.js b/test/sequential/test-util-debug.js index f791eb8a8d2513..6cad46f101c0af 100644 --- a/test/sequential/test-util-debug.js +++ b/test/sequential/test-util-debug.js @@ -117,6 +117,7 @@ function child(section) { Object.defineProperty(process.stderr, 'hasColors', { value: tty.WriteStream.prototype.hasColors }); + // eslint-disable-next-line no-restricted-syntax const debug = util.debuglog(section); debug('this', { is: 'a' }, /debugging/); debug('num=%d str=%s obj=%j', 1, 'a', { foo: 'bar' }); From 39e869d1856987e5d810894429505b495fbc5a68 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 9 Mar 2020 21:56:04 +0100 Subject: [PATCH 2/3] tools: remove obsolete no-restricted-syntax eslint rules These rules only apply for the test folder and will already be checked for. --- .eslintrc.js | 44 -------------------------------------------- 1 file changed, 44 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index ba94036b0f50bc..14c53277a19a47 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -189,34 +189,6 @@ module.exports = { // as well to lib/.eslintrc.yaml. 'no-restricted-syntax': [ 'error', - { - selector: "CallExpression[callee.property.name='deepStrictEqual'][arguments.2.type='Literal']", - message: 'Do not use a literal for the third argument of assert.deepStrictEqual()', - }, - { - selector: "CallExpression[callee.property.name='doesNotThrow']", - message: 'Do not use `assert.doesNotThrow()`. Write the code without the wrapper and add a comment instead.', - }, - { - selector: "CallExpression[callee.property.name='doesNotReject']", - message: 'Do not use `assert.doesNotReject()`. Write the code without the wrapper and add a comment instead.', - }, - { - selector: "CallExpression[callee.property.name='rejects'][arguments.length<2]", - message: '`assert.rejects()` must be invoked with at least two arguments.', - }, - { - selector: "CallExpression[callee.property.name='strictEqual'][arguments.2.type='Literal']", - message: 'Do not use a literal for the third argument of assert.strictEqual()', - }, - { - selector: "CallExpression[callee.property.name='throws'][arguments.1.type='Literal']:not([arguments.1.regex])", - message: 'Use an object as second argument of `assert.throws()`.', - }, - { - selector: "CallExpression[callee.property.name='throws'][arguments.length<2]", - message: '`assert.throws()` must be invoked with at least two arguments.', - }, { selector: "CallExpression[callee.name='setTimeout'][arguments.length<2]", message: '`setTimeout()` must be invoked with at least two arguments.', @@ -229,22 +201,6 @@ module.exports = { selector: 'ThrowStatement > CallExpression[callee.name=/Error$/]', message: 'Use `new` keyword when throwing an `Error`.', }, - { - selector: "CallExpression[callee.property.name='notDeepStrictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression']):not([arguments.1.type='UnaryExpression'])", - message: 'The first argument should be the `actual`, not the `expected` value.', - }, - { - selector: "CallExpression[callee.property.name='notStrictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression']):not([arguments.1.type='UnaryExpression'])", - message: 'The first argument should be the `actual`, not the `expected` value.', - }, - { - selector: "CallExpression[callee.property.name='deepStrictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression']):not([arguments.1.type='UnaryExpression'])", - message: 'The first argument should be the `actual`, not the `expected` value.', - }, - { - selector: "CallExpression[callee.property.name='strictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression']):not([arguments.1.type='UnaryExpression'])", - message: 'The first argument should be the `actual`, not the `expected` value.', - }, { selector: "CallExpression[callee.name='isNaN']", message: 'Use Number.isNaN() instead of the global isNaN() function.', From a5144ed429335ce47e429994ed2b4b1126695810 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sun, 10 May 2020 01:51:51 +0200 Subject: [PATCH 3/3] fixup! tools: add eslint rule to only pass through 'test' to debuglog Signed-off-by: Ruben Bridgewater --- test/.eslintrc.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/.eslintrc.yaml b/test/.eslintrc.yaml index 0d42b922c72ea3..f707caffdafde4 100644 --- a/test/.eslintrc.yaml +++ b/test/.eslintrc.yaml @@ -32,7 +32,7 @@ rules: message: "`setTimeout()` must be invoked with at least two arguments." - selector: "CallExpression[callee.name='setInterval'][arguments.length<2]" message: "`setInterval()` must be invoked with at least two arguments." - - selector: 'ThrowStatement > CallExpression[callee.name=/Error$/]' + - selector: "ThrowStatement > CallExpression[callee.name=/Error$/]" message: "Use `new` keyword when throwing an `Error`." - selector: "CallExpression:matches([callee.name='notDeepStrictEqual'], [callee.property.name='notDeepStrictEqual'])[arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression']):not([arguments.1.type='UnaryExpression'])" message: "The first argument should be the `actual`, not the `expected` value." @@ -44,8 +44,8 @@ rules: message: "The first argument should be the `actual`, not the `expected` value." - selector: "CallExpression[callee.name='isNaN']" message: "Use Number.isNaN() instead of the global isNaN() function." - - selector: "CallExpression:matches([callee.name='debuglog'], [callee.property.name='debuglog']):not([arguments.0.value='test'])" - message: "Only use 'test' as debuglog value inside of the tests folder." + - selector: "VariableDeclarator > CallExpression:matches([callee.name='debuglog'], [callee.property.name='debuglog']):not([arguments.0.value='test'])" + message: "Use 'test' as debuglog value in tests." # Custom rules in tools/eslint-rules node-core/prefer-assert-iferror: error