change implementation for discovering identifiers to handle even more…

… corner cases
elastic · Dec 10, 2024 · 21edadd · 21edadd
1 parent 8fec941
commit 21edadd
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 21 deletions.
diff --git a/packages/kbn-eslint-plugin-css/src/rules/no_css_color.test.ts b/packages/kbn-eslint-plugin-css/src/rules/no_css_color.test.ts
@@ -137,6 +137,16 @@ const invalid: RuleTester.InvalidTestCase[] = [
     }`,
     errors: [{ messageId: 'noCssColorSpecific' }],
   },
+  {
+    name: 'Raises an error when a CSS color for the color property is used in with the tagged template css function',
+    filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx',
+    code: `
+    import { css } from '@emotion/css';
+
+    const codeColor = css\` color: #dd4040; \`;
+    `,
+    errors: [{ messageId: 'noCssColor' }],
+  },
   {
     name: 'Raises an error when a CSS color for the color property is used in a JSX css attribute for EuiComponents with the css template function',
     filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx',
@@ -158,6 +168,24 @@ const invalid: RuleTester.InvalidTestCase[] = [
     import React from 'react';
     import { css } from '@emotion/css';
 
+    const codeCss = css({
+      color: '#dd4040',
+    })
+    
+    function TestComponent() {
+      return (
+        <EuiCode css={codeCss}>This is a test</EuiCode>
+      )
+    }`,
+    errors: [{ messageId: 'noCSSColorSpecificDeclaredVariable' }],
+  },
+  {
+    name: 'Raises an error when a CSS color for the color property is used in a JSX className attribute for EuiComponents with the css template function defined outside the scope of the component',
+    filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx',
+    code: `
+    import React from 'react';
+    import { css } from '@emotion/css';
+
     const codeCss = css\` color: #dd4040; \`
     
     function TestComponent() {
@@ -193,16 +221,6 @@ const invalid: RuleTester.InvalidTestCase[] = [
     }`,
     errors: [{ messageId: 'noCssColorSpecific' }],
   },
-  {
-    name: 'Raises an error when a CSS color for the color property is used in with the tagged template css function',
-    filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx',
-    code: `
-    import { css } from '@emotion/css';
-
-    const codeColor = css\` color: #dd4040; \`;
-    `,
-    errors: [{ messageId: 'noCssColor' }],
-  },
   {
     name: 'Raises an error when a CSS color for the color property is used in a JSX className attribute for EuiComponents with the css template function',
     filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx',

diff --git a/packages/kbn-eslint-plugin-css/src/rules/no_css_color.ts b/packages/kbn-eslint-plugin-css/src/rules/no_css_color.ts
@@ -211,6 +211,10 @@ export const NoCssColor: Rule.RuleModule = {
          * @example
          * const codeStyle = { color: '#dd4040' };
          * <EuiCode css={codeStyle}>This is an example</EuiCode>
+         *
+         * @example
+         * const codeStyle = css({ color: '#dd4040' });
+         * <EuiCode css={codeStyle}>This is an example</EuiCode>
          */
         if (
           node.value?.type === 'JSXExpressionContainer' &&
@@ -220,16 +224,9 @@ export const NoCssColor: Rule.RuleModule = {
 
           const nodeScope = context.sourceCode.getScope(node.value.expression);
 
-          let variableDeclarationMatches = nodeScope.variables.find(
-            (variable) => variable.name === styleVariableName
-          );
-
-          if (!variableDeclarationMatches) {
-            // identifier was probably not declared in the current scope, hence we'll give it another try to find it in the parent scope
-            variableDeclarationMatches = nodeScope.upper?.variables.find(
-              (variable) => variable.name === styleVariableName
-            );
-          }
+          const variableDeclarationMatches = nodeScope.references.find(
+            (ref) => ref.identifier.name === styleVariableName
+          )?.resolved;
 
           // we assume there's only one definition of the variable
           const variableInitializationNode = variableDeclarationMatches?.defs?.[0]?.node?.init;
@@ -238,7 +235,7 @@ export const NoCssColor: Rule.RuleModule = {
             // @ts-ignore
             variableInitializationNode.properties.forEach((property) => {
               handleObjectProperties(context, node, property, {
-                loc: node.loc,
+                loc: property.loc,
                 messageId: 'noCSSColorSpecificDeclaredVariable',
                 data: {
                   property:
@@ -250,6 +247,29 @@ export const NoCssColor: Rule.RuleModule = {
                 },
               });
             });
+          } else if (
+            variableInitializationNode.type === 'CallExpression' &&
+            variableInitializationNode.callee.name === 'css'
+          ) {
+            const cssFunctionArgument = variableInitializationNode.arguments[0];
+
+            if (cssFunctionArgument.type === 'ObjectExpression') {
+              // @ts-ignore
+              cssFunctionArgument.properties.forEach((property) => {
+                handleObjectProperties(context, node, property, {
+                  loc: node.loc,
+                  messageId: 'noCSSColorSpecificDeclaredVariable',
+                  data: {
+                    property:
+                      property.type === 'SpreadElement'
+                        ? String(property.argument.name)
+                        : String(property.key.name),
+                    variableName: styleVariableName,
+                    line: String(property.loc.start.line),
+                  },
+                });
+              });
+            }
           }
 
           return;