diff --git a/CHANGELOG.md b/CHANGELOG.md index 00061357535d..0f2581cacbc4 100644 --- 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 index aa06f1a72f08..4f780d112e43 100644 --- 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 index ff79c6fed46b..b69012cb61f9 100644 --- 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 new file mode 100644 index 000000000000..2d925bd67e2d --- /dev/null +++ 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 index 6dc26fe93f26..21c06b81e9e7 100644 --- 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 index d58e7819688f..b4bf6ac7da15 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 @@ -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 index 9da587545b1d..dbe20baac0ae 100644 --- 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 new file mode 100644 index 000000000000..ae99eacfba13 --- /dev/null +++ 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, + secondaryOptionObject: Record, + 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 index 65e236aaffad..b966f775e4d6 100644 --- 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 index cc97d13a030c..5467915cbabb 100644 --- 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; +export type FileBasedConfig = Record; export type ValueBasedConfig = Record< string, Record>