Respect nodeVersion option set in override block (#345)

Author: pvdlg

lib/options-manager.js

@@ -121,7 +121,7 @@ const mergeWithPkgConf = opts => {
 	opts.cwd = path.resolve(opts.cwd);
 	const conf = pkgConf.sync('xo', {cwd: opts.cwd, skipOnFalse: true});
 	const engines = pkgConf.sync('engines', {cwd: opts.cwd});
-	return Object.assign({}, conf, {engines}, opts);
+	return Object.assign({}, conf, {nodeVersion: engines && engines.node && semver.validRange(engines.node)}, opts);
 };
 
 const normalizeSpaces = opts => typeof opts.space === 'number' ? opts.space : 2;
@@ -176,11 +176,11 @@ const buildConfig = opts => {
 	);
 	const spaces = normalizeSpaces(opts);
 
-	if (opts.engines && opts.engines.node && semver.validRange(opts.engines.node)) {
+	if (opts.nodeVersion) {
 		for (const rule of Object.keys(ENGINE_RULES)) {
 			// Use the rule value for the highest version that is lower or equal to the oldest version of Node.js supported
 			for (const minVersion of Object.keys(ENGINE_RULES[rule]).sort(semver.compare)) {
-				if (!semver.intersects(opts.engines.node, `<${minVersion}`)) {
+				if (!semver.intersects(opts.nodeVersion, `<${minVersion}`)) {
 					config.rules[rule] = ENGINE_RULES[rule][minVersion];
 				}
 			}

main.js

@@ -129,10 +129,8 @@ if (input[0] === '-') {
 
 if (opts.nodeVersion) {
 	if (opts.nodeVersion === 'false') {
-		opts.engines = false;
-	} else if (semver.validRange(opts.nodeVersion)) {
-		opts.engines = {node: opts.nodeVersion};
-	} else {
+		opts.nodeVersion = false;
+	} else if (!semver.validRange(opts.nodeVersion)) {
 		console.error('The `node-engine` option must be a valid semver range (for example `>=6`)');
 		process.exit(1);
 	}

test/fixtures/engines-overrides/package.json

@@ -0,0 +1,15 @@
+{
+	"name": "application-name",
+	"version": "0.0.1",
+	"engines": {
+		"node": ">=6.0.0"
+	},
+	"xo": {
+		"overrides": [
+			{
+				"files": "*-transpile.js",
+				"nodeVersion": ">=8.0.0"
+			}
+		]
+	}
+}

test/fixtures/engines-overrides/promise-then-transpile.js

@@ -0,0 +1,7 @@
+const promise = new Promise(resolve => {
+	resolve('test');
+});
+
+function example() {
+	return promise.then(console.log);
+}

test/fixtures/engines-overrides/promise-then.js

@@ -0,0 +1,7 @@
+const promise = new Promise(resolve => {
+	resolve('test');
+});
+
+function example() {
+	return promise.then(console.log);
+}

test/fixtures/engines/package.json

@@ -2,6 +2,6 @@
 	"name": "application-name",
 	"version": "0.0.1",
 	"engines": {
-		"node": ">=6"
+		"node": ">=6.0.0"
 	}
 }

test/lint-files.js

@@ -4,6 +4,11 @@ import fn from '..';
 
 process.chdir(__dirname);
 
+const hasRule = (results, filePath, ruleId) => {
+	const result = results.find(x => x.filePath === filePath);
+	return result ? result.messages.some(x => x.ruleId === ruleId) : false;
+};
+
 test('only accepts whitelisted extensions', async t => {
 	// Markdown files will always produce linter errors and will not be going away
 	const mdGlob = path.join(__dirname, '..', '*.md');
@@ -94,3 +99,24 @@ test('multiple negative patterns should act as positive patterns', async t => {
 
 	t.deepEqual(paths, ['!!unicorn.js', '!unicorn.js']);
 });
+
+test('enable rules based on nodeVersion', async t => {
+	const {results} = await fn.lintFiles('**/*', {cwd: 'fixtures/engines-overrides'});
+
+	// The transpiled file (as specified in `overrides`) should use `await`
+	t.true(
+		hasRule(
+			results,
+			path.resolve('fixtures/engines-overrides/promise-then-transpile.js'),
+			'promise/prefer-await-to-then'
+		)
+	);
+	// The non transpiled files can use `.then`
+	t.false(
+		hasRule(
+			results,
+			path.resolve('fixtures/engines-overrides/promise-then.js'),
+			'promise/prefer-await-to-then'
+		)
+	);
+});

test/lint-text.js

@@ -196,3 +196,43 @@ test('lint negatively gitignored files', async t => {
 
 	t.true(results[0].errorCount > 0);
 });
+
+test('enable rules based on nodeVersion', async t => {
+	const cwd = path.join(__dirname, 'fixtures', 'engines-overrides');
+	const filename = path.join(cwd, 'promise-then.js');
+	const text = await readFile(filename, 'utf-8');
+
+	let {results} = fn.lintText(text, {nodeVersion: '>=8.0.0'});
+	t.true(hasRule(results, 'promise/prefer-await-to-then'));
+
+	({results} = fn.lintText(text, {nodeVersion: '>=6.0.0'}));
+	t.false(hasRule(results, 'promise/prefer-await-to-then'));
+});
+
+test('enable rules based on nodeVersion in override', async t => {
+	const cwd = path.join(__dirname, 'fixtures', 'engines-overrides');
+	const filename = path.join(cwd, 'promise-then.js');
+	const text = await readFile(filename, 'utf-8');
+
+	let {results} = fn.lintText(text, {
+		nodeVersion: '>=8.0.0',
+		filename: 'promise-then.js',
+		overrides: [
+			{
+				files: 'promise-*.js',
+				nodeVersion: '>=6.0.0'
+			}
+		]});
+	t.false(hasRule(results, 'promise/prefer-await-to-then'));
+
+	({results} = fn.lintText(text, {
+		nodeVersion: '>=6.0.0',
+		filename: 'promise-then.js',
+		overrides: [
+			{
+				files: 'promise-*.js',
+				nodeVersion: '>=8.0.0'
+			}
+		]}));
+	t.true(hasRule(results, 'promise/prefer-await-to-then'));
+});

test/options-manager.js

@@ -207,8 +207,8 @@ test('buildConfig: engines: undefined', t => {
 	t.is(config.rules['promise/prefer-await-to-then'], undefined);
 });
 
-test('buildConfig: engines: false', t => {
-	const config = manager.buildConfig({engines: false});
+test('buildConfig: nodeVersion: false', t => {
+	const config = manager.buildConfig({nodeVersion: false});
 
 	// Do not include any Node.js version specific rules
 	t.is(config.rules['prefer-spread'], undefined);
@@ -217,8 +217,8 @@ test('buildConfig: engines: false', t => {
 	t.is(config.rules['promise/prefer-await-to-then'], undefined);
 });
 
-test('buildConfig: engines: invalid range', t => {
-	const config = manager.buildConfig({engines: {node: '4'}});
+test('buildConfig: nodeVersion: invalid range', t => {
+	const config = manager.buildConfig({nodeVersion: '4'});
 
 	// Do not include any Node.js version specific rules
 	t.is(config.rules['prefer-spread'], undefined);
@@ -227,8 +227,8 @@ test('buildConfig: engines: invalid range', t => {
 	t.is(config.rules['promise/prefer-await-to-then'], undefined);
 });
 
-test('buildConfig: engines: >=8', t => {
-	const config = manager.buildConfig({engines: {node: '>=8'}});
+test('buildConfig: nodeVersion: >=8', t => {
+	const config = manager.buildConfig({nodeVersion: '>=8'});
 
 	// Include rules for Node.js 8 and above
 	t.is(config.rules['promise/prefer-await-to-then'], 'error');
@@ -400,47 +400,47 @@ test('groupConfigs', t => {
 test('mergeWithPkgConf: use child if closest', t => {
 	const cwd = path.resolve('fixtures', 'nested', 'child');
 	const result = manager.mergeWithPkgConf({cwd});
-	const expected = Object.assign({}, childConfig.xo, {cwd}, {engines: {}});
+	const expected = Object.assign({}, childConfig.xo, {cwd, nodeVersion: undefined});
 	t.deepEqual(result, expected);
 });
 
 test('mergeWithPkgConf: use parent if closest', t => {
 	const cwd = path.resolve('fixtures', 'nested');
 	const result = manager.mergeWithPkgConf({cwd});
-	const expected = Object.assign({}, parentConfig.xo, {cwd}, {engines: {}});
+	const expected = Object.assign({}, parentConfig.xo, {cwd, nodeVersion: undefined});
 	t.deepEqual(result, expected);
 });
 
 test('mergeWithPkgConf: use parent if child is ignored', t => {
 	const cwd = path.resolve('fixtures', 'nested', 'child-ignore');
 	const result = manager.mergeWithPkgConf({cwd});
-	const expected = Object.assign({}, parentConfig.xo, {cwd}, {engines: {}});
+	const expected = Object.assign({}, parentConfig.xo, {cwd, nodeVersion: undefined});
 	t.deepEqual(result, expected);
 });
 
 test('mergeWithPkgConf: use child if child is empty', t => {
 	const cwd = path.resolve('fixtures', 'nested', 'child-empty');
 	const result = manager.mergeWithPkgConf({cwd});
-	t.deepEqual(result, {cwd, engines: {}});
+	t.deepEqual(result, {nodeVersion: undefined, cwd});
 });
 
 test('mergeWithPkgConf: read engines from package.json', t => {
 	const cwd = path.resolve('fixtures', 'engines');
 	const result = manager.mergeWithPkgConf({cwd});
-	const expected = Object.assign({}, {engines: enginesConfig.engines}, {cwd});
+	const expected = {nodeVersion: enginesConfig.engines.node, cwd};
 	t.deepEqual(result, expected);
 });
 
 test('mergeWithPkgConf: XO engine options supersede package.json\'s', t => {
 	const cwd = path.resolve('fixtures', 'engines');
-	const result = manager.mergeWithPkgConf({cwd, engines: {node: '>=8'}});
-	const expected = Object.assign({}, {engines: {node: '>=8'}}, {cwd});
+	const result = manager.mergeWithPkgConf({cwd, nodeVersion: '>=8'});
+	const expected = {nodeVersion: '>=8', cwd};
 	t.deepEqual(result, expected);
 });
 
 test('mergeWithPkgConf: XO engine options false supersede package.json\'s', t => {
 	const cwd = path.resolve('fixtures', 'engines');
-	const result = manager.mergeWithPkgConf({cwd, engines: false});
-	const expected = Object.assign({}, {engines: false}, {cwd});
+	const result = manager.mergeWithPkgConf({cwd, nodeVersion: false});
+	const expected = {nodeVersion: false, cwd};
 	t.deepEqual(result, expected);
 });