languages: Prefer arrow callbacks in ES6

Also prefer single-line body-less style when possible.

Fixes #572

language/rules-es6.json

@@ -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"
 	}
 }

test/fixtures/mediawiki/qunit/invalid.js

@@ -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();
 

test/fixtures/mediawiki/qunit/valid.js

@@ -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() );
 	} );

test/fixtures/mocha/invalid-server.js

@@ -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 = {};

test/fixtures/mocha/valid-server.js

@@ -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 );

test/fixtures/qunit/invalid.js

@@ -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
@@ -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
@@ -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 () {};
@@ -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();
 
@@ -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 ) {
@@ -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

test/fixtures/qunit/valid.js

@@ -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';
 
@@ -43,4 +43,11 @@ QUnit.module( 'Module', ( hooks ) => {
 		} );
 	} );
 
+	// Valid: prefer-arrow-callback
+	// Non-arrow callback allowed when `this` is used
+	QUnit.test( 'this', function ( assert ) {
+		const x = this.foo === 3;
+		assert.true( x );
+	} );
+
 } );

test/fixtures/server/invalid.js

@@ -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;

test/fixtures/server/valid.js

@@ -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 } ) );
 

test/test.js

@@ -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;
 	}
@@ -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();
 			} );
@@ -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` );
 		} );
 	} );
@@ -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 ) => {
@@ -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 ) => {