Skip to content

Feat/NEWCLICKUI-4391 add new rule #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 62 additions & 6 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const RULE_USE_MIXINS = 'stylelint-core-vars/use-mixins';
const RULE_USE_ONE_OF_MIXINS = 'stylelint-core-vars/use-one-of-mixins';
const RULE_DO_NOT_USE_DARK_COLORS = 'stylelint-core-vars/do-not-use-dark-colors';
const RULE_DO_NOT_USE_OLD_VARS = 'stylelint-core-vars/do-not-use-old-vars';
const RULE_NO_USELESS_VARS_IMPORT = 'stylelint-core-vars/no-useless-vars-import';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А оно точно везде useless?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В контексте названия имеешь ввиду? Там где срабатывает - да, а где vars импорят совместно с миксинами, правило же как раз и не будет ругаться. Или на что-то лучше заменить?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я про то что во всех ли проектах компании это так, библиотека же глобальная

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@reme3d2y @SiebenSieben Привет!
Подскажите пожалуйста, ок ли вам по такому правилу?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нам бы обновить кодоунеров, reme3d2y уже с нами не работает, и я по факту не занимаюсь поддержкой линтера

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

По вопросу не уверен, что везде будет ок, предлагаю спросить в рокетчате javascript


const messages = {
[RULE_USE_VARS]: stylelint.utils.ruleMessages(RULE_USE_VARS, {
Expand Down Expand Up @@ -59,6 +60,11 @@ const messages = {
return 'Do not use dark colors directly. Only light and static colors are allowed';
},
}),
[RULE_NO_USELESS_VARS_IMPORT]: stylelint.utils.ruleMessages(RULE_NO_USELESS_VARS_IMPORT, {
expected: () => {
return 'Do not import vars.css unless you use a @mixin or @include';
},
}),
};

const checkVars = (decl, result, context, ruleName, matcher, shouldReport, options = {}) => {
Expand All @@ -81,7 +87,7 @@ const checkVars = (decl, result, context, ruleName, matcher, shouldReport, optio

const originalValueIndex = previousValues.reduce(
(acc, sub) => (acc > sub.index + sub.diff ? acc - sub.diff : acc),
substitution.index
substitution.index,
);

if (shouldReport(substitution.fixable, fixed)) {
Expand All @@ -91,7 +97,7 @@ const checkVars = (decl, result, context, ruleName, matcher, shouldReport, optio
message: messages[ruleName].expected(
substitution.variables,
substitution.value,
substitution.fixable
substitution.fixable,
),
node: decl,
word: value,
Expand Down Expand Up @@ -155,7 +161,7 @@ const checkTypography = (rule, result, context, ruleName) => {
}
};

const checkDarkColorsUsage = (decl, result, context, ruleName) => {
const checkDarkColorsUsage = (decl, result) => {
const { prop, raws } = decl;

let value = toOneLine(decl.value);
Expand All @@ -174,6 +180,46 @@ const checkDarkColorsUsage = (decl, result, context, ruleName) => {
}
};

const checkVarsImportUsage = (root, result, context, ruleName) => {
const isVarsCssImport = (params) => /\/?vars\.css(['"])?\s*;?$/.test(params);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Может стоит ограничить конкретным местом? Какая мотивация ограничения для всех vars.css?


let hasVarsImport = false;
let usesMixinOrInclude = false;
const importNodes = [];

root.walkAtRules((atRule) => {
if (atRule.name === 'import' && isVarsCssImport(atRule.params)) {
hasVarsImport = true;
importNodes.push(atRule);
}

if (
atRule.name === 'mixin' ||
atRule.name === 'include' ||
(atRule.name === 'use' && /mixin/.test(atRule.params))
) {
usesMixinOrInclude = true;
}
});

if (hasVarsImport && !usesMixinOrInclude) {
importNodes.forEach((atRule) => {
if (context.fix) {
atRule.remove();
} else {
stylelint.utils.report({
result,
ruleName,
message: messages[RULE_NO_USELESS_VARS_IMPORT].expected(),
node: atRule,
word: 'vars.css',
index: atRule.params.indexOf('vars.css'),
});
}
});
}
};

module.exports = [
stylelint.createPlugin(RULE_USE_VARS, (enabled, config, context) => {
if (!enabled || !VARS_AVAILABLE) {
Expand All @@ -191,7 +237,7 @@ module.exports = [
RULE_USE_VARS,
findVars,
(fixable, fixed) => fixable && !fixed,
{ allowNumericValues }
{ allowNumericValues },
);
});
};
Expand All @@ -212,7 +258,7 @@ module.exports = [
RULE_USE_ONE_OF_VARS,
findVars,
(fixable, fixed) => !fixable && !fixed,
{ allowNumericValues }
{ allowNumericValues },
);
});
};
Expand All @@ -230,7 +276,7 @@ module.exports = [
context,
RULE_DO_NOT_USE_OLD_VARS,
findOldVars,
(_, fixed) => !fixed
(_, fixed) => !fixed,
);
});
};
Expand Down Expand Up @@ -268,6 +314,15 @@ module.exports = [
});
};
}),
stylelint.createPlugin(RULE_NO_USELESS_VARS_IMPORT, (enabled, _, context) => {
if (!enabled || !VARS_AVAILABLE) {
return () => {};
}

return (root, result) => {
checkVarsImportUsage(root, result, context, RULE_NO_USELESS_VARS_IMPORT);
};
}),
];

