From dad4b2045a586804502e242484ca50901ebc8d0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Szab=C3=B3?= Date: Tue, 16 Feb 2021 19:25:22 +0100 Subject: [PATCH] Eslint plugin: Add suggestions to "data-no-store-string-literals" rule (#28974) * Add suggestion * Add tests * Assume we always camelize store names ; few refactors * Rename short variables --- .../data-no-store-string-literals.js | 63 ++++++++++- .../rules/data-no-store-string-literals.js | 107 ++++++++++++++++++ 2 files changed, 169 insertions(+), 1 deletion(-) 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 index 14614ac45580f1..ac74c7800a50a4 100644 --- a/packages/eslint-plugin/rules/__tests__/data-no-store-string-literals.js +++ b/packages/eslint-plugin/rules/__tests__/data-no-store-string-literals.js @@ -37,6 +37,21 @@ const valid = [ `import { controls as controlsAlias } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; controlsAlias.resolveSelect( store );`, ]; +const createSuggestionTestCase = ( code, output ) => ( { + code, + errors: [ + { + suggestions: [ + { + desc: + 'Replace literal with store definition. Import store if neccessary.', + output, + }, + ], + }, + ], +} ); + const invalid = [ // Callback functions `import { createRegistrySelector } from '@wordpress/data'; createRegistrySelector(( select ) => { select( 'core' ); });`, @@ -57,6 +72,50 @@ const invalid = [ `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' );`, + + // Direct function calls suggestions + // Replace core with coreStore and import coreStore + createSuggestionTestCase( + `import { select } from '@wordpress/data'; select( 'core' );`, + `import { select } from '@wordpress/data';\nimport { store as coreStore } from '@wordpress/core-data'; select( coreStore );` + ), + // Replace core with coreStore. A @wordpress/core-data already exists, so it should append the import to that one. + createSuggestionTestCase( + `import { select } from '@wordpress/data'; import { something } from '@wordpress/core-data'; select( 'core' );`, + `import { select } from '@wordpress/data'; import { something,store as coreStore } from '@wordpress/core-data'; select( coreStore );` + ), + // Replace core with coreStore. A @wordpress/core-data already exists, so it should append the import to that one. + // This time there is a comma after the import. + createSuggestionTestCase( + `import { select } from '@wordpress/data'; import { something, } from '@wordpress/core-data'; select( 'core' );`, + `import { select } from '@wordpress/data'; import { something,store as coreStore, } from '@wordpress/core-data'; select( coreStore );` + ), + // Replace core with coreStore. Store import already exists. It shouldn't modify the import, just replace the literal with the store definition. + createSuggestionTestCase( + `import { select } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; select( 'core' );`, + `import { select } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; select( coreStore );` + ), + // Replace core with coreStore. There are internal and WordPress dependencies. + // It should append the import after the last WordPress dependency import. + createSuggestionTestCase( + `import { a } from './a'; import { select } from '@wordpress/data'; import { b } from './b'; select( 'core' );`, + `import { a } from './a'; import { select } from '@wordpress/data';\nimport { store as coreStore } from '@wordpress/core-data'; import { b } from './b'; select( coreStore );` + ), + // Replace block-editor with blockEditorStore + createSuggestionTestCase( + `import { select } from '@wordpress/data'; select( 'core/block-editor' );`, + `import { select } from '@wordpress/data';\nimport { store as blockEditorStore } from '@wordpress/block-editor'; select( blockEditorStore );` + ), + // Replace notices with noticesStore + createSuggestionTestCase( + `import { select } from '@wordpress/data'; select( 'core/notices' );`, + `import { select } from '@wordpress/data';\nimport { store as noticesStore } from '@wordpress/notices'; select( noticesStore );` + ), + // Replace edit-post with editPostStore + createSuggestionTestCase( + `import { select } from '@wordpress/data'; select( 'core/edit-post' );`, + `import { select } from '@wordpress/data';\nimport { store as editPostStore } from '@wordpress/edit-post'; select( editPostStore );` + ), ]; const errors = [ { @@ -66,5 +125,7 @@ const errors = [ ruleTester.run( 'data-no-store-string-literals', rule, { valid: valid.map( ( code ) => ( { code } ) ), - invalid: invalid.map( ( code ) => ( { code, errors } ) ), + invalid: invalid.map( ( code ) => + typeof code === 'string' ? { code, errors } : code + ), } ); diff --git a/packages/eslint-plugin/rules/data-no-store-string-literals.js b/packages/eslint-plugin/rules/data-no-store-string-literals.js index 7fda8ecaf1d5ad..72dfc97654f2a2 100644 --- a/packages/eslint-plugin/rules/data-no-store-string-literals.js +++ b/packages/eslint-plugin/rules/data-no-store-string-literals.js @@ -1,3 +1,33 @@ +/** + * Converts store name to variable name. + * Removes dashes and uppercases the characters after dashes and appends `Store` at the end. + * + * @param {string} storeName + * @return {string} store name as variable name + */ +function storeNameToVariableNames( storeName ) { + return ( + storeName + .split( '-' ) + .map( ( value, index ) => + index === 0 + ? value.toLowerCase() + : value[ 0 ].toUpperCase() + value.slice( 1 ).toLowerCase() + ) + .join( '' ) + 'Store' + ); +} + +/** + * Returns last element of an array. + * + * @param {Array} array + * @return {*} last element of the array + */ +function arrayLast( array ) { + return array[ array.length - 1 ]; +} + function getReferences( context, specifiers ) { const variables = specifiers.reduce( ( acc, specifier ) => @@ -85,6 +115,82 @@ function collectAllNodesFromObjectPropertyFunctionCalls( context, node ) { return possibleCallExpressionNodes; } +function getSuggest( context, callNode ) { + return [ + { + desc: + 'Replace literal with store definition. Import store if neccessary.', + fix: ( fixer ) => getFixes( fixer, context, callNode ), + }, + ]; +} + +function getFixes( fixer, context, callNode ) { + const storeName = callNode.arguments[ 0 ].value; + const storeDefinitions = { + core: { + import: '@wordpress/core-data', + variable: 'coreStore', + }, + }; + let storeDefinition = storeDefinitions[ storeName ]; + if ( ! storeDefinition && storeName.startsWith( 'core/' ) ) { + const storeNameWithoutCore = storeName.substring( 5 ); + storeDefinition = { + import: `@wordpress/${ storeNameWithoutCore }`, + variable: storeNameToVariableNames( storeNameWithoutCore ), + }; + } + if ( ! storeDefinition ) { + return null; + } + const { variable: variableName, import: importName } = storeDefinition; + + const fixes = [ + fixer.replaceText( callNode.arguments[ 0 ], variableName ), + ]; + + const imports = context + .getAncestors()[ 0 ] + .body.filter( ( node ) => node.type === 'ImportDeclaration' ); + const packageImports = imports.filter( + ( node ) => node.source.value === importName + ); + const packageImport = + packageImports.length > 0 ? packageImports[ 0 ] : null; + if ( packageImport ) { + const alreadyHasStore = packageImport.specifiers.some( + ( specifier ) => specifier.imported.name === 'store' + ); + if ( ! alreadyHasStore ) { + const lastSpecifier = arrayLast( packageImport.specifiers ); + fixes.push( + fixer.insertTextAfter( + lastSpecifier, + `,store as ${ variableName }` + ) + ); + } + } else { + const wpImports = imports.filter( ( node ) => + node.source.value.startsWith( '@wordpress/' ) + ); + const lastImport = + wpImports.length > 0 + ? arrayLast( wpImports ) + : arrayLast( imports ); + + fixes.push( + fixer.insertTextAfter( + lastImport, + `\nimport { store as ${ variableName } } from '${ importName }';` + ) + ); + } + + return fixes; +} + module.exports = { meta: { type: 'problem', @@ -131,6 +237,7 @@ module.exports = { node: callNode.parent, messageId: 'doNotUseStringLiteral', data: { argument: callNode.arguments[ 0 ].value }, + suggest: getSuggest( context, callNode ), } ); } ); },