Merge pull request #721 from bryceosterhaus/pr-692

Author: bryceosterhaus

projects/eslint-plugin/configs/general.js

@@ -102,6 +102,7 @@ const config = {
 		'@liferay/no-abbreviations': 'error',
 		'@liferay/no-absolute-import': 'error',
 		'@liferay/no-anonymous-exports': 'error',
+		'@liferay/no-conditional-object-keys': 'error',
 		'@liferay/no-duplicate-imports': 'error',
 		'@liferay/no-dynamic-require': 'error',
 		'@liferay/no-get-data-attribute': 'error',

projects/eslint-plugin/rules/general/docs/rules/no-conditional-object-keys.md

@@ -0,0 +1,41 @@
+# Don't use negated `!Object.keys()` or double negated `!!Object.keys()` expressions as they always evaluate to `false` and `true`, respectively
+
+This rule enforces that engineers don't use a truthy or falsy value that `Object.keys()` can return. Instead, use an expression that is more precise.
+
+## Rule Details
+
+Examples of **incorrect** code for this rule:
+
+```js
+if (Object.keys({foo: 'bar'})) {
+	// do your magic
+}
+
+if (!Object.keys({foo: 'bar'})) {
+	// do your magic
+}
+
+!!Object.keys({foo: 'bar'}) && 'test';
+
+const negatedObjectKeys = !Object.keys({foo: 'bar'});
+
+!Object.keys({foo: 'bar'}) && true;
+```
+
+Examples of **correct** code for this rule:
+
+```js
+if (Object.keys({foo: 'bar'}).length) {
+	// do your magic
+}
+
+Object.keys({foo: 'bar'}).length && 'test';
+
+if (Object.keys({foo: 'bar'}).find((i) => true)) {
+	// do your magic
+}
+```
+
+## Further Reading
+
+-   https://github.com/liferay/liferay-frontend-projects/issues/10

projects/eslint-plugin/rules/general/index.js

@@ -15,6 +15,7 @@ module.exports = {
 	'no-absolute-import': require('./lib/rules/no-absolute-import'),
 	'no-anonymous-exports': require('./lib/rules/no-anonymous-exports'),
 	'no-arrow': require('./lib/rules/no-arrow'),
+	'no-conditional-object-keys': require('./lib/rules/no-conditional-object-keys'),
 	'no-duplicate-class-names': require('./lib/rules/no-duplicate-class-names'),
 	'no-duplicate-imports': require('./lib/rules/no-duplicate-imports'),
 	'no-dynamic-require': require('./lib/rules/no-dynamic-require'),

projects/eslint-plugin/rules/general/lib/rules/no-conditional-object-keys.js

@@ -0,0 +1,71 @@
+/**
+ * SPDX-FileCopyrightText: © 2017 Liferay, Inc. <https://liferay.com>
+ * SPDX-License-Identifier: MIT
+ */
+
+module.exports = {
+	create(context) {
+		return {
+			CallExpression(node) {
+				if (
+					node.callee.object &&
+					node.callee.object.name === 'Object' &&
+					node.callee.property &&
+					node.callee.property.name === 'keys'
+				) {
+					const {parent} = node;
+
+					if (parent.type === 'MemberExpression') {
+						return;
+					}
+
+					const grandParent = parent && parent.parent;
+
+					let message;
+
+					if (
+						parent.type === 'UnaryExpression' &&
+						parent.operator === '!'
+					) {
+						if (
+							grandParent &&
+							grandParent.type === 'UnaryExpression' &&
+							grandParent.operator === '!'
+						) {
+							message = `!!Object.keys({}) always evaluates to true`;
+						}
+						else {
+							message = `!Object.keys({}) always evaluates to false`;
+						}
+					}
+					else if (
+						parent.type === 'LogicalExpression' ||
+						parent.type === 'IfStatement'
+					) {
+						message = `Object.keys({}) always evaluates to truthy`;
+					}
+
+					if (message) {
+						context.report({
+							message,
+							node,
+						});
+					}
+				}
+			},
+		};
+	},
+
+	meta: {
+		docs: {
+			category: 'Best Practices',
+			description: 'Object.keys({}) always evaluates truthy',
+			recommended: false,
+			url:
+				'https://github.com/liferay/liferay-frontend-projects/issues/10',
+		},
+		fixable: 'code',
+		schema: [],
+		type: 'problem',
+	},
+};

projects/eslint-plugin/rules/general/tests/lib/rules/no-conditional-object-keys.js

@@ -0,0 +1,192 @@
+/**
+ * SPDX-FileCopyrightText: © 2017 Liferay, Inc. <https://liferay.com>
+ * SPDX-License-Identifier: MIT
+ */
+
+const MultiTester = require('../../../../../scripts/MultiTester');
+const rule = require('../../../lib/rules/no-conditional-object-keys');
+
+const parserOptions = {
+	parserOptions: {
+		ecmaVersion: 6,
+		sourceType: 'module',
+	},
+};
+
+const ruleTester = new MultiTester(parserOptions);
+
+const message = 'Object.keys({}) always evaluates to truthy';
+const negationMessage = '!Object.keys({}) always evaluates to false';
+const doubleNegationMessage = '!!Object.keys({}) always evaluates to true';
+
+ruleTester.run('no-conditional-object-keys', rule, {
+	invalid: [
+		{
+			code: `
+				if (Object.keys({foo: 'bar'})) {}
+			`,
+			errors: [
+				{
+					message,
+					type: 'CallExpression',
+				},
+			],
+		},
+		{
+			code: `
+				if (!Object.keys({foo: 'bar'})) {}
+			`,
+			errors: [
+				{
+					message: negationMessage,
+					type: 'CallExpression',
+				},
+			],
+		},
+		{
+			code: `
+				if (!!Object.keys({foo: 'bar'})) {}
+			`,
+			errors: [
+				{
+					message: doubleNegationMessage,
+					type: 'CallExpression',
+				},
+			],
+		},
+		{
+			code: `
+				!Object.keys({foo: 'bar'})
+			`,
+			errors: [
+				{
+					message: negationMessage,
+					type: 'CallExpression',
+				},
+			],
+		},
+		{
+			code: `
+				!!Object.keys({foo: 'bar'})
+			`,
+			errors: [
+				{
+					message: doubleNegationMessage,
+					type: 'CallExpression',
+				},
+			],
+		},
+		{
+			code: `
+				Object.keys({foo: 'bar'}) && 'test'
+			`,
+			errors: [
+				{
+					message,
+					type: 'CallExpression',
+				},
+			],
+		},
+		{
+			code: `
+				!Object.keys({foo: 'bar'}) && 'test'
+			`,
+			errors: [
+				{
+					message: negationMessage,
+					type: 'CallExpression',
+				},
+			],
+		},
+		{
+			code: `
+				!!Object.keys({foo: 'bar'}) && 'test'
+			`,
+			errors: [
+				{
+					message: doubleNegationMessage,
+					type: 'CallExpression',
+				},
+			],
+		},
+		{
+			code: `
+				const keysNegated = !Object.keys({foo: 'bar'});
+			`,
+			errors: [
+				{
+					message: negationMessage,
+					type: 'CallExpression',
+				},
+			],
+		},
+		{
+			code: `
+				const keysDoubleNegated = !!Object.keys({foo: 'bar'});
+			`,
+			errors: [
+				{
+					message: doubleNegationMessage,
+					type: 'CallExpression',
+				},
+			],
+		},
+	],
+	valid: [
+		{
+			code: `
+				if (Object.keys({foo: 'bar'}).length) {}
+			`,
+		},
+		{
+			code: `
+				if (!Object.keys({foo: 'bar'}).length) {}
+			`,
+		},
+		{
+			code: `
+				!Object.keys({foo: 'bar'}).length;
+			`,
+		},
+		{
+			code: `
+				!!Object.keys({foo: 'bar'}).length;
+			`,
+		},
+		{
+			code: `
+				Object.keys({foo: 'bar'}).length && 'test';
+			`,
+		},
+		{
+			code: `
+				!Object.keys({foo: 'bar'}).length && 'test';
+			`,
+		},
+		{
+			code: `
+				!!Object.keys({foo: 'bar'}).length && 'test';
+			`,
+		},
+		{
+			code: `
+				typeof Object.keys({foo: 'bar'});
+			`,
+		},
+		{
+			code: `
+				Object.keys({foo: 'bar'}).find(i => true);
+			`,
+		},
+		{
+			code: `
+				if (Object.keys({foo: 'bar'}).find(i => true)) {};
+			`,
+		},
+		{
+			code: `
+				const keys = Object.keys({foo: 'bar'});
+			`,
+		},
+	],
+});