From 749aea7b08d6ac3bc08db15625a33c0a8c1e8da2 Mon Sep 17 00:00:00 2001 From: Brett Zamir Date: Wed, 7 Aug 2024 20:05:54 +0800 Subject: [PATCH 1/2] fix: permit appropriate computed member expressions and prototype access --- __tests__/spec-only.js | 19 +++++++++++++++++++ rules/lib/is-promise.js | 2 +- rules/lib/promise-statics.js | 18 +++++++++--------- rules/no-new-statics.js | 2 +- rules/spec-only.js | 14 +++++++++++--- 5 files changed, 41 insertions(+), 14 deletions(-) diff --git a/__tests__/spec-only.js b/__tests__/spec-only.js index ea93a378..c73acaad 100644 --- a/__tests__/spec-only.js +++ b/__tests__/spec-only.js @@ -9,7 +9,10 @@ ruleTester.run('spec-only', rule, { 'Promise.resolve()', 'Promise.reject()', 'Promise.all()', + 'Promise["all"]', + 'Promise[method];', 'Promise.race()', + 'var ctch = Promise.prototype.catch', 'Promise.withResolvers()', 'new Promise(function (resolve, reject) {})', 'SomeClass.resolve()', @@ -22,6 +25,14 @@ ruleTester.run('spec-only', rule, { }, ], }, + { + code: 'Promise.prototype.permittedInstanceMethod', + options: [ + { + allowedMethods: ['permittedInstanceMethod'], + }, + ], + }, ], invalid: [ { @@ -53,5 +64,13 @@ ruleTester.run('spec-only', rule, { `, errors: [{ message: "Avoid using non-standard 'Promise.done'" }], }, + { + code: `var done = Promise.prototype.done`, + errors: [{ message: "Avoid using non-standard 'Promise.prototype'" }], + }, + { + code: `Promise["done"];`, + errors: [{ message: "Avoid using non-standard 'Promise.done'" }], + }, ], }) diff --git a/rules/lib/is-promise.js b/rules/lib/is-promise.js index 840dfb2f..a13f2bb7 100644 --- a/rules/lib/is-promise.js +++ b/rules/lib/is-promise.js @@ -29,7 +29,7 @@ function isPromise(expression) { expression.callee.type === 'MemberExpression' && expression.callee.object.type === 'Identifier' && expression.callee.object.name === 'Promise' && - PROMISE_STATICS[expression.callee.property.name] && + PROMISE_STATICS.has(expression.callee.property.name) && expression.callee.property.name !== 'withResolvers') ) } diff --git a/rules/lib/promise-statics.js b/rules/lib/promise-statics.js index e42205a0..3cf7415f 100644 --- a/rules/lib/promise-statics.js +++ b/rules/lib/promise-statics.js @@ -1,11 +1,11 @@ 'use strict' -module.exports = { - all: true, - allSettled: true, - any: true, - race: true, - reject: true, - resolve: true, - withResolvers: true, -} +module.exports = new Set([ + 'all', + 'allSettled', + 'any', + 'race', + 'reject', + 'resolve', + 'withResolvers', +]) diff --git a/rules/no-new-statics.js b/rules/no-new-statics.js index c2483ce4..5274fe2e 100644 --- a/rules/no-new-statics.js +++ b/rules/no-new-statics.js @@ -22,7 +22,7 @@ module.exports = { if ( node.callee.type === 'MemberExpression' && node.callee.object.name === 'Promise' && - PROMISE_STATICS[node.callee.property.name] + PROMISE_STATICS.has(node.callee.property.name) ) { context.report({ node, diff --git a/rules/spec-only.js b/rules/spec-only.js index e5ab90a9..8031d585 100644 --- a/rules/spec-only.js +++ b/rules/spec-only.js @@ -3,6 +3,8 @@ const PROMISE_STATICS = require('./lib/promise-statics') const getDocsUrl = require('./lib/get-docs-url') +const PROMISE_INSTANCE_METHODS = new Set(['then', 'catch', 'finally']) + module.exports = { meta: { type: 'problem', @@ -35,14 +37,20 @@ module.exports = { MemberExpression(node) { if ( node.object.type === 'Identifier' && + (!node.computed || node.property.type === 'Literal') && node.object.name === 'Promise' && - !(node.property.name in PROMISE_STATICS) && - !allowedMethods.includes(node.property.name) + ((node.property.name && !PROMISE_STATICS.has(node.property.name)) || + (node.property.value && + !PROMISE_STATICS.has(node.property.value))) && + (node.property.name !== 'prototype' || + (!PROMISE_INSTANCE_METHODS.has(node?.parent?.property?.name) && + !allowedMethods.includes(node?.parent?.property?.name))) && + !allowedMethods.includes(node.property.name ?? node.property.value) ) { context.report({ node, messageId: 'avoidNonStandard', - data: { name: node.property.name }, + data: { name: node.property.name ?? node.property.value }, }) } }, From fd72cbc0190f120249937ba180476a5b116d1aed Mon Sep 17 00:00:00 2001 From: Brett Zamir Date: Thu, 17 Oct 2024 03:38:18 +0800 Subject: [PATCH 2/2] permit Promise.prototype['then'] access and test for other prototype access --- __tests__/spec-only.js | 3 +++ rules/spec-only.js | 39 +++++++++++++++++++++++++++++++-------- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/__tests__/spec-only.js b/__tests__/spec-only.js index c73acaad..e54f1860 100644 --- a/__tests__/spec-only.js +++ b/__tests__/spec-only.js @@ -11,6 +11,9 @@ ruleTester.run('spec-only', rule, { 'Promise.all()', 'Promise["all"]', 'Promise[method];', + 'Promise.prototype;', + 'Promise.prototype[method];', + 'Promise.prototype["then"];', 'Promise.race()', 'var ctch = Promise.prototype.catch', 'Promise.withResolvers()', diff --git a/rules/spec-only.js b/rules/spec-only.js index 8031d585..4554e3cc 100644 --- a/rules/spec-only.js +++ b/rules/spec-only.js @@ -5,6 +5,28 @@ const getDocsUrl = require('./lib/get-docs-url') const PROMISE_INSTANCE_METHODS = new Set(['then', 'catch', 'finally']) +function isPermittedProperty(expression, standardSet, allowedMethods) { + // istanbul ignore if + if (expression.type !== 'MemberExpression') return false + + if (expression.property.type === 'Literal') + return ( + standardSet.has(expression.property.value) || + allowedMethods.includes(expression.property.value) + ) + + // istanbul ignore else + if (expression.property.type === 'Identifier') + return ( + expression.computed || + standardSet.has(expression.property.name) || + allowedMethods.includes(expression.property.name) + ) + + // istanbul ignore next + return false +} + module.exports = { meta: { type: 'problem', @@ -37,15 +59,16 @@ module.exports = { MemberExpression(node) { if ( node.object.type === 'Identifier' && - (!node.computed || node.property.type === 'Literal') && node.object.name === 'Promise' && - ((node.property.name && !PROMISE_STATICS.has(node.property.name)) || - (node.property.value && - !PROMISE_STATICS.has(node.property.value))) && - (node.property.name !== 'prototype' || - (!PROMISE_INSTANCE_METHODS.has(node?.parent?.property?.name) && - !allowedMethods.includes(node?.parent?.property?.name))) && - !allowedMethods.includes(node.property.name ?? node.property.value) + ((node.property.name !== 'prototype' && + !isPermittedProperty(node, PROMISE_STATICS, allowedMethods)) || + (node.property.name === 'prototype' && + node.parent.type === 'MemberExpression' && + !isPermittedProperty( + node.parent, + PROMISE_INSTANCE_METHODS, + allowedMethods, + ))) ) { context.report({ node,