refactor: port the cpplint runner to javascript (#14440)

* refactor: port the cpplint runner to javascript

* fix: address code review suggestions

* refactor: make .cc, bootstrapped test funcs inline

* refactor: make CC_ROOTS inline too

* fix: test process.mainModule before calling main()

* refactor: reverse logic order in findChangedFiles

* refactor: make findChangedFiles() more readable

* fix: copy-paste error introduced in 3b17400

* chore: fix grammar in log message

package.json

@@ -22,6 +22,7 @@
     "node-fetch": "^2.1.2",
     "nugget": "^2.0.1",
     "octicons": "^7.3.0",
+    "recursive-readdir": "^2.2.2",
     "remark-cli": "^4.0.0",
     "remark-preset-lint-markdown-style-guide": "^2.1.1",
     "request": "^2.68.0",
@@ -62,7 +63,7 @@
     "lint": "npm run lint:js && npm run lint:cpp && npm run lint:clang-format && npm run lint:py && npm run lint:docs",
     "lint:js": "standard && cd spec && standard",
     "lint:clang-format": "python script/run-clang-format.py -r -c atom/ chromium_src/ brightray/ || (echo \"\\nCode not formatted correctly.\" && exit 1)",
-    "lint:cpp": "python ./script/cpplint.py",
+    "lint:cpp": "node ./script/cpplint.js",
     "lint:py": "python ./script/pylint.py",
     "lint:docs": "remark docs -qf && npm run lint:js-in-markdown && npm run create-typescript-definitions && npm run lint:docs-relative-links",
     "lint:docs-relative-links": "python ./script/check-relative-doc-links.py",

script/cpplint.js

@@ -0,0 +1,115 @@
+#!/usr/bin/env node
+
+const { GitProcess } = require('dugite')
+const childProcess = require('child_process')
+const fs = require('fs')
+const klaw = require('klaw')
+const minimist = require('minimist')
+const path = require('path')
+
+const SOURCE_ROOT = path.normalize(path.dirname(__dirname))
+const LINTER_PATH = path.join(SOURCE_ROOT, 'vendor', 'depot_tools', 'cpplint.py')
+
+function callCpplint (filenames, args) {
+  if (args.verbose) console.log([LINTER_PATH, ...filenames].join(' '))
+  try {
+    console.log(String(childProcess.execFileSync(LINTER_PATH, filenames, {cwd: SOURCE_ROOT})))
+  } catch (e) {
+    process.exit(1)
+  }
+}
+
+function parseCommandLine () {
+  let help
+  const opts = minimist(process.argv.slice(2), {
+    boolean: ['help', 'onlyChanged', 'verbose'],
+    default: { help: false, onlyChanged: false, verbose: false },
+    alias: { c: 'onlyChanged', 'only-changed': 'onlyChanged', h: 'help', v: 'verbose' },
+    unknown: arg => { help = true }
+  })
+  if (help || opts.help) {
+    console.log('Usage: cpplint.js [-h|--help] [-c|--only-changed] [-v|--verbose]')
+    process.exit(0)
+  }
+  return opts
+}
+
+async function findChangedFiles (top) {
+  const result = await GitProcess.exec(['diff', '--name-only'], top)
+  if (result.exitCode !== 0) {
+    console.log('Failed to find changed files', GitProcess.parseError(result.stderr))
+    process.exit(1)
+  }
+  const relativePaths = result.stdout.split(/\r\n|\r|\n/g)
+  const absolutePaths = relativePaths.map(x => path.join(top, x))
+  return new Set(absolutePaths)
+}
+
+async function findFiles (top, test) {
+  return new Promise((resolve, reject) => {
+    const matches = []
+    klaw(top)
+      .on('end', () => resolve(matches))
+      .on('data', item => {
+        if (test(item.path)) matches.push(item.path)
+      })
+  })
+}
+
+const blacklist = new Set([
+  ['atom', 'browser', 'mac', 'atom_application.h'],
+  ['atom', 'browser', 'mac', 'atom_application_delegate.h'],
+  ['atom', 'browser', 'resources', 'win', 'resource.h'],
+  ['atom', 'browser', 'ui', 'cocoa', 'atom_menu_controller.h'],
+  ['atom', 'browser', 'ui', 'cocoa', 'atom_ns_window.h'],
+  ['atom', 'browser', 'ui', 'cocoa', 'atom_ns_window_delegate.h'],
+  ['atom', 'browser', 'ui', 'cocoa', 'atom_preview_item.h'],
+  ['atom', 'browser', 'ui', 'cocoa', 'atom_touch_bar.h'],
+  ['atom', 'browser', 'ui', 'cocoa', 'touch_bar_forward_declarations.h'],
+  ['atom', 'browser', 'ui', 'cocoa', 'NSColor+Hex.h'],
+  ['atom', 'browser', 'ui', 'cocoa', 'NSString+ANSI.h'],
+  ['atom', 'common', 'api', 'api_messages.h'],
+  ['atom', 'common', 'common_message_generator.cc'],
+  ['atom', 'common', 'common_message_generator.h'],
+  ['atom', 'common', 'node_includes.h'],
+  ['atom', 'node', 'osfhandle.cc'],
+  ['brightray', 'browser', 'mac', 'bry_inspectable_web_contents_view.h'],
+  ['brightray', 'browser', 'mac', 'event_dispatching_window.h'],
+  ['brightray', 'browser', 'mac', 'notification_center_delegate.h'],
+  ['brightray', 'browser', 'win', 'notification_presenter_win7.h'],
+  ['brightray', 'browser', 'win', 'win32_desktop_notifications', 'common.h'],
+  ['brightray', 'browser', 'win', 'win32_desktop_notifications',
+    'desktop_notification_controller.cc'],
+  ['brightray', 'browser', 'win', 'win32_desktop_notifications',
+    'desktop_notification_controller.h'],
+  ['brightray', 'browser', 'win', 'win32_desktop_notifications', 'toast.h'],
+  ['brightray', 'browser', 'win', 'win32_notification.h']
+].map(tokens => path.join(SOURCE_ROOT, ...tokens)))
+
+async function main () {
+  if (!fs.existsSync(LINTER_PATH)) {
+    print('[INFO] Skipping cpplint, dependencies have not been bootstrapped')
+    return
+  }
+
+  const args = parseCommandLine()
+
+  let filenames = []
+  const isCCFile = filename => filename.endsWith('.cc') || filename.endsWith('.h')
+  const CC_ROOTS = ['atom', 'brightray'].map(x => path.join(SOURCE_ROOT, x))
+  for (const root of CC_ROOTS) {
+    const files = await findFiles(root, isCCFile)
+    filenames.push(...files)
+  }
+
+  filenames = filenames.filter(x => !blacklist.has(x))
+
+  if (args.onlyChanged) {
+    const whitelist = await findChangedFiles(SOURCE_ROOT)
+    filenames = filenames.filter(x => whitelist.has(x))
+  }
+
+  if (filenames.length) callCpplint(filenames, args)
+}
+
+if (process.mainModule === module) main()

script/cpplint.py

@@ -1,110 +0,0 @@
-#!/usr/bin/env python
-
-import argparse
-import os
-import sys
-
-from lib.config import enable_verbose_mode
-from lib.util import execute
-
-IGNORE_FILES = set(os.path.join(*components) for components in [
-  ['atom', 'browser', 'mac', 'atom_application.h'],
-  ['atom', 'browser', 'mac', 'atom_application_delegate.h'],
-  ['atom', 'browser', 'resources', 'win', 'resource.h'],
-  ['atom', 'browser', 'ui', 'cocoa', 'atom_menu_controller.h'],
-  ['atom', 'browser', 'ui', 'cocoa', 'atom_ns_window.h'],
-  ['atom', 'browser', 'ui', 'cocoa', 'atom_ns_window_delegate.h'],
-  ['atom', 'browser', 'ui', 'cocoa', 'atom_preview_item.h'],
-  ['atom', 'browser', 'ui', 'cocoa', 'atom_touch_bar.h'],
-  ['atom', 'browser', 'ui', 'cocoa', 'touch_bar_forward_declarations.h'],
-  ['atom', 'browser', 'ui', 'cocoa', 'NSColor+Hex.h'],
-  ['atom', 'browser', 'ui', 'cocoa', 'NSString+ANSI.h'],
-  ['atom', 'common', 'api', 'api_messages.h'],
-  ['atom', 'common', 'common_message_generator.cc'],
-  ['atom', 'common', 'common_message_generator.h'],
-  ['atom', 'common', 'node_includes.h'],
-  ['atom', 'node', 'osfhandle.cc'],
-  ['brightray', 'browser', 'mac', 'bry_inspectable_web_contents_view.h'],
-  ['brightray', 'browser', 'mac', 'event_dispatching_window.h'],
-  ['brightray', 'browser', 'mac', 'notification_center_delegate.h'],
-  ['brightray', 'browser', 'win', 'notification_presenter_win7.h'],
-  ['brightray', 'browser', 'win', 'win32_desktop_notifications', 'common.h'],
-  ['brightray', 'browser', 'win', 'win32_desktop_notifications',
-   'desktop_notification_controller.cc'],
-  ['brightray', 'browser', 'win', 'win32_desktop_notifications',
-   'desktop_notification_controller.h'],
-  ['brightray', 'browser', 'win', 'win32_desktop_notifications', 'toast.h'],
-  ['brightray', 'browser', 'win', 'win32_notification.h']
-])
-
-SOURCE_ROOT = os.path.abspath(os.path.dirname(os.path.dirname(__file__)))
-
-
-def main():
-
-  parser = argparse.ArgumentParser(
-    description="Run cpplint on Electron's C++ files",
-    formatter_class=argparse.RawTextHelpFormatter
-  )
-  parser.add_argument(
-    '-c', '--only-changed',
-    action='store_true',
-    default=False,
-    dest='only_changed',
-    help='only run on changed files'
-  )
-  parser.add_argument(
-    '-v', '--verbose',
-    action='store_true',
-    default=False,
-    dest='verbose',
-    help='show cpplint output'
-  )
-  args = parser.parse_args()
-
-  if not os.path.isfile(cpplint_path()):
-    print("[INFO] Skipping cpplint, dependencies has not been bootstrapped")
-    return
-
-  if args.verbose:
-    enable_verbose_mode()
-
-  os.chdir(SOURCE_ROOT)
-  files = find_files(['atom', 'brightray'], is_cpp_file)
-  files -= IGNORE_FILES
-  if args.only_changed:
-    files &= find_changed_files()
-  call_cpplint(files)
-
-
-def find_files(roots, test):
-  matches = set()
-  for root in roots:
-    for parent, _, children, in os.walk(root):
-      for child in children:
-        filename = os.path.join(parent, child)
-        if test(filename):
-          matches.add(filename)
-  return matches
-
-
-def is_cpp_file(filename):
-  return filename.endswith('.cc') or filename.endswith('.h')
-
-
-def find_changed_files():
-  return set(execute(['git', 'diff', '--name-only']).splitlines())
-
-
-def call_cpplint(files):
-  if files:
-    cpplint = cpplint_path()
-    execute([sys.executable, cpplint] + list(files))
-
-
-def cpplint_path():
-  return os.path.join(SOURCE_ROOT, 'vendor', 'depot_tools', 'cpplint.py')
-
-
-if __name__ == '__main__':
-  sys.exit(main())