Skip to content

Commit

Permalink
Set ESLint rules as errors while preventing build tools from failing. (
Browse files Browse the repository at this point in the history
…#6945)

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
  • Loading branch information
nssw authored and nosolosw committed Mar 22, 2017
1 parent 109b74e commit a5889f9
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 104 deletions.
8 changes: 8 additions & 0 deletions .eslines.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"rulesNotToDowngrade": ["no-unused-vars"],
"remote": "origin/master",
"processors": {
"default": "lines-modified",
"master": "parsing-errors"
}
}
69 changes: 2 additions & 67 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
};
28 changes: 9 additions & 19 deletions bin/pre-commit
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
68 changes: 53 additions & 15 deletions bin/run-lint
Original file line number Diff line number Diff line change
@@ -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 );
});
20 changes: 18 additions & 2 deletions npm-shrinkwrap.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -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",
Expand Down

0 comments on commit a5889f9

Please sign in to comment.