refactor: use one script to launch all linters (#14622)
* refactor: use one script to launch py, js, cc linters * lint changed files as precommit hook * fix py linter in GN build * add overlooked files to linter search path
This commit is contained in:
Charles Kerr
GitHub
parent
ab47ad7bbc
commit
eeeeae3bff
7 changed files with 182 additions and 154 deletions
|
|
@ -1,4 +0,0 @@
|
|||
vendor
|
||||
node_modules
|
||||
spec/node_modules
|
||||
spec/static
|
||||
|
|
@ -8,10 +8,12 @@ pool:
|
|||
|
||||
steps:
|
||||
- bash: |
|
||||
git clone https://chromium.googlesource.com/chromium/tools/depot_tools.git "${AGENT_BUILDDIRECTORY}/depot_tools"
|
||||
git clone https://chromium.googlesource.com/chromium/tools/depot_tools.git "${AGENT_BUILDDIRECTORY}/third_party/depot_tools"
|
||||
echo "##vso[task.setvariable variable=PATH]$PATH:${AGENT_BUILDDIRECTORY}/depot_tools"
|
||||
name: Setup_depot_tools
|
||||
|
||||
- bash: |
|
||||
export PATH="$PATH:${AGENT_BUILDDIRECTORY}/third_party/depot_tools"
|
||||
echo "##vso[task.setvariable variable=PATH]$PATH"
|
||||
npm install
|
||||
npm run lint
|
||||
|
|
|
|||
10
package.json
10
package.json
|
|
@ -43,18 +43,18 @@
|
|||
"bump-version": "./script/bump-version.py",
|
||||
"check-tls": "python ./script/tls.py",
|
||||
"clang-format": "find atom/ brightray/ chromium_src/ -iname *.h -o -iname *.cc -o -iname *.mm | xargs clang-format -i",
|
||||
"lint": "npm run lint:js && npm run lint:cpp && npm run lint:clang-format && npm run lint:py && npm run lint:docs",
|
||||
"lint:js": "eslint . --cache",
|
||||
"lint": "./script/lint.js && npm run lint:clang-format && npm run lint:docs",
|
||||
"lint:js": "./script/lint.js --js",
|
||||
"lint:clang-format": "python script/run-clang-format.py -r -c atom/ chromium_src/ brightray/ || (echo \"\\nCode not formatted correctly.\" && exit 1)",
|
||||
"lint:cpp": "node ./script/cpplint.js",
|
||||
"lint:py": "python ./script/pylint.py",
|
||||
"lint:cpp": "./script/lint.js --cc",
|
||||
"lint:py": "./script/lint.js --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",
|
||||
"lint:js-in-markdown": "standard-markdown docs",
|
||||
"create-api-json": "electron-docs-linter docs --outfile=electron-api.json",
|
||||
"create-typescript-definitions": "npm run create-api-json && electron-typescript-definitions --in=electron-api.json --out=electron.d.ts",
|
||||
"preinstall": "node -e 'process.exit(0)'",
|
||||
"precommit": "python script/run-clang-format.py -r -c atom/ chromium_src/ brightray/ && node ./script/cpplint.js -c && npm run lint:js && remark docs -qf || (echo \"Code not formatted correctly.\" && exit 1)",
|
||||
"precommit": "python script/run-clang-format.py -r -c atom/ chromium_src/ brightray/ && ./script/lint.js -c && remark docs -qf || (echo \"Code not formatted correctly.\" && exit 1)",
|
||||
"prepack": "check-for-leaks",
|
||||
"prepush": "check-for-leaks",
|
||||
"repl": "node ./script/start.js --interactive",
|
||||
|
|
|
|||
|
|
@ -1,120 +0,0 @@
|
|||
#!/usr/bin/env node
|
||||
|
||||
const { GitProcess } = require('dugite')
|
||||
const childProcess = require('child_process')
|
||||
const klaw = require('klaw')
|
||||
const minimist = require('minimist')
|
||||
const path = require('path')
|
||||
|
||||
const SOURCE_ROOT = path.normalize(path.dirname(__dirname))
|
||||
|
||||
function callCpplint (filenames, args) {
|
||||
const linter = 'cpplint.py'
|
||||
if (args.verbose) console.log([linter, ...filenames].join(' '))
|
||||
try {
|
||||
childProcess.execFile(linter, filenames, { cwd: SOURCE_ROOT }, error => {
|
||||
if (error) {
|
||||
for (const line of error.message.split(/[\r\n]+/)) {
|
||||
if (line.length && !line.startsWith('Done processing ')) {
|
||||
console.warn(line)
|
||||
}
|
||||
}
|
||||
if (error.message.includes('Command failed')) {
|
||||
process.exit(1)
|
||||
}
|
||||
}
|
||||
})
|
||||
} 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', 'HEAD', '--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 () {
|
||||
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()
|
||||
|
|
@ -25,7 +25,6 @@ def start():
|
|||
if __name__ == '__main__':
|
||||
start()
|
||||
try:
|
||||
print(sys.argv)
|
||||
subprocess.check_call(sys.argv[1:])
|
||||
finally:
|
||||
stop()
|
||||
|
|
|
|||
174
script/lint.js
Executable file
174
script/lint.js
Executable file
|
|
@ -0,0 +1,174 @@
|
|||
#!/usr/bin/env node
|
||||
|
||||
const { GitProcess } = require('dugite')
|
||||
const childProcess = require('child_process')
|
||||
const klaw = require('klaw')
|
||||
const minimist = require('minimist')
|
||||
const path = require('path')
|
||||
|
||||
const SOURCE_ROOT = path.normalize(path.dirname(__dirname))
|
||||
|
||||
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'],
|
||||
['spec', 'static', 'jquery-2.0.3.min.js']
|
||||
].map(tokens => path.join(SOURCE_ROOT, ...tokens)))
|
||||
|
||||
function spawnAndCheckExitCode (cmd, args, opts) {
|
||||
opts = Object.assign({ stdio: 'inherit' }, opts)
|
||||
const status = childProcess.spawnSync(cmd, args, opts).status
|
||||
if (status) process.exit(status)
|
||||
}
|
||||
|
||||
const LINTERS = [ {
|
||||
key: 'c++',
|
||||
roots: ['atom', 'brightray'],
|
||||
test: filename => filename.endsWith('.cc') || filename.endsWith('.h'),
|
||||
run: (opts, filenames) => {
|
||||
const result = childProcess.spawnSync('cpplint.py', filenames, { encoding: 'utf8' })
|
||||
// cpplint.py writes EVERYTHING to stderr, including status messages
|
||||
if (result.stderr) {
|
||||
for (const line of result.stderr.split(/[\r\n]+/)) {
|
||||
if (line.length && !line.startsWith('Done processing ') && line !== 'Total errors found: 0') {
|
||||
console.warn(line)
|
||||
}
|
||||
}
|
||||
}
|
||||
if (result.status) process.exit(result.status)
|
||||
}
|
||||
}, {
|
||||
key: 'python',
|
||||
roots: ['script'],
|
||||
test: filename => filename.endsWith('.py'),
|
||||
run: (opts, filenames) => {
|
||||
const rcfile = path.normalize(path.join(SOURCE_ROOT, '..', 'third_party', 'depot_tools', 'pylintrc'))
|
||||
const args = ['--rcfile=' + rcfile, ...filenames]
|
||||
const env = Object.assign({ PYTHONPATH: path.join(SOURCE_ROOT, 'script') }, process.env)
|
||||
spawnAndCheckExitCode('pylint.py', args, { env })
|
||||
}
|
||||
}, {
|
||||
key: 'javascript',
|
||||
roots: ['lib', 'spec', 'script', 'default_app'],
|
||||
test: filename => filename.endsWith('.js'),
|
||||
run: (opts, filenames) => {
|
||||
const cmd = path.join(SOURCE_ROOT, 'node_modules', '.bin', 'eslint')
|
||||
const args = [ '--cache', ...filenames ]
|
||||
if (opts.fix) args.unshift('--fix')
|
||||
spawnAndCheckExitCode(cmd, args, { cwd: SOURCE_ROOT })
|
||||
}
|
||||
}]
|
||||
|
||||
function parseCommandLine () {
|
||||
let help
|
||||
const opts = minimist(process.argv.slice(2), {
|
||||
boolean: [ 'c++', 'javascript', 'python', 'help', 'changed', 'fix', 'verbose' ],
|
||||
alias: { 'c++': ['cc', 'cpp', 'cxx'], javascript: ['js', 'es'], python: 'py', changed: 'c', help: 'h', verbose: 'v' },
|
||||
unknown: arg => { help = true }
|
||||
})
|
||||
if (help || opts.help) {
|
||||
console.log('Usage: script/lint.js [--cc] [--js] [--py] [-c|--changed] [-h|--help] [-v|--verbose] [--fix]')
|
||||
process.exit(0)
|
||||
}
|
||||
return opts
|
||||
}
|
||||
|
||||
async function findChangedFiles (top) {
|
||||
const result = await GitProcess.exec(['diff', 'HEAD', '--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 findMatchingFiles (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)
|
||||
}
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
async function findFiles (args, linter) {
|
||||
let filenames = []
|
||||
let whitelist = null
|
||||
|
||||
// build the whitelist
|
||||
if (args.changed) {
|
||||
whitelist = await findChangedFiles(SOURCE_ROOT)
|
||||
if (!whitelist.size) {
|
||||
return filenames
|
||||
}
|
||||
}
|
||||
|
||||
// accumulate the raw list of files
|
||||
for (const root of linter.roots) {
|
||||
const files = await findMatchingFiles(path.join(SOURCE_ROOT, root), linter.test)
|
||||
filenames.push(...files)
|
||||
}
|
||||
|
||||
// remove blacklisted files
|
||||
filenames = filenames.filter(x => !BLACKLIST.has(x))
|
||||
|
||||
// if a whitelist exists, remove anything not in it
|
||||
if (whitelist) {
|
||||
filenames = filenames.filter(x => whitelist.has(x))
|
||||
}
|
||||
|
||||
return filenames
|
||||
}
|
||||
|
||||
async function main () {
|
||||
const opts = parseCommandLine()
|
||||
|
||||
// no mode specified? run 'em all
|
||||
if (!opts['c++'] && !opts.javascript && !opts.python) {
|
||||
opts['c++'] = opts.javascript = opts.python = true
|
||||
}
|
||||
|
||||
const linters = LINTERS.filter(x => opts[x.key])
|
||||
|
||||
for (const linter of linters) {
|
||||
const filenames = await findFiles(opts, linter)
|
||||
if (filenames.length) {
|
||||
if (opts.verbose) { console.log(`linting ${filenames.length} ${linter.key} ${filenames.length === 1 ? 'file' : 'files'}`) }
|
||||
linter.run(opts, filenames)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (process.mainModule === module) {
|
||||
main()
|
||||
}
|
||||
|
|
@ -1,23 +0,0 @@
|
|||
#!/usr/bin/env python
|
||||
|
||||
import glob
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
SOURCE_ROOT = os.path.abspath(os.path.dirname(os.path.dirname(__file__)))
|
||||
|
||||
|
||||
def main():
|
||||
os.chdir(SOURCE_ROOT)
|
||||
|
||||
pylint = os.path.join(SOURCE_ROOT, 'vendor', 'depot_tools', 'pylint.py')
|
||||
settings = ['--rcfile=vendor/depot_tools/pylintrc']
|
||||
pys = glob.glob('script/*.py')
|
||||
env = os.environ.copy()
|
||||
env['PYTHONPATH'] = 'script'
|
||||
subprocess.check_call([sys.executable, pylint] + settings + pys, env=env)
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
sys.exit(main())
|
||||
Loading…
Reference in a new issue