[Stylelint] Add no_restricted_values linter rule (#4413) (#4763)

* Add no_restricted_values linter rule Signed-off-by: Matt Provost <provomat@amazon.com> * Update changelog Signed-off-by: Matt Provost <provomat@amazon.com> * Add explanation for file based configs Signed-off-by: Matt Provost <provomat@amazon.com> * Update error messages Signed-off-by: Matt Provost <provomat@amazon.com> --------- Signed-off-by: Matt Provost <provomat@amazon.com> Signed-off-by: Matt Provost <mattprovost6@gmail.com> Co-authored-by: Ashwin P Chandran <ashwinpc@amazon.com> Co-authored-by: Anan Zhuang <ananzh@amazon.com> Co-authored-by: Josh Romero <rmerqg@amazon.com> (cherry picked from commit 06a4903) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Josh Romero <rmerqg@amazon.com>
opensearch-project · Aug 29, 2023 · a453c2e · a453c2e
1 parent 687c523
commit a453c2e
Show file tree

Hide file tree

Showing 9 changed files with 150 additions and 12 deletions.
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[] }>>