Skip to content

Commit

Permalink
languages: Prefer arrow callbacks in ES6
Browse files Browse the repository at this point in the history
Also prefer single-line body-less style when possible.

Fixes #572
  • Loading branch information
edg2s committed May 17, 2024
1 parent 1e8adec commit 47059d1
Show file tree
Hide file tree
Showing 10 changed files with 61 additions and 51 deletions.
5 changes: 4 additions & 1 deletion language/rules-es6.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
"no-var": "error",
"one-var": "off",
"vars-on-top": "off",
"prefer-const": "error"
"prefer-const": "error",
"prefer-arrow-callback": "error",
"implicit-arrow-linebreak": "error",
"arrow-body-style": "error"
}
}
2 changes: 1 addition & 1 deletion test/fixtures/mediawiki/qunit/invalid.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
QUnit.module( 'example' );

QUnit.test( 'test', function ( assert ) {
QUnit.test( 'test', ( assert ) => {
// eslint-disable-next-line compat/compat
const data = navigator.getBattery();

Expand Down
4 changes: 1 addition & 3 deletions test/fixtures/mediawiki/qunit/valid.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ QUnit.module( 'example', () => {
$( '<div>contents</div>' );

// Global: sinon
sinon.stub( data, 'isValid', function () {
return true;
} );
sinon.stub( data, 'isValid', () => true );

assert.true( data.save() );
} );
Expand Down
29 changes: 15 additions & 14 deletions test/fixtures/mocha/invalid-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,54 +2,55 @@

// Recommended
// eslint-disable-next-line mocha/no-async-describe
describe( 'async', async function () {
describe( 'async', async () => {
} );

describe( 'foo', function () {
describe( 'foo', () => {
const foo = function () {};
// eslint-disable-next-line mocha/no-setup-in-describe
foo();

// eslint-disable-next-line mocha/handle-done-callback, no-unused-vars
it( 'async', function ( done ) {
it( 'async', ( done ) => {
// done not called
} );

it( 'return-and-callback', function ( done ) {
it( 'return-and-callback', ( done ) => {
foo();
// eslint-disable-next-line mocha/no-return-and-callback
return foo( done );
} );

before( function () {} );
before( () => {} );
// eslint-disable-next-line mocha/no-sibling-hooks
before( function () {} );
before( () => {} );

// eslint-disable-next-line mocha/no-exclusive-tests
it.only( 'only', function () {
it.only( 'only', () => {
// eslint-disable-next-line mocha/no-nested-tests
it( 'nested', function () {} );
it( 'nested', () => {} );
} );

// eslint-disable-next-line mocha/no-pending-tests
it( 'pending' );
} );

// TODO: This should also trigger mocha/no-global-tests?
it( 'global', function () {} );
it( 'global', () => {} );

// eslint-disable-next-line mocha/no-top-level-hooks
before( function () {} );
before( () => {} );

// TODO: This should also trigger mocha/max-top-level-suites?
// eslint-disable-next-line mocha/no-identical-title
describe( 'foo', function () {
it( 'test', function () {} );
describe( 'foo', () => {
it( 'test', () => {} );
// eslint-disable-next-line mocha/no-identical-title
it( 'test', function () {} );
it( 'test', () => {} );
} );

// eslint-disable-next-line mocha/no-empty-description
describe( '', function () { } );
describe( '', () => { } );

// eslint-disable-next-line mocha/no-exports
module.exports = {};
17 changes: 7 additions & 10 deletions test/fixtures/mocha/valid-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,23 @@ describe.skip( () => {

// Recommended
// Off: mocha/valid-suite-description
describe( 'Suite', function () {
describe( 'Suite', () => {
// Off: mocha/no-hooks
it( 'title', async function () {
return 1;
} );
it( 'title', async () => 1 );

// Off: mocha/valid-test-description
it( 'async', async function ( done ) {
it( 'async', async ( done ) => {
foo();
// Off: mocha/no-return-from-async
return done();
} );

// Off: mocha/no-synchronous-tests
it( 'test', function () {
return;
it( 'test', () => {
foo();
} );

} );

// Off: mocha/prefer-arrow-callback
foo( function ( a ) {
return a;
} );
foo( ( a ) => a );
24 changes: 12 additions & 12 deletions test/fixtures/qunit/invalid.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ QUnit.module( 'Example' );

// Local rules
// eslint-disable-next-line qunit/require-expect
QUnit.test( '.foo()', function ( assert ) {
QUnit.test( '.foo()', ( assert ) => {
const x = 'bar';

// eslint-disable-next-line qunit/no-loose-assertions
Expand Down Expand Up @@ -45,7 +45,7 @@ QUnit.test( '.foo()', ( assert ) => {
assert.strictEqual( done > 3, true );

// eslint-disable-next-line qunit/no-throws-string
assert.throws( function () {
assert.throws( () => {
}, 'error message', 'An error should have been thrown' );

// eslint-disable-next-line qunit/require-object-in-propequal
Expand All @@ -56,7 +56,7 @@ QUnit.test( '.foo()', ( assert ) => {
// QUnit.test( '.bar()', function ( assert ) {} );

// eslint-disable-next-line qunit/no-only
QUnit.module.only( '.foo()', function () {} );
QUnit.module.only( '.foo()', () => {} );

// eslint-disable-next-line qunit/no-reassign-log-callbacks
QUnit.log = function () {};
Expand All @@ -71,7 +71,7 @@ QUnit.init();
QUnit.jsDump( {} );

// eslint-disable-next-line qunit/no-async-test, qunit/no-test-expect-argument, qunit/require-expect
QUnit.asyncTest( 'Asynchronous test', 3, function () {
QUnit.asyncTest( 'Asynchronous test', 3, () => {
// eslint-disable-next-line qunit/no-qunit-start-in-tests
QUnit.start();

Expand All @@ -89,8 +89,8 @@ QUnit.asyncTest( 'Asynchronous test', 3, function () {
} );

// eslint-disable-next-line qunit/no-async-module-callbacks
QUnit.module( 'An async module', async function () {
QUnit.test( 'a passing test', function ( assert ) {
QUnit.module( 'An async module', async () => {
QUnit.test( 'a passing test', ( assert ) => {
// Shadowing is only allowed for the variable name "hooks" (#532)
// eslint-disable-next-line no-shadow
function checkAssert( assert ) {
Expand All @@ -102,21 +102,21 @@ QUnit.module( 'An async module', async function () {

await Promise.resolve();

QUnit.test( 'another passing test', function ( assert ) {
QUnit.test( 'another passing test', ( assert ) => {
assert.true( true );
} );
} );

QUnit.module( 'outer module', function ( hooks ) {
QUnit.module( 'inner module', function () {
QUnit.module( 'outer module', ( hooks ) => {
QUnit.module( 'inner module', () => {
// eslint-disable-next-line qunit/no-hooks-from-ancestor-modules
hooks.beforeEach( function () {} );
hooks.beforeEach( () => {} );
} );
} );

QUnit.test( 'Parent', function () {
QUnit.test( 'Parent', () => {
// eslint-disable-next-line qunit/no-nested-tests
QUnit.test( 'Child', function () {} );
QUnit.test( 'Child', () => {} );
} );

// eslint-disable-next-line qunit/no-qunit-push
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/qunit/valid.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ QUnit.module( 'Module', ( hooks ) => {
QUnit.test( '.foo()', () => {
} );

QUnit.test( '.bar()', function ( assert ) {
QUnit.test( '.bar()', ( assert ) => {
const x = 'bar',
y = 'baz';

Expand Down
11 changes: 10 additions & 1 deletion test/fixtures/server/invalid.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,25 @@
const object = { ... ( foo || {} ) };

// eslint-disable-next-line arrow-parens, arrow-spacing
Object.keys( foo ).map( x=> {
Object.keys( foo ).map( x=> x + 1 );

// eslint-disable-next-line prefer-arrow-callback
f( function ( x ) {
return x + 1;
} );

f( ( x ) =>
// eslint-disable-next-line implicit-arrow-linebreak
x + 1
);

// eslint-disable-next-line no-useless-concat, no-unused-expressions
'a' + 'b';

// eslint-disable-next-line template-curly-spacing, no-unused-expressions
`${global.foo}`;

// eslint-disable-next-line arrow-body-style
const promise = new Promise( () => {
// eslint-disable-next-line no-promise-executor-return
return 1;
Expand Down
3 changes: 3 additions & 0 deletions test/fixtures/server/valid.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@

// Valid: arrow-parens
// Valid: arrow-spacing
// Valid: prefer-arrow-callback
// Valid: implicit-arrow-linebreak
// Valid: arrow-body-style
global.then( ( data ) => Math.pow( data, 2 ) );
global.then( ( data ) => ( { d: data } ) );

Expand Down
15 changes: 7 additions & 8 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const fs = require( 'fs' ),
{ ESLint } = require( 'eslint' ),
eslint = new ESLint();

packageFiles.forEach( function ( packageFile ) {
packageFiles.forEach( ( packageFile ) => {
if ( !fs.existsSync( packageFile ) ) {
return;
}
Expand Down Expand Up @@ -94,8 +94,8 @@ QUnit.module( 'ignorePatterns', () => {
{ path: '.dot.folder/.foo.js', ignored: true }
];
const done = assert.async( tests.length );
tests.forEach( function ( test ) {
eslint.isPathIgnored( test.path ).then( function ( ignored ) {
tests.forEach( ( test ) => {
eslint.isPathIgnored( test.path ).then( ( ignored ) => {
assert.strictEqual( ignored, test.ignored, `Is ${ test.path } ignored` );
done();
} );
Expand All @@ -121,7 +121,7 @@ QUnit.module( 'package.json', () => {
} );
} );
QUnit.test( 'All listed files exist', ( assert ) => {
packageFiles.forEach( function ( file ) {
packageFiles.forEach( ( file ) => {
assert.true( fs.existsSync( file ), `"${ file }" found` );
} );
} );
Expand Down Expand Up @@ -216,8 +216,8 @@ configs.forEach( ( configPath ) => {
// Invalid examples are required for every rule that is enabled,
// as reportUnusedDisableDirectives ensures the disable directives
// are actually being used.
const invalidFixtures = invalidFixturesFiles.map( ( file ) =>
fs.readFileSync( file ).toString()
const invalidFixtures = invalidFixturesFiles.map(
( file ) => fs.readFileSync( file ).toString()
).join( '' );

Object.keys( rules ).forEach( ( rule ) => {
Expand All @@ -231,8 +231,7 @@ configs.forEach( ( configPath ) => {
}
} );

const validFixtures = validFixturesFiles.map( ( file ) =>
fs.readFileSync( file ).toString()
const validFixtures = validFixturesFiles.map( ( file ) => fs.readFileSync( file ).toString()
).join( '' );

QUnit.test( 'Valid fixtures contain no disables', ( assert ) => {
Expand Down

0 comments on commit 47059d1

Please sign in to comment.