Improve XML parsing error messages

constants.js

@@ -6,7 +6,10 @@ const cloudCIOnlyMavenProperties = [
   "anypoint.platform.client_secret"
 ];
 
+const encoding = "utf8";
+
 module.exports = {
   propertyPlaceholderRegEx,
-  cloudCIOnlyMavenProperties
+  cloudCIOnlyMavenProperties,
+  encoding
 };

mulint.js

@@ -15,7 +15,7 @@ const validateDataWeaveFiles = require("./validateDataWeaveFiles");
 const assert = require("./assert");
 
 program
-  .version("1.3.0")
+  .version("1.3.1")
   .description("Mule project linter")
   .arguments("<apiBasePath>")
   .on("--help", () => {

pomParser.js

@@ -1,27 +1,13 @@
-const fs = require("fs");
-const xml2js = require("xml2js");
-const error = require("./error");
+const xmlParser = require("./xmlParser");
 
 const pomParser = pomFile => {
-  let contents = fs.readFileSync(pomFile);
-  let parser = new xml2js.Parser();
+  let { xml } = xmlParser(pomFile);
+  let xmlProperties = xml.project.properties[0];
   let properties = new Map();
-  let xml;
 
-  // synchronous by default in current version of xml2js
-  parser.parseString(contents, (err, result) => {
-    if (err) {
-      error.fatal(err);
-    }
-
-    xml = result;
-
-    let xmlProperties = xml.project.properties[0];
-
-    for (let property in xmlProperties) {
-      properties.set(property, xmlProperties[property][0]);
-    }
-  });
+  for (let property in xmlProperties) {
+    properties.set(property, xmlProperties[property][0]);
+  }
 
   let isOnPrem = properties.get("deployment.type") === "arm";
 

validateApiFiles.js

@@ -1,7 +1,5 @@
-const fs = require("fs");
+const xmlParser = require("./xmlParser");
 const path = require("path");
-const xml2js = require("xml2js");
-const error = require("./error");
 const assert = require("./assert");
 
 const expectedOnPremListenerConfig = "standardHTTPS";
@@ -10,58 +8,51 @@ const expectedListenerPathRegEx = /^\/(?:console|api)\/.+\/v1\/(?:.+\/)?\*$/;
 const validateApiFiles = (folderInfo, pomInfo) => {
   folderInfo.apiFiles.forEach(apiFile => {
     let apiFileName = path.basename(apiFile);
-    let contents = fs.readFileSync(apiFile);
-    let parser = new xml2js.Parser();
+    let { xml } = xmlParser(apiFile);
 
-    parser.parseString(contents, (err, result) => {
-      if (err) {
-        error.fatal(err);
-      }
-
-      result.mule.flow.map(flow => {
-        let listener = flow["http:listener"];
+    xml.mule.flow.map(flow => {
+      let listener = flow["http:listener"];
 
-        if (listener) {
-          let listenerAttributes = listener[0]["$"];
+      if (listener) {
+        let listenerAttributes = listener[0]["$"];
 
-          if (pomInfo.isOnPrem) {
-            assert.equals(
-              expectedOnPremListenerConfig,
-              listenerAttributes["config-ref"],
-              `${apiFileName} http:listener config`
-            );
-          }
-
-          assert.matches(
-            expectedListenerPathRegEx,
-            listenerAttributes["path"],
-            `${apiFileName} http:listener path`
+        if (pomInfo.isOnPrem) {
+          assert.equals(
+            expectedOnPremListenerConfig,
+            listenerAttributes["config-ref"],
+            `${apiFileName} http:listener config`
           );
         }
 
-        let exceptionStrategy = flow["exception-strategy"];
-
-        assert.isTrue(
-          exceptionStrategy,
-          `${apiFileName}: Missing exception strategy`
+        assert.matches(
+          expectedListenerPathRegEx,
+          listenerAttributes["path"],
+          `${apiFileName} http:listener path`
         );
+      }
 
-        if (exceptionStrategy) {
-          let exceptionStrategyAttributes = exceptionStrategy[0]["$"];
-
-          assert.equals(
-            "ChoiceExceptionStrategy",
-            exceptionStrategyAttributes.ref,
-            `${apiFileName} exception-strategy ref`
-          );
-        }
-      });
+      let exceptionStrategy = flow["exception-strategy"];
 
       assert.isTrue(
-        !result.mule["apikit:mapping-exception-strategy"],
-        `${apiFileName}: APIkit exception strategy not removed`
+        exceptionStrategy,
+        `${apiFileName}: Missing exception strategy`
       );
+
+      if (exceptionStrategy) {
+        let exceptionStrategyAttributes = exceptionStrategy[0]["$"];
+
+        assert.equals(
+          "ChoiceExceptionStrategy",
+          exceptionStrategyAttributes.ref,
+          `${apiFileName} exception-strategy ref`
+        );
+      }
     });
+
+    assert.isTrue(
+      !xml.mule["apikit:mapping-exception-strategy"],
+      `${apiFileName}: APIkit exception strategy not removed`
+    );
   });
 };
 

validateDataWeaveFiles.js

@@ -2,6 +2,7 @@ const fs = require("fs");
 const path = require("path");
 const os = require("os");
 const assert = require("./assert");
+const { encoding } = require("./constants");
 
 // Possible false positive - might be line comment
 // as opposed to commented-out code.
@@ -13,8 +14,7 @@ const validateDataWeaveFiles = folderInfo => {
     let context = path.basename(dataWeaveFile);
 
     let lines = fs
-      .readFileSync(dataWeaveFile)
-      .toString()
+      .readFileSync(dataWeaveFile, encoding)
       .split(os.EOL);
 
     let isCommentedOutLine = false;

validateGitignore.js

@@ -1,8 +1,9 @@
 const fs = require("fs");
 const assert = require("./assert");
+const { encoding } = require("./constants");
 
 const validateGitignore = folderInfo => {
-  let contents = fs.readFileSync(folderInfo.gitignoreFile).toString();
+  let contents = fs.readFileSync(folderInfo.gitignoreFile, encoding);
 
   assert.isTrue(
     /^\.project\s*$/m.test(contents),

validateGlobal.js

@@ -1,96 +1,87 @@
 const { propertyPlaceholderRegEx } = require("./constants");
-const fs = require("fs");
-const xml2js = require("xml2js");
-const error = require("./error");
+const xmlParser = require("./xmlParser");
 const assert = require("./assert");
 
 const expectedTlsContext = "clientTlsContext";
 
 const validateGlobal = folderInfo => {
-  let contents = fs.readFileSync(folderInfo.globalFile);
-  let parser = new xml2js.Parser();
+  let { contents, xml } = xmlParser(folderInfo.globalFile);
 
   assert.isTrue(
     !contents.includes("<db:dynamic-query>"),
     "Global: Dynamic query is not permitted - vulnerable to SQL injection"
   );
 
-  parser.parseString(contents, (err, result) => {
-    if (err) {
-      error.fatal(err);
-    }
-
-    assert.isTrue(
-      result.mule["api-platform-gw:api"] &&
-        result.mule["api-platform-gw:api"][0]["$"]["doc:name"] ===
-          "API Autodiscovery",
-      "Global: API Autodiscovery not configured"
-    );
-
-    let requestConfigs = result.mule["http:request-config"];
+  assert.isTrue(
+    xml.mule["api-platform-gw:api"] &&
+      xml.mule["api-platform-gw:api"][0]["$"]["doc:name"] ===
+        "API Autodiscovery",
+    "Global: API Autodiscovery not configured"
+  );
 
-    if (requestConfigs) {
-      requestConfigs.forEach(requestConfig => {
-        let requestConfigAttributes = requestConfig["$"];
+  let requestConfigs = xml.mule["http:request-config"];
 
-        let protocol = requestConfigAttributes.protocol;
-        let host = requestConfigAttributes["host"];
-        let usesMockService = host && host.includes("mock");
+  if (requestConfigs) {
+    requestConfigs.forEach(requestConfig => {
+      let requestConfigAttributes = requestConfig["$"];
 
-        if (usesMockService) {
-          return; // continue forEach, skip remaining checks
-        }
+      let protocol = requestConfigAttributes.protocol;
+      let host = requestConfigAttributes["host"];
+      let usesMockService = host && host.includes("mock");
 
-        if (protocol === "HTTPS") {
-          let tlsContext = requestConfigAttributes["tlsContext-ref"];
+      if (usesMockService) {
+        return; // continue forEach, skip remaining checks
+      }
 
-          assert.equals(
-            expectedTlsContext,
-            tlsContext,
-            `Global ${requestConfigAttributes.name} tlsContext`
-          );
-        }
+      if (protocol === "HTTPS") {
+        let tlsContext = requestConfigAttributes["tlsContext-ref"];
 
-        assert.matches(
-          propertyPlaceholderRegEx,
-          requestConfigAttributes.host,
-          `Global ${requestConfigAttributes.name} host`
+        assert.equals(
+          expectedTlsContext,
+          tlsContext,
+          `Global ${requestConfigAttributes.name} tlsContext`
         );
-
-        assert.matches(
-          propertyPlaceholderRegEx,
-          requestConfigAttributes.port,
-          `Global ${requestConfigAttributes.name} port`
+      }
+
+      assert.matches(
+        propertyPlaceholderRegEx,
+        requestConfigAttributes.host,
+        `Global ${requestConfigAttributes.name} host`
+      );
+
+      assert.matches(
+        propertyPlaceholderRegEx,
+        requestConfigAttributes.port,
+        `Global ${requestConfigAttributes.name} port`
+      );
+    });
+  }
+
+  let templateQueries = xml.mule["db:template-query"];
+
+  if (templateQueries) {
+    templateQueries.forEach(templateQuery => {
+      let query = templateQuery["db:parameterized-query"];
+
+      if (query) {
+        let queryAttributes = query[0]["$"];
+        let isFileQuery = queryAttributes && queryAttributes.file;
+
+        assert.isTrue(
+          isFileQuery,
+          "Global: Inline SQL should be moved to file"
         );
-      });
-    }
-
-    let templateQueries = result.mule["db:template-query"];
 
-    if (templateQueries) {
-      templateQueries.forEach(templateQuery => {
-        let query = templateQuery["db:parameterized-query"];
-
-        if (query) {
-          let queryAttributes = query[0]["$"];
-          let isFileQuery = queryAttributes && queryAttributes.file;
-
-          assert.isTrue(
-            isFileQuery,
-            "Global: Inline SQL should be moved to file"
+        if (isFileQuery) {
+          assert.matches(
+            /^sql\//,
+            queryAttributes.file,
+            "Global: Database query file"
           );
-
-          if (isFileQuery) {
-            assert.matches(
-              /^sql\//,
-              queryAttributes.file,
-              "Global: Database query file"
-            );
-          }
         }
-      });
-    }
-  });
+      }
+    });
+  }
 };
 
 module.exports = validateGlobal;

validateImplementation.js

@@ -1,7 +1,5 @@
-const fs = require("fs");
 const path = require("path");
-const xml2js = require("xml2js");
-const error = require("./error");
+const xmlParser = require("./xmlParser");
 const assert = require("./assert");
 
 // XML elements immediately below the root other than
@@ -19,8 +17,7 @@ const permittedTopLevelElements = new Set([
 const validateImplementation = folderInfo => {
   folderInfo.implementationFiles.forEach(implementationFile => {
     let implementationFileName = path.basename(implementationFile);
-    let contents = fs.readFileSync(implementationFile);
-    let parser = new xml2js.Parser();
+    let { contents, xml } = xmlParser(implementationFile);
 
     assert.isTrue(
       !contents.includes("<db:dynamic-query>"),
@@ -32,18 +29,12 @@ const validateImplementation = folderInfo => {
       `${implementationFileName}: Inline SQL should be moved to file/template`
     );
 
-    parser.parseString(contents, (err, result) => {
-      if (err) {
-        error.fatal(err);
-      }
-
-      for (let topLevelElement in result.mule) {
-        assert.isTrue(
-          permittedTopLevelElements.has(topLevelElement),
-          `${implementationFileName}: ${topLevelElement} is not permitted`
-        );
-      }
-    });
+    for (let topLevelElement in xml.mule) {
+      assert.isTrue(
+        permittedTopLevelElements.has(topLevelElement),
+        `${implementationFileName}: ${topLevelElement} is not permitted`
+      );
+    }
   });
 };