From d55828d36dcd0bcabbd6450913277c2659833d70 Mon Sep 17 00:00:00 2001 From: Juan Pablo Garcia Ripa Date: Wed, 10 Apr 2024 09:39:47 +0200 Subject: [PATCH] add fixer logic for aliases Signed-off-by: Juan Pablo Garcia Ripa --- .changeset/witty-onions-jog.md | 5 + packages/eslint-plugin/package.json | 1 + .../no-top-level-material-ui-4-imports.js | 136 ++++++++++++++---- ...no-top-level-material-ui-4-imports.test.ts | 33 ++++- yarn.lock | 1 + 5 files changed, 147 insertions(+), 29 deletions(-) create mode 100644 .changeset/witty-onions-jog.md diff --git a/.changeset/witty-onions-jog.md b/.changeset/witty-onions-jog.md new file mode 100644 index 0000000000..534224a704 --- /dev/null +++ b/.changeset/witty-onions-jog.md @@ -0,0 +1,5 @@ +--- +'@backstage/eslint-plugin': patch +--- + +add fixer logic for import aliases diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index c4b3f0d47d..c543c82ee6 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -23,6 +23,7 @@ }, "devDependencies": { "@backstage/cli": "workspace:^", + "@types/estree": "^1.0.5", "eslint": "^8.33.0" } } diff --git a/packages/eslint-plugin/rules/no-top-level-material-ui-4-imports.js b/packages/eslint-plugin/rules/no-top-level-material-ui-4-imports.js index 8305615ca4..77adcad215 100644 --- a/packages/eslint-plugin/rules/no-top-level-material-ui-4-imports.js +++ b/packages/eslint-plugin/rules/no-top-level-material-ui-4-imports.js @@ -16,16 +16,75 @@ // @ts-check +/** + * @typedef {object} FixerValues + * @property {string} value + * @property {string} [alias] + * @property {string} [componentValue] + * @property {string} [componentAlias] + * @property {boolean} emitComponent + * @property {boolean} emitProp + */ + const KNOWN_STYLES = [ - 'makeStyles', - 'withStyles', - 'createStyles', - 'styled', - 'useTheme', + // TODO: add exports from colorManipulator and transitions + 'createTheme', + 'unstable_createMuiStrictModeTheme', + 'createMuiTheme', + 'ThemeOptions', 'Theme', + 'Direction', + 'PaletteColorOptions', + 'SimplePaletteColorOptions', + 'createStyles', + 'TypographyStyle', + 'TypographyVariant', + 'makeStyles', + 'responsiveFontSizes', + 'ComponentsPropsList', + 'useTheme', + 'withStyles', + 'WithStyles', + 'StyleRules', + 'StyleRulesCallback', + 'StyledComponentProps', + 'withTheme', + 'WithTheme', + 'styled', + 'ComponentCreator', + 'StyledProps', + 'createGenerateClassName', + 'jssPreset', + 'ServerStyleSheets', + 'StylesProvider', + 'MuiThemeProvider', 'ThemeProvider', + 'ThemeProviderProps', ]; +/** + * filter function to keep only ImportSpecifier nodes + * @param {import('estree').ImportSpecifier | import('estree').ImportDefaultSpecifier| import('estree').ImportNamespaceSpecifier} specifier + * @returns {specifier is import('estree').ImportSpecifier} + */ +function importSpecifiersFilter(specifier) { + return specifier.type === 'ImportSpecifier'; +} + +/** + * Gets the value of the named import depending on if it has an alias or not + * @param {FixerValues} values + * @returns {string} + * @example + * `import { ${getNamedImportValue({ value: 'SvgIcon', alias: 'Icon' })} } from 'x'` // import { Icon as SvgIcon } from 'x' + * `import { ${getNamedImportValue({ value: 'SvgIcon' })} } from 'x'` // import { SvgIcon } from 'x' + */ +function getNamedImportValue(values) { + return values.alias + ? `${values.value} as ${values.alias}` + : `${values.value}`; +} + /** @type {import('eslint').Rule.RuleModule} */ module.exports = { meta: { @@ -69,27 +128,39 @@ module.exports = { const replacements = []; const styles = []; - const specifiers = node.specifiers.filter( - s => s.type === 'ImportSpecifier', + const specifiers = node.specifiers.filter(importSpecifiersFilter); + + const specifiersMap = specifiers.map( + /** + * transform ImportSpecifier to FixerValues to have a simpler object to work with + * @returns {FixerValues} + */ + s => { + const value = s.imported.name; + const alias = s.local.name === value ? undefined : s.local.name; + + const propsMatch = /^([A-Z]\w+)Props$/.exec(value); + + const emitProp = propsMatch !== null; + const emitComponent = !emitProp; + const emitComponentAndProp = + emitProp && + specifiers.find(s => s.imported.name === propsMatch[1])?.local + .name; + + return { + emitComponent: emitComponent || Boolean(emitComponentAndProp), + emitProp, + value, + componentValue: propsMatch ? propsMatch[1] : undefined, + componentAlias: emitComponentAndProp + ? emitComponentAndProp + : undefined, + alias, + }; + }, ); - const specifiersMap = specifiers.map(s => { - const value = s.local.name; - const propsMatch = /^([A-Z]\w+)Props$/.exec(value); - - const emitProp = propsMatch !== null; - const emitComponent = !emitProp; - const emitComponentAndProp = - emitProp && specifiers.some(s => s.local.name === propsMatch[1]); - - return { - emitComponent: emitComponent || emitComponentAndProp, - emitProp, - value, - componentValue: propsMatch ? propsMatch[1] : undefined, - }; - }); - // Filter out duplicates where we have both component and component+prop const filteredMap = specifiersMap.filter( f => !specifiersMap.some(s => f.value === s.componentValue), @@ -104,27 +175,36 @@ module.exports = { // Just Component if (specifier.emitComponent && !specifier.emitProp) { if (KNOWN_STYLES.includes(specifier.value)) { - styles.push(specifier.value); + styles.push(getNamedImportValue(specifier)); } else { - const replacement = `import ${specifier.value} from '${node.source.value}/${specifier.value}';`; + const replacement = `import ${ + specifier.alias ?? specifier.value + } from '${node.source.value}/${specifier.value}';`; replacements.push(replacement); } } // Just Prop if (specifier.emitProp && !specifier.emitComponent) { - const replacement = `import { ${specifier.value} } from '@material-ui/core/${specifier.componentValue}';`; + const replacement = `import { ${getNamedImportValue( + specifier, + )} } from '@material-ui/core/${specifier.componentValue}';`; replacements.push(replacement); } // Component and Prop if (specifier.emitComponent && specifier.emitProp) { replacements.push( - `import ${specifier.componentValue}, { ${specifier.value} } from '@material-ui/core/${specifier.componentValue}';`, + `import ${ + specifier.componentAlias ?? specifier.componentValue + }, { ${getNamedImportValue( + specifier, + )} } from '@material-ui/core/${specifier.componentValue}';`, ); } } + // if we imports that should be moved to styles we added them here if (styles.length > 0) { const stylesReplacement = `import { ${styles.join( ', ', diff --git a/packages/eslint-plugin/src/no-top-level-material-ui-4-imports.test.ts b/packages/eslint-plugin/src/no-top-level-material-ui-4-imports.test.ts index 8acd2f8850..2d29c2f9e9 100644 --- a/packages/eslint-plugin/src/no-top-level-material-ui-4-imports.test.ts +++ b/packages/eslint-plugin/src/no-top-level-material-ui-4-imports.test.ts @@ -35,6 +35,9 @@ ruleTester.run('path-imports-rule', rule, { { code: `import { styled, withStyles } from '@material-ui/core/styles';`, }, + { + code: `import { WithStyles } from '@material-ui/core/styles';`, + }, { code: `import SvgIcon, { SvgIconProps } from '@material-ui/core/SvgIcon';`, }, @@ -62,6 +65,11 @@ import Typography from '@material-ui/core/Typography';`, errors: [{ messageId: 'topLevelImport' }], output: `import { ThemeProvider } from '@material-ui/core/styles';`, }, + { + code: `import { WithStyles } from '@material-ui/core';`, + errors: [{ messageId: 'topLevelImport' }], + output: `import { WithStyles } from '@material-ui/core/styles';`, + }, { code: `import { Grid, GridProps, Theme, makeStyles } from '@material-ui/core';`, errors: [{ messageId: 'topLevelImport' }], @@ -83,6 +91,8 @@ import SvgIcon, { SvgIconProps } from '@material-ui/core/SvgIcon';`, Grid, makeStyles, ThemeProvider, + WithStyles, + Tooltip as MaterialTooltip, } from '@material-ui/core';`, errors: [{ messageId: 'topLevelImport' }], output: `import Box from '@material-ui/core/Box'; @@ -90,7 +100,8 @@ import DialogActions from '@material-ui/core/DialogActions'; import DialogContent from '@material-ui/core/DialogContent'; import DialogTitle from '@material-ui/core/DialogTitle'; import Grid from '@material-ui/core/Grid'; -import { makeStyles, ThemeProvider } from '@material-ui/core/styles';`, +import MaterialTooltip from '@material-ui/core/Tooltip'; +import { makeStyles, ThemeProvider, WithStyles } from '@material-ui/core/styles';`, }, { code: `import { Box, Button, makeStyles } from '@material-ui/core';`, @@ -121,5 +132,25 @@ import { styled, withStyles } from '@material-ui/core/styles';`, errors: [{ messageId: 'topLevelImport' }], output: `import { TabProps } from '@material-ui/core/Tab';`, }, + { + code: `import { Tooltip as MaterialTooltip, } from '@material-ui/core';`, + errors: [{ messageId: 'topLevelImport' }], + output: `import MaterialTooltip from '@material-ui/core/Tooltip';`, + }, + { + code: `import { SvgIcon as Icon, SvgIconProps as IconProps } from '@material-ui/core';`, + errors: [{ messageId: 'topLevelImport' }], + output: `import Icon, { SvgIconProps as IconProps } from '@material-ui/core/SvgIcon';`, + }, + { + code: `import { SvgIconProps as IconProps } from '@material-ui/core';`, + errors: [{ messageId: 'topLevelImport' }], + output: `import { SvgIconProps as IconProps } from '@material-ui/core/SvgIcon';`, + }, + { + code: `import { styled as s } from '@material-ui/core';`, + errors: [{ messageId: 'topLevelImport' }], + output: `import { styled as s } from '@material-ui/core/styles';`, + }, ], }); diff --git a/yarn.lock b/yarn.lock index 69ca05c3e9..1754a7359c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4189,6 +4189,7 @@ __metadata: dependencies: "@backstage/cli": "workspace:^" "@manypkg/get-packages": ^1.1.3 + "@types/estree": ^1.0.5 eslint: ^8.33.0 minimatch: ^9.0.0 languageName: unknown