From 1df672e8143f7eaa62bd2495031456bb184dbc49 Mon Sep 17 00:00:00 2001
From: Pat Cavit <github@patcavit.com>
Date: Fri, 24 Feb 2023 03:06:20 -0800
Subject: [PATCH] feat: add optional attribute support (#75)

* feat: add optional attribute support

* fix: simplify required attribute conditional

* docs: add optional attr docs

Co-authored-by: Eric Cornelissen <ericornelissen@gmail.com>
---
 src/rules/attr.js | 35 +++++++++++++++++++----------------
 test/attr.spec.js | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/src/rules/attr.js b/src/rules/attr.js
index 72b04e5..55f8dc7 100644
--- a/src/rules/attr.js
+++ b/src/rules/attr.js
@@ -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.
@@ -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.
  */
 
 /**
@@ -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
@@ -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
+                );
             }
         }
     );
@@ -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) {
@@ -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`,
diff --git a/test/attr.spec.js b/test/attr.spec.js
index 06894bb..ed88b5f 100644
--- a/test/attr.spec.js
+++ b/test/attr.spec.js
@@ -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"],
@@ -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",