Skip to content

Commit

Permalink
fix: permit appropriate computed member expressions and prototype acc…
Browse files Browse the repository at this point in the history
…ess (#535)

Permits appropriate computed member expressions such as `Promise['all']` and suitable prototype access (e.g., `Promise.prototype.catch`) but errs on detectable problems (e.g., `Promise.prototype.done`).
  • Loading branch information
brettz9 authored Oct 26, 2024
1 parent 05c8a93 commit 4de9d43
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 14 deletions.
22 changes: 22 additions & 0 deletions __tests__/spec-only.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ ruleTester.run('spec-only', rule, {
'Promise.resolve()',
'Promise.reject()',
'Promise.all()',
'Promise["all"]',
'Promise[method];',
'Promise.prototype;',
'Promise.prototype[method];',
'Promise.prototype["then"];',
'Promise.race()',
'var ctch = Promise.prototype.catch',
'Promise.withResolvers()',
'new Promise(function (resolve, reject) {})',
'SomeClass.resolve()',
Expand All @@ -22,6 +28,14 @@ ruleTester.run('spec-only', rule, {
},
],
},
{
code: 'Promise.prototype.permittedInstanceMethod',
options: [
{
allowedMethods: ['permittedInstanceMethod'],
},
],
},
],
invalid: [
{
Expand Down Expand Up @@ -53,5 +67,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'" }],
},
],
})
2 changes: 1 addition & 1 deletion rules/lib/is-promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
)
}
Expand Down
18 changes: 9 additions & 9 deletions rules/lib/promise-statics.js
Original file line number Diff line number Diff line change
@@ -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',
])
2 changes: 1 addition & 1 deletion rules/no-new-statics.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
37 changes: 34 additions & 3 deletions rules/spec-only.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,30 @@
const PROMISE_STATICS = require('./lib/promise-statics')
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',
Expand Down Expand Up @@ -36,13 +60,20 @@ module.exports = {
if (
node.object.type === 'Identifier' &&
node.object.name === 'Promise' &&
!(node.property.name in PROMISE_STATICS) &&
!allowedMethods.includes(node.property.name)
((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,
messageId: 'avoidNonStandard',
data: { name: node.property.name },
data: { name: node.property.name ?? node.property.value },
})
}
},
Expand Down

0 comments on commit 4de9d43

Please sign in to comment.