feat: cli can now output json with the --json flag (#76)

* add command line option to have output be in JSON format * ensure reporter files are pushed, update test describe blocks * Adds docker-compose.test for running unit tests * update readme for options * dd json output option: fix build because of inproper test setup * raise error when attempting buildReport on Reporter object * cleanup CLI options, remove unused code and duplcated tests * dd json output option: remove function that was no longer being used * dd json output option: dry out usage messages * dd json output option: fix cli_reporter test that failed in docker-compose, remove extra line * indentation
replicatedhq · Jul 3, 2017 · c4b13d2 · c4b13d2
1 parent 9482bd9
commit c4b13d2
Show file tree

Hide file tree

Showing 9 changed files with 456 additions and 187 deletions.
diff --git a/README.md b/README.md
@@ -6,14 +6,37 @@
 `Dockerfilelint` is an node module that analyzes a Dockerfile and looks for common traps, mistakes and helps enforce best practices:
 
 ## Testing
-Start unit tests with `npm test` or `yarn run test`
+Start unit tests with `npm test`, `yarn run test`, or `docker-compose -f docker-compose.test.yml up`
 
 ## Running
 #### From the command line:
 ```shell
 ./bin/dockerfilelint <path/to/Dockerfile>
 ```
 
+#### Command Line options
+```shell
+Usage: dockerfilelint [files | content..] [options]
+
+Options:
+  -o, --output  Specify the format to use for output of linting results. Valid values
+                are `json` or `cli` (default).                               [string]
+  -j, --json    Output linting results as JSON, equivalent to `-o json`.    [boolean]
+  -h, --help    Show help                                                   [boolean]
+
+Examples:
+  dockerfilelint Dockerfile         Lint a Dockerfile in the current working
+                                    directory
+
+  dockerfilelint test/example/* -j  Lint all files in the test/example directory and
+                                    output results in JSON
+
+  dockerfilelint 'FROM latest'      Lint the contents given as a string on the
+                                    command line
+
+  dockerfilelint < Dockerfile       Lint the contents of Dockerfile via stdin
+```
+
 #### Configuring
 You can configure the linter by creating a `.dockerfilelintrc` with the following syntax:
 ```yaml

diff --git a/bin/dockerfilelint b/bin/dockerfilelint
@@ -4,14 +4,36 @@ var process = require('process');
 var fs = require('fs');
 var os = require("os");
 var path = require('path');
-var argv = require('yargs').argv;
+var usage = 'Usage: dockerfilelint [files | content..] [options]';
+var argv = require('yargs')
+  .usage(usage)
+  .option('o', {
+    alias: 'output',
+    desc: 'Specify the format to use for output of linting results. Valid values are `json` or `cli` (default).',
+    type: 'string'
+  })
+  .option('j', {
+    alias: 'json',
+    desc: 'Output linting results as JSON, equivalent to `-o json`.',
+    type: 'boolean'
+  })
+  .help().alias('h', 'help')
+  .example('dockerfilelint Dockerfile', 'Lint a Dockerfile in the current working directory\n')
+  .example('dockerfilelint test/example/* -j', 'Lint all files in the test/example directory and output results in JSON\n')
+  .example(`dockerfilelint 'FROM latest'`, 'Lint the contents given as a string on the command line\n')
+  .example('dockerfilelint < Dockerfile', 'Lint the contents of Dockerfile via stdin')
+  .wrap(85)
+  .check(argv => {
+    if (!argv.output && argv.json) argv.output = 'json'
+    return true
+  })
+  .argv;
+
 var dockerfilelint = require('../lib/index');
 var chalk = require('chalk');
 
-// reporter could be a command line option
-// but until we have others, just use CliReporter
-var CliReporter = require('../lib/cli_reporter');
-var reporter = new CliReporter();
+var Reporter = argv.output === 'json' ? require('../lib/reporter/json_reporter') : require('../lib/reporter/cli_reporter');
+var reporter = new Reporter();
 
 var fileContent, configFilePath;
 if (argv._.length === 0 || argv._[0] === '-') {
@@ -26,7 +48,7 @@ if (argv._.length === 0 || argv._[0] === '-') {
   });
   return process.stdin.on('end', function () {
     if (fileContent.length === 0) {
-      console.error('Usage: dockerfilelint <filename or dockerfile contents>');
+      console.error(usage);
       return process.exit(1);
     }
     processContent(configFilePath, '<stdin>', fileContent);

diff --git a/docker-compose.test.yml b/docker-compose.test.yml
@@ -0,0 +1,10 @@
+version: '2'
+services:
+  dockerfilelint:
+    build: .
+    image: dockerfilelint
+    entrypoint: npm test
+    volumes:
+      - ./bin:/dockerfilelint/bin
+      - ./lib:/dockerfilelint/lib
+      - ./test:/dockerfilelint/test
diff --git a/lib/cli_reporter.js → lib/reporter/cli_reporter.js b/lib/cli_reporter.js → lib/reporter/cli_reporter.js
@@ -1,8 +1,8 @@
 'use strict';
 
-const notDeepStrictEqual = require('assert').notDeepStrictEqual;
 const chalk = require('chalk');
 const cliui = require('cliui');
+const Reporter = require('./reporter');
 
 const DEFAULT_TOTAL_WIDTH = 110;
 const ISSUE_COL0_WIDTH = 5;
@@ -12,8 +12,9 @@ const ISSUE_TITLE_WIDTH_MAX = 40;
 const PAD_TOP0_LEFT2 = [0, 0, 0, 2];
 const PAD_TOP1_LEFT0 = [1, 0, 0, 0];
 
-class CliReporter {
+class CliReporter extends Reporter {
   constructor (opts) {
+    super(opts);
     opts = opts || { width: DEFAULT_TOTAL_WIDTH, wrap: true };
     opts.width = parseInt(opts.width, 10) || DEFAULT_TOTAL_WIDTH;
     this.ui = cliui(opts);
@@ -24,37 +25,6 @@ class CliReporter {
       'Clarity': chalk.cyan,
       'Optimization': chalk.cyan
     };
-    this.fileReports = {};
-  }
-
-  // group file items by line for easy reporting
-  addFile (file, fileContent, items) {
-    let self = this;
-    if (!file) return self;
-    let fileReport = self.fileReports[file] || {
-      itemsByLine: {},
-      uniqueIssues: 0,
-      contentArray: (fileContent || '').replace('\r', '').split('\n')
-    };
-    let ibl = fileReport.itemsByLine;
-    [].concat(items).forEach((item) => {
-      if (ibl[String(item.line)]) {
-        try {
-          ibl[String(item.line)].forEach((lineItem) => {
-            notDeepStrictEqual(item, lineItem);
-          });
-          ibl[String(item.line)].push(item);
-          fileReport.uniqueIssues = fileReport.uniqueIssues + 1;
-        } catch (err) {
-          // ignore duplicate
-        }
-      } else {
-        ibl[String(item.line)] = [ item ];
-        fileReport.uniqueIssues = fileReport.uniqueIssues + 1;
-      }
-    });
-    self.fileReports[file] = fileReport;
-    return self;
   }
 
   // build a report object for data given via addFile

diff --git a/lib/reporter/json_reporter.js b/lib/reporter/json_reporter.js
@@ -0,0 +1,71 @@
+// {
+//   "file": "file",
+//   "issues_count": 3,
+//   "issues" : [
+//     {
+//       "line": 4,
+//       "content": "FROM ...",
+//       "category": "clarity",
+//       "title": "...",
+//       "description": "...."
+//     }, ..., {}
+//   ]
+// }
+
+'use strict';
+
+const Reporter = require('./reporter');
+
+class JsonReporter extends Reporter {
+  constructor (opts) {
+    super(opts);
+
+    this.json = {};
+  }
+
+  // build a report object for data given via addFile
+  buildReport () {
+    let self = this;
+    let totalIssues = 0;
+    let reportFiles = [];
+
+    Object.keys(self.fileReports).forEach((file) => {
+      let fileReport = self.fileReports[file];
+      let linesWithItems = Object.keys(fileReport.itemsByLine);
+
+      let jsonReport = { file: file, issues_count: fileReport.uniqueIssues, issues: [] };
+
+      if (linesWithItems.length === 0) {
+        reportFiles.push(jsonReport);
+        return;
+      }
+
+      totalIssues += fileReport.uniqueIssues;
+
+      linesWithItems.forEach((lineNum) => {
+        let lineContent = fileReport.contentArray[parseInt(lineNum, 10) - 1];
+
+        fileReport.itemsByLine[lineNum].forEach((item) => {
+          let lineIssueJson = {
+            line: lineNum,
+            content: lineContent,
+            category: item.category,
+            title: item.title,
+            description: item.description
+          };
+
+          jsonReport.issues.push(lineIssueJson);
+        });
+      });
+
+      reportFiles.push(jsonReport);
+    });
+
+    this.json.files = reportFiles;
+    this.json.totalIssues = totalIssues;
+
+    return { toString: () => JSON.stringify(this.json), totalIssues: totalIssues };
+  }
+}
+
+module.exports = JsonReporter;
diff --git a/lib/reporter/reporter.js b/lib/reporter/reporter.js
@@ -0,0 +1,47 @@
+'use strict';
+
+const notDeepStrictEqual = require('assert').notDeepStrictEqual;
+
+class Reporter {
+  constructor (opts) {
+    this.fileReports = {};
+  }
+
+  // group file items by line for easy reporting
+  addFile (file, fileContent, items) {
+    let self = this;
+    if (!file) return self;
+    let fileReport = self.fileReports[file] || {
+      itemsByLine: {},
+      uniqueIssues: 0,
+      contentArray: (fileContent || '').replace('\r', '').split('\n')
+    };
+    let ibl = fileReport.itemsByLine;
+    [].concat(items).forEach((item) => {
+      if (ibl[String(item.line)]) {
+        try {
+          ibl[String(item.line)].forEach((lineItem) => {
+            notDeepStrictEqual(item, lineItem);
+          });
+          ibl[String(item.line)].push(item);
+          fileReport.uniqueIssues = fileReport.uniqueIssues + 1;
+        } catch (err) {
+          // ignore duplicate
+        }
+      } else {
+        ibl[String(item.line)] = [ item ];
+        fileReport.uniqueIssues = fileReport.uniqueIssues + 1;
+      }
+    });
+    self.fileReports[file] = fileReport;
+    return self;
+  }
+
+  // build a report object for data given via addFile
+  buildReport () {
+    // TO BE OVERRIDDEN BY SUB CLASSES
+    throw new Error("#buildReport() must be defined in a child class");
+  }
+}
+
+module.exports = Reporter;