module.exports.messages = messages;
Expand All @@ -277,3 +332,4 @@ module.exports.RULE_USE_MIXINS = RULE_USE_MIXINS;
module.exports.RULE_USE_ONE_OF_MIXINS = RULE_USE_ONE_OF_MIXINS;
module.exports.RULE_DO_NOT_USE_DARK_COLORS = RULE_DO_NOT_USE_DARK_COLORS;
module.exports.RULE_DO_NOT_USE_OLD_VARS = RULE_DO_NOT_USE_OLD_VARS;
module.exports.RULE_NO_USELESS_VARS_IMPORT = RULE_NO_USELESS_VARS_IMPORT;
62 changes: 50 additions & 12 deletions test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const {
RULE_USE_ONE_OF_MIXINS,
RULE_DO_NOT_USE_DARK_COLORS,
RULE_DO_NOT_USE_OLD_VARS,
RULE_NO_USELESS_VARS_IMPORT,
messages,
} = require('../lib');

Expand Down Expand Up @@ -181,7 +182,7 @@ testRule({
description: 'hardcode singleline shadow',
message: messages[RULE_USE_VARS].expected(
['--shadow-xs'],
'0 0 4px rgba(11, 31, 53, 0.02), 0 2px 4px rgba(11, 31, 53, 0.04)'
'0 0 4px rgba(11, 31, 53, 0.02), 0 2px 4px rgba(11, 31, 53, 0.04)',
),
line: 2,
column: 17,
Expand All @@ -193,7 +194,7 @@ testRule({
description: 'hardcode multiline shadow',
message: messages[RULE_USE_VARS].expected(
['--shadow-xs-hard'],
'0 0 4px rgba(11, 31, 53, 0.02), 0 2px 4px rgba(11, 31, 53, 0.04), 0 2px 4px rgba(11, 31, 53, 0.16)'
'0 0 4px rgba(11, 31, 53, 0.02), 0 2px 4px rgba(11, 31, 53, 0.04), 0 2px 4px rgba(11, 31, 53, 0.16)',
),
line: 2,
column: 17,
Expand All @@ -219,7 +220,7 @@ testRule({
{
message: messages[RULE_USE_VARS].expected(
['--shadow-xs'],
'0 0 4px rgba(11, 31, 53, 0.02), 0 2px 4px rgba(11, 31, 53, 0.04)'
'0 0 4px rgba(11, 31, 53, 0.02), 0 2px 4px rgba(11, 31, 53, 0.04)',
),
line: 3,
column: 17,
Expand Down Expand Up @@ -429,7 +430,7 @@ testRule({
'--color-light-specialbg-secondary-grouped',
'--color-light-graphic-primary-inverted',
],
'#fff'
'#fff',
),
},
{
Expand All @@ -441,7 +442,7 @@ testRule({
'--color-light-specialbg-secondary-grouped',
'--color-light-graphic-primary-inverted',
],
'#fff'
'#fff',
),
},
{
Expand All @@ -453,7 +454,7 @@ testRule({
'--color-light-graphic-primary',
'--color-light-bg-primary-inverted',
],
'#0b1f35'
'#0b1f35',
),
},
],
Expand Down Expand Up @@ -485,7 +486,7 @@ testRule({
'--color-light-specialbg-secondary-grouped',
'--color-light-graphic-primary-inverted',
],
'#fff'
'#fff',
),
},
{
Expand All @@ -497,7 +498,7 @@ testRule({
'--color-light-specialbg-secondary-grouped',
'--color-light-graphic-primary-inverted',
],
'#fff'
'#fff',
),
},
{
Expand All @@ -509,7 +510,7 @@ testRule({
'--color-light-graphic-primary',
'--color-light-bg-primary-inverted',
],
'#0b1f35'
'#0b1f35',
),
},
],
Expand Down Expand Up @@ -544,7 +545,7 @@ testRule({
fixed: `.class {\n background-color: var(--color-light-bg-tertiary);\n}`,
message: messages[RULE_DO_NOT_USE_OLD_VARS].expected(
['--color-light-bg-tertiary'],
'--color-dark-indigo-10-flat'
'--color-dark-indigo-10-flat',
),
line: 2,
column: 27,
Expand Down Expand Up @@ -576,7 +577,7 @@ testRule({
message: messages[RULE_DO_NOT_USE_OLD_VARS].expected(
['--color-light-bg-negative-muted'],
'--color-red-brand-10-flat',
false
false,
),
},
{
Expand All @@ -585,7 +586,7 @@ testRule({
message: messages[RULE_DO_NOT_USE_OLD_VARS].expected(
['--color-light-graphic-negative'],
'--color-red-dark',
true
true,
),
},
],
Expand Down Expand Up @@ -859,3 +860,40 @@ testRule({
},
],
});

testRule({
plugins: [RULE_NO_USELESS_VARS_IMPORT],
ruleName: RULE_NO_USELESS_VARS_IMPORT,
config: true,
accept: [
{
code: `@import '../../../shared/styles/vars.css';\n.css { @include flex-center; }`,
description: 'mixin in use',
},
{
code: `.class { padding: 8px; }`,
description: 'not have import',
},
{
code: `
/* stylelint-disable */
@import '../../../shared/styles/vars.css';

.class {
font-size: 48px;
}
`,
description: 'linter disabled',
},
],
reject: [
{
code: `@import '../../../shared/styles/vars.css';\n.class { color: red; }`,
description: 'without using mixin/include',
message: messages[RULE_NO_USELESS_VARS_IMPORT].expected(),
line: 1,
column: 25,
fixed: `.class { color: red; }`,
},
],
});