[Stylelint] Add no_restricted_values linter rule (#4413)

* Add no_restricted_values linter rule Signed-off-by: Matt Provost <[email protected]> * Update changelog Signed-off-by: Matt Provost <[email protected]> * Add explanation for file based configs Signed-off-by: Matt Provost <[email protected]> * Update error messages Signed-off-by: Matt Provost <[email protected]> --------- Signed-off-by: Matt Provost <[email protected]> Signed-off-by: Matt Provost <[email protected]> Co-authored-by: Ashwin P Chandran <[email protected]> Co-authored-by: Anan Zhuang <[email protected]> Co-authored-by: Josh Romero <[email protected]>
opensearch-project · Aug 17, 2023 · 06a4903 · 06a4903
1 parent 86a3f1e
commit 06a4903
Show file tree

Hide file tree

Showing 10 changed files with 151 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -145,6 +145,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Upgrade the backport workflow ([#4343](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4343))
 - [Lint] Add custom stylelint rules and config to prevent unintended style overrides ([#4290](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4290))
 - [Lint] Add stylelint rule to define properties that are restricted from being used ([#4374](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4374))
+- [Lint] Add stylelint rule to define values that are restricted from being used ([#4413](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4413))
 - [Lint] Add typing to Stylelint rules ([#4392](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4392))
 - [CI] Split build and verify into parallel jobs ([#4467](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4467))
 

diff --git a/packages/osd-stylelint-config/.stylelintrc.js b/packages/osd-stylelint-config/.stylelintrc.js
@@ -15,26 +15,34 @@ module.exports = {
   ],
 
   rules: {
-    '@osd/stylelint/no_restricted_properties': [
+    '@osd/stylelint/no_modifying_global_selectors': [
       {
-        config: "./../../../osd-stylelint-config/config/restricted_properties.json"
+        config: "./../../../osd-stylelint-config/config/global_selectors.json"
       },
       {
         severity: "error"
       }
     ],
-    '@osd/stylelint/no_modifying_global_selectors': [
+    '@osd/stylelint/no_custom_colors': [
       {
-        config: "./../../../osd-stylelint-config/config/global_selectors.json"
+        config: './../../../osd-stylelint-config/config/colors.json'
+      },
+    ],
+    '@osd/stylelint/no_restricted_properties': [
+      {
+        config: "./../../../osd-stylelint-config/config/restricted_properties.json"
       },
       {
         severity: "error"
       }
     ],
-    '@osd/stylelint/no_custom_colors': [
+    '@osd/stylelint/no_restricted_values': [
       {
-        config: './../../../osd-stylelint-config/config/colors.json'
+        config: "./../../../osd-stylelint-config/config/restricted_values.json"
       },
-    ]
+      {
+        severity: "error"
+      }
+    ],
   }
 }
diff --git a/packages/osd-stylelint-config/config/restricted_properties.json b/packages/osd-stylelint-config/config/restricted_properties.json
@@ -1,5 +1,6 @@
 {
   "font-family": {
+    "explanation": "All \"font-family\" styles should be inherited from OUI themes and components. Remove the rule.",
     "approved": [
       "src/plugins/discover/public/application/_discover.scss",
       "src/plugins/maps_legacy/public/map/_leaflet_overrides.scss",

diff --git a/packages/osd-stylelint-config/config/restricted_values.json b/packages/osd-stylelint-config/config/restricted_values.json
@@ -0,0 +1,25 @@
+{
+  "/#[a-fA-F0-9]{3}(?:[a-fA-F0-9]{3})?/": {
+    "explanation": "All colors should be inherited from the OUI theme, not hard-coded. Either remove the custom color rule or use OUI SASS variables instead.",
+    "approved": [
+      "src/core/public/_variables.scss",
+      "src/core/public/styles/_ace_overrides.scss",
+      "packages/osd-ui-framework/src/components/tool_bar/_tool_bar_search.scss",
+      "packages/osd-ui-framework/src/components/view/_index.scss",
+      "src/core/public/chrome/ui/header/header_breadcrumbs.scss",
+      "src/plugins/vis_type_timeseries/public/application/components/vis_types/_vis_types.scss",
+      "src/plugins/vis_type_vislib/public/vislib/visualizations/point_series/_labels.scss"
+    ]
+  },
+  "/(?:rgba?|hsla?|hwb|lab|lch|oklab|oklch|color)\\([^)]*\\)/": {
+    "explanation": "All colors should be inherited from the OUI theme, not hard-coded. Either remove the custom color rule or use OUI SASS variables instead.",
+    "approved": [
+      "src/core/public/styles/_ace_overrides.scss",
+      "src/plugins/opensearch_dashboards_react/public/markdown/_markdown.scss",
+      "packages/osd-ui-framework/src/components/info_panel/_info_panel.scss",
+      "packages/osd-ui-framework/src/components/local_nav/_local_menu.scss",
+      "packages/osd-ui-framework/src/global_styling/mixins/_shadow.scss",
+      "packages/osd-ui-framework/src/global_styling/variables/_colors.scss"
+    ]
+  }
+}
diff --git a/packages/osd-stylelint-plugin-stylelint/src/rules/index.ts b/packages/osd-stylelint-plugin-stylelint/src/rules/index.ts
@@ -9,13 +9,15 @@
  * GitHub history for details.
  */
 
-import noRestrictedProperties from './no_restricted_properties';
 import noCustomColors from './no_custom_colors';
 import noModifyingGlobalSelectors from './no_modifying_global_selectors';
+import noRestrictedProperties from './no_restricted_properties';
+import noRestrictedValues from './no_restricted_values';
 
 // eslint-disable-next-line import/no-default-export
 export default {
   no_custom_colors: noCustomColors,
   no_modifying_global_selectors: noModifyingGlobalSelectors,
   no_restricted_properties: noRestrictedProperties,
+  no_restricted_values: noRestrictedValues,
 };
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
@@ -79,7 +79,10 @@ const ruleFunction: stylelint.Rule = (
       }
 
       reportInfo.message = messages.expected(
-        getNotCompliantMessage(`Modifying global selector "${rule.selector}" not allowed.`)
+        getNotCompliantMessage(
+          `Modifying the global selector "${rule.selector}" is not allowed.`,
+          selectorRule.explanation
+        )
       );
       report(reportInfo);
     });

diff --git a/packages/osd-stylelint-plugin-stylelint/src/rules/no_restricted_properties/index.ts b/packages/osd-stylelint-plugin-stylelint/src/rules/no_restricted_properties/index.ts
@@ -73,7 +73,10 @@ const ruleFunction: stylelint.Rule = (
       }
 
       reportInfo.message = messages.expected(
-        getNotCompliantMessage(`Usage of property "${decl.prop}" is not allowed.`)
+        getNotCompliantMessage(
+          `Specifying the "${decl.prop}" property is not allowed.`,
+          propertyRule.explanation
+        )
       );
       report(reportInfo);
     });

diff --git a/packages/osd-stylelint-plugin-stylelint/src/rules/no_restricted_values/index.ts b/packages/osd-stylelint-plugin-stylelint/src/rules/no_restricted_values/index.ts
@@ -0,0 +1,90 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import stylelint from 'stylelint';
+import { NAMESPACE } from '../..';
+import {
+  getNotCompliantMessage,
+  getRuleFromConfig,
+  getRulesFromConfig,
+  isValidOptions,
+  FileBasedConfig,
+} from '../../utils';
+
+const { ruleMessages, report } = stylelint.utils;
+
+const ruleName = 'no_restricted_values';
+const messages = ruleMessages(ruleName, {
+  expected: (message) => `${message}`,
+});
+
+const ruleFunction: stylelint.Rule = (
+  primaryOption: Record<string, any>,
+  secondaryOptionObject: Record<string, any>,
+  context
+) => {
+  return (postcssRoot, postcssResult) => {
+    const validOptions = isValidOptions(postcssResult, ruleName, primaryOption);
+    if (!validOptions) {
+      return;
+    }
+
+    const rules: FileBasedConfig = getRulesFromConfig(primaryOption.config);
+
+    const isAutoFixing = Boolean(context.fix);
+
+    postcssRoot.walkDecls((decl) => {
+      const valueRule = getRuleFromConfig(rules, decl.value);
+      if (!valueRule) {
+        return;
+      }
+
+      let shouldReport = false;
+
+      const file = postcssRoot.source?.input.file;
+      if (!file) {
+        return;
+      }
+
+      const approvedFiles = valueRule.approved;
+
+      const reportInfo = {
+        ruleName: `${NAMESPACE}/${ruleName}`,
+        result: postcssResult,
+        node: decl,
+        message: '',
+      };
+
+      if (approvedFiles) {
+        shouldReport = !approvedFiles.some((inspectedFile) => {
+          return file.includes(inspectedFile);
+        });
+      }
+
+      if (shouldReport && isAutoFixing) {
+        decl.remove();
+        return;
+      }
+
+      if (!shouldReport) {
+        return;
+      }
+
+      reportInfo.message = messages.expected(
+        getNotCompliantMessage(
+          `Using the value "${decl.value}" is not allowed.`,
+          valueRule.explanation
+        )
+      );
+      report(reportInfo);
+    });
+  };
+};
+
+ruleFunction.ruleName = ruleName;
+ruleFunction.messages = messages;
+
+// eslint-disable-next-line import/no-default-export
+export default ruleFunction;
diff --git a/packages/osd-stylelint-plugin-stylelint/src/utils/get_message.ts b/packages/osd-stylelint-plugin-stylelint/src/utils/get_message.ts
@@ -15,4 +15,10 @@ export const getUntrackedMessage = (nodeInfo: { selector: string; prop: string;
 export const getTrackedMessage = (nodeInfo: { selector: string; prop: string; value: string }) =>
   `Tracked but missing approval: "${nodeInfo.selector}.${nodeInfo.prop}: ${nodeInfo.value}"`;
 
-export const getNotCompliantMessage = (message: string) => `${message}`;
+export const getNotCompliantMessage = (message: string, explanation?: string) => {
+  if (explanation) {
+    return `${message} ${explanation}`;
+  }
+
+  return message;
+};
diff --git a/packages/osd-stylelint-plugin-stylelint/src/utils/get_rules_from_config.ts b/packages/osd-stylelint-plugin-stylelint/src/utils/get_rules_from_config.ts
@@ -13,7 +13,7 @@ import path from 'path';
 import { readFileSync } from 'fs';
 import { matches } from './matches';
 
-export type FileBasedConfig = Record<string, { approved?: string[] }>;
+export type FileBasedConfig = Record<string, { approved?: string[]; explanation?: string }>;
 export type ValueBasedConfig = Record<
   string,
   Record<string, Array<{ approved?: string; rejected?: string[] }>>