Skip to content

Commit

Permalink
improve implementation to prevent false positives
Browse files Browse the repository at this point in the history
  • Loading branch information
eokoneyo committed Dec 10, 2024
1 parent ffdf8e7 commit fb36928
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 26 deletions.
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1561,6 +1561,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",
Expand Down Expand Up @@ -1711,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.15.2",
"cypress-axe": "^1.5.0",
Expand Down
103 changes: 77 additions & 26 deletions packages/kbn-eslint-plugin-css/src/rules/no_css_color.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,41 @@
*/

import type { Rule } from 'eslint';
import { CSSStyleDeclaration } from 'cssstyle';
import type { TSESTree } from '@typescript-eslint/typescript-estree';

/**
* @description List of css properties that can that can apply color to html box element and text node
* @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', 'backgroundColor', 'borderColor'];
const propertiesSupportingCssColor = ['color', 'background', 'border'];

/**
* @description Builds off the existing color definition regex to match css declarations that can apply color to
* @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('|')}):\s?.*;`
String.raw`(${propertiesSupportingCssColor.join('|')})`
);

const checkPropertySpecifiesInvalidCSSColor = (property: string, value: string) => {
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)
);

// 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, its' 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 Boolean(resolvedColorProperty && typeof style[resolvedColorProperty] === 'string');
};

const resolveMemberExpressionRoot = (node: TSESTree.MemberExpression): TSESTree.Identifier => {
if (node.object.type === 'MemberExpression') {
return resolveMemberExpressionRoot(node.object);
Expand All @@ -36,18 +56,28 @@ const raiseReportIfPropertyHasInvalidCssColor = (
propertyNode: TSESTree.Property,
messageToReport: Rule.ReportDescriptor
) => {
let didReport: boolean;
let didReport = false;

if (
propertyNode.key.type === 'Identifier' &&
propertiesSupportingCssColor.indexOf(propertyNode.key.name) < 0
!htmlElementColorDeclarationRegex.test(propertyNode.key.name)
) {
return;
}

if ((didReport = Boolean(propertyNode.value.type === 'Literal'))) {
// in trying to keep this rule simple, if a string is used to define a color we simply mark it as invalid
context.report(messageToReport);
if (propertyNode.value.type === 'Literal') {
if (
(didReport = checkPropertySpecifiesInvalidCSSColor(
// @ts-expect-error the key name is present in this scenario
propertyNode.key.name,
// @ts-expect-error we already ascertained that the value here is a literal
propertyNode.value.value
))
) {
context.report(messageToReport);
}

return;
} else if (propertyNode.value.type === 'Identifier') {
const identifierDeclaration = context.sourceCode
// @ts-expect-error
Expand All @@ -56,7 +86,14 @@ const raiseReportIfPropertyHasInvalidCssColor = (
(variable) => variable.name === (propertyNode.value as TSESTree.Identifier).name!
);

if (identifierDeclaration?.defs[0].node.init.type === 'Literal') {
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',
Expand Down Expand Up @@ -179,21 +216,30 @@ export const NoCssColor: Rule.RuleModule = {
return {
// accounts for instances where declarations are created using the template tagged css function
TaggedTemplateExpression(node) {
if (node.tag.type === 'Identifier' && node.tag.name !== 'css') {
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)) {
context.report({
node: declarationTemplateNode,
messageId: 'noCssColor',
});
const cssText = declarationTemplateNode.value.raw.replace(/(\{|\}|\\n)/g, '').trim();

break;
}
cssText.split(';').forEach((declaration) => {
if (declaration.length > 0) {
const [prop, value] = declaration.split(':');

if (checkPropertySpecifiesInvalidCSSColor(prop.trim(), value.trim())) {
context.report({
node: declarationTemplateNode,
messageId: 'noCssColor',
});
}
}
});
}
},
JSXAttribute(node: TSESTree.JSXAttribute) {
Expand Down Expand Up @@ -323,14 +369,19 @@ export const NoCssColor: Rule.RuleModule = {
for (let i = 0; i < node.value.expression.quasis.length; i++) {
const declarationTemplateNode = node.value.expression.quasis[i];

if (htmlElementColorDeclarationRegex.test(declarationTemplateNode.value.raw)) {
context.report({
node: declarationTemplateNode,
messageId: 'noCssColor',
});

break;
}
const cssText = declarationTemplateNode.value.raw.replace(/(\{|\}|\\n)/g, '').trim();

cssText.split(';').forEach((declaration) => {
if (declaration.length > 0) {
const [prop, value] = declaration.split(':');
if (checkPropertySpecifiesInvalidCSSColor(prop.trim(), value.trim())) {
context.report({
node: declarationTemplateNode,
messageId: 'noCssColor',
});
}
}
});
}
}

Expand Down
17 changes: 17 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -11296,6 +11296,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"
Expand Down Expand Up @@ -16238,6 +16243,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"

[email protected]:
version "3.1.2"
resolved "https://registry.yarnpkg.com/csstype/-/csstype-3.1.2.tgz#1d4bf9d572f11c14031f0436e1c10bc1f571f50b"
Expand Down Expand Up @@ -28803,6 +28815,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"
Expand Down

0 comments on commit fb36928

Please sign in to comment.