From 7370cc712ee2bf23f4cf53eef2e74942340f761c Mon Sep 17 00:00:00 2001 From: "Eyo O. Eyo" <7893459+eokoneyo@users.noreply.github.com> Date: Mon, 16 Dec 2024 15:56:52 +0000 Subject: [PATCH] ES Lint rules for css-in-js declarations within Kibana (#200703) ## Summary Closes https://github.com/elastic/kibana-team/issues/1272 This PR adds implementation for eslint rules to help facilitate the migration away from SASS files to leveraging the design tokens EUI provides for styling. The introduced rules in this PR are as follows; - #### No CSS Color values Consider; ```tsx Hello World! ``` this expression because it specifies the css color property, with a valid [CSS color value](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value), when the aforementioned rule is enabled depending on the set report level set the user would get a feedback, see screenshot below; Screenshot 2024-11-20 at 12 46 17 This rule also works for variables defined elsewhere in the code and referenced as a value to the style prop, see screenshot below; Screenshot 2024-11-26 at 13 29 45 feedback will also be provided when some variable that is a literal value is specified as a value for any earmarked property that should not specify literal values. Screenshot 2024-11-28 at 19 00 08 feedback will be provided for referencing a member prop of some object defined elsewhere as a value to any earmarked property that we have deemed to not specify literal values Screenshot 2024-11-29 at 11 36 44 Supports; - object values - object references - template literals - tagged templates This approach does not penalize variable declarations, only the usages of any said variable when it doesn't conform to expectation - #### Prefer CSS attributes for EUI components (optional) Consider; ```tsx Hello World! ``` A declaration like the one above, will be regarded as an error and can be fixed, when it's fixed it will be re-written as ```tsx Hello World! ``` --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine --- .eslintrc.js | 1 - .github/CODEOWNERS | 1 + package.json | 3 + packages/kbn-eslint-config/.eslintrc.js | 2 + packages/kbn-eslint-plugin-css/README.mdx | 129 +++++ packages/kbn-eslint-plugin-css/index.ts | 20 + packages/kbn-eslint-plugin-css/jest.config.js | 14 + packages/kbn-eslint-plugin-css/kibana.jsonc | 6 + packages/kbn-eslint-plugin-css/package.json | 6 + .../src/rules/no_css_color.test.ts | 249 ++++++++++ .../src/rules/no_css_color.ts | 453 ++++++++++++++++++ ...r_css_attribute_for_eui_components.test.ts | 85 ++++ ...prefer_css_attribute_for_eui_components.ts | 62 +++ packages/kbn-eslint-plugin-css/tsconfig.json | 11 + tsconfig.base.json | 6 +- yarn.lock | 21 + 16 files changed, 1065 insertions(+), 4 deletions(-) create mode 100644 packages/kbn-eslint-plugin-css/README.mdx create mode 100644 packages/kbn-eslint-plugin-css/index.ts create mode 100644 packages/kbn-eslint-plugin-css/jest.config.js create mode 100644 packages/kbn-eslint-plugin-css/kibana.jsonc create mode 100644 packages/kbn-eslint-plugin-css/package.json create mode 100644 packages/kbn-eslint-plugin-css/src/rules/no_css_color.test.ts create mode 100644 packages/kbn-eslint-plugin-css/src/rules/no_css_color.ts create mode 100644 packages/kbn-eslint-plugin-css/src/rules/prefer_css_attribute_for_eui_components.test.ts create mode 100644 packages/kbn-eslint-plugin-css/src/rules/prefer_css_attribute_for_eui_components.ts create mode 100644 packages/kbn-eslint-plugin-css/tsconfig.json diff --git a/.eslintrc.js b/.eslintrc.js index dd397193cf425..89896ff74a2e2 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -764,7 +764,6 @@ module.exports = { ], }, }, - /** * Jest specific rules */ diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 8b376c45ea49f..8f658aef3ac14 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -337,6 +337,7 @@ packages/kbn-es-errors @elastic/kibana-core packages/kbn-es-query @elastic/kibana-data-discovery packages/kbn-es-types @elastic/kibana-core @elastic/obs-knowledge-team packages/kbn-eslint-config @elastic/kibana-operations +packages/kbn-eslint-plugin-css @elastic/appex-sharedux packages/kbn-eslint-plugin-disable @elastic/kibana-operations packages/kbn-eslint-plugin-eslint @elastic/kibana-operations packages/kbn-eslint-plugin-i18n @elastic/obs-knowledge-team @elastic/kibana-operations diff --git a/package.json b/package.json index ebc74c2bb03d6..29e87c578ad44 100644 --- a/package.json +++ b/package.json @@ -1451,6 +1451,7 @@ "@kbn/es": "link:packages/kbn-es", "@kbn/es-archiver": "link:packages/kbn-es-archiver", "@kbn/eslint-config": "link:packages/kbn-eslint-config", + "@kbn/eslint-plugin-css": "link:packages/kbn-eslint-plugin-css", "@kbn/eslint-plugin-disable": "link:packages/kbn-eslint-plugin-disable", "@kbn/eslint-plugin-eslint": "link:packages/kbn-eslint-plugin-eslint", "@kbn/eslint-plugin-i18n": "link:packages/kbn-eslint-plugin-i18n", @@ -1566,6 +1567,7 @@ "@types/classnames": "^2.2.9", "@types/cli-progress": "^3.11.5", "@types/color": "^3.0.3", + "@types/cssstyle": "^2.2.4", "@types/cytoscape": "^3.14.0", "@types/d3": "^3.5.43", "@types/d3-array": "^2.12.1", @@ -1714,6 +1716,7 @@ "css-loader": "^3.4.2", "cssnano": "^5.1.12", "cssnano-preset-default": "^5.2.12", + "cssstyle": "^4.1.0", "csstype": "^3.0.2", "cypress": "13.15.2", "cypress-axe": "^1.5.0", diff --git a/packages/kbn-eslint-config/.eslintrc.js b/packages/kbn-eslint-config/.eslintrc.js index e38b6cc48c443..57a116a2b9b68 100644 --- a/packages/kbn-eslint-config/.eslintrc.js +++ b/packages/kbn-eslint-config/.eslintrc.js @@ -28,6 +28,7 @@ module.exports = { '@kbn/eslint-plugin-imports', '@kbn/eslint-plugin-telemetry', '@kbn/eslint-plugin-i18n', + '@kbn/eslint-plugin-css', 'eslint-plugin-depend', 'prettier', ], @@ -332,6 +333,7 @@ module.exports = { '@kbn/imports/no_boundary_crossing': 'error', '@kbn/imports/no_group_crossing_manifests': 'error', '@kbn/imports/no_group_crossing_imports': 'error', + '@kbn/css/no_css_color': 'warn', 'no-new-func': 'error', 'no-implied-eval': 'error', 'no-prototype-builtins': 'error', diff --git a/packages/kbn-eslint-plugin-css/README.mdx b/packages/kbn-eslint-plugin-css/README.mdx new file mode 100644 index 0000000000000..1e121657bc571 --- /dev/null +++ b/packages/kbn-eslint-plugin-css/README.mdx @@ -0,0 +1,129 @@ +--- +id: kibSharedUXEslintPluginCSS +slug: /kibana-dev-docs/shared-ux/packages/kbn-eslint-plugin-css +title: '@kbn/eslint-plugin-design-tokens' +description: Custom ESLint rules to guardrails for using eui in the Kibana repository +date: 2024-11-19 +tags: ['kibana', 'dev', 'contributor', 'shared_ux', 'eslint', 'eui'] +--- + +# Summary + +`@kbn/eslint-plugin-css` is an ESLint plugin providing custom ESLint rules to help setup guardrails for using eui in the Kibana repo especially around styling. + +The aim of this package is to help engineers to modify EUI components in a much complaint way. + +If a rule does not behave as you expect or you have an idea of how these rules can be improved, please reach out to the Shared UX team. + +# Rules + +## `@kbn/css/no_css_color` + +This rule warns engineers to not use literal css color in the codebase, particularly for CSS properties that apply color to +either the html element or text nodes, but rather urge users to defer to using the color tokens provided by EUI. + +This rule kicks in on the following JSXAttributes; `style`, `className` and `css` and supports various approaches to providing styling declarations. + +### Example + +The following code: + +``` +// Filename: /x-pack/plugins/observability_solution/observability/public/my_component.tsx + +import React from 'react'; +import { EuiText } from '@elastic/eui'; + +function MyComponent() { + return ( + You know, for search + ) +} +``` + +``` +// Filename: /x-pack/plugins/observability_solution/observability/public/my_component.tsx + +import React from 'react'; +import { EuiText } from '@elastic/eui'; + +function MyComponent() { + + const style = { + color: 'red' + } + + return ( + You know, for search + ) +} +``` + +``` +// Filename: /x-pack/plugins/observability_solution/observability/public/my_component.tsx + +import React from 'react'; +import { EuiText } from '@elastic/eui'; + +function MyComponent() { + const colorValue = '#dd4040'; + + return ( + You know, for search + ) +} +``` + +will all raise an eslint report with an appropriate message of severity that matches the configuration of the rule, further more all the examples above +will also match for when the attribute in question is `css`. The `css` attribute will also raise a report the following cases below; + +``` +// Filename: /x-pack/plugins/observability_solution/observability/public/my_component.tsx + +import React from 'react'; +import { css } from '@emotion/css'; +import { EuiText } from '@elastic/eui'; + +function MyComponent() { + return ( + You know, for search + ) +} +``` + +``` +// Filename: /x-pack/plugins/observability_solution/observability/public/my_component.tsx + +import React from 'react'; +import { EuiText } from '@elastic/eui'; + +function MyComponent() { + return ( + ({ color: '#dd4040' })}>You know, for search + ) +} +``` + +A special case is also covered for the `className` attribute, where the rule will also raise a report for the following case below; + + +``` +// Filename: /x-pack/plugins/observability_solution/observability/public/my_component.tsx + +import React from 'react'; +import { css } from '@emotion/css'; +import { EuiText } from '@elastic/eui'; + +function MyComponent() { + return ( + You know, for search + ) +} +``` + +it's worth pointing out that although the examples provided are specific to EUI components, this rule applies to all JSX elements. + +## `@kbn/css/prefer_css_attributes_for_eui_components` + +This rule warns engineers to use the `css` attribute for EUI components instead of the `style` attribute. + diff --git a/packages/kbn-eslint-plugin-css/index.ts b/packages/kbn-eslint-plugin-css/index.ts new file mode 100644 index 0000000000000..9ea4bcc67619f --- /dev/null +++ b/packages/kbn-eslint-plugin-css/index.ts @@ -0,0 +1,20 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { NoCssColor } from './src/rules/no_css_color'; +import { PreferCSSAttributeForEuiComponents } from './src/rules/prefer_css_attribute_for_eui_components'; + +/** + * Custom ESLint rules, included as `'@kbn/eslint-plugin-design-tokens'` in the kibana eslint config + * @internal + */ +export const rules = { + no_css_color: NoCssColor, + prefer_css_attributes_for_eui_components: PreferCSSAttributeForEuiComponents, +}; diff --git a/packages/kbn-eslint-plugin-css/jest.config.js b/packages/kbn-eslint-plugin-css/jest.config.js new file mode 100644 index 0000000000000..c8ae20237eae8 --- /dev/null +++ b/packages/kbn-eslint-plugin-css/jest.config.js @@ -0,0 +1,14 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +module.exports = { + preset: '@kbn/test', + rootDir: '../..', + roots: ['/packages/kbn-eslint-plugin-css'], +}; diff --git a/packages/kbn-eslint-plugin-css/kibana.jsonc b/packages/kbn-eslint-plugin-css/kibana.jsonc new file mode 100644 index 0000000000000..3ee8bff8736f6 --- /dev/null +++ b/packages/kbn-eslint-plugin-css/kibana.jsonc @@ -0,0 +1,6 @@ +{ + "type": "shared-common", + "id": "@kbn/eslint-plugin-css", + "devOnly": true, + "owner": "@elastic/appex-sharedux" +} diff --git a/packages/kbn-eslint-plugin-css/package.json b/packages/kbn-eslint-plugin-css/package.json new file mode 100644 index 0000000000000..c811f06f27cb7 --- /dev/null +++ b/packages/kbn-eslint-plugin-css/package.json @@ -0,0 +1,6 @@ +{ + "name": "@kbn/eslint-plugin-css", + "version": "1.0.0", + "private": true, + "license": "Elastic License 2.0 OR AGPL-3.0-only OR SSPL-1.0" +} 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 new file mode 100644 index 0000000000000..e1f683b09814f --- /dev/null +++ b/packages/kbn-eslint-plugin-css/src/rules/no_css_color.test.ts @@ -0,0 +1,249 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { RuleTester } from 'eslint'; +import { NoCssColor } from './no_css_color'; + +const tsTester = [ + '@typescript-eslint/parser', + new RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), + parserOptions: { + sourceType: 'module', + ecmaVersion: 2018, + ecmaFeatures: { + jsx: true, + }, + }, + }), +] as const; + +const babelTester = [ + '@babel/eslint-parser', + new RuleTester({ + parser: require.resolve('@babel/eslint-parser'), + parserOptions: { + sourceType: 'module', + ecmaVersion: 2018, + requireConfigFile: false, + babelOptions: { + presets: ['@kbn/babel-preset/node_preset'], + }, + }, + }), +] as const; + +const invalid: RuleTester.InvalidTestCase[] = [ + { + name: 'Raises an error when a CSS color is used in a JSX style attribute', + filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx', + code: ` + import React from 'react'; + + function TestComponent() { + return ( + This is a test + ) + }`, + errors: [{ messageId: 'noCssColorSpecific' }], + }, + { + name: 'Raises an error when a CSS color references a string variable that is passed to style prop of a JSX element', + filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx', + code: ` + import React from 'react'; + + function TestComponent() { + const codeColor = '#dd4040'; + return ( + This is a test + ) + }`, + errors: [{ messageId: 'noCSSColorSpecificDeclaredVariable' }], + }, + { + name: 'Raises an error when a CSS color is used in an object variable that is passed to style prop of a JSX element', + filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx', + code: ` + import React from 'react'; + + function TestComponent() { + const codeStyle = { color: '#dd4040' }; + return ( + This is a test + ) + }`, + errors: [{ messageId: 'noCSSColorSpecificDeclaredVariable' }], + }, + { + name: 'Raises an error when an object property that is a literal CSS color is used for the background property in a JSX style attribute', + filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx', + code: ` + import React from 'react'; + + function TestComponent() { + const baseStyle = { background: 'rgb(255, 255, 255)' }; + + return ( + This is a test + ) + }`, + errors: [{ messageId: 'noCSSColorSpecificDeclaredVariable' }], + }, + { + name: 'Raises an error when a CSS color is used in a variable that is spread into another variable that is passed to style prop of a JSX element', + filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx', + code: ` + import React from 'react'; + + function TestComponent() { + const baseStyle = { background: 'rgb(255, 255, 255)' }; + const codeStyle = { margin: '5px', ...baseStyle }; + return ( + This is a test + ) + }`, + errors: [{ messageId: 'noCSSColorSpecificDeclaredVariable' }], + }, + { + name: 'Raises an error when a CSS color is used for the background property in a JSX style attribute', + filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx', + code: ` + import React from 'react'; + + function TestComponent() { + return ( + This is a test + ) + }`, + errors: [{ messageId: 'noCssColorSpecific' }], + }, + { + name: 'Raises an error when a CSS color for the color property is used in a JSX css attribute for EuiComponents', + filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx', + code: ` + import React from 'react'; + + function TestComponent() { + return ( + This is a test + ) + }`, + 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', + code: ` + import React from 'react'; + import { css } from '@emotion/css'; + + function TestComponent() { + return ( + This is a test + ) + }`, + 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 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() { + 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() { + return ( + This is a test + ) + }`, + 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 an arrow function', + filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx', + code: ` + import React from 'react'; + + function TestComponent() { + return ( + ({ color: '#dd4040' })}>This is a test + ) + }`, + errors: [{ messageId: 'noCssColorSpecific' }], + }, + { + name: 'Raises an error when a CSS color for the color property is used in a JSX css attribute for EuiComponents with a regular function', + filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx', + code: ` + import React from 'react'; + + function TestComponent() { + return ( + This is a test + ) + }`, + errors: [{ messageId: 'noCssColorSpecific' }], + }, + { + 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', + code: ` + import React from 'react'; + import { css } from '@emotion/css'; + + function TestComponent() { + return ( + This is a test + ) + }`, + errors: [{ messageId: 'noCssColor' }], + }, +]; + +const valid: RuleTester.ValidTestCase[] = []; + +for (const [name, tester] of [tsTester, babelTester]) { + describe(name, () => { + tester.run('@kbn/no_css_color', NoCssColor, { + valid, + invalid, + }); + }); +} 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 new file mode 100644 index 0000000000000..c453e5edfcd74 --- /dev/null +++ b/packages/kbn-eslint-plugin-css/src/rules/no_css_color.ts @@ -0,0 +1,453 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import type { Rule } from 'eslint'; +import { CSSStyleDeclaration } from 'cssstyle'; +import type { TSESTree } from '@typescript-eslint/typescript-estree'; + +/** + * @description List of superset css properties that can apply color to html box element elements and text nodes, leveraging the + * css style package allows us to directly singly check for these properties even if the actual declaration was written using the shorthand form + */ +const propertiesSupportingCssColor = ['color', 'background', 'border']; + +/** + * @description Builds off the existing color definition to match css declarations that can apply color to + * html elements and text nodes for string declarations + */ +const htmlElementColorDeclarationRegex = RegExp( + String.raw`(${propertiesSupportingCssColor.join('|')})` +); + +const checkPropertySpecifiesInvalidCSSColor = ([property, value]: string[]) => { + if (!property || !value) return false; + + const style = new CSSStyleDeclaration(); + + // @ts-ignore the types for this packages specifics an index signature of number, alongside other valid CSS properties + style[property] = value; + + const anchor = propertiesSupportingCssColor.find((resolvedProperty) => + property.includes(resolvedProperty) + ); + + if (!anchor) return false; + + // build the resolved color property to check if the value is a string after parsing the style declaration + const resolvedColorProperty = anchor === 'color' ? 'color' : anchor + 'Color'; + + // in trying to keep this rule simple, it's enough if a string is used to define a color to mark it as invalid + // @ts-ignore the types for this packages specifics an index signature of number, alongside other valid CSS properties + return typeof style[resolvedColorProperty] === 'string'; +}; + +const resolveMemberExpressionRoot = (node: TSESTree.MemberExpression): TSESTree.Identifier => { + if (node.object.type === 'MemberExpression') { + return resolveMemberExpressionRoot(node.object); + } + + return node.object as TSESTree.Identifier; +}; + +/** + * @description method to inspect values of interest found on an object + */ +const raiseReportIfPropertyHasInvalidCssColor = ( + context: Rule.RuleContext, + propertyNode: TSESTree.Property, + messageToReport: Rule.ReportDescriptor +) => { + let didReport = false; + + if ( + propertyNode.key.type === 'Identifier' && + !htmlElementColorDeclarationRegex.test(propertyNode.key.name) + ) { + return didReport; + } + + if (propertyNode.value.type === 'Literal') { + if ( + (didReport = checkPropertySpecifiesInvalidCSSColor([ + // @ts-expect-error the key name is present in this scenario + propertyNode.key.name, + propertyNode.value.value, + ])) + ) { + context.report(messageToReport); + } + } else if (propertyNode.value.type === 'Identifier') { + const identifierDeclaration = context.sourceCode + // @ts-expect-error + .getScope(propertyNode) + .variables.find( + (variable) => variable.name === (propertyNode.value as TSESTree.Identifier).name! + ); + + if ( + identifierDeclaration?.defs[0].node.init?.type === 'Literal' && + checkPropertySpecifiesInvalidCSSColor([ + // @ts-expect-error the key name is present in this scenario + propertyNode.key.name, + (identifierDeclaration.defs[0].node.init as TSESTree.Literal).value as string, + ]) + ) { + context.report({ + loc: propertyNode.value.loc, + messageId: 'noCSSColorSpecificDeclaredVariable', + data: { + // @ts-expect-error the key name is always present else this code will not execute + property: String(propertyNode.key.name), + line: String(propertyNode.value.loc.start.line), + variableName: propertyNode.value.name, + }, + }); + + didReport = true; + } + } else if (propertyNode.value.type === 'MemberExpression') { + // @ts-expect-error we ignore the case where this node could be a private identifier + const MemberExpressionLeafName = propertyNode.value.property.name; + const memberExpressionRootName = resolveMemberExpressionRoot(propertyNode.value).name; + + const expressionRootDeclaration = context.sourceCode + // @ts-expect-error + .getScope(propertyNode) + .variables.find((variable) => variable.name === memberExpressionRootName); + + const expressionRootDeclarationInit = expressionRootDeclaration?.defs[0].node.init; + + if (expressionRootDeclarationInit?.type === 'ObjectExpression') { + (expressionRootDeclarationInit as TSESTree.ObjectExpression).properties.forEach( + (property) => { + // This is a naive approach expecting the value to be at depth 1, we should actually be traversing the object to the same depth as the expression + if ( + property.type === 'Property' && + property.key.type === 'Identifier' && + property.key?.name === MemberExpressionLeafName + ) { + raiseReportIfPropertyHasInvalidCssColor(context, property, { + loc: propertyNode.value.loc, + messageId: 'noCSSColorSpecificDeclaredVariable', + data: { + // @ts-expect-error the key name is always present else this code will not execute + property: String(propertyNode.key.name), + line: String(propertyNode.value.loc.start.line), + variableName: memberExpressionRootName, + }, + }); + } + } + ); + } else if (expressionRootDeclarationInit?.type === 'CallExpression') { + // TODO: if this object was returned from invoking a function the best we can do is probably validate that the method invoked is one that returns an euitheme object + } + } + + return didReport; +}; + +/** + * + * @description style object declaration have a depth of 1, this function handles the properties of the object + */ +const handleObjectProperties = ( + context: Rule.RuleContext, + propertyParentNode: TSESTree.JSXAttribute, + property: TSESTree.ObjectLiteralElement, + reportMessage: Rule.ReportDescriptor +) => { + if (property.type === 'Property') { + raiseReportIfPropertyHasInvalidCssColor(context, property, reportMessage); + } else if (property.type === 'SpreadElement') { + const spreadElementIdentifierName = (property.argument as TSESTree.Identifier).name; + + const spreadElementDeclaration = context.sourceCode + // @ts-expect-error + .getScope(propertyParentNode!.value.expression!) + .references.find((ref) => ref.identifier.name === spreadElementIdentifierName)?.resolved; + + if (!spreadElementDeclaration) { + return; + } + + reportMessage = { + loc: propertyParentNode.loc, + messageId: 'noCSSColorSpecificDeclaredVariable', + data: { + // @ts-expect-error the key name is always present else this code will not execute + property: String(property.argument.name), + variableName: spreadElementIdentifierName, + line: String(property.loc.start.line), + }, + }; + + const spreadElementDeclarationNode = spreadElementDeclaration.defs[0].node.init; + + // evaluate only statically defined declarations, other possibilities like callExpressions in this context complicate things + if (spreadElementDeclarationNode?.type === 'ObjectExpression') { + (spreadElementDeclarationNode as TSESTree.ObjectExpression).properties.forEach( + (spreadProperty) => { + handleObjectProperties(context, propertyParentNode, spreadProperty, reportMessage); + } + ); + } + } +}; + +export const NoCssColor: Rule.RuleModule = { + meta: { + type: 'suggestion', + docs: { + description: 'Use color definitions from eui theme as opposed to CSS color values', + category: 'Best Practices', + recommended: true, + url: 'https://eui.elastic.co/#/theming/colors/values', + }, + messages: { + noCSSColorSpecificDeclaredVariable: + 'Avoid using a literal CSS color value for "{{property}}", use an EUI theme color instead in declared variable {{variableName}} on line {{line}}', + noCssColorSpecific: + 'Avoid using a literal CSS color value for "{{property}}", use an EUI theme color instead', + noCssColor: 'Avoid using a literal CSS color value, use an EUI theme color instead', + }, + schema: [], + }, + create(context) { + return { + // accounts for instances where declarations are created using the template tagged css function + TaggedTemplateExpression(node) { + if ( + node.tag.type !== 'Identifier' || + (node.tag.type === 'Identifier' && node.tag.name !== 'css') + ) { + return; + } + + for (let i = 0; i < node.quasi.quasis.length; i++) { + const declarationTemplateNode = node.quasi.quasis[i]; + + if (htmlElementColorDeclarationRegex.test(declarationTemplateNode.value.raw)) { + const cssText = declarationTemplateNode.value.raw.replace(/(\{|\}|\\n)/g, '').trim(); + + cssText.split(';').forEach((declaration) => { + if ( + declaration.length > 0 && + checkPropertySpecifiesInvalidCSSColor(declaration.split(':')) + ) { + context.report({ + node: declarationTemplateNode, + messageId: 'noCssColor', + }); + } + }); + } + } + }, + JSXAttribute(node: TSESTree.JSXAttribute) { + if (!(node.name.name === 'style' || node.name.name === 'css')) { + return; + } + + /** + * @description Accounts for instances where a variable is used to define a style object + * + * @example + * const codeStyle = { color: '#dd4040' }; + * This is an example + * + * @example + * const codeStyle = { color: '#dd4040' }; + * This is an example + * + * @example + * const codeStyle = css({ color: '#dd4040' }); + * This is an example + */ + if ( + node.value?.type === 'JSXExpressionContainer' && + node.value.expression.type === 'Identifier' + ) { + const styleVariableName = node.value.expression.name; + + const nodeScope = context.sourceCode.getScope(node.value.expression); + + const variableDeclarationMatches = nodeScope.references.find( + (ref) => ref.identifier.name === styleVariableName + )?.resolved; + + let variableInitializationNode; + + if ((variableInitializationNode = variableDeclarationMatches?.defs?.[0]?.node?.init)) { + if (variableInitializationNode.type === 'ObjectExpression') { + // @ts-ignore + variableInitializationNode.properties.forEach((property) => { + handleObjectProperties(context, node, property, { + loc: property.loc, + messageId: 'noCSSColorSpecificDeclaredVariable', + data: { + property: + property.type === 'SpreadElement' + ? String(property.argument.name) + : String(property.key.name), + variableName: styleVariableName, + line: String(property.loc.start.line), + }, + }); + }); + } 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; + } + + /** + * + * @description Accounts for instances where a style object is inlined in the JSX attribute + * + * @example + * This is an example + * + * @example + * This is an example + * + * @example + * const styleRules = { color: '#dd4040' }; + * This is an example + * + * @example + * const styleRules = { color: '#dd4040' }; + * This is an example + */ + if ( + node.value?.type === 'JSXExpressionContainer' && + node.value.expression.type === 'ObjectExpression' + ) { + const declarationPropertiesNode = node.value.expression.properties; + + declarationPropertiesNode?.forEach((property) => { + handleObjectProperties(context, node, property, { + loc: property.loc, + messageId: 'noCssColorSpecific', + data: { + property: + property.type === 'SpreadElement' + ? // @ts-expect-error the key name is always present else this code will not execute + String(property.argument.name) + : // @ts-expect-error the key name is always present else this code will not execute + String(property.key.name), + }, + }); + }); + + return; + } + + if (node.name.name === 'css' && node.value?.type === 'JSXExpressionContainer') { + /** + * @example + * This is an example + */ + if (node.value.expression.type === 'TemplateLiteral') { + for (let i = 0; i < node.value.expression.quasis.length; i++) { + const declarationTemplateNode = node.value.expression.quasis[i]; + + if (htmlElementColorDeclarationRegex.test(declarationTemplateNode.value.raw)) { + const cssText = declarationTemplateNode.value.raw + .replace(/(\{|\}|\\n)/g, '') + .trim(); + + cssText.split(';').forEach((declaration) => { + if ( + declaration.length > 0 && + checkPropertySpecifiesInvalidCSSColor(declaration.split(':')) + ) { + context.report({ + node: declarationTemplateNode, + messageId: 'noCssColor', + }); + } + }); + } + } + } + + /** + * @example + * ({ color: '#dd4040' })}>This is an example + */ + if ( + node.value.expression.type === 'FunctionExpression' || + node.value.expression.type === 'ArrowFunctionExpression' + ) { + let declarationPropertiesNode: TSESTree.Property[] = []; + + if (node.value.expression.body.type === 'ObjectExpression') { + // @ts-expect-error + declarationPropertiesNode = node.value.expression.body.properties; + } + + if (node.value.expression.body.type === 'BlockStatement') { + const functionReturnStatementNode = node.value.expression.body.body?.find((_node) => { + return _node.type === 'ReturnStatement'; + }); + + if (!functionReturnStatementNode) { + return; + } + + declarationPropertiesNode = // @ts-expect-error + (functionReturnStatementNode as TSESTree.ReturnStatement).argument?.properties; + } + + if (!declarationPropertiesNode.length) { + return; + } + + declarationPropertiesNode.forEach((property) => { + handleObjectProperties(context, node, property, { + loc: property.loc, + messageId: 'noCssColorSpecific', + data: { + // @ts-expect-error the key name is always present else this code will not execute + property: property.key.name, + }, + }); + }); + + return; + } + } + }, + }; + }, +}; diff --git a/packages/kbn-eslint-plugin-css/src/rules/prefer_css_attribute_for_eui_components.test.ts b/packages/kbn-eslint-plugin-css/src/rules/prefer_css_attribute_for_eui_components.test.ts new file mode 100644 index 0000000000000..f1534ec5c861f --- /dev/null +++ b/packages/kbn-eslint-plugin-css/src/rules/prefer_css_attribute_for_eui_components.test.ts @@ -0,0 +1,85 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { RuleTester } from 'eslint'; +import { PreferCSSAttributeForEuiComponents } from './prefer_css_attribute_for_eui_components'; + +const tsTester = [ + '@typescript-eslint/parser', + new RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), + parserOptions: { + sourceType: 'module', + ecmaVersion: 2018, + ecmaFeatures: { + jsx: true, + }, + }, + }), +] as const; + +const babelTester = [ + '@babel/eslint-parser', + new RuleTester({ + parser: require.resolve('@babel/eslint-parser'), + parserOptions: { + sourceType: 'module', + ecmaVersion: 2018, + requireConfigFile: false, + babelOptions: { + presets: ['@kbn/babel-preset/node_preset'], + }, + }, + }), +] as const; + +const invalid: RuleTester.InvalidTestCase[] = [ + { + name: 'Prefer the JSX css attribute for EUI components', + filename: '/x-pack/plugins/observability_solution/observability/public/test_component.tsx', + code: ` + import React from 'react'; + + function TestComponent() { + return ( + This is a test + ) + }`, + errors: [ + { + messageId: 'preferCSSAttributeForEuiComponents', + }, + ], + output: ` + import React from 'react'; + + function TestComponent() { + return ( + This is a test + ) + }`, + }, +]; + +const valid: RuleTester.ValidTestCase[] = [ + { + name: invalid[0].name, + filename: invalid[0].filename, + code: invalid[0].output as string, + }, +]; + +for (const [name, tester] of [tsTester, babelTester]) { + describe(name, () => { + tester.run('@kbn/prefer_css_attribute_for_eui_components', PreferCSSAttributeForEuiComponents, { + valid, + invalid, + }); + }); +} diff --git a/packages/kbn-eslint-plugin-css/src/rules/prefer_css_attribute_for_eui_components.ts b/packages/kbn-eslint-plugin-css/src/rules/prefer_css_attribute_for_eui_components.ts new file mode 100644 index 0000000000000..f2b8bfd7b7d74 --- /dev/null +++ b/packages/kbn-eslint-plugin-css/src/rules/prefer_css_attribute_for_eui_components.ts @@ -0,0 +1,62 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import type { Rule } from 'eslint'; +import type { TSESTree } from '@typescript-eslint/typescript-estree'; +import type { Identifier, Node } from 'estree'; + +export const PreferCSSAttributeForEuiComponents: Rule.RuleModule = { + meta: { + type: 'suggestion', + docs: { + description: 'Prefer the JSX css attribute for EUI components', + category: 'Best Practices', + recommended: true, + }, + messages: { + preferCSSAttributeForEuiComponents: 'Prefer the css attribute for EUI components', + }, + fixable: 'code', + schema: [], + }, + create(context) { + const isNamedEuiComponentRegex = /^Eui[A-Z]*/; + + return { + JSXOpeningElement(node: TSESTree.JSXOpeningElement) { + if (isNamedEuiComponentRegex.test((node.name as unknown as Identifier).name)) { + let styleAttrNode: TSESTree.JSXAttribute | undefined; + + if ( + // @ts-expect-error the returned result is somehow typed as a union of JSXAttribute and JSXAttributeSpread + (styleAttrNode = node.attributes.find( + (attr) => attr.type === 'JSXAttribute' && attr.name.name === 'style' + )) + ) { + context.report({ + node: styleAttrNode?.parent! as Node, + messageId: 'preferCSSAttributeForEuiComponents', + fix(fixer) { + const cssAttr = node.attributes.find( + (attr) => attr.type === 'JSXAttribute' && attr.name.name === 'css' + ); + + if (cssAttr) { + return null; + } + + return fixer.replaceTextRange(styleAttrNode?.name?.range!, 'css'); + }, + }); + } + } + }, + }; + }, +}; diff --git a/packages/kbn-eslint-plugin-css/tsconfig.json b/packages/kbn-eslint-plugin-css/tsconfig.json new file mode 100644 index 0000000000000..a6dec1c1a62c5 --- /dev/null +++ b/packages/kbn-eslint-plugin-css/tsconfig.json @@ -0,0 +1,11 @@ +{ + "extends": "../../tsconfig.base.json", + "compilerOptions": { + "outDir": "target/types", + "types": ["jest", "node"], + "lib": ["es2021"] + }, + "include": ["**/*.ts"], + "exclude": ["target/**/*"], + "kbn_references": [] +} diff --git a/tsconfig.base.json b/tsconfig.base.json index de3155031aadf..67eebb9aaac4e 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -850,6 +850,8 @@ "@kbn/es-ui-shared-plugin/*": ["src/platform/plugins/shared/es_ui_shared/*"], "@kbn/eslint-config": ["packages/kbn-eslint-config"], "@kbn/eslint-config/*": ["packages/kbn-eslint-config/*"], + "@kbn/eslint-plugin-css": ["packages/kbn-eslint-plugin-css"], + "@kbn/eslint-plugin-css/*": ["packages/kbn-eslint-plugin-css/*"], "@kbn/eslint-plugin-disable": ["packages/kbn-eslint-plugin-disable"], "@kbn/eslint-plugin-disable/*": ["packages/kbn-eslint-plugin-disable/*"], "@kbn/eslint-plugin-eslint": ["packages/kbn-eslint-plugin-eslint"], @@ -2080,9 +2082,7 @@ "@kbn/zod-helpers/*": ["packages/kbn-zod-helpers/*"], // END AUTOMATED PACKAGE LISTING // Allows for importing from `kibana` package for the exported types. - "@emotion/core": [ - "typings/@emotion" - ] + "@emotion/core": ["typings/@emotion"] }, // Support .tsx files and transform JSX into calls to React.createElement "jsx": "react", diff --git a/yarn.lock b/yarn.lock index 94b56bedc8c57..c2c2aa526ea31 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5517,6 +5517,10 @@ version "0.0.0" uid "" +"@kbn/eslint-plugin-css@link:packages/kbn-eslint-plugin-css": + version "0.0.0" + uid "" + "@kbn/eslint-plugin-disable@link:packages/kbn-eslint-plugin-disable": version "0.0.0" uid "" @@ -11345,6 +11349,11 @@ resolved "https://registry.yarnpkg.com/@types/cookiejar/-/cookiejar-2.1.5.tgz#14a3e83fa641beb169a2dd8422d91c3c345a9a78" integrity sha512-he+DHOWReW0nghN24E1WUqM0efK4kI9oTqDm6XmK8ZPe2djZ90BSNdGnIyCLzCPw7/pogPlGbzI2wHGGmi4O/Q== +"@types/cssstyle@^2.2.4": + version "2.2.4" + resolved "https://registry.yarnpkg.com/@types/cssstyle/-/cssstyle-2.2.4.tgz#3d333ab9f8e6c40183ad1d6ebeebfcb8da2bfe4b" + integrity sha512-FTGMeuHZtLB7hRm+NGvOLZElslR1UkKvZmEmFevOZe/e7Av0nFleka1s8ZwoX+QvbJ2y7r9NDZXIzyqpRWDJXQ== + "@types/cytoscape@^3.14.0": version "3.14.0" resolved "https://registry.yarnpkg.com/@types/cytoscape/-/cytoscape-3.14.0.tgz#346b5430a7a1533784bcf44fcbe6c5255b948d36" @@ -16338,6 +16347,13 @@ cssstyle@^2.3.0: dependencies: cssom "~0.3.6" +cssstyle@^4.1.0: + version "4.1.0" + resolved "https://registry.yarnpkg.com/cssstyle/-/cssstyle-4.1.0.tgz#161faee382af1bafadb6d3867a92a19bcb4aea70" + integrity sha512-h66W1URKpBS5YMI/V8PyXvTMFT8SupJ1IzoIV8IeBC/ji8WVmrO8dGlTi+2dh6whmdk6BiKJLD/ZBkhWbcg6nA== + dependencies: + rrweb-cssom "^0.7.1" + csstype@3.1.2: version "3.1.2" resolved "https://registry.yarnpkg.com/csstype/-/csstype-3.1.2.tgz#1d4bf9d572f11c14031f0436e1c10bc1f571f50b" @@ -28894,6 +28910,11 @@ robust-predicates@^3.0.0: resolved "https://registry.yarnpkg.com/robust-predicates/-/robust-predicates-3.0.1.tgz#ecde075044f7f30118682bd9fb3f123109577f9a" integrity sha512-ndEIpszUHiG4HtDsQLeIuMvRsDnn8c8rYStabochtUeCvfuvNptb5TUbVD68LRAILPX7p9nqQGh4xJgn3EHS/g== +rrweb-cssom@^0.7.1: + version "0.7.1" + resolved "https://registry.yarnpkg.com/rrweb-cssom/-/rrweb-cssom-0.7.1.tgz#c73451a484b86dd7cfb1e0b2898df4b703183e4b" + integrity sha512-TrEMa7JGdVm0UThDJSx7ddw5nVm3UJS9o9CCIZ72B1vSyEZoziDqBYP3XIoi/12lKrJR8rE3jeFHMok2F/Mnsg== + rst-selector-parser@^2.2.3: version "2.2.3" resolved "https://registry.yarnpkg.com/rst-selector-parser/-/rst-selector-parser-2.2.3.tgz#81b230ea2fcc6066c89e3472de794285d9b03d91"