Add eslint rule for preventing string literals in select/dispatch/useDispatch

.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': [

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)
 

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                                        | ✓           |

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();
+```

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 } ) ),
+} );

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 },
+						} );
+					} );
+			},
+		};
+	},
+};