Fix review comments: remove auto-fixing, remove sx prop, add experimental import test

Co-authored-by: pksjce <[email protected]>

Author: Copilot

docs/rules/no-deprecated-flash.md

@@ -21,11 +21,7 @@ import {Flash} from '@primer/react'
 
 function ExampleComponent() {
   return (
-    <Flash 
-      variant="warning" 
-      sx={{fontSize: 0, borderRadius: 0, py: 2, px: 3}}
-      className="custom-class"
-    >
+    <Flash variant="warning" sx={{fontSize: 0, borderRadius: 0, py: 2, px: 3}} className="custom-class">
       Banner content
     </Flash>
   )
@@ -47,11 +43,7 @@ import {Banner} from '@primer/react/experimental'
 
 function ExampleComponent() {
   return (
-    <Banner 
-      variant="warning" 
-      sx={{fontSize: 0, borderRadius: 0, py: 2, px: 3}}
-      className="custom-class"
-    >
+    <Banner variant="warning" className="custom-class">
       Banner content
     </Banner>
   )
@@ -61,8 +53,9 @@ function ExampleComponent() {
 ## Auto-fix
 
 This rule provides automatic fixes that:
+
 - Replace `Flash` component usage with `Banner`
 - Update import statements from `@primer/react` to `@primer/react/experimental`
 - Preserve all props, attributes, and children content
 - Handle mixed imports appropriately
-- Avoid duplicate Banner imports when they already exist
+- Avoid duplicate Banner imports when they already exist

src/rules/__tests__/no-deprecated-flash.test.js

@@ -49,11 +49,6 @@ function Component() {
 
 function Component() {
   return <Flash variant="warning">Banner content</Flash>
-}`,
-      output: `import {Banner} from '@primer/react/experimental'
-
-function Component() {
-  return <Banner variant="warning">Banner content</Banner>
 }`,
       errors: [{messageId: 'flashDeprecated'}, {messageId: 'flashDeprecated'}],
     },
@@ -66,25 +61,11 @@ function Component() {
   return (
     <Flash 
       variant="warning" 
-      sx={{fontSize: 0, borderRadius: 0, py: 2, px: 3}}
       className="custom-class"
     >
       Banner content
     </Flash>
   )
-}`,
-      output: `import {Banner} from '@primer/react/experimental'
-
-function Component() {
-  return (
-    <Banner 
-      variant="warning" 
-      sx={{fontSize: 0, borderRadius: 0, py: 2, px: 3}}
-      className="custom-class"
-    >
-      Banner content
-    </Banner>
-  )
 }`,
       errors: [{messageId: 'flashDeprecated'}, {messageId: 'flashDeprecated'}],
     },
@@ -101,18 +82,6 @@ function Component() {
       <Text>Some text</Text>
     </div>
   )
-}`,
-      output: `import {Button, Text} from '@primer/react'
-import {Banner} from '@primer/react/experimental'
-
-function Component() {
-  return (
-    <div>
-      <Button>Click me</Button>
-      <Banner variant="error">Error message</Banner>
-      <Text>Some text</Text>
-    </div>
-  )
 }`,
       errors: [{messageId: 'flashDeprecated'}, {messageId: 'flashDeprecated'}],
     },
@@ -123,11 +92,6 @@ function Component() {
 
 function Component() {
   return <Flash>Just Flash</Flash>
-}`,
-      output: `import {Banner} from '@primer/react/experimental'
-
-function Component() {
-  return <Banner>Just Flash</Banner>
 }`,
       errors: [{messageId: 'flashDeprecated'}, {messageId: 'flashDeprecated'}],
     },
@@ -138,11 +102,6 @@ function Component() {
 
 function Component() {
   return <Flash variant="info" />
-}`,
-      output: `import {Banner} from '@primer/react/experimental'
-
-function Component() {
-  return <Banner variant="info" />
 }`,
       errors: [{messageId: 'flashDeprecated'}, {messageId: 'flashDeprecated'}],
     },
@@ -158,16 +117,6 @@ function Component() {
       <Flash variant="error">Error</Flash>
     </div>
   )
-}`,
-      output: `import {Banner} from '@primer/react/experimental'
-
-function Component() {
-  return (
-    <div>
-      <Banner variant="warning">Warning</Banner>
-      <Banner variant="error">Error</Banner>
-    </div>
-  )
 }`,
       errors: [{messageId: 'flashDeprecated'}, {messageId: 'flashDeprecated'}, {messageId: 'flashDeprecated'}],
     },
@@ -185,13 +134,19 @@ function Component() {
     </div>
   )
 }`,
-      output: `import {Banner} from '@primer/react/experimental'
+      errors: [{messageId: 'flashDeprecated'}, {messageId: 'flashDeprecated'}],
+    },
+
+    // Flash with existing experimental imports like TooltipV2
+    {
+      code: `import {Flash} from '@primer/react'
+import {TooltipV2} from '@primer/react/experimental'
 
 function Component() {
   return (
     <div>
-      <Banner variant="warning">Flash message</Banner>
-      <Banner variant="info">Banner message</Banner>
+      <Flash variant="warning">Flash message</Flash>
+      <TooltipV2>Tooltip content</TooltipV2>
     </div>
   )
 }`,

src/rules/no-deprecated-flash.js

@@ -15,7 +15,6 @@ module.exports = {
       recommended: true,
       url: url(module),
     },
-    fixable: 'code',
     schema: [],
     messages: {
       flashDeprecated: 'Flash component is deprecated. Use Banner from @primer/react/experimental instead.',
@@ -42,61 +41,6 @@ module.exports = {
         context.report({
           node: flashSpecifier,
           messageId: 'flashDeprecated',
-          *fix(fixer) {
-            // Check if there's already a Banner import from @primer/react/experimental
-            const program = node.parent
-            const existingBannerImport = program.body.find(
-              stmt =>
-                stmt.type === 'ImportDeclaration' &&
-                stmt.source.value === '@primer/react/experimental' &&
-                stmt.specifiers.some(spec => spec.imported?.name === 'Banner'),
-            )
-
-            // Remove Flash from current import
-            const otherSpecifiers = node.specifiers.filter(spec => spec !== flashSpecifier)
-
-            if (otherSpecifiers.length === 0) {
-              // If Flash was the only import, replace entire import with Banner
-              if (!existingBannerImport) {
-                yield fixer.replaceText(node, "import {Banner} from '@primer/react/experimental'")
-              } else {
-                // Banner import already exists, remove this import entirely including newline
-                const nextToken = sourceCode.getTokenAfter(node)
-                if (nextToken && sourceCode.getText().substring(node.range[1], nextToken.range[0]).includes('\n')) {
-                  // Remove including the newline after
-                  yield fixer.removeRange([node.range[0], nextToken.range[0]])
-                } else {
-                  yield fixer.remove(node)
-                }
-              }
-            } else {
-              // Remove Flash specifier and handle commas properly
-              const indexOfFlash = node.specifiers.indexOf(flashSpecifier)
-
-              if (indexOfFlash === 0) {
-                // Flash is first, remove Flash and the trailing comma
-                const tokenAfter = sourceCode.getTokenAfter(flashSpecifier)
-                if (tokenAfter && tokenAfter.value === ',') {
-                  yield fixer.removeRange([flashSpecifier.range[0], tokenAfter.range[1]])
-                } else {
-                  yield fixer.remove(flashSpecifier)
-                }
-              } else {
-                // Flash is not first, remove the preceding comma and Flash
-                const tokenBefore = sourceCode.getTokenBefore(flashSpecifier)
-                if (tokenBefore && tokenBefore.value === ',') {
-                  yield fixer.removeRange([tokenBefore.range[0], flashSpecifier.range[1]])
-                } else {
-                  yield fixer.remove(flashSpecifier)
-                }
-              }
-
-              // Add Banner import if it doesn't exist
-              if (!existingBannerImport) {
-                yield fixer.insertTextAfter(node, "\nimport {Banner} from '@primer/react/experimental'")
-              }
-            }
-          },
         })
       },
 
@@ -116,15 +60,6 @@ module.exports = {
         context.report({
           node: node.openingElement.name,
           messageId: 'flashDeprecated',
-          *fix(fixer) {
-            // Replace opening tag
-            yield fixer.replaceText(node.openingElement.name, 'Banner')
-
-            // Replace closing tag if it exists
-            if (node.closingElement) {
-              yield fixer.replaceText(node.closingElement.name, 'Banner')
-            }
-          },
         })
       },
     }