Linter refactor (#78)

* Adding new linter interface and runner
* Switch code and tests to new linter runner
* Added documentation
* Linter fixes
* Log linter results
* Remove obsolete files
* runner: return default report when there are no files to analyze
* travis: add cache dir to GOPATH
* baseline: remove leftovers
* Use 'safe' and 'vulnerable' to describe all instances of test files
* refactor: handle array of linters instead of individual ones
* Move report path resolution to runner

Signed-off-by: Antoine Salon <[email protected]>

.travis.yml

@@ -7,7 +7,7 @@ install:
   - yarn install
   - pip3 install --user -r requirements.txt
   - curl -sfL https://raw.githubusercontent.com/securego/gosec/master/install.sh | sh -s -- -b $GOPATH/bin 1.2.0
-  - export GOPATH=$GOPATH:${TRAVIS_BUILD_DIR}/test/fixtures/go
+  - export GOPATH=$GOPATH:${TRAVIS_BUILD_DIR}/test/fixtures/go:${TRAVIS_BUILD_DIR}/cache/go
   - npm install -g codecov
 
 script:

index.js

@@ -1,16 +1,10 @@
 // Copyright 2018 VMware, Inc.
 // SPDX-License-Identifier: BSD-2-Clause
 
-const runBandit = require('./bandit/bandit')
-const runGosec = require('./gosec/gosec')
-const generateBanditReport = require('./bandit/bandit_report')
-const generateGosecReport = require('./gosec/gosec_report')
-const mergeReports = require('./merge_reports')
 const cache = require('./cache')
 const { config } = require('./config')
 const apiHelper = require('./github_api_helper')
-
-const path = require('path')
+const { runLinters } = require('./runner')
 
 /**
  * @param {import('probot').Application} app - Probot's Application class.
@@ -80,17 +74,10 @@ async function runLinterFromPRData (pullRequests, context, headSha) {
     const PR = pullRequests[0]
     const inputFiles = resolvedPRs[0]
 
-    const banditResults = await runBandit(cache.getBranchPath(repoID, PR.id, 'bandit'), inputFiles)
-    const banditReport = generateBanditReport(banditResults, cache.getBranchPath(repoID, PR.id, 'bandit'))
-
-    const gosecResults = await runGosec(cache.getBranchPath(repoID, PR.id, 'gosec'), inputFiles)
-    const gosecReport = generateGosecReport(gosecResults, path.resolve(cache.getBranchPath(repoID, PR.id, 'gosec')))
+    const report = await runLinters(inputFiles, repoID, PR.id)
 
-    const output = mergeReports(banditReport, gosecReport)
-    const resolvedCheckRunResponse = await checkRunResponse
-    const runID = resolvedCheckRunResponse.data.id
-    // Send results using the octokit APIrunID
-    apiHelper.sendResults(context, runID, output)
+    const runID = (await checkRunResponse).data.id
+    apiHelper.sendResults(context, runID, report)
 
     if (config.cleanupAfterRun) {
       cache.clear(repoID, PR.id)

linters/bandit.js

@@ -0,0 +1,68 @@
+// Copyright 2018 VMware, Inc.
+// SPDX-License-Identifier: BSD-2-Clause
+
+const cache = require('../cache')
+
+const report = require('../bandit/bandit_report')
+
+module.exports = class Bandit {
+  get name () {
+    return 'bandit'
+  }
+
+  /**
+   * The name of the generated report file
+   */
+  get reportFile () {
+    return 'bandit.json'
+  }
+
+  get defaultReport () {
+    return report(null)
+  }
+
+  /**
+   * Retains files that can be analyzed by this linter
+   * @param {string[]} files Names of files to analyze
+   * @returns {string[]} Filtered list of file names
+   */
+  filter (files) {
+    return files.filter(name => name.endsWith('.py'))
+  }
+
+  /**
+   * Returns the working directory for this analysis
+   * @param {string} repoID Unique repository id
+   * @param {string} prID PR id in repository
+   */
+  workingDirectoryForPR (repoID, prID) {
+    return cache.getBranchPath(repoID, prID, 'bandit')
+  }
+
+  /**
+   * Builds the command line args to pass to the linter process
+   * @param {string[]} files List of files to analyze
+   * @param {string} reportPath Path to the report file relative to working directory
+   */
+  args (files, reportPath) {
+    return ['--format', 'json', '-o', reportPath, ...files]
+  }
+
+  /**
+   * Parses the linter results
+   * @param {Buffer} data The raw linter results data
+   */
+  parseResults (data) {
+    return JSON.parse(data)
+  }
+
+  /**
+   * Generates a report in the format expected by GitHub checks
+   * from the linter results
+   * @param {any} results Linter results
+   * @returns GitHub checks report
+   */
+  generateReport (results) {
+    return report(results)
+  }
+}

