Merge branch 'main' of github.com:apigee/apigeelint

apigee · Mar 4, 2024 · 6bf5362 · 6bf5362
2 parents 6227d50 + be07fcd
commit 6bf5362
Show file tree

Hide file tree

Showing 10 changed files with 94 additions and 15 deletions.
diff --git a/lib/package/plugins/PO034-am-hygiene.js b/lib/package/plugins/PO034-am-hygiene.js
@@ -424,8 +424,43 @@ const onPolicy = function (policy, cb) {
         });
       });
 
-      // children of Remove/Copy, if they exist, should have no text
+      // children of Remove/Copy, if they exist, should have no text,
+      // or the text should be a boolean.
       ["Remove", "Copy"].forEach((tag) => {
+        const checkSingleChildTextBoolean = (child, tag1, tag2) => {
+          //debug(child);
+          if (child.childNodes.length > 1) {
+            foundIssue = true;
+            _addIssue(
+              policy,
+              `there should be at most a single element under <${tag1}>/<${tag2}>.`,
+              child.lineNumber,
+              child.columnNumber
+            );
+          } else {
+            if (child.firstChild.nodeType != TEXT_NODE) {
+              foundIssue = true;
+              _addIssue(
+                policy,
+                `there should be no non-text elements under <${tag1}>/<${tag2}>.`,
+                child.lineNumber,
+                child.columnNumber
+              );
+            } else {
+              const text = child.firstChild.data.trim().toLowerCase();
+              if (text != "false" && text != "true") {
+                foundIssue = true;
+                _addIssue(
+                  policy,
+                  `if there is a text element under <${tag1}>/<${tag2}>, it should be a boolean.`,
+                  child.lineNumber,
+                  child.columnNumber
+                );
+              }
+            }
+          }
+        };
+
         const elements = xpath.select(`AssignMessage/${tag}`, policyRoot);
         elements.forEach((element) => {
           debug(`checking(3) ${element.tagName}...`);
@@ -498,14 +533,13 @@ const onPolicy = function (policy, cb) {
                         );
                       }
 
-                      // check that innerChild has no text value or child elements
                       if (innerChild.hasChildNodes()) {
-                        foundIssue = true;
-                        _addIssue(
-                          policy,
-                          `there should be no text or child elements under element <${child.tagName}>/<${innerChild.tagName}>.`,
-                          innerChild.lineNumber,
-                          innerChild.columnNumber
+                        // check that there is a single innerChild,
+                        // it is text, and it represents a boolean
+                        checkSingleChildTextBoolean(
+                          innerChild,
+                          `${tag}>/<${child.tagName}`,
+                          innerChild.tagName
                         );
                       }
                     }
@@ -514,13 +548,9 @@ const onPolicy = function (policy, cb) {
               } else {
                 // Payload, Verb, etc
                 if (child.hasChildNodes()) {
-                  foundIssue = true;
-                  _addIssue(
-                    policy,
-                    `there should be no text or child elements under element <${tag}>/<${child.tagName}>.`,
-                    child.lineNumber,
-                    child.columnNumber
-                  );
+                  // check that there is a single innerChild,
+                  // it is text, and it represents a boolean
+                  checkSingleChildTextBoolean(child, tag, child.tagName);
                 }
                 if (child.hasAttributes()) {
                   foundIssue = true;

diff --git a/test/fixtures/resources/PO034/fail/Remove-Headers-Header-Non-Boolean.xml b/test/fixtures/resources/PO034/fail/Remove-Headers-Header-Non-Boolean.xml
@@ -0,0 +1,7 @@
+<AssignMessage name="Remove-Headers-Header-Non-Boolean">
+  <Remove>
+    <Headers>
+      <Header name='foo'>non-bool-value</Header>
+    </Headers>
+  </Remove>
+</AssignMessage>
diff --git a/test/fixtures/resources/PO034/fail/Remove-Payload-NonBoolean.xml b/test/fixtures/resources/PO034/fail/Remove-Payload-NonBoolean.xml
@@ -0,0 +1,5 @@
+<AssignMessage name="Remove-Payload-NonBoolean">
+  <Remove>
+    <Payload>non-boolean-value</Payload>
+  </Remove>
+</AssignMessage>
diff --git a/test/fixtures/resources/PO034/fail/messages.js b/test/fixtures/resources/PO034/fail/messages.js
@@ -47,5 +47,11 @@ module.exports = {
   ],
   "Copy-with-multiple-attrs.xml": [
     "incorrect attribute (extraneous) on element <Copy>."
+  ],
+  "Remove-Payload-NonBoolean.xml": [
+    "if there is a text element under <Remove>/<Payload>, it should be a boolean."
+  ],
+  "Remove-Headers-Header-Non-Boolean.xml": [
+    "if there is a text element under <Remove>/<Headers>/<Header>, it should be a boolean."
   ]
 };
diff --git a/test/fixtures/resources/PO034/pass/Remove-Headers-Header-Empty.xml b/test/fixtures/resources/PO034/pass/Remove-Headers-Header-Empty.xml
@@ -0,0 +1,7 @@
+<AssignMessage name="Remove-Headers-Header-Empty">
+  <Remove>
+    <Headers>
+      <Header name='foo'/>
+    </Headers>
+  </Remove>
+</AssignMessage>
diff --git a/test/fixtures/resources/PO034/pass/Remove-Headers-Header-False.xml b/test/fixtures/resources/PO034/pass/Remove-Headers-Header-False.xml
@@ -0,0 +1,7 @@
+<AssignMessage name="Remove-Headers-Header-False">
+  <Remove>
+    <Headers>
+      <Header name='foo'>false</Header>
+    </Headers>
+  </Remove>
+</AssignMessage>
diff --git a/test/fixtures/resources/PO034/pass/Remove-Headers-Header-True.xml b/test/fixtures/resources/PO034/pass/Remove-Headers-Header-True.xml
@@ -0,0 +1,7 @@
+<AssignMessage name="Remove-Headers-Header-True">
+  <Remove>
+    <Headers>
+      <Header name='foo'>true</Header>
+    </Headers>
+  </Remove>
+</AssignMessage>
diff --git a/...s/resources/PO034/pass/Remove-Payload.xml → ...urces/PO034/pass/Remove-Payload-Empty.xml b/...s/resources/PO034/pass/Remove-Payload.xml → ...urces/PO034/pass/Remove-Payload-Empty.xml
diff --git a/test/fixtures/resources/PO034/pass/Remove-Payload-False.xml b/test/fixtures/resources/PO034/pass/Remove-Payload-False.xml
@@ -0,0 +1,5 @@
+<AssignMessage name="Remove-Payload-False">
+  <Remove>
+    <Payload>false</Payload>
+  </Remove>
+</AssignMessage>
diff --git a/test/fixtures/resources/PO034/pass/Remove-Payload-True.xml b/test/fixtures/resources/PO034/pass/Remove-Payload-True.xml
@@ -0,0 +1,5 @@
+<AssignMessage name="Remove-Payload-True">
+  <Remove>
+    <Payload>true</Payload>
+  </Remove>
+</AssignMessage>