feat: add support for eslint for code style enforcement (#33)

* feat: add eslint * feat: add eslint to js standards * chore: add comments * chore: add comments * chore: add comments * refactor: simplify and electrify * test: foobar * test: remove test * docs: add jsdoc * fix: use sourcetype module * chore: remove sort imports * refactor: rename const to severity * refactor: here we go * refactor: run standardisation tools as a stream * chore: add babel-eslint as dep * refactor: change variable names for readability * refactor: export git commands * chore: add performance measurements for hotspots * chore: improve logging in verbose * chore: print amount of files for total time * chore: consistent logging * fix: abort early if there are no files to work on * refactor: rearrange structure * docs: update readme * docs: fix ugly formatting in readme * refactor: de-duplicate the code that runs js tools * fix: change command to runner * fix: remove confusing name from runner of commitlint * chore: only filter for violations once * fix: change output to debug level * chore: rename underscored functions to camelcase * chore: rename snake case to camel case * fix: add boolean to toggle between check and apply * fix: only write and stage modified files * fix: rename summary to summarize (verb vs. noun) * fix: track modified source * fix: reduce confusing logging * refactor: improve logging * fix: only do things when there are things to do * fix: remove unused prop * fix: make commit check consistent * docs: improve jsdocs * fix: ignore vendor folders for now * fix: make eslint a bit stricter for now * fix: do not use optional filename arg to execute on text
dhis2 · Apr 24, 2019 · 85cf53a · 85cf53a
1 parent 2f1248c
commit 85cf53a
Show file tree

Hide file tree

Showing 17 changed files with 992 additions and 227 deletions.
diff --git a/README.md b/README.md
@@ -9,11 +9,17 @@
 [![travis.com build](https://img.shields.io/travis/com/dhis2/cli-style.svg)](https://travis-ci.com/dhis2/cli-style)
 [![Conventional Commits](https://img.shields.io/badge/Conventional%20Commits-1.0.0-yellow.svg)](https://conventionalcommits.org) [![Greenkeeper badge](https://badges.greenkeeper.io/dhis2/cli-style.svg)](https://greenkeeper.io/)
 
-This tool applies our configuration for:
+This tool enforces DHIS2 configuration for:
 
--   Git commit messages
--   Prettier (prettifies JavaScript)
--   Browserslist
+#### Git
+
+- Git commit messages
+
+#### JavaScript
+
+- Prettier
+- ESLint
+- Browserslist
 
 This tool is part of the [dhis2/cli](https://github.com/dhis2/cli)
 suite, but can also be used standalone which is useful for project level

diff --git a/config/eslint.config.js b/config/eslint.config.js
@@ -0,0 +1,39 @@
+const SEVERITY = 2
+
+module.exports = {
+    root: true,
+
+    parser: 'babel-eslint',
+
+    env: {
+        browser: true,
+        node: true,
+        jest: true,
+    },
+
+    parserOptions: {
+        // latest standard is ok, eq. to 9
+        ecmaVersion: 2018,
+        ecmaFeatures: {
+            jsx: true,
+            modules: true,
+        },
+    },
+
+    rules: {
+        'max-params': [
+            SEVERITY,
+            {
+                max: 3,
+            },
+        ],
+        'prefer-const': [
+            SEVERITY,
+            {
+                destructuring: 'any',
+                ignoreReadBeforeAssign: false,
+            },
+        ],
+        'no-mixed-spaces-and-tabs': [SEVERITY],
+    },
+}
diff --git a/package.json b/package.json
@@ -18,6 +18,9 @@
     "@commitlint/load": "^7.2.1",
     "@commitlint/read": "^7.1.2",
     "@dhis2/cli-helpers-engine": "^1.0.1",
+    "babel-eslint": "^10.0.1",
+    "eslint": "^5.16.0",
+    "perfy": "^1.1.5",
     "prettier": "^1.15.3",
     "yargs": "^13.1.0"
   },

diff --git a/src/cmds/commit_cmds/check.js b/src/cmds/commit_cmds/check.js
@@ -1,5 +1,5 @@
 const log = require('@dhis2/cli-helpers-engine').reporter
-const fmt = require('../../commitlint.js')
+const { runner } = require('../../tools/git')
 
 exports.command = 'check [msg]'
 
@@ -10,9 +10,11 @@ exports.builder = {}
 exports.handler = async function(argv) {
     const { msg } = argv
 
-    const report = await fmt(msg)
+    const report = await runner(msg)
 
-    if (report.valid) {
+    report.summarize()
+
+    if (report.hasViolations) {
         log.info('Commit message is valid')
         process.exit(0)
     } else {

diff --git a/src/cmds/js_cmds/apply.js b/src/cmds/js_cmds/apply.js
@@ -1,8 +1,11 @@
-const { collectFiles, jsFiles } = require('../../files.js')
+const path = require('path')
+
 const log = require('@dhis2/cli-helpers-engine').reporter
 
-const { apply_fmt } = require('../../prettier.js')
-const { stage_files, staged_files } = require('../../git.js')
+const { selectFiles } = require('../../files.js')
+const { stageFiles } = require('../../git-files.js')
+
+const { runner } = require('../../tools/js')
 
 exports.command = 'apply [files..]'
 
@@ -25,34 +28,22 @@ exports.builder = {
 
 exports.handler = argv => {
     const { all, stage, files } = argv
-    const root_dir = process.cwd()
-
-    let codeFiles
-    if (all) {
-        codeFiles = collectFiles(root_dir)
-    } else if (files) {
-        codeFiles = files
-    } else {
-        codeFiles = staged_files(root_dir)
-    }
 
-    // debug information about the folders
-    log.debug('rootDir?', root_dir)
-    log.debug('codeFiles?', codeFiles)
+    const root = process.cwd()
+    log.debug(`Root directory: ${root}`)
 
-    const js = jsFiles(codeFiles)
-    const prettyFiles = apply_fmt(js)
+    const codeFiles = selectFiles(files, all, root)
+    const report = runner(codeFiles, true)
 
-    if (prettyFiles.length === 0) {
-        if (js.length > 0) {
-            log.info(`${js.length} file(s) reformatted.`)
-        } else {
-            log.info('No files to format.')
-        }
+    report.summarize()
+
+    if (report.hasViolations) {
+        process.exit(1)
     }
 
-    if (stage) {
-        const stagedFiles = stage_files(prettyFiles, root_dir)
-        log.debug('Staged files', stagedFiles)
+    const fixed = report.fix()
+
+    if (stage && fixed.length > 0) {
+        stageFiles(fixed, root)
     }
 }
diff --git a/src/cmds/js_cmds/check.js b/src/cmds/js_cmds/check.js
@@ -1,9 +1,7 @@
-const path = require('path')
-const { collectFiles, jsFiles } = require('../../files.js')
 const log = require('@dhis2/cli-helpers-engine').reporter
 
-const { check_fmt } = require('../../prettier.js')
-const { staged_files } = require('../../git.js')
+const { runner } = require('../../tools/js')
+const { selectFiles } = require('../../files.js')
 
 exports.command = 'check [files..]'
 
@@ -19,40 +17,17 @@ exports.builder = {
 }
 
 exports.handler = argv => {
-    const { all, files } = argv
-    const root_dir = process.cwd()
-
-    let codeFiles
-    if (all) {
-        codeFiles = collectFiles(root_dir)
-    } else if (files) {
-        codeFiles = files
-    } else {
-        codeFiles = staged_files(root_dir)
-    }
+    const { files, all } = argv
 
-    // debug information about the folders
-    log.debug('rootDir?', root_dir)
-    log.debug('codeFiles?', codeFiles)
+    const root = process.cwd()
+    log.debug(`Root directory: ${root}`)
 
-    const js = jsFiles(codeFiles)
-    const prettyFiles = check_fmt(js)
+    const codeFiles = selectFiles(files, all, root)
+    const report = runner(codeFiles)
 
-    const success = prettyFiles.length > 0
+    report.summarize()
 
-    if (success) {
-        log.info(`${prettyFiles.length} file(s) needs to be formatted:`)
-        prettyFiles.forEach(f =>
-            log.print(`${path.relative(process.cwd(), f)}`)
-        )
-        log.print('')
+    if (report.hasViolations) {
         process.exit(1)
-    } else {
-        if (js.length > 0) {
-            log.info(`${js.length} file(s) passed the style check.`)
-        } else {
-            log.info('No files to check.')
-        }
-        process.exit(0)
     }
 }
diff --git a/src/config.js b/src/config.js
@@ -5,29 +5,29 @@ const log = require('@dhis2/cli-helpers-engine').reporter
 
 const { readFile, writeFile } = require('./files.js')
 
-function wipe_prop_cfg(repo) {
-    const pkg_path = path.join(repo, 'package.json')
+function wipeConfigProperties(repo) {
+    const pkgPath = path.join(repo, 'package.json')
 
     try {
-        const fd = fs.readFileSync(pkg_path)
+        const fd = fs.readFileSync(pkgPath)
         const data = JSON.parse(fd.toString('utf8'))
 
         delete data.prettier
 
         const out = JSON.stringify(data, null, 2) + '\n'
         try {
-            fs.writeFileSync(pkg_path, out)
-            log.debug(pkg_path + ' updated and saved')
+            fs.writeFileSync(pkgPath, out)
+            log.debug(pkgPath + ' updated and saved')
         } catch (err) {
             log.error('failed to get package.json for repo: ' + repo)
         }
     } catch (e) {
-        log.error('failed to get package.json: ' + pkg_path)
+        log.error('failed to get package.json: ' + pkgPath)
     }
 }
 
-function wipe_file_cfg(repo) {
-    const wipe_cfg_list = [
+function wipeConfigFiles(repo) {
+    const configs = [
         '.prettierrc',
         '.prettierrc.yaml',
         '.prettierrc.yml',
@@ -40,7 +40,7 @@ function wipe_file_cfg(repo) {
         '.editorconfig',
     ]
 
-    wipe_cfg_list.map(cfg => {
+    configs.map(cfg => {
         try {
             fs.unlinkSync(path.join(repo, cfg))
             log.debug(cfg + ' removed from repo: ' + repo)
@@ -51,8 +51,8 @@ function wipe_file_cfg(repo) {
 }
 
 function cleanup(repo) {
-    wipe_prop_cfg(repo)
-    wipe_file_cfg(repo)
+    wipeConfigProperties(repo)
+    wipeConfigFiles(repo)
 }
 
 function copy(from, to) {

diff --git a/src/files.js b/src/files.js
@@ -3,7 +3,9 @@ const path = require('path')
 
 const log = require('@dhis2/cli-helpers-engine').reporter
 
-const blacklist = ['node_modules', 'build', 'dist', 'target', '.git']
+const { stagedFiles } = require('./git-files.js')
+
+const blacklist = ['node_modules', 'build', 'dist', 'target', '.git', 'vendor']
 
 const whitelists = {
     js: ['.js', '.jsx', '.ts'],
@@ -48,6 +50,19 @@ function collectFiles(target) {
         .reduce((a, b) => a.concat(b), [])
 }
 
+function selectFiles(files, all, dir) {
+    let codeFiles
+    if (all) {
+        codeFiles = collectFiles(dir)
+    } else if (files) {
+        codeFiles = files
+    } else {
+        codeFiles = stagedFiles(dir)
+    }
+
+    return codeFiles
+}
+
 function readFile(fp) {
     try {
         const text = fs.readFileSync(fp, 'utf8')
@@ -72,6 +87,7 @@ module.exports = {
     collectFiles,
     collectAllFiles,
     collectJsFiles,
+    selectFiles,
     jsFiles,
     readFile,
     writeFile,

diff --git a/src/git-files.js b/src/git-files.js
@@ -0,0 +1,41 @@
+/**
+ * This requires that there is a Git binary on $PATH
+ */
+const { execSync } = require('child_process')
+
+const log = require('@dhis2/cli-helpers-engine').reporter
+
+const stageFile = (file, dir) => {
+    log.debug(`Staging ${file}...`)
+    const added = execSync(`git add ${file}`, {
+        cwd: dir,
+        encoding: 'utf8',
+    })
+    return file
+}
+
+const stageFiles = (files, dir) => {
+    const staged = files.map(f => stageFile(f, dir))
+    log.info(`Staged ${files.length} file(s).`)
+    return staged
+}
+
+const stagedFiles = dir => {
+    const files = execSync('git diff --cached --name-only --relative', {
+        cwd: dir,
+        encoding: 'utf8',
+    }).trim()
+    if (!!files) {
+        const s = files.split('\n')
+        log.info(`Fetching staged files: ${s.length} file(s).`)
+        return s
+    }
+    log.info('No staged files found.')
+    return []
+}
+
+module.exports = {
+    stagedFiles,
+    stageFiles,
+    stageFile,
+}
diff --git a/src/git.js b/src/git.js