From 6620a6b1feb3cd28863056edd3cb894c4e05bc48 Mon Sep 17 00:00:00 2001 From: Charles GRUENAIS Date: Tue, 17 Jan 2023 21:37:41 +0100 Subject: [PATCH 1/8] Add use-string-literal-names rule --- README.md | 1 + docs/rules/use-string-literal-names.md | 61 ++++++++++++++ lib/configs/recommended.ts | 1 + lib/rules/use-string-literal-names.ts | 84 +++++++++++++++++++ lib/utils/ast.ts | 1 + .../rules/use-string-literal-names.test.ts | 37 ++++++++ 6 files changed, 185 insertions(+) create mode 100644 docs/rules/use-string-literal-names.md create mode 100644 lib/rules/use-string-literal-names.ts create mode 100644 tests/lib/rules/use-string-literal-names.test.ts diff --git a/README.md b/README.md index 626cb28..e31260e 100644 --- a/README.md +++ b/README.md @@ -122,6 +122,7 @@ This plugin does not support MDX files. | [`storybook/story-exports`](./docs/rules/story-exports.md) | A story file must contain at least one story export | | | | [`storybook/use-storybook-expect`](./docs/rules/use-storybook-expect.md) | Use expect from `@storybook/jest` | 🔧 | | | [`storybook/use-storybook-testing-library`](./docs/rules/use-storybook-testing-library.md) | Do not use testing-library directly on stories | 🔧 | | +| [`storybook/use-string-literal-names`](./docs/rules/use-string-literal-names.md) | Use string literals to override a story name | | | diff --git a/docs/rules/use-string-literal-names.md b/docs/rules/use-string-literal-names.md new file mode 100644 index 0000000..24c32a5 --- /dev/null +++ b/docs/rules/use-string-literal-names.md @@ -0,0 +1,61 @@ +# use-string-literal-names + + + +**Included in these configurations**: + + + +## Rule Details + +When indexing stories extracted from CSF files, Storybook automatically titles them [based on the named export](https://storybook.js.org/docs/7.0/react/api/csf#named-story-exports). Story names can be overridden by setting the `name` property: + +```js +export const Simple = { + decorators: [...], + name: 'So simple!', + parameters: {...}, +} +``` + +One can be tempted to programmatically assign story names using code such as template literals, variable references, spread objects, function invocations, etc. However, because of limitations to static analysis, Storybook only picks `name` properties when they are string literals: it cannot evaluate code. + +Examples of **incorrect** code for this rule: + +```js +export const A = { name: '1994' + 'definitely not my credit card PIN' } +export const A = { name: `N` } +export const A = { name: String(1994) } + +const name = 'N' +export const A = { name } + +const A = { name: `N` } +export { A } + +const A = { name: String(1994) } +export { A } + +const name = 'N' +const A = { name } +export { A } +``` + +Examples of **correct** code for this rule: + +```js +export const A = { name: 'N' } +export const A = { name: 'N' } + +const A = { name: 'N' } +export { A } + +const A = { name: 'N' } +export { A } + +const A = { name } // Not a Story (not exported) +``` + +## Further Reading + +More discussion on issue [#111](https://github.com/storybookjs/eslint-plugin-storybook/issues/111) diff --git a/lib/configs/recommended.ts b/lib/configs/recommended.ts index 9996f33..384336b 100644 --- a/lib/configs/recommended.ts +++ b/lib/configs/recommended.ts @@ -19,6 +19,7 @@ export = { 'storybook/story-exports': 'error', 'storybook/use-storybook-expect': 'error', 'storybook/use-storybook-testing-library': 'error', + 'storybook/use-string-literal-names': 'error', }, }, { diff --git a/lib/rules/use-string-literal-names.ts b/lib/rules/use-string-literal-names.ts new file mode 100644 index 0000000..fedfcec --- /dev/null +++ b/lib/rules/use-string-literal-names.ts @@ -0,0 +1,84 @@ +/** + * @fileoverview Use string literals to override a story name + * @author Charles Gruenais + */ + +import { createStorybookRule } from '../utils/create-storybook-rule' +import { CategoryId } from '../utils/constants' +import { TSESTree } from '@typescript-eslint/utils' +import { + isExportNamedDeclaration, + isIdentifier, + isLiteral, + isVariableDeclaration, + isVariableDeclarator, +} from '../utils/ast' + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +const messageId = 'useStringLiteralName' as const + +export = createStorybookRule({ + name: 'use-string-literal-names', + defaultOptions: [], + meta: { + type: 'problem', + docs: { + description: 'Use string literals to override a story name', + categories: [CategoryId.RECOMMENDED], + recommended: 'error', + }, + messages: { + [messageId]: 'Fill me in', + }, + fixable: 'code', + hasSuggestions: true, + schema: [], + }, + + create(context) { + const TOP_VARIABLE_DECLARATION_DEPTH = 4 + const TOP_EXPORT_DECLARATION_DEPTH = 5 + const topLevelObjects: Map[0]> = new Map() + + const isTopLevelObjectProperty = () => + [TOP_VARIABLE_DECLARATION_DEPTH, TOP_EXPORT_DECLARATION_DEPTH].includes( + context.getAncestors().length + ) + + const isStoryNamePropertyCandidate = (node: TSESTree.Property) => + isIdentifier(node.key) && node.key.name === 'name' && isTopLevelObjectProperty() + + return { + Property: function (node) { + if (isStoryNamePropertyCandidate(node)) { + if (!isLiteral(node.value)) { + const descriptor = { node, messageId } + const [declaration, declarator] = context.getAncestors().slice(1, 3) + if ( + isVariableDeclaration(declaration) && + isVariableDeclarator(declarator) && + isIdentifier(declarator.id) + ) { + topLevelObjects.set(declarator.id.name, descriptor) + } else if (isExportNamedDeclaration(declaration)) { + context.report(descriptor) + } + } + } + }, + ExportNamedDeclaration: function (node) { + if (node.specifiers.length) { + node.specifiers.forEach((specifier) => { + const baseTopLevelObject = topLevelObjects.get(specifier.local.name) + if (baseTopLevelObject) { + context.report(baseTopLevelObject) + } + }) + } + }, + } + }, +}) diff --git a/lib/utils/ast.ts b/lib/utils/ast.ts index 4a6375b..9081fa9 100644 --- a/lib/utils/ast.ts +++ b/lib/utils/ast.ts @@ -16,6 +16,7 @@ export const isBlockStatement = isNodeOfType(AST_NODE_TYPES.BlockStatement) export const isCallExpression = isNodeOfType(AST_NODE_TYPES.CallExpression) export const isExpressionStatement = isNodeOfType(AST_NODE_TYPES.ExpressionStatement) export const isVariableDeclaration = isNodeOfType(AST_NODE_TYPES.VariableDeclaration) +export const isExportNamedDeclaration = isNodeOfType(AST_NODE_TYPES.ExportNamedDeclaration) export const isAssignmentExpression = isNodeOfType(AST_NODE_TYPES.AssignmentExpression) export const isSequenceExpression = isNodeOfType(AST_NODE_TYPES.SequenceExpression) export const isImportDeclaration = isNodeOfType(AST_NODE_TYPES.ImportDeclaration) diff --git a/tests/lib/rules/use-string-literal-names.test.ts b/tests/lib/rules/use-string-literal-names.test.ts new file mode 100644 index 0000000..31895d5 --- /dev/null +++ b/tests/lib/rules/use-string-literal-names.test.ts @@ -0,0 +1,37 @@ +/** + * @fileoverview Use string literals to override a story name + * @author Charles Gruenais + */ + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +import rule from '../../../lib/rules/use-string-literal-names' +import ruleTester from '../../utils/rule-tester' + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const errors = [{ messageId: 'useStringLiteralName' }] as const + +ruleTester.run('use-string-literal-names', rule, { + valid: [ + 'export const A = { name: "N" }', + "export const A = { name: 'N' }", + "const ABase = { parameters: {} }; export const A = { ...ABase, name: 'N' }", + 'const A = { name: "N" }; export { A }', + "const A = { name: 'N' }; export { A }", + 'const A = { name }', // Not a Story (not exported) + ], + invalid: [ + { code: 'export const A = { name: "1994" + "definitely not my credit card PIN" }', errors }, + { code: 'export const A = { name: `N` }', errors }, + { code: 'export const A = { name: String(1994) }', errors }, + { code: 'const name = "N"; export const A = { name }', errors }, + { code: 'const A = { name: `N` }; export { A }', errors }, + { code: 'const A = { name: String(1994) }; export { A }', errors }, + { code: 'const name = "N"; const A = { name }; export { A }', errors }, + ], +}) From ace1877992fc33e09dc8df90b1213f4084026489 Mon Sep 17 00:00:00 2001 From: Charles GRUENAIS Date: Tue, 17 Jan 2023 22:15:49 +0100 Subject: [PATCH 2/8] Add rule error message --- lib/rules/use-string-literal-names.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/rules/use-string-literal-names.ts b/lib/rules/use-string-literal-names.ts index fedfcec..ddfcfbe 100644 --- a/lib/rules/use-string-literal-names.ts +++ b/lib/rules/use-string-literal-names.ts @@ -31,10 +31,8 @@ export = createStorybookRule({ recommended: 'error', }, messages: { - [messageId]: 'Fill me in', + [messageId]: 'Story names can only be overriden by string literals', }, - fixable: 'code', - hasSuggestions: true, schema: [], }, From 19ffdc522f41b4f87a0e8a5963dd55a59822257a Mon Sep 17 00:00:00 2001 From: Charles GRUENAIS Date: Thu, 19 Jan 2023 01:44:02 +0100 Subject: [PATCH 3/8] Fixed typo --- lib/rules/use-string-literal-names.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/use-string-literal-names.ts b/lib/rules/use-string-literal-names.ts index ddfcfbe..2624e96 100644 --- a/lib/rules/use-string-literal-names.ts +++ b/lib/rules/use-string-literal-names.ts @@ -31,7 +31,7 @@ export = createStorybookRule({ recommended: 'error', }, messages: { - [messageId]: 'Story names can only be overriden by string literals', + [messageId]: 'Story names can only be overridden by string literals', }, schema: [], }, From 7feb4c2df32c05285c6f7fc265817b1b56de00e7 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Wed, 25 Jan 2023 08:58:42 +0100 Subject: [PATCH 4/8] use-string-literal-names: support storyName property --- lib/rules/use-string-literal-names.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/rules/use-string-literal-names.ts b/lib/rules/use-string-literal-names.ts index 2624e96..45a0b13 100644 --- a/lib/rules/use-string-literal-names.ts +++ b/lib/rules/use-string-literal-names.ts @@ -47,7 +47,9 @@ export = createStorybookRule({ ) const isStoryNamePropertyCandidate = (node: TSESTree.Property) => - isIdentifier(node.key) && node.key.name === 'name' && isTopLevelObjectProperty() + isIdentifier(node.key) && + (node.key.name === 'name' || node.key.name === 'storyName') && + isTopLevelObjectProperty() return { Property: function (node) { From d2a98f9cfc6ffe2f43a707fc5fab796129c3c39d Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Wed, 25 Jan 2023 08:58:50 +0100 Subject: [PATCH 5/8] add more test cases --- tests/lib/rules/use-string-literal-names.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/lib/rules/use-string-literal-names.test.ts b/tests/lib/rules/use-string-literal-names.test.ts index 31895d5..7db9ec2 100644 --- a/tests/lib/rules/use-string-literal-names.test.ts +++ b/tests/lib/rules/use-string-literal-names.test.ts @@ -24,6 +24,11 @@ ruleTester.run('use-string-literal-names', rule, { 'const A = { name: "N" }; export { A }', "const A = { name: 'N' }; export { A }", 'const A = { name }', // Not a Story (not exported) + 'const name = String(1994); export const A = { args: { name } }', + 'export const A = { args: { name: String(1994) } }', + 'export const A = { play: async () => { const name = String(1994) } }', + 'export const A = () => {}; A.name = "N"', + 'export const A = () => {}; A.storyName = "N"', ], invalid: [ { code: 'export const A = { name: "1994" + "definitely not my credit card PIN" }', errors }, @@ -33,5 +38,13 @@ ruleTester.run('use-string-literal-names', rule, { { code: 'const A = { name: `N` }; export { A }', errors }, { code: 'const A = { name: String(1994) }; export { A }', errors }, { code: 'const name = "N"; const A = { name }; export { A }', errors }, + { code: 'export const A = { storyName: String(1994) };', errors }, + { code: 'export const A = () => {}; A.name = String(1994)', errors }, + { code: 'export const A = () => {}; A.storyName = String(1994)', errors }, + // Not sure how often this actually happens. If too complex, don't take care of it and delete this test + { + code: 'const ABase = { name: String(1994) }; export const A = { ...ABase, parameters: {} }', + errors, + }, ], }) From fca12e406940072410bfb3425fa1e75f3f6ad7c0 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Wed, 25 Jan 2023 09:01:51 +0100 Subject: [PATCH 6/8] use-string-literal-names: tweak docs --- docs/rules/use-string-literal-names.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/rules/use-string-literal-names.md b/docs/rules/use-string-literal-names.md index 24c32a5..2b08fb5 100644 --- a/docs/rules/use-string-literal-names.md +++ b/docs/rules/use-string-literal-names.md @@ -8,17 +8,18 @@ ## Rule Details -When indexing stories extracted from CSF files, Storybook automatically titles them [based on the named export](https://storybook.js.org/docs/7.0/react/api/csf#named-story-exports). Story names can be overridden by setting the `name` property: +When indexing stories extracted from CSF files, Storybook automatically titles them [based on the named export](https://storybook.js.org/docs/react/api/csf#named-story-exports). Story names can be overridden by setting the `name` property: ```js export const Simple = { decorators: [...], - name: 'So simple!', parameters: {...}, + // Displays "So Simple" instead of "Simple" in Storybook's sidebar + name: 'So simple!', } ``` -One can be tempted to programmatically assign story names using code such as template literals, variable references, spread objects, function invocations, etc. However, because of limitations to static analysis, Storybook only picks `name` properties when they are string literals: it cannot evaluate code. +One can be tempted to programmatically assign story names using code such as template literals, variable references, spread objects, function invocations, etc. However, because of limitations to static analysis, which Storybook relies on, Storybook only picks `name` properties when they are string literals: it cannot evaluate code. Examples of **incorrect** code for this rule: From 74c5f0cf73066f4b571b932c74ca6a398f422045 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Wed, 25 Jan 2023 09:25:56 +0100 Subject: [PATCH 7/8] remove invalid test case --- tests/lib/rules/use-string-literal-names.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/lib/rules/use-string-literal-names.test.ts b/tests/lib/rules/use-string-literal-names.test.ts index 7db9ec2..e8c596f 100644 --- a/tests/lib/rules/use-string-literal-names.test.ts +++ b/tests/lib/rules/use-string-literal-names.test.ts @@ -38,7 +38,6 @@ ruleTester.run('use-string-literal-names', rule, { { code: 'const A = { name: `N` }; export { A }', errors }, { code: 'const A = { name: String(1994) }; export { A }', errors }, { code: 'const name = "N"; const A = { name }; export { A }', errors }, - { code: 'export const A = { storyName: String(1994) };', errors }, { code: 'export const A = () => {}; A.name = String(1994)', errors }, { code: 'export const A = () => {}; A.storyName = String(1994)', errors }, // Not sure how often this actually happens. If too complex, don't take care of it and delete this test From 3b4e7a02ca509d3e01140e14289ead037ab49bc4 Mon Sep 17 00:00:00 2001 From: Charles GRUENAIS Date: Sat, 4 Feb 2023 15:40:24 +0100 Subject: [PATCH 8/8] Added a wrapper to easily validate Stories and their properties - added support for missing cases --- lib/rules/use-string-literal-names.ts | 59 +----- lib/types/ast.ts | 23 +++ lib/types/index.ts | 2 + lib/types/stories.ts | 45 ++++ lib/utils/StoryIndexer.ts | 134 ++++++++++++ lib/utils/ast.ts | 45 +++- lib/utils/stories.ts | 194 ++++++++++++++++++ .../rules/use-string-literal-names.test.ts | 5 +- 8 files changed, 455 insertions(+), 52 deletions(-) create mode 100644 lib/types/ast.ts create mode 100644 lib/types/stories.ts create mode 100644 lib/utils/StoryIndexer.ts create mode 100644 lib/utils/stories.ts diff --git a/lib/rules/use-string-literal-names.ts b/lib/rules/use-string-literal-names.ts index 45a0b13..11f4346 100644 --- a/lib/rules/use-string-literal-names.ts +++ b/lib/rules/use-string-literal-names.ts @@ -5,14 +5,8 @@ import { createStorybookRule } from '../utils/create-storybook-rule' import { CategoryId } from '../utils/constants' -import { TSESTree } from '@typescript-eslint/utils' -import { - isExportNamedDeclaration, - isIdentifier, - isLiteral, - isVariableDeclaration, - isVariableDeclarator, -} from '../utils/ast' +import { isLiteral } from '../utils/ast' +import { extractStories } from '../utils/stories' //------------------------------------------------------------------------------ // Rule Definition @@ -37,48 +31,13 @@ export = createStorybookRule({ }, create(context) { - const TOP_VARIABLE_DECLARATION_DEPTH = 4 - const TOP_EXPORT_DECLARATION_DEPTH = 5 - const topLevelObjects: Map[0]> = new Map() - - const isTopLevelObjectProperty = () => - [TOP_VARIABLE_DECLARATION_DEPTH, TOP_EXPORT_DECLARATION_DEPTH].includes( - context.getAncestors().length - ) - - const isStoryNamePropertyCandidate = (node: TSESTree.Property) => - isIdentifier(node.key) && - (node.key.name === 'name' || node.key.name === 'storyName') && - isTopLevelObjectProperty() - - return { - Property: function (node) { - if (isStoryNamePropertyCandidate(node)) { - if (!isLiteral(node.value)) { - const descriptor = { node, messageId } - const [declaration, declarator] = context.getAncestors().slice(1, 3) - if ( - isVariableDeclaration(declaration) && - isVariableDeclarator(declarator) && - isIdentifier(declarator.id) - ) { - topLevelObjects.set(declarator.id.name, descriptor) - } else if (isExportNamedDeclaration(declaration)) { - context.report(descriptor) - } - } - } - }, - ExportNamedDeclaration: function (node) { - if (node.specifiers.length) { - node.specifiers.forEach((specifier) => { - const baseTopLevelObject = topLevelObjects.get(specifier.local.name) - if (baseTopLevelObject) { - context.report(baseTopLevelObject) - } - }) + return extractStories(context, (output) => { + const properties = output.getProperties(['name', 'storyName']) + properties.forEach(({ valueNode: node }) => { + if (!isLiteral(node)) { + context.report({ node, messageId }) } - }, - } + }) + }) }, }) diff --git a/lib/types/ast.ts b/lib/types/ast.ts new file mode 100644 index 0000000..4e650da --- /dev/null +++ b/lib/types/ast.ts @@ -0,0 +1,23 @@ +import type { TSESTree } from '@typescript-eslint/utils' + +export type TypedNode = TSESTree.Node & { type: NodeType } + +export type SpreadProperty = TSESTree.SpreadElement & { + argument: TSESTree.Identifier + parent: { parent: NamedVariableDeclarator } +} + +export type NamedProperty = TSESTree.Property & { key: TSESTree.Identifier } + +export type NamedExportSpecifier = TSESTree.ExportSpecifier & { local: TSESTree.Identifier } + +export type NamedVariableDeclarator = TSESTree.VariableDeclarator & { id: TSESTree.Identifier } + +export type NamedVariableDeclaration = NamedExportSpecifier | NamedVariableDeclarator + +export type NamedAssignment = TSESTree.AssignmentExpression & { + left: TSESTree.MemberExpression & { + property: TSESTree.Identifier + object: TSESTree.Identifier + } +} diff --git a/lib/types/index.ts b/lib/types/index.ts index 538df07..df22321 100644 --- a/lib/types/index.ts +++ b/lib/types/index.ts @@ -1,6 +1,8 @@ import { TSESLint } from '@typescript-eslint/utils' import { CategoryId } from '../utils/constants' +export type RuleContext = Readonly> + export type RuleModule = TSESLint.RuleModule<'', []> & { meta: { hasSuggestions?: boolean; docs: { recommendedConfig?: 'error' | 'warn' } } } diff --git a/lib/types/stories.ts b/lib/types/stories.ts new file mode 100644 index 0000000..5baabe2 --- /dev/null +++ b/lib/types/stories.ts @@ -0,0 +1,45 @@ +import { TSESTree } from '@typescript-eslint/utils' +import { NamedVariableDeclaration } from './ast' + +type StoryDeclaration = NamedVariableDeclaration | TSESTree.Expression + +export type StoryPropertyDeclaration = { + storyName: string + nameNode: TSESTree.Node + valueNode: TSESTree.Node +} + +export type SpreadsMap = Map + +export type StoriesMap = Map + +export type PropertiesMap = Map + +export type StoryExports = Map< + StoryExportName, + NamedVariableDeclaration | undefined +> + +export type PropertyDefinition = { + parentName: string + propertyName: string + propertyNameNode: TSESTree.Node + propertyValueNode: TSESTree.Node +} + +export type StoryPropertyCandidates = Map< + StoryPropertyKey, + PropertyDefinition +> + +export type StoryDeclarationCandidates = Map< + DeclarationName, + { + declarationValue: TSESTree.Expression | null + } +> + +export type SpreadContentCandidates = Map< + VariableName, + ParentName[] +> diff --git a/lib/utils/StoryIndexer.ts b/lib/utils/StoryIndexer.ts new file mode 100644 index 0000000..540f97d --- /dev/null +++ b/lib/utils/StoryIndexer.ts @@ -0,0 +1,134 @@ +import { + PropertiesMap, + PropertyDefinition, + SpreadContentCandidates, + SpreadsMap, + StoriesMap, + StoryDeclarationCandidates, + StoryExports, + StoryPropertyCandidates, + StoryPropertyDeclaration, +} from '../types/stories' + +export class StoryIndexer { + /** List of all Stories (indexed by their exported name). */ + stories: StoriesMap = new Map() + + /** List of all Stories properties (indexed by name). */ + properties: PropertiesMap = new Map() + + /** + * List of all spreads used on Stories (indexed by their name). + * Each spread item resolves to the exported name of the Story they are + * attached to. This is used internally and has no reason to be public. + */ + private spreads: SpreadsMap = new Map() + + constructor( + propertyCandidates: StoryPropertyCandidates, + declarationCandidates: StoryDeclarationCandidates, + spreadCandidates: SpreadContentCandidates, + storyExports: StoryExports + ) { + this.registerStories(storyExports, declarationCandidates) + this.registerSpreads(spreadCandidates) + this.registerProperties(propertyCandidates) + } + + /** Output list of stories. May be filtered by one or several names. */ + public getStories(q?: string | string[]) { + const stories = [...this.stories.entries()] + const outputStories = (list: typeof stories) => + list.map(([, storyDeclaration]) => storyDeclaration) + + if (!q) return outputStories(stories) + + const search = Array.isArray(q) ? q : [q] + return outputStories( + stories.filter(([storyName]) => { + return search.includes(storyName) + }) + ) + } + + /** + * Output list of properties. May be filtered by one or several names. + * Each output property is an object containing the following: + * - `storyName`: Name of the Story the property is attached to. + * - `valueNode`: Node of property's value. + * - `nameNode`: Node of property's name. + */ + public getProperties(q?: string | string[]) { + const properties = [...this.properties.entries()] + const search = Array.isArray(q) ? q : [q] + return properties.reduce((pickedProperties, [propertyName, propertyInstances]) => { + propertyInstances.forEach((propertyInstance) => { + if (!q || search.includes(propertyName)) { + pickedProperties.push(propertyInstance) + } + }) + return pickedProperties + }, [] as StoryPropertyDeclaration[]) + } + + /** Registers stories and map them to their declaration. */ + private registerStories( + storyExports: StoryExports, + declarationCandidates: StoryDeclarationCandidates + ) { + const exports = [...storyExports.entries()] + exports.forEach(([name, declaration]) => { + if (declaration) { + this.stories.set(name, declaration) + } else { + const declaration = declarationCandidates.get(name)?.declarationValue + if (declaration) this.stories.set(name, declaration) + } + }) + } + + /** Registers spread elements used on Stories. */ + private registerSpreads(spreadCandidates: SpreadContentCandidates) { + const possibleSpreads = [...spreadCandidates.entries()] + possibleSpreads.forEach(([spreadName, parentNames]) => { + parentNames.forEach((parentName) => { + if (this.stories.has(parentName)) { + this.spreads.set(spreadName, parentNames) + } + }) + }) + } + + /** Registers story properties. */ + private registerProperties(propertyCandidates: StoryPropertyCandidates) { + const possibleProperties = [...propertyCandidates.values()] + + possibleProperties.forEach((property) => { + const { parentName } = property + + // This is indeed a Story property. + if (this.stories.has(parentName)) { + this.addProperty(property) + } + + // Property's parent object is spread within a Story declaration. + if (this.spreads.has(parentName)) { + const [storyName] = this.spreads.get(parentName) as [string] + this.addProperty({ ...property, parentName: storyName }) + } + }) + } + + /** Adds property to list of properties. */ + private addProperty({ + parentName, + propertyName: name, + propertyNameNode: nameNode, + propertyValueNode: valueNode, + }: PropertyDefinition) { + this.properties.set(name, [ + ...(this.properties.get(name) ?? []), + { storyName: parentName, nameNode, valueNode }, + ]) + } +} diff --git a/lib/utils/ast.ts b/lib/utils/ast.ts index 9081fa9..6deea69 100644 --- a/lib/utils/ast.ts +++ b/lib/utils/ast.ts @@ -1,9 +1,11 @@ import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils' +import { RuleContext } from '../types' +import { NamedVariableDeclarator, TypedNode } from '../types/ast' export { ASTUtils } from '@typescript-eslint/utils' const isNodeOfType = (nodeType: NodeType) => - (node: TSESTree.Node | null | undefined): node is TSESTree.Node & { type: NodeType } => + (node: TSESTree.Node | null | undefined): node is TypedNode => node?.type === nodeType export const isAwaitExpression = isNodeOfType(AST_NODE_TYPES.AwaitExpression) @@ -41,3 +43,44 @@ export const isTSAsExpression = isNodeOfType(AST_NODE_TYPES.TSAsExpression) export const isTSSatisfiesExpression = isNodeOfType(AST_NODE_TYPES.TSSatisfiesExpression) export const isTSNonNullExpression = isNodeOfType(AST_NODE_TYPES.TSNonNullExpression) export const isMetaProperty = isNodeOfType(AST_NODE_TYPES.MetaProperty) + +export const TOP_VARIABLE_DECLARATOR_DEPTH = 2 +export const TOP_VARIABLE_PROP_DECLARATION_DEPTH = 4 +export const TOP_EXPORT_PROP_DECLARATION_DEPTH = 5 +export const TOP_PARAM_ASSIGNMENT_DEPTH = 2 +export const PROPERTY_DECLARATOR_OFFSET = -2 + +export function getPropertyParentName(context: RuleContext) { + const [objectDeclaration] = context.getAncestors().slice(PROPERTY_DECLARATOR_OFFSET) + if (!isVariableDeclarator(objectDeclaration) || !isNamedVariableDeclaration(objectDeclaration)) { + throw new Error("Could not get property's parent object name") + } + + return objectDeclaration.id.name +} + +export function isNamedVariableDeclaration( + declaration: TSESTree.Node +): declaration is NamedVariableDeclarator { + return isVariableDeclarator(declaration) && isIdentifier(declaration.id) +} + +export function isTopLevelVariableDeclarator( + node: TSESTree.Node, + context: RuleContext +): node is NamedVariableDeclarator { + return ( + isNamedVariableDeclaration(node) && + context.getAncestors().length === TOP_VARIABLE_DECLARATOR_DEPTH + ) +} + +export function isTopLevelObjectProperty(context: RuleContext) { + return [TOP_VARIABLE_PROP_DECLARATION_DEPTH, TOP_EXPORT_PROP_DECLARATION_DEPTH].includes( + context.getAncestors().length + ) +} + +export function isTopLevelParamAssignment(context: RuleContext) { + return context.getAncestors().length === TOP_PARAM_ASSIGNMENT_DEPTH +} diff --git a/lib/utils/stories.ts b/lib/utils/stories.ts new file mode 100644 index 0000000..41769a3 --- /dev/null +++ b/lib/utils/stories.ts @@ -0,0 +1,194 @@ +import type { TSESLint, TSESTree } from '@typescript-eslint/utils' +import type { RuleContext } from '../types' +import type { NamedAssignment, NamedProperty, SpreadProperty } from '../types/ast' +import type { + SpreadContentCandidates, + StoryDeclarationCandidates, + StoryExports, + StoryPropertyCandidates, +} from '../types/stories' +import { + getPropertyParentName, + isIdentifier, + isMemberExpression, + isNamedVariableDeclaration, + isTopLevelObjectProperty, + isTopLevelParamAssignment, + isTopLevelVariableDeclarator, + isVariableDeclaration, +} from './ast' +import { StoryIndexer } from './StoryIndexer' + +const STORY_KEY_SEP = '.' +const getKey = (...args: string[]) => args.join(STORY_KEY_SEP) + +export function isStoryPropertyCandidate( + node: TSESTree.Property, + context: RuleContext +): node is NamedProperty { + return isIdentifier(node.key) && isTopLevelObjectProperty(context) +} + +export function isStoryAssignmentCandidate( + node: TSESTree.AssignmentExpression, + context: RuleContext +): node is NamedAssignment { + return ( + isTopLevelParamAssignment(context) && + isMemberExpression(node.left) && + isIdentifier(node.left.object) + ) +} + +export function isSpreadContentCandidate( + node: TSESTree.SpreadElement, + context: RuleContext +): node is SpreadProperty { + return isTopLevelObjectProperty(context) && isIdentifier(node.argument) +} + +/** + * Wrapper utility to extract list of stories and their properties while ESLint + * traverses the document. It can be used to specifically target Story-related + * pieces of code when running validation. + * + * @param context ESLint's `RuleContext`. + * + * @param callback Extraction callback function fired once ESLint has finished + * traversing the document. Its first and only argument exposes an instance of + * `StoryIndexer`, which provides a small API to help with running CSF-related + * validation. + * + * @param listener Some rules may benefit from this wrapper, but still need to + * run additional logic while traversing the document. This argument addresses + * that need. It accepts ESLint's regular `RuleListener`, so that one's custom + * logic is run along with the Story extraction logic. + */ +export function extractStories( + context: Context, + callback: (output: StoryIndexer) => void, + listener: TSESLint.RuleListener = {} +): TSESLint.RuleListener { + const spreadContentCandidates: SpreadContentCandidates = new Map() + const storyPropertyCandidates: StoryPropertyCandidates = new Map() + const storyDeclarationCandidates: StoryDeclarationCandidates = new Map() + const storyExports: StoryExports = new Map() + + return { + ...listener, + + /** + * In CSF, `ExportNamedDeclaration`s translate to stories. Because exports + * can be specified before actual declarations, we need to track them all. + */ + ExportNamedDeclaration: function (node) { + // e.g. `export { A, B }` (see `VariableDeclaration` for further notes) + if (node.specifiers.length) { + node.specifiers.forEach((specifier) => { + storyExports.set(specifier.local.name, undefined) + }) + } + + // e.g. `export const A = {}, B = {}` + if (node.declaration && isVariableDeclaration(node.declaration)) { + node.declaration.declarations.forEach((declaration) => { + if (isNamedVariableDeclaration(declaration)) { + storyExports.set(declaration.id.name, declaration) + } + }) + } + + listener.ExportNamedDeclaration?.(node) + }, + + /** + * Because of modules' flexibility, stories could be declared before they are + * actually exported and therefore considered as Stories. Since we do want to + * attach any report to declarations instead of exports, tracking is required. + */ + VariableDeclarator: function (node) { + if (isTopLevelVariableDeclarator(node, context)) { + storyDeclarationCandidates.set(node.id.name, { + declarationValue: node.init, + }) + } + + listener.VariableDeclarator?.(node) + }, + + /** + * Because of static analysis limitations, only top-level objects' properties + * are considered to actually be potential story properties. Let's track them. + */ + Property: function (node) { + // e.g. `const A = { property: 'foo' }; export const B = { property: 'bar' }` + if (isStoryPropertyCandidate(node, context)) { + const parentName = getPropertyParentName(context) + const propertyName = node.key.name + storyPropertyCandidates.set(getKey(parentName, propertyName), { + parentName, + propertyName, + propertyNameNode: node.key, + propertyValueNode: node.value, + }) + } + + listener.Property?.(node) + }, + + /** + * Story properties can also be mutated after the story was defined. This was + * a common pattern back with CSF2, which still needs to be supported for now. + */ + AssignmentExpression: function (node) { + // e.g. `A.property = …` + if (isStoryAssignmentCandidate(node, context)) { + const parentName = node.left.object.name + const propertyName = node.left.property.name + storyPropertyCandidates.set(getKey(parentName, propertyName), { + parentName, + propertyName, + propertyNameNode: node.left.property, + propertyValueNode: node.right, + }) + } + + listener.AssignmentExpression?.(node) + }, + + /** + * Story properties may also be set by spreading declared variables. Since we + * do want to also validate spread properties, we have to track all top-level + * spread elements. + */ + SpreadElement: function (node) { + if (isSpreadContentCandidate(node, context)) { + const parentName = node.parent.parent.id.name + const spreadName = node.argument.name + spreadContentCandidates.set(spreadName, [ + ...(spreadContentCandidates.get(spreadName) ?? []), + parentName, + ]) + } + + listener.SpreadElement?.(node) + }, + + /** + * Once the whole file has been traversed and analyzed, we shall cross all + * gathered data to output a map of exported stories, and their respective + * properties. + */ + 'Program:exit': function (program) { + callback( + new StoryIndexer( + storyPropertyCandidates, + storyDeclarationCandidates, + spreadContentCandidates, + storyExports + ) + ) + listener['Program:exit']?.(program) + }, + } +} diff --git a/tests/lib/rules/use-string-literal-names.test.ts b/tests/lib/rules/use-string-literal-names.test.ts index e8c596f..6de3546 100644 --- a/tests/lib/rules/use-string-literal-names.test.ts +++ b/tests/lib/rules/use-string-literal-names.test.ts @@ -40,10 +40,13 @@ ruleTester.run('use-string-literal-names', rule, { { code: 'const name = "N"; const A = { name }; export { A }', errors }, { code: 'export const A = () => {}; A.name = String(1994)', errors }, { code: 'export const A = () => {}; A.storyName = String(1994)', errors }, - // Not sure how often this actually happens. If too complex, don't take care of it and delete this test { code: 'const ABase = { name: String(1994) }; export const A = { ...ABase, parameters: {} }', errors, }, + { + code: 'const ABase = { name: String(1994) }; const A = { ...ABase, parameters: {} }; export { A }', + errors, + }, ], })