Skip to content

Commit

Permalink
[Stylelint] Add no_restricted_values linter rule (#4413)
Browse files Browse the repository at this point in the history
* 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]>
  • Loading branch information
4 people authored Aug 17, 2023
1 parent 86a3f1e commit 06a4903
Show file tree
Hide file tree
Showing 10 changed files with 151 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
22 changes: 15 additions & 7 deletions packages/osd-stylelint-config/.stylelintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
],
}
}
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
25 changes: 25 additions & 0 deletions packages/osd-stylelint-config/config/restricted_values.json
Original file line number Diff line number Diff line change
@@ -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"
]
}
}
4 changes: 3 additions & 1 deletion packages/osd-stylelint-plugin-stylelint/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Original file line number Diff line number Diff line change
Expand Up @@ -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[] }>>
Expand Down

0 comments on commit 06a4903

Please sign in to comment.