-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add eslint rule for preventing string literals in select/dispatch/use…
…Dispatch (#28726) * Add rule * Add rules for controls.select/dispatch and registry.select/dispatch * Create rule and add as warning * Rework logic * Fix TypeError * Add tests * Rename variables * Update comment * Docs: Document the new ESLint rule added * Rename and change error message Co-authored-by: Grzegorz Ziolkowski <grzegorz@gziolo.pl>
- Loading branch information
1 parent
5bc5c39
commit 6f7542c
Showing
6 changed files
with
236 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
22 changes: 22 additions & 0 deletions
22
packages/eslint-plugin/docs/rules/data-no-store-string-literals.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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(); | ||
| ``` |
70 changes: 70 additions & 0 deletions
70
packages/eslint-plugin/rules/__tests__/data-no-store-string-literals.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 } ) ), | ||
| } ); |
139 changes: 139 additions & 0 deletions
139
packages/eslint-plugin/rules/data-no-store-string-literals.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 }, | ||
| } ); | ||
| } ); | ||
| }, | ||
| }; | ||
| }, | ||
| }; |