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..2b08fb5 --- /dev/null +++ b/docs/rules/use-string-literal-names.md @@ -0,0 +1,62 @@ +# 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/react/api/csf#named-story-exports). Story names can be overridden by setting the `name` property: + +```js +export const Simple = { + decorators: [...], + 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, 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: + +```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..11f4346 --- /dev/null +++ b/lib/rules/use-string-literal-names.ts @@ -0,0 +1,43 @@ +/** + * @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 { isLiteral } from '../utils/ast' +import { extractStories } from '../utils/stories' + +//------------------------------------------------------------------------------ +// 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]: 'Story names can only be overridden by string literals', + }, + schema: [], + }, + + create(context) { + 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 4a6375b..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) @@ -16,6 +18,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) @@ -40,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 new file mode 100644 index 0000000..6de3546 --- /dev/null +++ b/tests/lib/rules/use-string-literal-names.test.ts @@ -0,0 +1,52 @@ +/** + * @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) + '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 }, + { 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 }, + { code: 'export const A = () => {}; A.name = String(1994)', errors }, + { code: 'export const A = () => {}; A.storyName = String(1994)', errors }, + { + 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, + }, + ], +})