From 21edadd2444cca200b53eea8f5db462a8d839731 Mon Sep 17 00:00:00 2001 From: Eyo Okon Eyo Date: Thu, 5 Dec 2024 19:02:13 +0100 Subject: [PATCH] change implementation for discovering identifiers to handle even more corner cases --- .../src/rules/no_css_color.test.ts | 38 ++++++++++++----- .../src/rules/no_css_color.ts | 42 ++++++++++++++----- 2 files changed, 59 insertions(+), 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 index d95160083c117..e1f683b09814f 100644 --- 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 ( + This is a test + ) + }`, + 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 index 5462114dad9c2..5583bce9fcf54 100644 --- 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' }; * This is an example + * + * @example + * const codeStyle = css({ color: '#dd4040' }); + * This is an example */ 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;