From e499569eb6bba99624086e483b37af769347d2eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Maneiro?= Date: Wed, 22 Mar 2017 13:18:43 +0100 Subject: [PATCH] Set ESLint rules as errors while preventing build tools from failing. The gist of this PR is: eslint -f json | eslines With these changes in place, the linting task will report: * only parsing errors, when running on master branch. * for other branches, any ESLint error in * lines modified within that branch * any linted files for the rule no-unused-vars --- .eslines.json | 8 ++++++ .eslintrc.js | 69 ++------------------------------------------- bin/pre-commit | 28 ++++++------------ bin/run-lint | 68 ++++++++++++++++++++++++++++++++++---------- npm-shrinkwrap.json | 20 +++++++++++-- package.json | 3 +- 6 files changed, 92 insertions(+), 104 deletions(-) create mode 100644 .eslines.json diff --git a/.eslines.json b/.eslines.json new file mode 100644 index 0000000000000..379bf0a676a97 --- /dev/null +++ b/.eslines.json @@ -0,0 +1,8 @@ +{ + "rulesNotToDowngrade": ["no-unused-vars"], + "remote": "origin/master", + "processors": { + "default": "lines-modified", + "master": "parsing-errors" + } +} diff --git a/.eslintrc.js b/.eslintrc.js index e4c74c1084592..11b7db5eff888 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -10,73 +10,8 @@ module.exports = { globals: { asyncRequire: true }, - // Ideally, we should not have a `rules` block here at all, save for some - // Calypso-specific rules (no-unused-expressions, camelcase). The remainder - // are rules we cannot yet flag as errors, and should be removed over time - // as outstanding issues are resolved. rules: { - 'array-bracket-spacing': [ 1, 'always' ], - 'brace-style': [ 1, '1tbs' ], - // REST API objects include underscores - camelcase: 0, - 'comma-spacing': 1, - curly: 1, - 'computed-property-spacing': [ 1, 'always' ], - 'func-call-spacing': 1, - indent: [ 1, 'tab', { SwitchCase: 1 } ], - 'jsx-quotes': [ 1, 'prefer-double' ], - 'key-spacing': 1, - 'keyword-spacing': 1, - 'max-len': [ 1, { code: 140 } ], - 'new-cap': [ 1, { capIsNew: false, newIsCap: true } ], - 'no-else-return': 1, - 'no-extra-semi': 1, - 'no-multiple-empty-lines': [ 1, { max: 1 } ], - 'no-multi-spaces': 1, - 'no-restricted-imports': [ 1, 'lib/sites-list', 'lib/mixins/data-observe' ], - 'no-restricted-modules': [ 1, 'lib/sites-list', 'lib/mixins/data-observe' ], - 'no-shadow': 1, - 'no-spaced-func': 1, - 'no-trailing-spaces': 1, - // Allows Chai `expect` expressions - 'no-unused-expressions': 0, - 'no-var': 1, - 'object-curly-spacing': [ 1, 'always' ], - 'operator-linebreak': [ 1, 'after', { overrides: { - '?': 'before', - ':': 'before' - } } ], - 'padded-blocks': [ 1, 'never' ], - 'prefer-const': 1, - 'quote-props': [ 1, 'as-needed', { keywords: true } ], - quotes: [ 1, 'single', 'avoid-escape' ], - 'react/jsx-curly-spacing': [ 1, 'always' ], - 'react/jsx-no-bind': 1, - 'react/jsx-space-before-closing': 1, - 'react/no-did-mount-set-state': 1, - 'react/no-did-update-set-state': 1, - 'react/no-is-mounted': 1, - 'react/prefer-es6-class': 1, - semi: 1, - 'semi-spacing': 1, - 'space-before-blocks': [ 1, 'always' ], - 'space-before-function-paren': [ 1, 'never' ], - 'space-in-parens': [ 1, 'always' ], - 'space-infix-ops': [ 1, { int32Hint: false } ], - 'space-unary-ops': [ 1, { - overrides: { - '!': true - } - } ], - 'template-curly-spacing': [ 1, 'always' ], - 'valid-jsdoc': [ 1, { requireReturn: false } ], - 'wpcalypso/i18n-ellipsis': 1, - 'wpcalypso/i18n-no-collapsible-whitespace': 1, - 'wpcalypso/i18n-no-this-translate': 1, - 'wpcalypso/i18n-no-variables': 1, - 'wpcalypso/i18n-mismatched-placeholders': 1, - 'wpcalypso/import-docblock': 1, - 'wpcalypso/jsx-gridicon-size': 1, - 'wpcalypso/jsx-classname-namespace': 1, + camelcase: 0, // REST API objects include underscores + 'no-unused-expressions': 0, // Allows Chai `expect` expressions } }; diff --git a/bin/pre-commit b/bin/pre-commit index 64a71826bb527..f8a4abf95a2f5 100755 --- a/bin/pre-commit +++ b/bin/pre-commit @@ -1,7 +1,5 @@ #!/bin/sh -PATH="$PATH:/usr/local/bin" - printf "\nBy contributing to this project, you license the materials you contribute under the GNU General Public License v2 (or later). All materials must have GPLv2 compatible licenses — see .github/CONTRIBUTING.md for details.\n\n" # Make quick pass over config files on every change @@ -16,21 +14,13 @@ pass=true printf "\nValidating .jsx and .js:\n" -for file in ${files}; do - ./node_modules/.bin/eslint --cache ${file} - if [ $? -ne 0 ]; then - printf "\033[31meslint Failed: %s\033[0m\n" "${file}" - pass=false - else - printf "\033[32meslint Passed: %s\033[0m\n" "${file}" - fi -done - -printf "\neslint validation complete\n" - -if ! $pass; then - printf "\n\033[41mCOMMIT FAILED:\033[0m Your commit contains files that should pass validation tests but do not. Please fix the errors and try again.\n\n" - exit 1 -else - printf "\n\033[42mCOMMIT SUCCEEDED\033[0m\n\n" +./bin/run-lint $(git diff --cached --name-only client/ server/ test/ | grep ".jsx*$") -- --diff=index +linter_exit_code=$? +if [ ! 0 = "$linter_exit_code" ] +then + printf "\n" + printf "\033[41mCOMMIT ABORTED:\033[0m the linter reported some problems. If you are aware of them and it is OK, repeat the commit command with --no-verify to avoid this check. You may also want to execute the linter after the errors have been solved: ./bin/run-lint \$(git diff --cached --name-only client/ server/ test/) -- --diff=index" + printf "\n" fi + +exit $linter_exit_code diff --git a/bin/run-lint b/bin/run-lint index 38bb63832e52a..5ef0692a7ac4a 100755 --- a/bin/run-lint +++ b/bin/run-lint @@ -1,21 +1,59 @@ #!/usr/bin/env node -const child_process = require( 'child_process' ); + const path = require( 'path' ); -var args = [ '--cache', '--quiet', '--ext=.js', '--ext=.jsx' ]; -if ( process.argv.length > 2 ) { - args = args.concat( process.argv.slice( 2 ) ); +const child_process = require( 'child_process' ); + +var argsESLint = [ '--cache', '--quiet', '--ext=.js,.jsx', '--format=json' ]; +var argsESLines = [ ]; + +const markerIndex = process.argv.indexOf('--'); +if ( ( process.argv.length > 2 ) && ( markerIndex > 2 )) { + // use -- as a marker for end of options, + // so any value that follows the marker is considered an option for ESLines. + if ( markerIndex > -1 ) { + argsESLint = argsESLint.concat( process.argv.slice( 2, markerIndex ) ); + argsESLines = argsESLines.concat( process.argv.slice( markerIndex + 1 ) ); + } else { + argsESLint = argsESLint.concat( process.argv.slice( 2 ) ); + } } else { - args = args.concat( [ '.' ] ); + process.stdout.write( 'No files to lint\n' ); + process.exit( 0 ); } -const results = child_process.spawnSync( path.join( '.', 'node_modules', '.bin', 'eslint' ), args ); +const eslint = child_process.spawn( path.join( '.', 'node_modules', '.bin', 'eslint' ), argsESLint ); +const eslines = child_process.spawn( path.join( '.', 'node_modules', '.bin', 'eslines' ), argsESLines ); -if ( results.stdout ) { - process.stdout.write( results.stdout ); -} -if ( results.stderr ) { - process.stderr.write( results.stderr ); -} -process.on( 'exit', function() { - process.exit( results.status ); -} ); +eslint.stdout.on( 'data', ( data ) => { + eslines.stdin.write( data ); +}); + +var eslintStdErr = 0; +eslint.stderr.on( 'data', ( data ) => { + eslintStdErr = 1; + process.stderr.write( data ); +}); + +eslint.on('close', ( code ) => { + eslines.stdin.end(); +}); + +eslines.stdout.on( 'data', ( data ) => { + process.stdout.write( data ); +}); + +eslines.stderr.on( 'data', ( data ) => { + process.stderr.write( data ); +}); + +eslines.on( 'close', ( code ) => { + // since the goal of eslines is to downgrade errors + // on non-modified lines, we can't count on eslint's + // exit code, but we assume that if eslint output + // something on its stderr, this means it encountered + // some other error + if ( eslintStdErr === 1 ) { + process.exit( 1 ); + } + process.exit( code ); +}); diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 2f06366487a2e..060c3abc4f4ca 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1294,6 +1294,10 @@ "version": "1.0.1", "dev": true }, + "eslines": { + "version": "0.0.10", + "dev": true + }, "eslint": { "version": "3.8.1", "dev": true, @@ -1306,6 +1310,14 @@ "version": "1.5.0", "dev": true }, + "fast-levenshtein": { + "version": "2.0.6", + "dev": true + }, + "optionator": { + "version": "0.8.2", + "dev": true + }, "strip-bom": { "version": "3.0.0", "dev": true @@ -1321,6 +1333,10 @@ "user-home": { "version": "2.0.0", "dev": true + }, + "wordwrap": { + "version": "1.0.0", + "dev": true } } }, @@ -1466,7 +1482,7 @@ "version": "1.0.2" }, "fast-levenshtein": { - "version": "2.0.6", + "version": "1.1.4", "dev": true }, "fast-luhn": { @@ -2754,7 +2770,7 @@ "version": "0.6.1" }, "optionator": { - "version": "0.8.2", + "version": "0.8.1", "dev": true, "dependencies": { "wordwrap": { diff --git a/package.json b/package.json index f56621659a98c..24f5e5077c657 100644 --- a/package.json +++ b/package.json @@ -151,7 +151,7 @@ "test-client:watch": "nodemon -e js,jsx --exec npm run test-client", "test-server:watch": "nodemon -e js,jsx --exec npm run test-server", "test-test:watch": "nodemon -e js,jsx --exec npm run test-test", - "lint": "bin/run-lint", + "lint": "bin/run-lint .", "css-lint": "stylelint 'client/**/*.scss' --syntax scss" }, "devDependencies": { @@ -167,6 +167,7 @@ "esformatter-quotes": "1.0.3", "esformatter-semicolons": "1.1.1", "esformatter-special-bangs": "1.0.1", + "eslines": "0.0.10", "eslint": "3.8.1", "eslint-config-wpcalypso": "0.6.0", "eslint-plugin-react": "6.4.1",