Skip to content

Commit

Permalink
feat(no-inline-implementation): detect inline implementation inside s…
Browse files Browse the repository at this point in the history
…pawn calls with xstate v4

Also prevent false positives on inline implementations within the second argument to createMachine.
  • Loading branch information
rlaffers committed Aug 16, 2023
1 parent bee4ad5 commit 9654443
Show file tree
Hide file tree
Showing 5 changed files with 324 additions and 167 deletions.
25 changes: 21 additions & 4 deletions docs/rules/no-inline-implementation.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,12 @@ createMachine({
actions: () => {} // inlined action
}
},
activities: () => {} // inlined activity
activities: () => {}, // inlined activity
entry: assign({
childActor: () => spawn(
() => {}, // inlined actor
)
}),
}
}
})
Expand All @@ -53,7 +58,12 @@ createMachine({
actions: huffAndPuff // defined elsewhere
}
},
activities: beep // defined elsewhere
activities: beep, // defined elsewhere
entry: assign({
childActor: () => spawn(
childMachine, // inlined actor
)
}),
}
}
})
Expand All @@ -77,11 +87,17 @@ createMachine(
actions: ['huffAndPuff', 'log'] // arrays are ok too
}
},
activities: 'beep'
activities: 'beep',
entry: assign({
childActor: () => spawn(
'childMachine',
)
}),
}
}
},
{
// "services" in XState v4, renamed to "actors" in XState v5
services: {
someMachine: () => {}
},
Expand All @@ -92,6 +108,7 @@ createMachine(
huffAndPuff: () => {},
log: () => {}
},
// only in XState v4
activities: {
beep: () => {}
}
Expand All @@ -106,7 +123,7 @@ createMachine({
entry: assign({ count: 1 }),
on: {
TRIGGER: {
actions: [assign({ count: 0 }), send('EVENT')] // arrays are ok too
actions: [assign({ count: 0 }), raise({ type: 'EVENT' })] // arrays are ok too
}
}
}
Expand Down
26 changes: 21 additions & 5 deletions lib/rules/no-inline-implementation.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ const {
isIdentifier,
isStringLiteral,
isCallExpression,
isObjectExpression,
isKnownActionCreatorCall,
isArrayExpression,
} = require('../utils/predicates')
const { getTypeProperty } = require('../utils/selectors')
const { anyPass } = require('../utils/combinators')
const getSettings = require('../utils/getSettings')
const XStateDetector = require('../utils/XStateDetector')

function isArrayWithFunctionExpressionOrIdentifier(node) {
return (
Expand Down Expand Up @@ -78,10 +78,10 @@ const activitiesProperty =
'CallExpression[callee.name=/^createMachine$|^Machine$/] Property[key.name="activities"]'

const propertyOfChoosableActionObject =
'CallExpression[callee.name=/^createMachine$|^Machine$/] CallExpression[callee.name="choose"] > ArrayExpression > ObjectExpression > Property'
'CallExpression[callee.name=/^createMachine$|^Machine$/] > ObjectExpression:first-child CallExpression[callee.name="choose"] > ArrayExpression > ObjectExpression > Property'

const propertyOfChoosableActionObjectAlt =
'CallExpression[callee.name=/^createMachine$|^Machine$/] CallExpression[callee.type="MemberExpression"][callee.object.name="actions"][callee.property.name="choose"] > ArrayExpression > ObjectExpression > Property'
'CallExpression[callee.name=/^createMachine$|^Machine$/] > ObjectExpression:first-child CallExpression[callee.type="MemberExpression"][callee.object.name="actions"][callee.property.name="choose"] > ArrayExpression > ObjectExpression > Property'

const defaultOptions = {
allowKnownActionCreators: false,
Expand Down Expand Up @@ -110,7 +110,7 @@ module.exports = {
type: 'suggestion',
docs: {
description:
'Suggest refactoring actions, guards, services, activities into options',
'Suggest refactoring actions, guards, actors, activities into options',
category: 'Best Practices',
url: getDocsUrl('no-inline-implementation'),
recommended: 'warn',
Expand Down Expand Up @@ -158,7 +158,8 @@ module.exports = {
const { version } = getSettings(context)
const options = context.options[0] || defaultOptions

// TODO also check what is passed to spawn
const xstateDetector = new XStateDetector()

function checkActorSrc(node) {
if (isStringLiteral(node.value)) {
return
Expand Down Expand Up @@ -246,6 +247,7 @@ module.exports = {
}

return {
...xstateDetector.visitors,
[propertyOfEventTransition]: checkTransitionProperty,
[propertyOfEventTransitionInArray]: checkTransitionProperty,
[propertyOfAltEventTransitionInArray]: checkTransitionProperty,
Expand Down Expand Up @@ -304,6 +306,20 @@ module.exports = {
})
}
},

'CallExpression[callee.name=/^createMachine$|^Machine$/] > ObjectExpression:first-child CallExpression':
function (node) {
if (!xstateDetector.isSpawnCallExpression(node)) {
return
}

if (node.arguments[0] && !isStringLiteral(node.arguments[0])) {
context.report({
node: node.arguments[0],
messageId: 'moveActorToOptions',
})
}
},
}
},
}
122 changes: 10 additions & 112 deletions lib/rules/spawn-usage.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
'use strict'

const getDocsUrl = require('../utils/getDocsUrl')

const { isFunctionExpression, isIIFE } = require('../utils/predicates')
const XStateDetector = require('../utils/XStateDetector')

function isAssignCall(node) {
return node.type === 'CallExpression' && node.callee.name === 'assign'
Expand Down Expand Up @@ -54,124 +54,22 @@ module.exports = {
},

create: function (context) {
// This will remain empty unless spawn is imported from the XState library
const spawnIdentifiers = []
// This may get populated if the whole xstate module is imported
let xstateIdentifier = null

function captureSpawnIdentifierFromXStateMemberExpression(node) {
if (
xstateIdentifier != null &&
node.init.object.name === xstateIdentifier
) {
spawnIdentifiers.push(node.id.name)
}
}
const xstateDetector = new XStateDetector()

return {
'ImportDeclaration[source.value="xstate"]': function (node) {
const importSpecifier = node.specifiers.find(
(s) => s.type === 'ImportSpecifier' && s.imported.name === 'spawn'
)
if (importSpecifier) {
spawnIdentifiers.push(importSpecifier.local.name)
return
}
const importDefaultSpecifier = node.specifiers.find(
(s) => s.type === 'ImportDefaultSpecifier'
)
if (importDefaultSpecifier) {
xstateIdentifier = importDefaultSpecifier.local.name
return
}
const importNamespaceSpecifier = node.specifiers.find(
(s) => s.type === 'ImportNamespaceSpecifier'
)
if (importNamespaceSpecifier) {
xstateIdentifier = importNamespaceSpecifier.local.name
}
},
// commonjs imports
// const { spawn } = require('xstate')
// const { spawn: xspawn } = require('xstate')
// const xstate = require('xstate')
'VariableDeclarator[init.callee.name="require"][init.arguments.0.value="xstate"]':
function (node) {
if (xstateIdentifier !== null || spawnIdentifiers.length > 0) {
return
}
if (node.id.type === 'ObjectPattern') {
const spawnProperty = node.id.properties.find(
(p) => p.key.name === 'spawn'
)
if (spawnProperty) {
spawnIdentifiers.push(spawnProperty.value.name)
}
return
}
if (node.id.type === 'Identifier') {
xstateIdentifier = node.id.name
}
},
// const spawn = require('xstate').spawn
// const spawn = require('xstate')['spawn']
'VariableDeclarator[init.object.callee.name="require"][init.object.arguments.0.value="xstate"]':
function (node) {
if (
node.init.property.name === 'spawn' ||
node.init.property.value === 'spawn'
) {
spawnIdentifiers.push(node.id.name)
}
},
// const { spawn } = xstate
// Ignores it if the xstateIdentifier has not been previously resolved
'VariableDeclarator[id.type="ObjectPattern"] > ObjectPattern > Property[key.name="spawn"]':
function (node) {
const varDeclarator = node.parent.parent
if (
xstateIdentifier != null &&
xstateIdentifier === varDeclarator.init.name
) {
spawnIdentifiers.push(node.value.name)
}
},
// const spawn = xstate.spawn
// const spawn = xstate['spawn']
'VariableDeclarator[init.type="MemberExpression"][init.property.name="spawn"]':
captureSpawnIdentifierFromXStateMemberExpression,
'VariableDeclarator[init.type="MemberExpression"][init.property.value="spawn"]':
captureSpawnIdentifierFromXStateMemberExpression,
...xstateDetector.visitors,

// Ignores it if the spawnIdentifier has not been previously resolved to "spawn" (imported from xstate)
CallExpression: function (node) {
if (spawnIdentifiers.length < 1 && xstateIdentifier == null) {
if (!xstateDetector.isSpawnCallExpression(node)) {
return
}

if (
node.callee.type === 'Identifier' &&
spawnIdentifiers.includes(node.callee.name)
) {
if (!isInsideAssignerFunction(node)) {
context.report({
node,
messageId: 'invalidCallContext',
})
}
return
}
if (
node.callee.type === 'MemberExpression' &&
node.callee.object.name === xstateIdentifier &&
(node.callee.property.name === 'spawn' ||
node.callee.property.value === 'spawn')
) {
if (!isInsideAssignerFunction(node)) {
context.report({
node,
messageId: 'invalidCallContext',
})
}
if (!isInsideAssignerFunction(node)) {
context.report({
node,
messageId: 'invalidCallContext',
})
}
},
}
Expand Down
111 changes: 111 additions & 0 deletions lib/utils/XStateDetector.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/**
* XStateDetector can be used to capture information about xstate imports.
*/
module.exports = class XStateDetector {
constructor() {
this.xstateIdentifier = null
this.spawnIdentifiers = []

const captureSpawnIdentifierFromXStateMemberExpression = (node) => {
if (
this.xstateIdentifier != null &&
node.init.object.name === this.xstateIdentifier
) {
this.spawnIdentifiers.push(node.id.name)
}
}

this.visitors = {
'ImportDeclaration[source.value="xstate"]': (node) => {
const importSpecifier = node.specifiers.find(
(s) => s.type === 'ImportSpecifier' && s.imported.name === 'spawn'
)
if (importSpecifier) {
this.spawnIdentifiers.push(importSpecifier.local.name)
return
}
const importDefaultSpecifier = node.specifiers.find(
(s) => s.type === 'ImportDefaultSpecifier'
)
if (importDefaultSpecifier) {
this.xstateIdentifier = importDefaultSpecifier.local.name
return
}
const importNamespaceSpecifier = node.specifiers.find(
(s) => s.type === 'ImportNamespaceSpecifier'
)
if (importNamespaceSpecifier) {
this.xstateIdentifier = importNamespaceSpecifier.local.name
}
},
// commonjs imports
// const { spawn } = require('xstate')
// const { spawn: xspawn } = require('xstate')
// const xstate = require('xstate')
'VariableDeclarator[init.callee.name="require"][init.arguments.0.value="xstate"]':
(node) => {
if (
this.xstateIdentifier !== null ||
this.spawnIdentifiers.length > 0
) {
return
}
if (node.id.type === 'ObjectPattern') {
const spawnProperty = node.id.properties.find(
(p) => p.key.name === 'spawn'
)
if (spawnProperty) {
this.spawnIdentifiers.push(spawnProperty.value.name)
}
return
}
if (node.id.type === 'Identifier') {
this.xstateIdentifier = node.id.name
}
},
// const spawn = require('xstate').spawn
// const spawn = require('xstate')['spawn']
'VariableDeclarator[init.object.callee.name="require"][init.object.arguments.0.value="xstate"]':
(node) => {
if (
node.init.property.name === 'spawn' ||
node.init.property.value === 'spawn'
) {
this.spawnIdentifiers.push(node.id.name)
}
},
// const { spawn } = xstate
// Ignores it if the xstateIdentifier has not been previously resolved
'VariableDeclarator[id.type="ObjectPattern"] > ObjectPattern > Property[key.name="spawn"]':
(node) => {
const varDeclarator = node.parent.parent
if (
this.xstateIdentifier != null &&
this.xstateIdentifier === varDeclarator.init.name
) {
this.spawnIdentifiers.push(node.value.name)
}
},
// const spawn = xstate.spawn
// const spawn = xstate['spawn']
'VariableDeclarator[init.type="MemberExpression"][init.property.name="spawn"]':
captureSpawnIdentifierFromXStateMemberExpression,
'VariableDeclarator[init.type="MemberExpression"][init.property.value="spawn"]':
captureSpawnIdentifierFromXStateMemberExpression,
}
}

isSpawnCallExpression(node) {
if (this.spawnIdentifiers.length < 1 && this.xstateIdentifier == null) {
return false
}
return (
(node.callee.type === 'Identifier' &&
this.spawnIdentifiers.includes(node.callee.name)) ||
(node.callee.type === 'MemberExpression' &&
node.callee.object.name === this.xstateIdentifier &&
(node.callee.property.name === 'spawn' ||
node.callee.property.value === 'spawn'))
)
}
}
Loading

0 comments on commit 9654443

Please sign in to comment.