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
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
oandregal committed Mar 22, 2017
1 parent 4349b5e commit e499569
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 e499569

Please sign in to comment.