linters/gosec.js

@@ -0,0 +1,68 @@
+// Copyright 2018 VMware, Inc.
+// SPDX-License-Identifier: BSD-2-Clause
+
+const cache = require('../cache')
+
+const report = require('../gosec/gosec_report')
+
+module.exports = class Gosec {
+  get name () {
+    return 'gosec'
+  }
+
+  /**
+   * The name of the generated report file
+   */
+  get reportFile () {
+    return 'gosec.json'
+  }
+
+  get defaultReport () {
+    return report(null)
+  }
+
+  /**
+   * Retains files that can be analyzed by this linter
+   * @param {string[]} files Names of files to analyze
+   * @returns {string[]} Filtered list of file names
+   */
+  filter (files) {
+    return files.filter(name => name.endsWith('.go'))
+  }
+
+  /**
+   * Returns the working directory for this analysis
+   * @param {string} repoID Unique repository id
+   * @param {string} prID PR id in repository
+   */
+  workingDirectoryForPR (repoID, prID) {
+    return cache.getBranchPath(repoID, prID, 'gosec')
+  }
+
+  /**
+   * Builds the command line args to pass to the linter process
+   * @param {string[]} files List of files to analyze
+   * @param {string} reportPath Path to the report file relative to working directory
+   */
+  args (files, reportPath) {
+    return ['-fmt=json', '-out', reportPath, './...']
+  }
+
+  /**
+   * Parses the linter results
+   * @param {Buffer} data The raw linter results data
+   */
+  parseResults (data) {
+    return JSON.parse(data)
+  }
+
+  /**
+   * Generates a report in the format expected by GitHub checks
+   * from the linter results
+   * @param {any} results Linter results
+   * @returns GitHub checks report
+   */
+  generateReport (results) {
+    return report(results)
+  }
+}

linters/index.js

@@ -0,0 +1,10 @@
+// Copyright 2018 VMware, Inc.
+// SPDX-License-Identifier: BSD-2-Clause
+
+const Bandit = require('./bandit')
+const Gosec = require('./gosec')
+
+module.exports = {
+  BANDIT: new Bandit(),
+  GOSEC: new Gosec()
+}

runner.js

@@ -0,0 +1,69 @@
+// Copyright 2018 VMware, Inc.
+// SPDX-License-Identifier: BSD-2-Clause
+
+const { spawn } = require('child_process')
+const fs = require('fs')
+const path = require('path')
+
+const merge = require('./merge_reports')
+const linters = require('./linters')
+
+/**
+ * Run all linters on specified files
+ * @param {string[]} files Files to analyze
+ * @param {string} repoID
+ * @param {string} prID
+ */
+async function runLinters (files, repoID, prID) {
+  // TODO: Sync directory with file download location resolution
+  const reports = Object.values(linters).map((linter) => run(linter, linter.workingDirectoryForPR(repoID, prID), files))
+  const resolved = await Promise.all(reports)
+
+  // TODO: rewrite merge to handle list of reports
+  return merge(resolved[0], resolved[1])
+}
+
+/**
+ * Linter driver logic: spawn a child process, gather the results and build
+ * a report
+ * @param {*} linter A linter instance
+ * @param {string} workingDirectory The path to the process working directory
+ * @param {string[]} files Files to analyze
+ * @returns {Promise<any>} A promise for the report object with the analysis results
+ */
+function run (linter, workingDirectory, files) {
+  const filtered = linter.filter(files)
+
+  if (filtered.length === 0) { return linter.defaultReport }
+
+  const reportFilePath = path.join(workingDirectory, '..', linter.reportFile)
+  const process = spawn(linter.name, linter.args(filtered, path.join('..', linter.reportFile)), { cwd: workingDirectory })
+
+  let errorLogs = ''
+  process.stderr.on('data', (chunk) => {
+    errorLogs += chunk.toString()
+  })
+
+  // Promise report generation
+  return new Promise((resolve, reject) => {
+    process.on('error', reject)
+    process.on('close', () => reportHandler(linter, reportFilePath, resolve, reject, errorLogs))
+  })
+}
+
+function reportHandler (linter, reportFilePath, resolve, reject, logs) {
+  fs.readFile(reportFilePath, 'utf8', (err, data) => {
+    if (err) {
+      console.log('Could not read linter results: ' + reportFilePath)
+      console.log('stderr: ' + logs)
+      return reject(err)
+    } else {
+      const results = linter.parseResults(data)
+      const report = linter.generateReport(results)
+      return resolve(report)
+    }
+  })
+}
+
+module.exports.runLinters = runLinters
+module.exports.run = run