From 2fc16f97ee0f9aa142f143345f14149e33fe6d07 Mon Sep 17 00:00:00 2001 From: Matt Provost Date: Mon, 26 Jun 2023 16:47:58 +0000 Subject: [PATCH 1/3] Add typing to Stylelint rules Signed-off-by: Matt Provost --- .../src/rules/no_custom_colors/index.ts | 12 ++++++++---- .../src/rules/no_modifying_global_selectors/index.ts | 9 +++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/osd-stylelint-plugin-stylelint/src/rules/no_custom_colors/index.ts b/packages/osd-stylelint-plugin-stylelint/src/rules/no_custom_colors/index.ts index b8061f7ff9bd..cf7e041505a3 100644 --- a/packages/osd-stylelint-plugin-stylelint/src/rules/no_custom_colors/index.ts +++ b/packages/osd-stylelint-plugin-stylelint/src/rules/no_custom_colors/index.ts @@ -10,6 +10,7 @@ */ import stylelint from 'stylelint'; +import { Rule } from 'postcss'; import { NAMESPACE } from '../..'; import { ComplianceEngine, @@ -31,12 +32,12 @@ const messages = ruleMessages(ruleName, { expected: (message) => `${message}`, }); -const ruleFunction = ( +const ruleFunction: stylelint.Rule = ( primaryOption: Record, secondaryOptionObject: Record, context ) => { - return (postcssRoot: any, postcssResult: any) => { + return (postcssRoot, postcssResult) => { const validOptions = isValidOptions(postcssResult, ruleName, primaryOption); if (!validOptions) { return; @@ -46,15 +47,18 @@ const ruleFunction = ( const isAutoFixing = Boolean(context.fix); - postcssRoot.walkDecls((decl: any) => { + postcssRoot.walkDecls((decl) => { if (!isColorProperty(decl.prop)) { return; } + // We know this is a rule, because we discriminate the property in the conditional above. This means we only have 1 choice on its type. + const parent = decl.parent as Rule; + let shouldReport = false; const nodeInfo = { - selector: decl.parent.selector, + selector: parent.selector, prop: decl.prop, value: decl.value, }; diff --git a/packages/osd-stylelint-plugin-stylelint/src/rules/no_modifying_global_selectors/index.ts b/packages/osd-stylelint-plugin-stylelint/src/rules/no_modifying_global_selectors/index.ts index 2c6ee035aec1..a019c46dc144 100644 --- a/packages/osd-stylelint-plugin-stylelint/src/rules/no_modifying_global_selectors/index.ts +++ b/packages/osd-stylelint-plugin-stylelint/src/rules/no_modifying_global_selectors/index.ts @@ -25,12 +25,12 @@ const messages = ruleMessages(ruleName, { expected: (message) => `${message}`, }); -const ruleFunction = ( +const ruleFunction: stylelint.Rule = ( primaryOption: Record, secondaryOptionObject: Record, context ) => { - return (postcssRoot: any, postcssResult: any) => { + return (postcssRoot, postcssResult) => { const validOptions = isValidOptions(postcssResult, ruleName, primaryOption); if (!validOptions) { return; @@ -40,7 +40,7 @@ const ruleFunction = ( const isAutoFixing = Boolean(context.fix); - postcssRoot.walkRules((rule: any) => { + postcssRoot.walkRules((rule) => { const selectorRule = getSelectorRule(rules, rule); if (!selectorRule) { return; @@ -48,7 +48,8 @@ const ruleFunction = ( let shouldReport = false; - const file = postcssRoot.source.input.file; + // We can safely unwrap these possibly undefined variables because we get these values from Stylelint, which grabs them from source files. + const file = postcssRoot.source!.input.file!; const approvedFiles = selectorRule.approved; const reportInfo = { From b9460c4205e1bfbbc4e736e7119a7c8465ebfda0 Mon Sep 17 00:00:00 2001 From: Matt Provost Date: Mon, 26 Jun 2023 19:45:19 +0000 Subject: [PATCH 2/3] Extract get color property parent into function Signed-off-by: Matt Provost --- packages/osd-stylelint-plugin-stylelint/package.json | 1 + .../src/rules/no_custom_colors/index.ts | 9 +++------ .../src/utils/is_color_property.ts | 10 ++++++++++ 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/osd-stylelint-plugin-stylelint/package.json b/packages/osd-stylelint-plugin-stylelint/package.json index 2d2a6e661d5d..49947331fb59 100644 --- a/packages/osd-stylelint-plugin-stylelint/package.json +++ b/packages/osd-stylelint-plugin-stylelint/package.json @@ -13,6 +13,7 @@ "devOnly": true }, "peerDependencies": { + "postcss": "^8.4.12", "stylelint": "^14.5.2" }, "devDependencies": { diff --git a/packages/osd-stylelint-plugin-stylelint/src/rules/no_custom_colors/index.ts b/packages/osd-stylelint-plugin-stylelint/src/rules/no_custom_colors/index.ts index cf7e041505a3..7aac69986216 100644 --- a/packages/osd-stylelint-plugin-stylelint/src/rules/no_custom_colors/index.ts +++ b/packages/osd-stylelint-plugin-stylelint/src/rules/no_custom_colors/index.ts @@ -10,7 +10,6 @@ */ import stylelint from 'stylelint'; -import { Rule } from 'postcss'; import { NAMESPACE } from '../..'; import { ComplianceEngine, @@ -18,7 +17,7 @@ import { getUntrackedMessage, getNotCompliantMessage, getRulesFromConfig, - isColorProperty, + getColorPropertyParent, isValidOptions, } from '../../utils'; @@ -48,13 +47,11 @@ const ruleFunction: stylelint.Rule = ( const isAutoFixing = Boolean(context.fix); postcssRoot.walkDecls((decl) => { - if (!isColorProperty(decl.prop)) { + const parent = getColorPropertyParent(decl); + if (!parent) { return; } - // We know this is a rule, because we discriminate the property in the conditional above. This means we only have 1 choice on its type. - const parent = decl.parent as Rule; - let shouldReport = false; const nodeInfo = { diff --git a/packages/osd-stylelint-plugin-stylelint/src/utils/is_color_property.ts b/packages/osd-stylelint-plugin-stylelint/src/utils/is_color_property.ts index d52cb43cfc32..ef7ad9118f81 100644 --- a/packages/osd-stylelint-plugin-stylelint/src/utils/is_color_property.ts +++ b/packages/osd-stylelint-plugin-stylelint/src/utils/is_color_property.ts @@ -9,6 +9,8 @@ * GitHub history for details. */ +import { Rule, Declaration } from 'postcss'; + const COLOR_PROPERTIES = [ 'all', 'animation', @@ -58,3 +60,11 @@ const COLOR_PROPERTIES = [ export const isColorProperty = (prop: string) => { return COLOR_PROPERTIES.includes(prop); }; + +export const getColorPropertyParent = (decl: Declaration) => { + if (!isColorProperty(decl.prop)) { + return undefined; + } + + return decl.parent as Rule; +}; From 0c7b99b547157cc26ab566b2b4d6d6616a54b469 Mon Sep 17 00:00:00 2001 From: Matt Provost Date: Mon, 26 Jun 2023 19:48:26 +0000 Subject: [PATCH 3/3] Optchain instead of unwrapping source file Signed-off-by: Matt Provost --- .../src/rules/no_modifying_global_selectors/index.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/osd-stylelint-plugin-stylelint/src/rules/no_modifying_global_selectors/index.ts b/packages/osd-stylelint-plugin-stylelint/src/rules/no_modifying_global_selectors/index.ts index a019c46dc144..de1a5131da87 100644 --- a/packages/osd-stylelint-plugin-stylelint/src/rules/no_modifying_global_selectors/index.ts +++ b/packages/osd-stylelint-plugin-stylelint/src/rules/no_modifying_global_selectors/index.ts @@ -48,8 +48,11 @@ const ruleFunction: stylelint.Rule = ( let shouldReport = false; - // We can safely unwrap these possibly undefined variables because we get these values from Stylelint, which grabs them from source files. - const file = postcssRoot.source!.input.file!; + const file = postcssRoot.source?.input.file; + if (!file) { + return; + } + const approvedFiles = selectorRule.approved; const reportInfo = {