diff --git a/.eslintrc.js b/.eslintrc.js index fafce927f828..9f8569cad19e 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 023857932186..853b3de2d26c 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -426,6 +426,7 @@ packages/kbn-es-query @elastic/kibana-data-discovery packages/kbn-es-types @elastic/kibana-core @elastic/obs-knowledge-team src/platform/plugins/shared/es_ui_shared @elastic/kibana-management 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 4b781517aee8..4c5c44259b5b 100644 --- a/package.json +++ b/package.json @@ -1447,6 +1447,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", @@ -1561,6 +1562,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", @@ -1710,6 +1712,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.6.3", "cypress-axe": "^1.5.0", diff --git a/packages/kbn-eslint-config/.eslintrc.js b/packages/kbn-eslint-config/.eslintrc.js index 14ede0f4db45..1814f3f47fec 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', ], @@ -328,6 +329,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 000000000000..1e121657bc57 --- /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 000000000000..9ea4bcc67619 --- /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 000000000000..c8ae20237eae --- /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 000000000000..3ee8bff8736f --- /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 000000000000..c811f06f27cb --- /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 000000000000..e1f683b09814 --- /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 000000000000..c453e5edfcd7 --- /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 000000000000..f1534ec5c861 --- /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 000000000000..f2b8bfd7b7d7 --- /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 000000000000..a6dec1c1a62c --- /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 d7b729f0aa34..036acb822e20 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -846,6 +846,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"], @@ -2070,9 +2072,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 0429712b792c..79c16abf9c9c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5499,6 +5499,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 "" @@ -11315,6 +11319,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" @@ -16320,6 +16329,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" @@ -28908,6 +28924,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"