Refactor code

* move most code outside of lib/htmllint.js
* rename a few variables and methods to be more descriptive
* return early in parseErrorMessages

Author: XhmikosR

Gruntfile.js

@@ -1,6 +1,6 @@
 'use strict';
 
-module.exports = function(grunt) {
+module.exports = grunt => {
   require('time-grunt')(grunt);
 
   grunt.initConfig({

lib/htmllint.js

@@ -4,125 +4,32 @@ const os = require('os');
 const path = require('path');
 const { execFile } = require('child_process');
 const async = require('async');
-const jar = require('vnu-jar');
 const chunkify = require('./chunkify.js');
-const javadetect = require('./javadetect.js');
+const javaDetect = require('./javaDetect.js');
+const javaArgs = require('./javaArgs.js');
+const parseErrorMessages = require('./parseErrorMessages.js');
+const processErrorMessages = require('./processErrorMessages.js');
 
 // chunkify's max characters
 const MAX_CHARS = 5000;
 
-// Increase child process buffer to accommodate large amounts of
-// validation output. (the default is a paltry 200k)
-// https://nodejs.org/api/child_process.html#child_process_child_process_exec_command_options_callback
+/**
+ * Increase child process buffer to accommodate large amounts of validation output.
+ * The default is a paltry 200k:
+ * https://nodejs.org/api/child_process.html#child_process_child_process_exec_command_options_callback
+ */
 const MAX_BUFFER = 20_000 * 1024;
 
 // eslint-disable-next-line unicorn/explicit-length-check
 const CPUS = os.cpus() && os.cpus().length;
 const THREADS = CPUS > 2 ? CPUS - 1 : 1;
 
-// Replace left/right quotation marks with normal quotation marks
-function normalizeQuotationMarks(string) {
-  if (string) {
-    string = string.replace(/[\u201C\u201D]/g, '"');
-  }
-
-  return string;
-}
-
-// Parse and, if needed, normalize error messages from HttpClient to java -jar format
-// java -jar: one object containing messages for all files
-//   { messages: [{ message, type, url, ... }, ...] }
-// HttpClient: one object per file, separated by a newline,
-// each object containing messages for only that file
-//   { messages: [{ message, type, ...}, ...], url }\n{ ... }
-function parseErrorMessages(errors, config) {
-  const parsed = JSON.parse(config.server ? `[${errors.trim().replace(/\n/g, ',')}]` : errors);
-  let { messages } = parsed;
-
-  if (config.server) {
-    // Extract "messages" property from each object and set the url of each message.
-    // This results in an array of arrays instead of array of objects, which is then
-    // flattened
-    messages = parsed.flatMap(file => {
-      return file.messages.map(message => {
-        message.url = file.url;
-        return message;
-      });
-    });
-  }
-
-  return messages;
-}
-
-function processErrorMessages(errors, config) {
-  const result = errors.map(message => {
-    if (message.url) {
-      message.file = path.relative('.', message.url.replace(path.sep === '\\' ? 'file:/' : 'file:', ''));
-    }
-
-    if (config.absoluteFilePathsForReporter) {
-      message.file = path.resolve(message.file);
-    }
-
-    return message;
-  });
-
-  if (!config.ignore) {
-    return result;
-  }
-
-  const ignore = Array.isArray(config.ignore) ? config.ignore : [config.ignore];
-
-  return result.filter(({ message }) => {
-    // Iterate over the ignore rules and test the message against each rule.
-    // A match should return false, which causes every() to return false and
-    // the message to be filtered out.
-    return ignore.every(
-      currentValue => currentValue instanceof RegExp ?
-        !currentValue.test(message) :
-        normalizeQuotationMarks(currentValue) !== normalizeQuotationMarks(message)
-    );
-  });
-}
-
-// Determine proper jarfile arguments
-function javaArgs(java, chunk, config) {
-  const args = [];
-
-  // Increase the default stack size for ia32 versions
-  if (java.arch === 'ia32') {
-    args.push('-Xss512k');
-  }
-
-  args.push(config.server ? '-cp' : '-jar', `"${jar}"`);
-
-  if (config.server) {
-    if (config.server.host) {
-      args.push(`-Dnu.validator.client.host=${config.server.host}`);
-    }
-
-    if (config.server.port) {
-      args.push(`-Dnu.validator.client.port=${config.server.port}`);
-    }
-
-    args.push('-Dnu.validator.client.out=json nu.validator.client.HttpClient');
-  } else {
-    args.push('--format', 'json');
-  }
-
-  if (config.noLangDetect) {
-    args.push('--no-langdetect');
-  }
-
-  return [...args, chunk];
-}
-
 function htmllint(config, done) {
   if (config.files.length === 0) {
     return done(null, []);
   }
 
-  javadetect((err, java) => {
+  javaDetect((err, java) => {
     if (err) {
       throw err;
     }

lib/javaArgs.js

@@ -0,0 +1,37 @@
+'use strict';
+
+const jar = require('vnu-jar');
+
+// Determine proper jarfile arguments
+function javaArgs(java, chunk, config) {
+  const args = [];
+
+  // Increase the default stack size for ia32 versions
+  if (java.arch === 'ia32') {
+    args.push('-Xss512k');
+  }
+
+  args.push(config.server ? '-cp' : '-jar', `"${jar}"`);
+
+  if (config.server) {
+    if (config.server.host) {
+      args.push(`-Dnu.validator.client.host=${config.server.host}`);
+    }
+
+    if (config.server.port) {
+      args.push(`-Dnu.validator.client.port=${config.server.port}`);
+    }
+
+    args.push('-Dnu.validator.client.out=json nu.validator.client.HttpClient');
+  } else {
+    args.push('--format', 'json');
+  }
+
+  if (config.noLangDetect) {
+    args.push('--no-langdetect');
+  }
+
+  return [...args, chunk];
+}
+
+module.exports = javaArgs;

lib/javadetect.js lib/javaDetect.jslib/javadetect.js renamed to lib/javaDetect.js

@@ -2,7 +2,7 @@
 
 const { execFile } = require('child_process');
 
-function javadetect(callback) {
+function javaDetect(callback) {
   execFile('java', ['-version'], (error, stdout, stderr) => {
     if (error) {
       return callback(error);
@@ -15,4 +15,4 @@ function javadetect(callback) {
   });
 }
 
-module.exports = javadetect;
+module.exports = javaDetect;

lib/parseErrorMessages.js

@@ -0,0 +1,30 @@
+'use strict';
+
+/**
+ * Parse and, if needed, normalize error messages from HttpClient to java -jar format
+ * java -jar: one object containing messages for all files
+ *   { messages: [{ message, type, url, ... }, ...] }
+ * HttpClient: one object per file, separated by a newline,
+ * each object containing messages for only that file
+ *   { messages: [{ message, type, ...}, ...], url }\n{ ... }
+ */
+function parseErrorMessages(errors, config) {
+  const normalizedErrors = config.server ? `[${errors.trim().replace(/\n/g, ',')}]` : errors;
+  const parsedErrors = JSON.parse(normalizedErrors);
+
+  // If server is not used, return the messages as is
+  if (!config.server) {
+    return parsedErrors.messages;
+  }
+
+  // Extract the "messages" property from each object and set the url of each message.
+  // This results in an array of arrays instead of array of objects, which is then flattened.
+  return parsedErrors.flatMap(file => {
+    return file.messages.map(message => {
+      message.url = file.url;
+      return message;
+    });
+  });
+}
+
+module.exports = parseErrorMessages;

lib/processErrorMessages.js

@@ -0,0 +1,41 @@
+'use strict';
+
+const path = require('path');
+const { normalizeQuotationMarks } = require('./util.js');
+
+const processedMessages = (errors, config) => {
+  return errors.map(message => {
+    if (message.url) {
+      message.file = path.relative('.', message.url.replace(path.sep === '\\' ? 'file:/' : 'file:', ''));
+    }
+
+    if (config.absoluteFilePathsForReporter) {
+      message.file = path.resolve(message.file);
+    }
+
+    return message;
+  });
+};
+
+function processErrorMessages(errors, config) {
+  const messages = processedMessages(errors, config);
+
+  if (!config.ignore) {
+    return messages;
+  }
+
+  const ignore = Array.isArray(config.ignore) ? config.ignore : [config.ignore];
+
+  // Iterate over the ignore rules and test the message against each rule.
+  // A match should return false, which causes every() to return false and
+  // the message to be filtered out.
+  return messages.filter(({ message }) => {
+    return ignore.every(
+      currentValue => currentValue instanceof RegExp ?
+        !currentValue.test(message) :
+        normalizeQuotationMarks(currentValue) !== normalizeQuotationMarks(message)
+    );
+  });
+}
+
+module.exports = processErrorMessages;

lib/util.js

@@ -18,6 +18,16 @@ function encode(string) {
   return string || '';
 }
 
+// Replace left/right quotation marks with normal quotation marks
+function normalizeQuotationMarks(string) {
+  if (string) {
+    string = string.replace(/[\u201C\u201D]/g, '"');
+  }
+
+  return string;
+}
+
 module.exports = {
-  encode
+  encode,
+  normalizeQuotationMarks
 };

tasks/html.js

@@ -12,7 +12,7 @@ const path = require('path');
 const htmllint = require('../lib/htmllint.js');
 const reporters = require('../lib/reporters.js');
 
-module.exports = function(grunt) {
+module.exports = grunt => {
   grunt.registerMultiTask('htmllint', 'Validate html files', function() {
     const done = this.async();
     const files = grunt.file.expand(this.filesSrc);