diff --git a/.eslintrc.js b/.eslintrc.js index d955d57846f933..b78d72bde774c7 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -61,6 +61,7 @@ module.exports = { }, ], '@wordpress/no-unsafe-wp-apis': 'off', + '@wordpress/data-no-store-string-literals': 'warn', 'import/default': 'error', 'import/named': 'error', 'no-restricted-imports': [ diff --git a/packages/eslint-plugin/CHANGELOG.md b/packages/eslint-plugin/CHANGELOG.md index f2c6f8060a9642..aa9192cfab5a0b 100644 --- a/packages/eslint-plugin/CHANGELOG.md +++ b/packages/eslint-plugin/CHANGELOG.md @@ -5,6 +5,7 @@ ### New Features - Enabled `import/default` and `import/named` rules in the `recommended` ruleset. [#28513](https://github.com/WordPress/gutenberg/pull/28513) +- Add new rule `@wordpress/data-no-store-string-literals` to discourage passing string literals to reference data stores ([#28726](https://github.com/WordPress/gutenberg/pull/28726)). ## 8.0.1 (2021-01-28) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index a831e6ce810798..c0559e0735dfc6 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -58,13 +58,14 @@ The granular rulesets will not define any environment globals. As such, if they | Rule | Description | Recommended | | -------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------- | ----------- | +| [data-no-store-string-literals](/packages/eslint-plugin/docs/rules/data-no-store-string-literals.md) | Discourage passing string literals to reference data stores | | | [dependency-group](/packages/eslint-plugin/docs/rules/dependency-group.md) | Enforce dependencies docblocks formatting | ✓ | | [gutenberg-phase](docs/rules/gutenberg-phase.md) | Governs the use of the `process.env.GUTENBERG_PHASE` constant | ✓ | +| [no-base-control-with-label-without-id](/packages/eslint-plugin/docs/rules/no-base-control-with-label-without-id.md) | Disallow the usage of BaseControl component with a label prop set but omitting the id property | ✓ | +| [no-unguarded-get-range-at](/packages/eslint-plugin/docs/rules/no-unguarded-get-range-at.md) | Disallow the usage of unguarded `getRangeAt` calls | ✓ | | [no-unused-vars-before-return](/packages/eslint-plugin/docs/rules/no-unused-vars-before-return.md) | Disallow assigning variable values if unused before a return | ✓ | | [react-no-unsafe-timeout](/packages/eslint-plugin/docs/rules/react-no-unsafe-timeout.md) | Disallow unsafe `setTimeout` in component | | [valid-sprintf](/packages/eslint-plugin/docs/rules/valid-sprintf.md) | Enforce valid sprintf usage | ✓ | -| [no-base-control-with-label-without-id](/packages/eslint-plugin/docs/rules/no-base-control-with-label-without-id.md) | Disallow the usage of BaseControl component with a label prop set but omitting the id property | ✓ | -| [no-unguarded-get-range-at](/packages/eslint-plugin/docs/rules/no-unguarded-get-range-at.md) | Disallow the usage of unguarded `getRangeAt` calls | ✓ | | [i18n-ellipsis](/packages/eslint-plugin/docs/rules/i18n-ellipsis.md) | Disallow using three dots in translatable strings | ✓ | | [i18n-no-collapsible-whitespace](/packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md) | Disallow collapsible whitespace in translatable strings | ✓ | | [i18n-no-placeholders-only](/packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md) | Prevent using only placeholders in translatable strings | ✓ | diff --git a/packages/eslint-plugin/docs/rules/data-no-store-string-literals.md b/packages/eslint-plugin/docs/rules/data-no-store-string-literals.md new file mode 100644 index 00000000000000..c7135832f4cba1 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/data-no-store-string-literals.md @@ -0,0 +1,22 @@ +# Discourage passing string literals to reference data stores (data-no-store-string-literals) + +Ensures that string literals aren't used for accessing `@wordpress/data` stores when using API methods. The store definition is promoted as the preferred way of referencing registered stores. + +## Rule details + +Examples of **incorrect** code for this rule: + +```js +import { select } from '@wordpress/data'; + +const blockTypes = select( 'core/blocks' ).getBlockTypes(); +``` + +Examples of **correct** code for this rule: + +```js +import { store as blocksStore } from '@wordpress/blocks'; +import { select } from '@wordpress/data'; + +const blockTypes = select( blocksStore ).getBlockTypes(); +``` diff --git a/packages/eslint-plugin/rules/__tests__/data-no-store-string-literals.js b/packages/eslint-plugin/rules/__tests__/data-no-store-string-literals.js new file mode 100644 index 00000000000000..14614ac45580f1 --- /dev/null +++ b/packages/eslint-plugin/rules/__tests__/data-no-store-string-literals.js @@ -0,0 +1,70 @@ +/** + * External dependencies + */ +import { RuleTester } from 'eslint'; + +/** + * Internal dependencies + */ +import rule from '../data-no-store-string-literals'; + +const ruleTester = new RuleTester( { + parserOptions: { + sourceType: 'module', + ecmaVersion: 6, + }, +} ); + +const valid = [ + // Callback functions + `import { createRegistrySelector } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; createRegistrySelector(( select ) => { select(store); });`, + `import { useSelect } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; useSelect(( select ) => { select(store); });`, + `import { withSelect } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; withSelect(( select ) => { select(store); });`, + `import { withDispatch } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; withDispatch(( select ) => { select(store); });`, + `import { withDispatch as withDispatchAlias } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; withDispatchAlias(( select ) => { select(store); });`, + + // Direct function calls + `import { useDispatch } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; useDispatch( store );`, + `import { dispatch } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; dispatch( store );`, + `import { select } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; select( store );`, + `import { resolveSelect } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; resolveSelect( store );`, + `import { resolveSelect as resolveSelectAlias } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; resolveSelectAlias( store );`, + + // Object property function calls + `import { controls } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; controls.select( store );`, + `import { controls } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; controls.dispatch( store );`, + `import { controls } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; controls.resolveSelect( store );`, + `import { controls as controlsAlias } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; controlsAlias.resolveSelect( store );`, +]; + +const invalid = [ + // Callback functions + `import { createRegistrySelector } from '@wordpress/data'; createRegistrySelector(( select ) => { select( 'core' ); });`, + `import { useSelect } from '@wordpress/data'; useSelect(( select ) => { select( 'core' ); });`, + `import { withSelect } from '@wordpress/data'; withSelect(( select ) => { select( 'core' ); });`, + `import { withDispatch } from '@wordpress/data'; withDispatch(( select ) => { select( 'core' ); });`, + `import { withDispatch as withDispatchAlias } from '@wordpress/data'; withDispatchAlias(( select ) => { select( 'core' ); });`, + + // Direct function calls + `import { useDispatch } from '@wordpress/data'; useDispatch( 'core' );`, + `import { dispatch } from '@wordpress/data'; dispatch( 'core' );`, + `import { select } from '@wordpress/data'; select( 'core' );`, + `import { resolveSelect } from '@wordpress/data'; resolveSelect( 'core' );`, + `import { resolveSelect as resolveSelectAlias } from '@wordpress/data'; resolveSelectAlias( 'core' );`, + + // Object property function calls + `import { controls } from '@wordpress/data'; controls.select( 'core' );`, + `import { controls } from '@wordpress/data'; controls.dispatch( 'core' );`, + `import { controls } from '@wordpress/data'; controls.resolveSelect( 'core' );`, + `import { controls as controlsAlias } from '@wordpress/data'; controlsAlias.resolveSelect( 'core' );`, +]; +const errors = [ + { + message: `Do not use string literals ( 'core' ) for accessing @wordpress/data stores. Pass the store definition instead`, + }, +]; + +ruleTester.run( 'data-no-store-string-literals', rule, { + valid: valid.map( ( code ) => ( { code } ) ), + invalid: invalid.map( ( code ) => ( { code, errors } ) ), +} ); diff --git a/packages/eslint-plugin/rules/data-no-store-string-literals.js b/packages/eslint-plugin/rules/data-no-store-string-literals.js new file mode 100644 index 00000000000000..7fda8ecaf1d5ad --- /dev/null +++ b/packages/eslint-plugin/rules/data-no-store-string-literals.js @@ -0,0 +1,139 @@ +function getReferences( context, specifiers ) { + const variables = specifiers.reduce( + ( acc, specifier ) => + acc.concat( context.getDeclaredVariables( specifier ) ), + [] + ); + const references = variables.reduce( + ( acc, variable ) => acc.concat( variable.references ), + [] + ); + return references; +} + +function collectAllNodesFromCallbackFunctions( context, node ) { + const functionSpecifiers = node.specifiers.filter( + ( specifier ) => + specifier.imported && + [ + 'createRegistrySelector', + 'useSelect', + 'withSelect', + 'withDispatch', + ].includes( specifier.imported.name ) + ); + const functionReferences = getReferences( context, functionSpecifiers ); + + const functionArgumentVariables = functionReferences.reduce( + ( acc, { identifier: { parent } } ) => + parent && parent.arguments && parent.arguments.length > 0 + ? acc.concat( + context.getDeclaredVariables( parent.arguments[ 0 ] ) + ) + : acc, + [] + ); + const functionArgumentReferences = functionArgumentVariables.reduce( + ( acc, variable ) => acc.concat( variable.references ), + [] + ); + const possibleCallExpressionNodes = functionArgumentReferences + .filter( ( reference ) => reference.identifier.parent ) + .map( ( reference ) => reference.identifier.parent ); + + return possibleCallExpressionNodes; +} + +function collectAllNodesFromDirectFunctionCalls( context, node ) { + const specifiers = node.specifiers.filter( + ( specifier ) => + specifier.imported && + [ 'useDispatch', 'dispatch', 'select', 'resolveSelect' ].includes( + specifier.imported.name + ) + ); + const references = getReferences( context, specifiers ); + const possibleCallExpressionNodes = references + .filter( ( reference ) => reference.identifier.parent ) + .map( ( reference ) => reference.identifier.parent ); + + return possibleCallExpressionNodes; +} + +function collectAllNodesFromObjectPropertyFunctionCalls( context, node ) { + const specifiers = node.specifiers.filter( + ( specifier ) => + specifier.imported && + [ 'controls' ].includes( specifier.imported.name ) + ); + const references = getReferences( context, specifiers ); + const referencesWithPropertyCalls = references.filter( + ( reference ) => + reference.identifier.parent.property && + [ 'select', 'resolveSelect', 'dispatch' ].includes( + reference.identifier.parent.property.name + ) + ); + const possibleCallExpressionNodes = referencesWithPropertyCalls + .filter( + ( reference ) => + reference.identifier.parent && + reference.identifier.parent.parent + ) + .map( ( reference ) => reference.identifier.parent.parent ); + + return possibleCallExpressionNodes; +} + +module.exports = { + meta: { + type: 'problem', + schema: [], + messages: { + doNotUseStringLiteral: `Do not use string literals ( '{{ argument }}' ) for accessing @wordpress/data stores. Pass the store definition instead`, + }, + }, + create( context ) { + return { + ImportDeclaration( node ) { + if ( node.source.value !== '@wordpress/data' ) { + return; + } + + const callbackFunctionNodes = collectAllNodesFromCallbackFunctions( + context, + node + ); + const directNodes = collectAllNodesFromDirectFunctionCalls( + context, + node + ); + const objectPropertyCallNodes = collectAllNodesFromObjectPropertyFunctionCalls( + context, + node + ); + + const allNodes = [ + ...callbackFunctionNodes, + ...directNodes, + ...objectPropertyCallNodes, + ]; + allNodes + .filter( + ( callNode ) => + callNode && + callNode.type === 'CallExpression' && + callNode.arguments.length > 0 && + callNode.arguments[ 0 ].type === 'Literal' + ) + .forEach( ( callNode ) => { + context.report( { + node: callNode.parent, + messageId: 'doNotUseStringLiteral', + data: { argument: callNode.arguments[ 0 ].value }, + } ); + } ); + }, + }; + }, +};