From 24567bdff66843d522489c05888bfea3e8be9651 Mon Sep 17 00:00:00 2001 From: Charles GRUENAIS Date: Tue, 10 Jan 2023 17:53:06 +0100 Subject: [PATCH 1/2] Initial commit --- lib/types/index.ts | 4 ++- lib/utils/index.ts | 75 ++++++++++++++++++++++++++++++++-------------- 2 files changed, 56 insertions(+), 23 deletions(-) diff --git a/lib/types/index.ts b/lib/types/index.ts index 538df07..a056489 100644 --- a/lib/types/index.ts +++ b/lib/types/index.ts @@ -1,4 +1,4 @@ -import { TSESLint } from '@typescript-eslint/utils' +import { TSESLint, TSESTree } from '@typescript-eslint/utils' import { CategoryId } from '../utils/constants' export type RuleModule = TSESLint.RuleModule<'', []> & { @@ -39,3 +39,5 @@ export type StorybookRuleMeta = Omit< // schema: [], // docs, // } + +export type NamedVariable = TSESTree.VariableDeclarator & { id: TSESTree.Identifier } diff --git a/lib/utils/index.ts b/lib/utils/index.ts index efb7225..82afa26 100644 --- a/lib/utils/index.ts +++ b/lib/utils/index.ts @@ -1,11 +1,14 @@ import { IncludeExcludeOptions, isExportStory } from '@storybook/csf' import { ASTUtils, TSESTree, TSESLint } from '@typescript-eslint/utils' +import { NamedVariable } from '../types' import { isFunctionDeclaration, isIdentifier, + isLiteral, isObjectExpression, + isProperty, isSpreadElement, isTSAsExpression, isTSSatisfiesExpression, @@ -61,10 +64,11 @@ export const getDescriptor = ( return t.value }) case 'Literal': - // @ts-expect-error TODO: Investigation needed. Type systems says, that "RegExpLiteral" does not exist - case 'RegExpLiteral': - // @ts-ignore - return property.value.value + // // TODO: Investigation needed. Type systems says, that "RegExpLiteral" does not exist + // // @ts-ignore + // case 'RegExpLiteral': + // // @ts-ignore + return property.value.value as any default: throw new Error(`Unexpected descriptor: ${type}`) } @@ -76,7 +80,22 @@ export const isValidStoryExport = ( ) => isExportStory(node.name, nonStoryExportsConfig) && node.name !== '__namedExportsOrder' export const getAllNamedExports = (node: TSESTree.ExportNamedDeclaration) => { - // e.g. `export { MyStory }` + const namedReferences = getExportNamedReferences(node) + if (namedReferences) return namedReferences + + const namedIdentifier = getExportNamedIdentifierDeclaration(node) + if (namedIdentifier?.id) return [namedIdentifier.id] + + const namedFunction = getExportNamedFunctionDeclaration(node) + if (namedFunction?.id) return [namedFunction.id] + + return [] +} + +/** e.g. `export { First, Two }` => `['First', 'Two']`*/ +export const getExportNamedReferences = ( + node: TSESTree.ExportNamedDeclaration +): TSESTree.Identifier[] | undefined => { if (!node.declaration && node.specifiers) { return node.specifiers.reduce((acc, specifier) => { if (isIdentifier(specifier.exported)) { @@ -85,26 +104,38 @@ export const getAllNamedExports = (node: TSESTree.ExportNamedDeclaration) => { return acc }, [] as TSESTree.Identifier[]) } +} - const decl = node.declaration - if (isVariableDeclaration(decl)) { - const declaration = decl.declarations[0] - - if (declaration) { - const { id } = declaration - // e.g. `export const MyStory` - if (isIdentifier(id)) { - return [id] - } - } +/** e.g `export function MyStory() { } => 'MyStory'` */ +export const getExportNamedFunctionDeclaration = ( + node: TSESTree.ExportNamedDeclaration +): TSESTree.FunctionDeclaration | undefined => { + const { declaration } = node + if (isFunctionDeclaration(declaration) && isIdentifier(declaration.id)) { + return declaration } +} - if (isFunctionDeclaration(decl)) { - // e.g. `export function MyStory() {}` - if (isIdentifier(decl.id)) { - return [decl.id] +/** e.g `export const MyStory = () => {}` => `"MyStory"` */ +export const getExportNamedIdentifierDeclaration = ( + node: TSESTree.ExportNamedDeclaration +): NamedVariable | undefined => { + const { declaration } = node + if (isVariableDeclaration(declaration)) { + const [decl] = declaration.declarations + if (decl && isIdentifier(decl.id)) { + return decl as NamedVariable } } - - return [] } + +export const getObjectLiteralProperty = ( + properties: TSESTree.ObjectLiteralElement[], + name: string +) => + properties.find( + (property) => isProperty(property) && isIdentifier(property.key) && property.key.name === name + ) + +export const getObjectLiteralPropertyValue = (property: TSESTree.ObjectLiteralElement) => + !isSpreadElement(property) && isLiteral(property.value) && property.value.value From 8da36359b251e7fb5405c7777b79c791d056117e Mon Sep 17 00:00:00 2001 From: Charles GRUENAIS Date: Tue, 10 Jan 2023 23:55:22 +0100 Subject: [PATCH 2/2] Additional changes --- lib/rules/hierarchy-separator.ts | 9 +-- lib/rules/no-redundant-story-name.ts | 44 ++++-------- lib/rules/no-title-property-in-meta.ts | 7 +- lib/rules/no-uninstalled-addons.ts | 62 ++++++---------- lib/rules/prefer-pascal-case.ts | 21 +++--- lib/types/index.ts | 6 ++ lib/utils/ast.ts | 3 +- lib/utils/index.ts | 97 ++++++++++++++------------ 8 files changed, 112 insertions(+), 137 deletions(-) diff --git a/lib/rules/hierarchy-separator.ts b/lib/rules/hierarchy-separator.ts index ab32bc0..9bbda33 100644 --- a/lib/rules/hierarchy-separator.ts +++ b/lib/rules/hierarchy-separator.ts @@ -3,9 +3,8 @@ * @author Yann Braga */ -import { TSESTree } from '@typescript-eslint/utils' -import { getMetaObjectExpression } from '../utils' -import { isLiteral, isSpreadElement } from '../utils/ast' +import { getMetaObjectExpression, getObjectBareProperty } from '../utils' +import { isLiteral } from '../utils/ast' import { CategoryId } from '../utils/constants' import { createStorybookRule } from '../utils/create-storybook-rule' @@ -40,9 +39,7 @@ export = createStorybookRule({ return null } - const titleNode = meta.properties.find( - (prop) => !isSpreadElement(prop) && 'name' in prop.key && prop.key?.name === 'title' - ) as TSESTree.MethodDefinition | TSESTree.Property | undefined + const titleNode = getObjectBareProperty(meta.properties, 'title') if (!titleNode || !isLiteral(titleNode.value)) { return diff --git a/lib/rules/no-redundant-story-name.ts b/lib/rules/no-redundant-story-name.ts index d652a12..24091d4 100644 --- a/lib/rules/no-redundant-story-name.ts +++ b/lib/rules/no-redundant-story-name.ts @@ -4,16 +4,17 @@ */ import { storyNameFromExport } from '@storybook/csf' +import { + getExportNamedIdentifierDeclaration, + getObjectBareProperty, + getObjectBarePropertyValue, +} from '../utils' import { isExpressionStatement, - isLiteral, isIdentifier, isObjectExpression, - isProperty, - isVariableDeclaration, isMetaProperty, - isSpreadElement, } from '../utils/ast' import { CategoryId } from '../utils/constants' import { createStorybookRule } from '../utils/create-storybook-rule' @@ -50,34 +51,17 @@ export = createStorybookRule({ return { // CSF3 ExportNamedDeclaration: function (node) { - // if there are specifiers, node.declaration should be null - if (!node.declaration) return - - const decl = node.declaration - if (isVariableDeclaration(decl)) { - const declaration = decl.declarations[0] - if (declaration == null) return - const { id, init } = declaration - if (isIdentifier(id) && isObjectExpression(init)) { - const storyNameNode = init.properties.find( - (prop) => - isProperty(prop) && - isIdentifier(prop.key) && - (prop.key?.name === 'name' || prop.key?.name === 'storyName') - ) - - if (!storyNameNode) { - return - } + const declaration = getExportNamedIdentifierDeclaration(node) + if (declaration && isObjectExpression(declaration.init)) { + const storyNameNode = + getObjectBareProperty(declaration.init.properties, 'name') || + getObjectBareProperty(declaration.init.properties, 'storyName') - const { name } = id - const resolvedStoryName = storyNameFromExport(name) + if (storyNameNode) { + const resolvedStoryName = storyNameFromExport(declaration.id.name) + const storyName = getObjectBarePropertyValue(storyNameNode) - if ( - !isSpreadElement(storyNameNode) && - isLiteral(storyNameNode.value) && - storyNameNode.value.value === resolvedStoryName - ) { + if (storyName === resolvedStoryName) { context.report({ node: storyNameNode, messageId: 'storyNameIsRedundant', diff --git a/lib/rules/no-title-property-in-meta.ts b/lib/rules/no-title-property-in-meta.ts index 76909fd..c13065f 100644 --- a/lib/rules/no-title-property-in-meta.ts +++ b/lib/rules/no-title-property-in-meta.ts @@ -4,10 +4,9 @@ */ import { TSESTree } from '@typescript-eslint/utils' -import { getMetaObjectExpression } from '../utils' +import { getMetaObjectExpression, getObjectBareProperty } from '../utils' import { CategoryId } from '../utils/constants' import { createStorybookRule } from '../utils/create-storybook-rule' -import { isSpreadElement } from '../utils/ast' //------------------------------------------------------------------------------ // Rule Definition @@ -39,9 +38,7 @@ export = createStorybookRule({ return null } - const titleNode = meta.properties.find( - (prop) => !isSpreadElement(prop) && 'name' in prop.key && prop.key?.name === 'title' - ) + const titleNode = getObjectBareProperty(meta.properties, 'title') if (titleNode) { context.report({ diff --git a/lib/rules/no-uninstalled-addons.ts b/lib/rules/no-uninstalled-addons.ts index a3fb179..b98fada 100644 --- a/lib/rules/no-uninstalled-addons.ts +++ b/lib/rules/no-uninstalled-addons.ts @@ -9,16 +9,10 @@ import { resolve, relative, sep } from 'path' import { createStorybookRule } from '../utils/create-storybook-rule' import { CategoryId } from '../utils/constants' -import { - isObjectExpression, - isProperty, - isIdentifier, - isArrayExpression, - isLiteral, - isVariableDeclarator, - isVariableDeclaration, -} from '../utils/ast' +import { isObjectExpression, isArrayExpression, isLiteral } from '../utils/ast' import { TSESTree } from '@typescript-eslint/utils' +import { getExportNamedIdentifierDeclaration, getObjectBareProperty } from '../utils' +import { Maybe, NamedVariable, ObjectLiteralItem } from '../types' //------------------------------------------------------------------------------ // Rule Definition @@ -152,10 +146,8 @@ export = createStorybookRule({ const nodesWithAddonsInObj = addonsExpression.elements .map((elem) => (isObjectExpression(elem) ? elem : { properties: [] })) .map((elem) => { - const property: TSESTree.Property = elem.properties.find( - (prop) => isProperty(prop) && isIdentifier(prop.key) && prop.key.name === 'name' - ) as TSESTree.Property - return isLiteral(property?.value) + const property = getObjectBareProperty(elem.properties, 'name') + return property && isLiteral(property?.value) ? { value: property.value.value, node: property.value } : undefined }) @@ -208,6 +200,18 @@ export = createStorybookRule({ } } + function checkAddonInstall< + AddonsProp extends ObjectLiteralItem | NamedVariable, + Property = AddonsProp extends ObjectLiteralItem ? 'value' : 'init' + >(addonsProp: AddonsProp, prop: Property extends keyof AddonsProp ? Property : never) { + if (addonsProp && addonsProp[prop]) { + const node = addonsProp[prop] as Maybe + if (isArrayExpression(node)) { + reportUninstalledAddons(node) + } + } + } + //---------------------------------------------------------------------- // Public //---------------------------------------------------------------------- @@ -215,39 +219,19 @@ export = createStorybookRule({ return { AssignmentExpression: function (node) { if (isObjectExpression(node.right)) { - const addonsProp = node.right.properties.find( - (prop): prop is TSESTree.Property => - isProperty(prop) && isIdentifier(prop.key) && prop.key.name === 'addons' - ) - - if (addonsProp && addonsProp.value && isArrayExpression(addonsProp.value)) { - reportUninstalledAddons(addonsProp.value) - } + const addonsProp = getObjectBareProperty(node.right.properties, 'addons') + if (addonsProp) checkAddonInstall(addonsProp, 'value') } }, ExportDefaultDeclaration: function (node) { if (isObjectExpression(node.declaration)) { - const addonsProp = node.declaration.properties.find( - (prop): prop is TSESTree.Property => - isProperty(prop) && isIdentifier(prop.key) && prop.key.name === 'addons' - ) - - if (addonsProp && addonsProp.value && isArrayExpression(addonsProp.value)) { - reportUninstalledAddons(addonsProp.value) - } + const addonsProp = getObjectBareProperty(node.declaration.properties, 'addons') + if (addonsProp) checkAddonInstall(addonsProp, 'value') } }, ExportNamedDeclaration: function (node) { - const addonsProp = - isVariableDeclaration(node.declaration) && - node.declaration.declarations.find( - (decl) => - isVariableDeclarator(decl) && isIdentifier(decl.id) && decl.id.name === 'addons' - ) - - if (addonsProp && isArrayExpression(addonsProp.init)) { - reportUninstalledAddons(addonsProp.init) - } + const addonsProp = getExportNamedIdentifierDeclaration(node, 'addons') + if (addonsProp) checkAddonInstall(addonsProp, 'init') }, } }, diff --git a/lib/rules/prefer-pascal-case.ts b/lib/rules/prefer-pascal-case.ts index 882bbbb..e22228e 100644 --- a/lib/rules/prefer-pascal-case.ts +++ b/lib/rules/prefer-pascal-case.ts @@ -6,8 +6,11 @@ import { ASTUtils, TSESTree } from '@typescript-eslint/utils' import { IncludeExcludeOptions, isExportStory } from '@storybook/csf' -import { getDescriptor, getMetaObjectExpression } from '../utils' -import { isIdentifier, isVariableDeclaration } from '../utils/ast' +import { + getDescriptor, + getExportNamedIdentifierDeclarations, + getMetaObjectExpression, +} from '../utils' import { CategoryId } from '../utils/constants' import { createStorybookRule } from '../utils/create-storybook-rule' @@ -102,7 +105,7 @@ export = createStorybookRule({ let meta let nonStoryExportsConfig: IncludeExcludeOptions - const namedExports: TSESTree.Identifier[] = [] + let namedExports: TSESTree.Identifier[] = [] let hasStoriesOfImport = false return { @@ -127,16 +130,8 @@ export = createStorybookRule({ ExportNamedDeclaration: function (node: TSESTree.ExportNamedDeclaration) { // if there are specifiers, node.declaration should be null if (!node.declaration) return - - const decl = node.declaration - if (isVariableDeclaration(decl)) { - const declaration = decl.declarations[0] - if (declaration == null) return - const { id } = declaration - if (isIdentifier(id)) { - namedExports.push(id) - } - } + const declarations = (getExportNamedIdentifierDeclarations(node) ?? []).map(({ id }) => id) + namedExports = [...namedExports, ...declarations] }, 'Program:exit': function () { if (namedExports.length && !hasStoriesOfImport) { diff --git a/lib/types/index.ts b/lib/types/index.ts index a056489..6943661 100644 --- a/lib/types/index.ts +++ b/lib/types/index.ts @@ -41,3 +41,9 @@ export type StorybookRuleMeta = Omit< // } export type NamedVariable = TSESTree.VariableDeclarator & { id: TSESTree.Identifier } + +export type ObjectLiteralItem = Exclude + +export type StoryDescriptor = string[] | RegExp + +export type Maybe = T | null | undefined diff --git a/lib/utils/ast.ts b/lib/utils/ast.ts index 4a6375b..0b162d5 100644 --- a/lib/utils/ast.ts +++ b/lib/utils/ast.ts @@ -1,9 +1,10 @@ import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils' +import { Maybe } from '../types' export { ASTUtils } from '@typescript-eslint/utils' const isNodeOfType = (nodeType: NodeType) => - (node: TSESTree.Node | null | undefined): node is TSESTree.Node & { type: NodeType } => + (node: Maybe): node is TSESTree.Node & { type: NodeType } => node?.type === nodeType export const isAwaitExpression = isNodeOfType(AST_NODE_TYPES.AwaitExpression) diff --git a/lib/utils/index.ts b/lib/utils/index.ts index 82afa26..918bc6c 100644 --- a/lib/utils/index.ts +++ b/lib/utils/index.ts @@ -1,7 +1,7 @@ import { IncludeExcludeOptions, isExportStory } from '@storybook/csf' import { ASTUtils, TSESTree, TSESLint } from '@typescript-eslint/utils' -import { NamedVariable } from '../types' +import { NamedVariable, ObjectLiteralItem, StoryDescriptor } from '../types' import { isFunctionDeclaration, @@ -38,39 +38,36 @@ export const getMetaObjectExpression = ( return isObjectExpression(meta) ? meta : null } +/** + * Descriptors support regexes and arrays of strings + * https://github.com/storybookjs/storybook/blob/next/code/lib/csf-tools/src/CsfFile.ts#L16 + */ export const getDescriptor = ( metaDeclaration: TSESTree.ObjectExpression, propertyName: string -): string[] | RegExp | undefined => { - const property = - metaDeclaration && - metaDeclaration.properties.find( - (p) => 'key' in p && 'name' in p.key && p.key.name === propertyName - ) - - if (!property || isSpreadElement(property)) { +): StoryDescriptor | undefined => { + const value = + metaDeclaration && getObjectBareProperty(metaDeclaration.properties, propertyName)?.value + + if (!value) { return undefined } - const { type } = property.value - - switch (type) { + switch (value.type) { case 'ArrayExpression': - return property.value.elements.map((t) => { - if (!['StringLiteral', 'Literal'].includes(t.type)) { - throw new Error(`Unexpected descriptor element: ${t.type}`) + return value.elements.map((element) => { + if (!isLiteral(element) || typeof element.value !== 'string') { + throw new Error(`Unexpected descriptor array element: ${element.type}`) } - // @ts-expect-error TODO: t should be only StringLiteral or Literal, and the type is not resolving correctly - return t.value + return element.value }) case 'Literal': - // // TODO: Investigation needed. Type systems says, that "RegExpLiteral" does not exist - // // @ts-ignore - // case 'RegExpLiteral': - // // @ts-ignore - return property.value.value as any + if (!(value.value instanceof RegExp)) { + throw new Error(`Unexpected descriptor: ${value.type}`) + } + return value.value default: - throw new Error(`Unexpected descriptor: ${type}`) + throw new Error(`Unexpected descriptor: ${value.type}`) } } @@ -83,8 +80,8 @@ export const getAllNamedExports = (node: TSESTree.ExportNamedDeclaration) => { const namedReferences = getExportNamedReferences(node) if (namedReferences) return namedReferences - const namedIdentifier = getExportNamedIdentifierDeclaration(node) - if (namedIdentifier?.id) return [namedIdentifier.id] + const namedIdentifiers = getExportNamedIdentifierDeclarations(node) + if (namedIdentifiers) return namedIdentifiers.map(({ id }) => id) const namedFunction = getExportNamedFunctionDeclaration(node) if (namedFunction?.id) return [namedFunction.id] @@ -92,7 +89,7 @@ export const getAllNamedExports = (node: TSESTree.ExportNamedDeclaration) => { return [] } -/** e.g. `export { First, Two }` => `['First', 'Two']`*/ +/** e.g. `export { First, Two } `*/ export const getExportNamedReferences = ( node: TSESTree.ExportNamedDeclaration ): TSESTree.Identifier[] | undefined => { @@ -106,7 +103,7 @@ export const getExportNamedReferences = ( } } -/** e.g `export function MyStory() { } => 'MyStory'` */ +/** e.g. `export function MyStory() { } => 'MyStory'` */ export const getExportNamedFunctionDeclaration = ( node: TSESTree.ExportNamedDeclaration ): TSESTree.FunctionDeclaration | undefined => { @@ -116,26 +113,40 @@ export const getExportNamedFunctionDeclaration = ( } } -/** e.g `export const MyStory = () => {}` => `"MyStory"` */ -export const getExportNamedIdentifierDeclaration = ( - node: TSESTree.ExportNamedDeclaration -): NamedVariable | undefined => { +/** e.g. `export const MyStory = () => {}` => `"MyStory", MyOtherStory = () => {}` */ +export const getExportNamedIdentifierDeclarations = ( + node: TSESTree.ExportNamedDeclaration, + name?: string +): NamedVariable[] | undefined => { const { declaration } = node + const matchesNameFilter = (exportName: string) => !name || exportName === name if (isVariableDeclaration(declaration)) { - const [decl] = declaration.declarations - if (decl && isIdentifier(decl.id)) { - return decl as NamedVariable + const declarations = declaration.declarations.filter( + (decl) => isIdentifier(decl.id) && matchesNameFilter(decl.id.name) + ) as NamedVariable[] + if (declarations.length > 0) { + return declarations } } } -export const getObjectLiteralProperty = ( - properties: TSESTree.ObjectLiteralElement[], - name: string -) => - properties.find( - (property) => isProperty(property) && isIdentifier(property.key) && property.key.name === name - ) +export const getExportNamedIdentifierDeclaration = ( + node: TSESTree.ExportNamedDeclaration, + name?: string +): NamedVariable | undefined => { + const [declaration] = getExportNamedIdentifierDeclarations(node, name) ?? [] + return declaration +} -export const getObjectLiteralPropertyValue = (property: TSESTree.ObjectLiteralElement) => - !isSpreadElement(property) && isLiteral(property.value) && property.value.value +/** e.g. `{ myProperty: {…}, myMethod(){…} }` */ +export const getObjectBareProperty = (properties: TSESTree.ObjectLiteralElement[], name: string) => + properties.find( + (property) => + isProperty(property) && + isIdentifier(property.key) && + !isSpreadElement(property) && + property.key.name === name + ) as ObjectLiteralItem | undefined + +export const getObjectBarePropertyValue = (property: ObjectLiteralItem) => + isLiteral(property.value) && property.value.value