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

feat: add optional attr support #75

Merged
merged 7 commits into from
Feb 24, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 19 additions & 16 deletions src/rules/attr.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const logger = Logger("rule:attr");

/**
* @typedef {Object<string,string|string[]|boolean|RegExp>} AttrConfig
*
* The key represents the attribute name. The value has the following meanings:
* - `{Boolean}` If true, the attr must exist. If false, it must not exist.
* - `{String}` The attr value must match this exactly. It must also exist.
Expand All @@ -17,6 +18,7 @@ const logger = Logger("rule:attr");
* - `{ "rule::selector": {String} }` Default "*". The matching elements must fulfill the other configs.
* - `{ "rule::whitelist": {Boolean} }` Default `false`. If true, no other attributes can exist than those specified by the other configs.
* - `{ "rule::order": {Array<String> | Boolean} }` Default `null`. As array, attributes must be defined in the provided order. As `true`, attributes must be defined in alphabetical order.
* - `{ "<attribute>?": {Boolean|String|RegExp|Array<String>} }` Appending a `?` to an attribute name will make that attribute optional, and it will not error if it is missing when `rule::whitelist` is set.
*/

/**
Expand All @@ -29,11 +31,15 @@ const logger = Logger("rule:attr");
* - If it has a config:
* - If allowed, remove it from the attr list
* - If disallowed, error and remove it from the attr list
* - If whitelist is true, error if there are attributes left
* - If whitelist is true, error if there are non-optional attributes left
*/

const SPECIAL_ATTRIBS = ["rule::selector", "rule::whitelist", "rule::order"];

const OPTIONAL_SUFFIX = "?";

const isAttrOptional = (attr) => attr.endsWith(OPTIONAL_SUFFIX);

/**
* Executes on a single element.
* @param {Cheerio} $elm The cheerio element to execute on
Expand All @@ -49,19 +55,15 @@ function executeOnElm($elm, config, reporter, ast) {
attrib => {
// do nothing with special configs
if (SPECIAL_ATTRIBS.includes(attrib)) { return; }
// if it must exist
const conf = config[attrib];
if (conf === true
|| conf instanceof Array
|| typeof conf === "string"
|| conf instanceof RegExp) {
if (attrs[attrib] === undefined) {
reporter.error(
`Expected attribute '${attrib}', didn't find it`,
$elm,
ast
);
}
// do nothing with optional attributes
if (isAttrOptional(attrib)) { return; }
// if defined and not false it must exist
if (config[attrib] && !(attrib in attrs)) {
reporter.error(
`Expected attribute '${attrib}', didn't find it`,
$elm,
ast
);
}
}
);
Expand Down Expand Up @@ -104,7 +106,7 @@ function executeOnElm($elm, config, reporter, ast) {
Object.keys(attrs).forEach(
attrib => {
const value = attrs[attrib];
const expected = config[attrib];
const expected = typeof config[attrib] !== "undefined" ? config[attrib] : config[`${attrib}${OPTIONAL_SUFFIX}`];
let handled = false;
// check each type
switch (typeof expected) {
Expand Down Expand Up @@ -170,7 +172,8 @@ function executeOnElm($elm, config, reporter, ast) {
);

if (config["rule::whitelist"]) {
const remaining = Object.keys(attrs);
const remaining = Object.keys(attrs).filter((attr) => !isAttrOptional(attr));

if (remaining.length) {
reporter.error(
`Found extra attributes ${JSON.stringify(remaining)} with whitelisting enabled`,
Expand Down
32 changes: 32 additions & 0 deletions test/attr.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,28 @@ describe("Rule: attr", function(){
"rule::whitelist": true,
});
});

it("should succeed in whitelist-mode when all required attributes match", function(){
return testSucceeds({
"width": true,
"height": true,
"style": true,
"x?": true,
"rule::selector": "rect",
"rule::whitelist": true,
});
});

it("should succeed in whitelist-mode when all required and optional attributes match", function(){
return testSucceeds({
"width": true,
"height": true,
"style?": true,
"rule::selector": "rect",
"rule::whitelist": true,
});
});

it("should fail in whitelist-mode when not all attributes are allowed", function(){
return testFails({
"role": ["img", "progressbar"],
Expand All @@ -177,6 +199,16 @@ describe("Rule: attr", function(){
"rule::whitelist": true,
});
});

it("should fail in whitelist-mode with an invalid value for an optional attribute", function(){
return testFails({
"role": ["img", "progressbar"],
"viewBox?": "0 0 25 25",
"rule::selector": "svg",
"rule::whitelist": true,
});
});

it("should succeed in whitelist-mode without attributes", function(){
return testSucceeds({
"rule::selector": "circle",
Expand Down