diff --git a/.changeset/tidy-beds-juggle.md b/.changeset/tidy-beds-juggle.md new file mode 100644 index 00000000..1982ff51 --- /dev/null +++ b/.changeset/tidy-beds-juggle.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-primer-react": minor +--- + +Add ESLint rule to replace deprecated Octicon component with specific icons and remove unused imports diff --git a/src/index.js b/src/index.js index 1dc3315e..b9fb371f 100644 --- a/src/index.js +++ b/src/index.js @@ -20,6 +20,7 @@ module.exports = { 'enforce-css-module-identifier-casing': require('./rules/enforce-css-module-identifier-casing'), 'enforce-css-module-default-import': require('./rules/enforce-css-module-default-import'), 'use-styled-react-import': require('./rules/use-styled-react-import'), + 'no-deprecated-octicon': require('./rules/no-deprecated-octicon'), }, configs: { recommended: require('./configs/recommended'), diff --git a/src/rules/__tests__/no-deprecated-octicon.test.js b/src/rules/__tests__/no-deprecated-octicon.test.js new file mode 100644 index 00000000..6c9055ba --- /dev/null +++ b/src/rules/__tests__/no-deprecated-octicon.test.js @@ -0,0 +1,266 @@ +'use strict' + +const {RuleTester} = require('eslint') +const rule = require('../no-deprecated-octicon') + +const ruleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 'latest', + sourceType: 'module', + parserOptions: { + ecmaFeatures: { + jsx: true, + }, + }, + }, +}) + +ruleTester.run('no-deprecated-octicon', rule, { + valid: [ + // Not an Octicon component + { + code: `import {Button} from '@primer/react' +export default function App() { + return +}`, + }, + + // Already using direct icon import + { + code: `import {XIcon} from '@primer/octicons-react' +export default function App() { + return +}`, + }, + + // Octicon without icon prop (edge case - can't transform) + { + code: `import {Octicon} from '@primer/react/deprecated' +export default function App() { + return +}`, + }, + ], + + invalid: [ + // Basic case: simple Octicon with icon prop + { + code: `import {Octicon} from '@primer/react/deprecated' +import {XIcon} from '@primer/octicons-react' +export default function App() { + return +}`, + output: `import {XIcon} from '@primer/octicons-react' +export default function App() { + return +}`, + errors: [ + { + messageId: 'replaceDeprecatedOcticon', + }, + ], + }, + + // Octicon with additional props + { + code: `import {Octicon} from '@primer/react/deprecated' +import {XIcon} from '@primer/octicons-react' +export default function App() { + return +}`, + output: `import {XIcon} from '@primer/octicons-react' +export default function App() { + return +}`, + errors: [ + { + messageId: 'replaceDeprecatedOcticon', + }, + ], + }, + + // Octicon with spread props + { + code: `import {Octicon} from '@primer/react/deprecated' +import {XIcon} from '@primer/octicons-react' +export default function App() { + const props = { size: 16 } + return +}`, + output: `import {XIcon} from '@primer/octicons-react' +export default function App() { + const props = { size: 16 } + return +}`, + errors: [ + { + messageId: 'replaceDeprecatedOcticon', + }, + ], + }, + + // Octicon with closing tag + { + code: `import {Octicon} from '@primer/react/deprecated' +import {XIcon} from '@primer/octicons-react' +export default function App() { + return + Content + +}`, + output: `import {XIcon} from '@primer/octicons-react' +export default function App() { + return + Content + +}`, + errors: [ + { + messageId: 'replaceDeprecatedOcticon', + }, + ], + }, + + // Multiple Octicons + { + code: `import {Octicon} from '@primer/react/deprecated' +import {XIcon, CheckIcon} from '@primer/octicons-react' +export default function App() { + return ( +
+ + +
+ ) +}`, + output: `import {Octicon} from '@primer/react/deprecated' +import {XIcon, CheckIcon} from '@primer/octicons-react' +export default function App() { + return ( +
+ + +
+ ) +}`, + errors: [ + { + messageId: 'replaceDeprecatedOcticon', + }, + { + messageId: 'replaceDeprecatedOcticon', + }, + ], + }, + + // Complex conditional case - now provides autofix + { + code: `import {Octicon} from '@primer/react/deprecated' +import {XIcon, CheckIcon} from '@primer/octicons-react' +export default function App() { + return +}`, + output: `import {XIcon, CheckIcon} from '@primer/octicons-react' +export default function App() { + return condition ? : +}`, + errors: [ + { + messageId: 'replaceDeprecatedOcticon', + }, + ], + }, + + // Complex conditional case with props - applies props to both components + { + code: `import {Octicon} from '@primer/react/deprecated' +import {XIcon, CheckIcon} from '@primer/octicons-react' +export default function App() { + return +}`, + output: `import {XIcon, CheckIcon} from '@primer/octicons-react' +export default function App() { + return condition ? : +}`, + errors: [ + { + messageId: 'replaceDeprecatedOcticon', + }, + ], + }, + + // Dynamic icon access - now provides autofix + { + code: `import {Octicon} from '@primer/react/deprecated' +export default function App() { + const icons = { x: XIcon } + return +}`, + output: `export default function App() { + const icons = { x: XIcon } + return React.createElement(icons.x, {}) +}`, + errors: [ + { + messageId: 'replaceDeprecatedOcticon', + }, + ], + }, + + // Dynamic icon access with props + { + code: `import {Octicon} from '@primer/react/deprecated' +export default function App() { + const icons = { x: XIcon } + return +}`, + output: `export default function App() { + const icons = { x: XIcon } + return React.createElement(icons.x, {size: 16, className: "btn-icon"}) +}`, + errors: [ + { + messageId: 'replaceDeprecatedOcticon', + }, + ], + }, + + // Test partial import removal - Octicon removed but other imports remain + { + code: `import {Octicon, Button} from '@primer/react/deprecated' +import {XIcon} from '@primer/octicons-react' +export default function App() { + return +}`, + output: `import {Button} from '@primer/react/deprecated' +import {XIcon} from '@primer/octicons-react' +export default function App() { + return +}`, + errors: [ + { + messageId: 'replaceDeprecatedOcticon', + }, + ], + }, + + // Test partial import removal - Octicon in middle of import list + { + code: `import {Button, Octicon, TextField} from '@primer/react/deprecated' +import {XIcon} from '@primer/octicons-react' +export default function App() { + return +}`, + output: `import {Button, TextField} from '@primer/react/deprecated' +import {XIcon} from '@primer/octicons-react' +export default function App() { + return +}`, + errors: [ + { + messageId: 'replaceDeprecatedOcticon', + }, + ], + }, + ], +}) diff --git a/src/rules/no-deprecated-octicon.js b/src/rules/no-deprecated-octicon.js new file mode 100644 index 00000000..d57d6067 --- /dev/null +++ b/src/rules/no-deprecated-octicon.js @@ -0,0 +1,382 @@ +'use strict' + +const {getJSXOpeningElementAttribute} = require('../utils/get-jsx-opening-element-attribute') +const {getJSXOpeningElementName} = require('../utils/get-jsx-opening-element-name') +const url = require('../url') + +/** + * @type {import('eslint').Rule.RuleModule} + */ +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'Replace deprecated `Octicon` component with specific icon imports from `@primer/octicons-react`', + recommended: true, + url: url(module), + }, + fixable: 'code', + schema: [], + messages: { + replaceDeprecatedOcticon: + 'Replace deprecated `Octicon` component with the specific icon from `@primer/octicons-react`', + }, + }, + create(context) { + const sourceCode = context.getSourceCode() + + // Track Octicon imports and usages + const octiconImports = [] + let totalOcticonUsages = 0 + let processedOcticonUsages = 0 + + // Count total Octicon usages with icon props at the start + const sourceText = sourceCode.getText() + const octiconMatches = sourceText.match(/]*icon=/g) + totalOcticonUsages = octiconMatches ? octiconMatches.length : 0 + + return { + ImportDeclaration(node) { + if (node.source.value !== '@primer/react/deprecated') { + return + } + + const hasOcticon = node.specifiers.some( + specifier => specifier.imported && specifier.imported.name === 'Octicon', + ) + + if (hasOcticon) { + octiconImports.push(node) + } + }, + + JSXElement(node) { + const {openingElement, closingElement} = node + const elementName = getJSXOpeningElementName(openingElement) + + if (elementName !== 'Octicon') { + return + } + + // Get the icon prop + const iconProp = getJSXOpeningElementAttribute(openingElement, 'icon') + if (!iconProp) { + // No icon prop - can't determine what to replace with + return + } + + let iconName = null + let isConditional = false + let isMemberExpression = false + let conditionalExpression = null + let memberExpression = null + + // Analyze the icon prop to determine the icon name and type + if (iconProp.value?.type === 'JSXExpressionContainer') { + const expression = iconProp.value.expression + + if (expression.type === 'Identifier') { + // Simple case: icon={XIcon} + iconName = expression.name + } else if (expression.type === 'ConditionalExpression') { + // Conditional case: icon={condition ? XIcon : YIcon} + isConditional = true + conditionalExpression = expression + } else if (expression.type === 'MemberExpression') { + // Dynamic lookup: icon={icons.x} + isMemberExpression = true + memberExpression = expression + } + } + + if (!iconName && !isConditional && !isMemberExpression) { + return + } + + // Get all props except the icon prop to preserve them + const otherProps = openingElement.attributes.filter(attr => attr !== iconProp) + const propsText = otherProps.map(attr => sourceCode.getText(attr)).join(' ') + + // For simple cases, we can provide an autofix + if (iconName) { + context.report({ + node: openingElement, + messageId: 'replaceDeprecatedOcticon', + *fix(fixer) { + // Replace opening element name + yield fixer.replaceText(openingElement.name, iconName) + + // Replace closing element name if it exists + if (closingElement) { + yield fixer.replaceText(closingElement.name, iconName) + } + + // Remove the icon prop with proper whitespace handling + // Use the JSXAttribute node's properties to determine proper removal boundaries + const attributes = openingElement.attributes + const iconIndex = attributes.indexOf(iconProp) + + if (iconIndex === 0 && attributes.length === 1) { + // Only attribute: remove with leading space + const beforeIcon = sourceCode.getTokenBefore(iconProp) + const startPos = + beforeIcon && /\s/.test(sourceCode.getText().substring(beforeIcon.range[1], iconProp.range[0])) + ? beforeIcon.range[1] + : iconProp.range[0] + yield fixer.removeRange([startPos, iconProp.range[1]]) + } else if (iconIndex === 0) { + // First attribute: remove including trailing whitespace/comma + const afterIcon = attributes[1] + const afterPos = sourceCode.getText().substring(iconProp.range[1], afterIcon.range[0]) + const whitespaceMatch = /^\s*/.exec(afterPos) + const endPos = whitespaceMatch ? iconProp.range[1] + whitespaceMatch[0].length : iconProp.range[1] + yield fixer.removeRange([iconProp.range[0], endPos]) + } else { + // Not first attribute: remove including leading whitespace/comma + const beforeIcon = attributes[iconIndex - 1] + const beforePos = sourceCode.getText().substring(beforeIcon.range[1], iconProp.range[0]) + const whitespaceMatch = /\s*$/.exec(beforePos) + const startPos = whitespaceMatch + ? beforeIcon.range[1] + beforePos.length - whitespaceMatch[0].length + : iconProp.range[0] + yield fixer.removeRange([startPos, iconProp.range[1]]) + } + + // Import removal: only enabled for single Octicon cases to prevent ESLint fix conflicts + // For multiple Octicons, JSX transformation works but import remains (can be cleaned up manually) + processedOcticonUsages++ + if ( + processedOcticonUsages === totalOcticonUsages && + totalOcticonUsages === 1 && + octiconImports.length > 0 + ) { + const importNode = octiconImports[0] + const octiconSpecifier = importNode.specifiers.find( + specifier => specifier.imported && specifier.imported.name === 'Octicon', + ) + + if (importNode.specifiers.length === 1) { + // Octicon is the only import, remove the entire import statement + // Also remove trailing newline if present + const nextToken = sourceCode.getTokenAfter(importNode) + const importEnd = importNode.range[1] + const nextStart = nextToken ? nextToken.range[0] : sourceCode.getText().length + const textBetween = sourceCode.getText().substring(importEnd, nextStart) + const hasTrailingNewline = /^\s*\n/.test(textBetween) + + if (hasTrailingNewline) { + const newlineMatch = textBetween.match(/^\s*\n/) + const endRange = importEnd + newlineMatch[0].length + yield fixer.removeRange([importNode.range[0], endRange]) + } else { + yield fixer.remove(importNode) + } + } else { + // Remove just the Octicon specifier from the import + const previousToken = sourceCode.getTokenBefore(octiconSpecifier) + const nextToken = sourceCode.getTokenAfter(octiconSpecifier) + const hasTrailingComma = nextToken && nextToken.value === ',' + const hasLeadingComma = previousToken && previousToken.value === ',' + + let rangeToRemove + if (hasTrailingComma) { + rangeToRemove = [octiconSpecifier.range[0], nextToken.range[1] + 1] + } else if (hasLeadingComma) { + rangeToRemove = [previousToken.range[0], octiconSpecifier.range[1]] + } else { + rangeToRemove = [octiconSpecifier.range[0], octiconSpecifier.range[1]] + } + yield fixer.removeRange(rangeToRemove) + } + } + }, + }) + } else if (isConditional) { + // Handle conditional expressions: icon={condition ? XIcon : YIcon} + // Transform to: condition ? : + context.report({ + node: openingElement, + messageId: 'replaceDeprecatedOcticon', + *fix(fixer) { + const test = sourceCode.getText(conditionalExpression.test) + const consequentName = + conditionalExpression.consequent.type === 'Identifier' + ? conditionalExpression.consequent.name + : sourceCode.getText(conditionalExpression.consequent) + const alternateName = + conditionalExpression.alternate.type === 'Identifier' + ? conditionalExpression.alternate.name + : sourceCode.getText(conditionalExpression.alternate) + + const propsString = propsText ? ` ${propsText}` : '' + let replacement = `${test} ? <${consequentName}${propsString} /> : <${alternateName}${propsString} />` + + // If it has children, we need to include them in both branches + if (node.children && node.children.length > 0) { + const childrenText = node.children.map(child => sourceCode.getText(child)).join('') + replacement = `${test} ? <${consequentName}${propsString}>${childrenText} : <${alternateName}${propsString}>${childrenText}` + } + + yield fixer.replaceText(node, replacement) + + // Import removal: only enabled for single Octicon cases to prevent ESLint fix conflicts + // For multiple Octicons, JSX transformation works but import remains (can be cleaned up manually) + processedOcticonUsages++ + if ( + processedOcticonUsages === totalOcticonUsages && + totalOcticonUsages === 1 && + octiconImports.length > 0 + ) { + const importNode = octiconImports[0] + const octiconSpecifier = importNode.specifiers.find( + specifier => specifier.imported && specifier.imported.name === 'Octicon', + ) + + if (importNode.specifiers.length === 1) { + // Octicon is the only import, remove the entire import statement + // Also remove trailing newline if present + const nextToken = sourceCode.getTokenAfter(importNode) + const importEnd = importNode.range[1] + const nextStart = nextToken ? nextToken.range[0] : sourceCode.getText().length + const textBetween = sourceCode.getText().substring(importEnd, nextStart) + const hasTrailingNewline = /^\s*\n/.test(textBetween) + + if (hasTrailingNewline) { + const newlineMatch = textBetween.match(/^\s*\n/) + const endRange = importEnd + newlineMatch[0].length + yield fixer.removeRange([importNode.range[0], endRange]) + } else { + yield fixer.remove(importNode) + } + } else { + // Remove just the Octicon specifier from the import + const previousToken = sourceCode.getTokenBefore(octiconSpecifier) + const nextToken = sourceCode.getTokenAfter(octiconSpecifier) + const hasTrailingComma = nextToken && nextToken.value === ',' + const hasLeadingComma = previousToken && previousToken.value === ',' + + let rangeToRemove + if (hasTrailingComma) { + rangeToRemove = [octiconSpecifier.range[0], nextToken.range[1] + 1] + } else if (hasLeadingComma) { + rangeToRemove = [previousToken.range[0], octiconSpecifier.range[1]] + } else { + rangeToRemove = [octiconSpecifier.range[0], octiconSpecifier.range[1]] + } + yield fixer.removeRange(rangeToRemove) + } + } + }, + }) + } else if (isMemberExpression) { + // Handle member expressions: icon={icons.x} + // Transform to: React.createElement(icons.x, otherProps) + context.report({ + node: openingElement, + messageId: 'replaceDeprecatedOcticon', + *fix(fixer) { + const memberText = sourceCode.getText(memberExpression) + + // Build props object + let propsObject = '{}' + if (otherProps.length > 0) { + const propStrings = otherProps.map(attr => { + if (attr.type === 'JSXSpreadAttribute') { + return `...${sourceCode.getText(attr.argument)}` + } else { + const name = attr.name.name + const value = attr.value + if (!value) { + return `${name}: true` + } else if (value.type === 'Literal') { + return `${name}: ${JSON.stringify(value.value)}` + } else if (value.type === 'JSXExpressionContainer') { + return `${name}: ${sourceCode.getText(value.expression)}` + } + return `${name}: ${sourceCode.getText(value)}` + } + }) + propsObject = `{${propStrings.join(', ')}}` + } + + let replacement = `React.createElement(${memberText}, ${propsObject})` + + // If it has children, include them as additional arguments + if (node.children && node.children.length > 0) { + const childrenArgs = node.children + .map(child => { + if (child.type === 'JSXText') { + return JSON.stringify(child.value.trim()).replace(/\n\s*/g, ' ') + } else { + return sourceCode.getText(child) + } + }) + .filter(child => child !== '""') // Filter out empty text nodes + + if (childrenArgs.length > 0) { + replacement = `React.createElement(${memberText}, ${propsObject}, ${childrenArgs.join(', ')})` + } + } + + yield fixer.replaceText(node, replacement) + + // Import removal: only enabled for single Octicon cases to prevent ESLint fix conflicts + // For multiple Octicons, JSX transformation works but import remains (can be cleaned up manually) + processedOcticonUsages++ + if ( + processedOcticonUsages === totalOcticonUsages && + totalOcticonUsages === 1 && + octiconImports.length > 0 + ) { + const importNode = octiconImports[0] + const octiconSpecifier = importNode.specifiers.find( + specifier => specifier.imported && specifier.imported.name === 'Octicon', + ) + + if (importNode.specifiers.length === 1) { + // Octicon is the only import, remove the entire import statement + // Also remove trailing newline if present + const nextToken = sourceCode.getTokenAfter(importNode) + const importEnd = importNode.range[1] + const nextStart = nextToken ? nextToken.range[0] : sourceCode.getText().length + const textBetween = sourceCode.getText().substring(importEnd, nextStart) + const hasTrailingNewline = /^\s*\n/.test(textBetween) + + if (hasTrailingNewline) { + const newlineMatch = textBetween.match(/^\s*\n/) + const endRange = importEnd + newlineMatch[0].length + yield fixer.removeRange([importNode.range[0], endRange]) + } else { + yield fixer.remove(importNode) + } + } else { + // Remove just the Octicon specifier from the import + const previousToken = sourceCode.getTokenBefore(octiconSpecifier) + const nextToken = sourceCode.getTokenAfter(octiconSpecifier) + const hasTrailingComma = nextToken && nextToken.value === ',' + const hasLeadingComma = previousToken && previousToken.value === ',' + + let rangeToRemove + if (hasTrailingComma) { + rangeToRemove = [octiconSpecifier.range[0], nextToken.range[1] + 1] + } else if (hasLeadingComma) { + rangeToRemove = [previousToken.range[0], octiconSpecifier.range[1]] + } else { + rangeToRemove = [octiconSpecifier.range[0], octiconSpecifier.range[1]] + } + yield fixer.removeRange(rangeToRemove) + } + } + }, + }) + } else { + // For other complex cases, just report without autofix + context.report({ + node: openingElement, + messageId: 'replaceDeprecatedOcticon', + }) + } + }, + } + }, +}