Skip to content
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

Fix false negatives for invalid options in *-list #5924

Merged
merged 1 commit into from Feb 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions lib/rules/at-rule-property-required-list/index.js
Expand Up @@ -3,8 +3,9 @@
const isStandardSyntaxAtRule = require('../../utils/isStandardSyntaxAtRule');
const report = require('../../utils/report');
const ruleMessages = require('../../utils/ruleMessages');
const validateObjectWithArrayProps = require('../../utils/validateObjectWithArrayProps');
const validateOptions = require('../../utils/validateOptions');
const { isPlainObject } = require('../../utils/validateTypes');
const { isString } = require('../../utils/validateTypes');

const ruleName = 'at-rule-property-required-list';

Expand All @@ -21,7 +22,7 @@ const rule = (primary) => {
return (root, result) => {
const validOptions = validateOptions(result, ruleName, {
actual: primary,
possible: [isPlainObject],
possible: [validateObjectWithArrayProps(isString)],
});

if (!validOptions) {
Expand Down
5 changes: 3 additions & 2 deletions lib/rules/declaration-property-unit-allowed-list/index.js
Expand Up @@ -8,8 +8,9 @@ const matchesStringOrRegExp = require('../../utils/matchesStringOrRegExp');
const optionsMatches = require('../../utils/optionsMatches');
const report = require('../../utils/report');
const ruleMessages = require('../../utils/ruleMessages');
const validateObjectWithArrayProps = require('../../utils/validateObjectWithArrayProps');
const validateOptions = require('../../utils/validateOptions');
const { isPlainObject } = require('../../utils/validateTypes');
const { isString } = require('../../utils/validateTypes');
const vendor = require('../../utils/vendor');

const ruleName = 'declaration-property-unit-allowed-list';
Expand All @@ -30,7 +31,7 @@ const rule = (primary, secondaryOptions) => {
ruleName,
{
actual: primary,
possible: [isPlainObject],
possible: [validateObjectWithArrayProps(isString)],
},
{
actual: secondaryOptions,
Expand Down
5 changes: 3 additions & 2 deletions lib/rules/declaration-property-unit-disallowed-list/index.js
Expand Up @@ -7,8 +7,9 @@ const getUnitFromValueNode = require('../../utils/getUnitFromValueNode');
const matchesStringOrRegExp = require('../../utils/matchesStringOrRegExp');
const report = require('../../utils/report');
const ruleMessages = require('../../utils/ruleMessages');
const validateObjectWithArrayProps = require('../../utils/validateObjectWithArrayProps');
const validateOptions = require('../../utils/validateOptions');
const { isPlainObject } = require('../../utils/validateTypes');
const { isString } = require('../../utils/validateTypes');
const vendor = require('../../utils/vendor');

const ruleName = 'declaration-property-unit-disallowed-list';
Expand All @@ -26,7 +27,7 @@ const rule = (primary) => {
return (root, result) => {
const validOptions = validateOptions(result, ruleName, {
actual: primary,
possible: [isPlainObject],
possible: [validateObjectWithArrayProps(isString)],
});

if (!validOptions) {
Expand Down
14 changes: 5 additions & 9 deletions lib/rules/declaration-property-value-allowed-list/index.js
@@ -1,10 +1,12 @@
'use strict';

const matchesStringOrRegExp = require('../../utils/matchesStringOrRegExp');
const optionsMatches = require('../../utils/optionsMatches');
const report = require('../../utils/report');
const ruleMessages = require('../../utils/ruleMessages');
const validateObjectWithArrayProps = require('../../utils/validateObjectWithArrayProps');
const validateOptions = require('../../utils/validateOptions');
const { isPlainObject } = require('../../utils/validateTypes');
const { isString, isRegExp } = require('../../utils/validateTypes');
const vendor = require('../../utils/vendor');

const ruleName = 'declaration-property-value-allowed-list';
Expand All @@ -22,7 +24,7 @@ const rule = (primary) => {
return (root, result) => {
const validOptions = validateOptions(result, ruleName, {
actual: primary,
possible: [isPlainObject],
possible: [validateObjectWithArrayProps([isString, isRegExp])],
});

if (!validOptions) {
Expand All @@ -42,13 +44,7 @@ const rule = (primary) => {
return;
}

const propList = primary[propKey];

if (!propList || propList.length === 0) {
return;
}

if (matchesStringOrRegExp(value, propList)) {
if (optionsMatches(primary, propKey, value)) {
return;
}

Expand Down
14 changes: 5 additions & 9 deletions lib/rules/declaration-property-value-disallowed-list/index.js
@@ -1,10 +1,12 @@
'use strict';

const matchesStringOrRegExp = require('../../utils/matchesStringOrRegExp');
const optionsMatches = require('../../utils/optionsMatches');
const report = require('../../utils/report');
const ruleMessages = require('../../utils/ruleMessages');
const validateObjectWithArrayProps = require('../../utils/validateObjectWithArrayProps');
const validateOptions = require('../../utils/validateOptions');
const { isPlainObject } = require('../../utils/validateTypes');
const { isString, isRegExp } = require('../../utils/validateTypes');
const vendor = require('../../utils/vendor');

const ruleName = 'declaration-property-value-disallowed-list';
Expand All @@ -22,7 +24,7 @@ const rule = (primary) => {
return (root, result) => {
const validOptions = validateOptions(result, ruleName, {
actual: primary,
possible: [isPlainObject],
possible: [validateObjectWithArrayProps([isString, isRegExp])],
});

if (!validOptions) {
Expand All @@ -42,13 +44,7 @@ const rule = (primary) => {
return;
}

const propList = primary[propKey];

if (!propList || propList.length === 0) {
return;
}

if (!matchesStringOrRegExp(value, propList)) {
if (!optionsMatches(primary, propKey, value)) {
return;
}

Expand Down
14 changes: 5 additions & 9 deletions lib/rules/media-feature-name-value-allowed-list/index.js
Expand Up @@ -5,11 +5,13 @@ const mediaParser = require('postcss-media-query-parser').default;
const atRuleParamIndex = require('../../utils/atRuleParamIndex');
const isRangeContextMediaFeature = require('../../utils/isRangeContextMediaFeature');
const matchesStringOrRegExp = require('../../utils/matchesStringOrRegExp');
const optionsMatches = require('../../utils/optionsMatches');
const rangeContextNodeParser = require('../rangeContextNodeParser');
const report = require('../../utils/report');
const ruleMessages = require('../../utils/ruleMessages');
const validateObjectWithArrayProps = require('../../utils/validateObjectWithArrayProps');
const validateOptions = require('../../utils/validateOptions');
const { isPlainObject } = require('../../utils/validateTypes');
const { isString, isRegExp } = require('../../utils/validateTypes');
const vendor = require('../../utils/vendor');

const ruleName = 'media-feature-name-value-allowed-list';
Expand All @@ -27,7 +29,7 @@ const rule = (primary) => {
return (root, result) => {
const validOptions = validateOptions(result, ruleName, {
actual: primary,
possible: [isPlainObject],
possible: [validateObjectWithArrayProps([isString, isRegExp])],
});

if (!validOptions) {
Expand Down Expand Up @@ -78,13 +80,7 @@ const rule = (primary) => {
return;
}

const allowedValues = primary[allowedValuesKey];

if (allowedValues == null) {
return;
}

if (matchesStringOrRegExp(value, allowedValues)) {
if (optionsMatches(primary, allowedValuesKey, value)) {
return;
}

Expand Down
5 changes: 3 additions & 2 deletions lib/rules/rule-selector-property-disallowed-list/index.js
Expand Up @@ -4,8 +4,9 @@ const isStandardSyntaxRule = require('../../utils/isStandardSyntaxRule');
const matchesStringOrRegExp = require('../../utils/matchesStringOrRegExp');
const report = require('../../utils/report');
const ruleMessages = require('../../utils/ruleMessages');
const validateObjectWithArrayProps = require('../../utils/validateObjectWithArrayProps');
const validateOptions = require('../../utils/validateOptions');
const { isPlainObject } = require('../../utils/validateTypes');
const { isString, isRegExp } = require('../../utils/validateTypes');

const ruleName = 'rule-selector-property-disallowed-list';

Expand All @@ -22,7 +23,7 @@ const rule = (primary) => {
return (root, result) => {
const validOptions = validateOptions(result, ruleName, {
actual: primary,
possible: [isPlainObject],
possible: [validateObjectWithArrayProps([isString, isRegExp])],
});

if (!validOptions) {
Expand Down
4 changes: 4 additions & 0 deletions lib/utils/__tests__/matchesStringOrRegExp.test.js
Expand Up @@ -102,3 +102,7 @@ it('matchesStringOrRegExp comparing with a actual RegExp comparisonValue', () =>
pattern: /FOO/,
});
});

it('matchesStringOrRegExp comparing with an empty array', () => {
expect(matchesStringOrRegExp('.foo', [])).toBeFalsy();
});
8 changes: 8 additions & 0 deletions lib/utils/__tests__/optionsMatches.test.js
Expand Up @@ -29,3 +29,11 @@ it('optionsMatches matches a RegExp', () => {
expect(optionsMatches({ foo: ['/\\.bar$/', '.baz'] }, 'foo', '.baz')).toBeTruthy();
expect(optionsMatches({ foo: ['/\\.bar$/', 'qux'] }, 'foo', '.baz')).toBeFalsy();
});

it('optionsMatches does not match any value without the property', () => {
expect(optionsMatches({}, 'foo', 'bar')).toBeFalsy();
});

it('optionsMatches does not match any value with the empty array property', () => {
expect(optionsMatches({ foo: [] }, 'foo', 'bar')).toBeFalsy();
});
46 changes: 24 additions & 22 deletions lib/utils/validateObjectWithArrayProps.js
Expand Up @@ -3,39 +3,41 @@
const { isPlainObject } = require('./validateTypes');

/**
* @template T
* @typedef {(i: T) => boolean} Validator
*/

/**
* Check whether the variable is an object and all its properties are arrays of string values:
* Check whether the variable is an object and all its properties are arrays of values
* that satisfy the specified validator(s):
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] Fixed the inaccurate comment.

*
* @example
* ignoreProperties = {
* value1: ["item11", "item12", "item13"],
* value2: ["item21", "item22", "item23"],
* value3: ["item31", "item32", "item33"],
* }
* @template T
* @param {Validator<T>|Validator<T>[]} validator
* };
* validateObjectWithArrayProps(isString)(ignoreProperties);
* //=> true
*
* @template {(value: unknown) => boolean} Validator
* @param {Validator | Validator[]} validator
* @returns {(value: unknown) => boolean}
*/
module.exports = (validator) => (value) => {
if (!isPlainObject(value)) {
return false;
}

return Object.values(value).every((array) => {
if (!Array.isArray(array)) {
module.exports = function validateObjectWithArrayProps(validator) {
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] Giving the function name changed indentation.

return (value) => {
if (!isPlainObject(value)) {
return false;
}

// Make sure the array items are strings
return array.every((item) => {
if (Array.isArray(validator)) {
return validator.some((v) => v(item));
return Object.values(value).every((array) => {
if (!Array.isArray(array)) {
return false;
}

return validator(item);
// Make sure the array items are strings
return array.every((item) => {
if (Array.isArray(validator)) {
return validator.some((v) => v(item));
}

return validator(item);
});
});
});
};
};