Merge 0b09f97 into 379df8f

README.md

@@ -66,6 +66,7 @@ Each rule has emojis denoting:
 | :two: | [no-test-expect-argument](./docs/rules/no-test-expect-argument.md) | disallow the expect argument in QUnit.test|
 | :white_check_mark::two: | [no-throws-string](./docs/rules/no-throws-string.md) | disallow assert.throws() with block, string, and message args|
 | :white_check_mark: | [require-expect](./docs/rules/require-expect.md) | enforce that `expect` is called|
+|  | [require-object-in-propequal](./docs/rules/require-object-in-propequal.md) | enforce use of objects as expected value in `assert.propEqual`|
 | :white_check_mark: | [resolve-async](./docs/rules/resolve-async.md) | require that async calls are resolved|
 
 <!--RULES_TABLE_END-->

docs/rules/require-object-in-propequal.md

@@ -0,0 +1,41 @@
+# Enforce use of objects as expected value in `assert.propEqual` (require-object-in-propequal)
+
+The `assert.propEqual` assertion is for the strict-equality comparison of own-properties
+of two objects. If the expected value is a string or other non-object, the assertion
+result can be unexpected.
+
+For string comparisons, `assert.strictEqual` should be used. For arrays,
+`assert.deepStrictEqual` should be used.
+
+## Rule Details
+
+The following patterns are considered warnings:
+
+```js
+assert.propEqual(actual, 0);
+
+assert.propEqual(actual, "foo");
+
+assert.propEqual(actual, `foo`);
+
+assert.propEqual(actual, /regex/);
+```
+
+The following patterns are not considered warnings:
+
+```js
+assert.propEqual(actual, { foo: "bar" });
+
+assert.propEqual(actual, ["array"]);
+
+// Variables are not checked
+assert.propEqual(actual, foo);
+```
+
+## When Not to Use It
+
+This rule should be reasonably safe to use in all circumstances.
+
+## Further Reading
+
+[`assert.propEqual`](https://api.qunitjs.com/assert/propEqual/)

lib/rules/require-object-in-propequal.js

@@ -0,0 +1,109 @@
+/**
+ * @fileoverview Enforce use of objects as expected values in `assert.propEqual`
+ * @author Kevin Partington
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const assert = require("assert"),
+    utils = require("../utils");
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = {
+    meta: {
+        type: "problem",
+        docs: {
+            description: "enforce use of objects as expected value in `assert.propEqual`",
+            category: "Possible Errors",
+            url: "https://github.com/platinumazure/eslint-plugin-qunit/blob/master/docs/rules/require-object-in-propequal.md"
+        },
+        messages: {
+            useObject: "Use object as propEqual expected value (found: {{ value }})."
+        },
+        schema: []
+    },
+
+    create: function (context) {
+        // Declare a test stack in case of nested test cases (not currently supported by QUnit).
+        const sourceCode = context.getSourceCode(),
+            testStack = [];
+
+        function getCurrentAssertContextVariable() {
+            assert(testStack.length, "Test stack should not be empty");
+
+            return testStack[testStack.length - 1].assertVar;
+        }
+
+        function isGlobalPropEqual(calleeNode) {
+            return calleeNode &&
+                calleeNode.type === "Identifier" &&
+                calleeNode.name === "propEqual";
+        }
+
+        function isAssertPropEqual(calleeNode) {
+            return calleeNode &&
+                calleeNode.type === "MemberExpression" &&
+                calleeNode.property.type === "Identifier" &&
+                calleeNode.property.name === "propEqual" &&
+                calleeNode.object.type === "Identifier" &&
+                calleeNode.object.name === getCurrentAssertContextVariable();
+        }
+
+        function isPropEqual(calleeNode) {
+            return isAssertPropEqual(calleeNode) || isGlobalPropEqual(calleeNode);
+        }
+
+        function isValidExpectedValue(argNode) {
+            switch (argNode.type) {
+                case "Literal":
+                case "TemplateLiteral":
+                case "JSXElement":
+                case "UpdateExpression":
+                case "UnaryExpression":
+                    return false;
+
+                default:
+                    return true;
+            }
+        }
+
+        function hasNonObjectExpectedValue(callExpressionNode) {
+            return callExpressionNode &&
+                callExpressionNode.arguments &&
+                callExpressionNode.arguments.length >= 2 &&
+                !isValidExpectedValue(callExpressionNode.arguments[1]);
+        }
+
+        return {
+            "CallExpression": function (node) {
+                /* istanbul ignore else: correctly does nothing */
+                if (utils.isTest(node.callee) || utils.isAsyncTest(node.callee)) {
+                    testStack.push({
+                        assertVar: utils.getAssertContextNameForTest(node.arguments)
+                    });
+                } else if (testStack.length && isPropEqual(node.callee) && hasNonObjectExpectedValue(node)) {
+                    context.report({
+                        node,
+                        messageId: "useObject",
+                        data: {
+                            value: sourceCode.getText(node.arguments[1])
+                        }
+                    });
+                }
+            },
+
+            "CallExpression:exit": function (node) {
+                /* istanbul ignore else: correctly does nothing */
+                if (utils.isTest(node.callee) || utils.isAsyncTest(node.callee)) {
+                    testStack.pop();
+                }
+            }
+        };
+    }
+};

tests/lib/rules/require-object-in-propequal.js

@@ -0,0 +1,109 @@
+/**
+ * @fileoverview Enforce use of objects as expected values in `assert.propEqual`
+ * @author Kevin Partington
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require("../../../lib/rules/require-object-in-propequal"),
+    RuleTester = require("eslint").RuleTester;
+
+//------------------------------------------------------------------------------
+// Helpers
+//------------------------------------------------------------------------------
+
+function wrap(assertionCode, testName) {
+    testName = testName || "Name";
+    return `QUnit.test('${testName}', function (assert) { ${assertionCode} });`;
+}
+
+function createInvalid(assertionCode, invalidValue) {
+    return {
+        code: wrap(assertionCode),
+        errors: [{
+            messageId: "useObject",
+            data: {
+                value: invalidValue
+            }
+        }]
+    };
+}
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester({
+    parserOptions: {
+        ecmaVersion: 2015,
+        ecmaFeatures: { jsx: true }
+    }
+});
+
+ruleTester.run("require-object-in-propequal", rule, {
+    valid: [
+        // Object expressions/array expressions
+        wrap("assert.propEqual(actual, { foo: 'bar' });"),
+        wrap("assert.propEqual(actual, ['string']);"),
+
+        // Identifiers, member expressions, calls, and new expressions are fine
+        wrap("assert.propEqual(actual, someVar);"),
+        wrap("assert.propEqual(actual, obj.prop);"),
+        wrap("assert.propEqual(actual, func());"),
+        wrap("assert.propEqual(actual, new Foo());"),
+
+        // this is fine
+        wrap("assert.propEqual(actual, this);"),
+
+        // Global assertion
+        wrap("propEqual(actual, { foo: 'bar' });"),
+
+        // Not propEqual
+        wrap("assert.deepEqual(actual, { foo: 'bar' });"),
+        wrap("assert.deepEqual(actual, 0);"),
+        wrap("assert.deepEqual(actual, -1);"),
+        wrap("assert.deepEqual(actual, 'string');"),
+        wrap("assert.deepEqual(actual, `template`);"),
+        wrap("assert.deepEqual(actual, true);"),
+        wrap("assert.deepEqual(actual, false);"),
+        wrap("assert.deepEqual(actual, null);"),
+        wrap("assert.deepEqual(actual, /regex/);"),
+        wrap("assert.deepEqual(actual, ++foo);"),
+        wrap("assert.deepEqual(actual, foo++);"),
+        wrap("assert.deepEqual(actual, --foo);"),
+        wrap("assert.deepEqual(actual, foo--);"),
+        wrap("assert.deepEqual(actual, <JSX />);")
+
+        // eslint-disable-next-line no-warning-comments
+        // TODO: Uncomment when support for ESLint 5 is dropped
+        // wrap("assert.deepEqual(actual, 0n);"),
+
+        // eslint-disable-next-line no-warning-comments
+        // TODO: Uncomment when support for ESLint 5 is dropped
+        // wrap("assert.propEqual(actual, foo?.bar);"),
+        // wrap("assert.propEqual(actual, foo?.bar?.());")
+    ],
+
+    invalid: [
+        createInvalid("assert.propEqual(actual, 0);", "0"),
+        createInvalid("assert.propEqual(actual, -1);", "-1"),
+        createInvalid("assert.propEqual(actual, 'string');", "'string'"),
+        createInvalid("assert.propEqual(actual, `template`);", "`template`"),
+        createInvalid("assert.propEqual(actual, true);", "true"),
+        createInvalid("assert.propEqual(actual, false);", "false"),
+        createInvalid("assert.propEqual(actual, null);", "null"),
+        createInvalid("assert.propEqual(actual, /regex/);", "/regex/"),
+        createInvalid("assert.propEqual(actual, ++foo);", "++foo"),
+        createInvalid("assert.propEqual(actual, foo++);", "foo++"),
+        createInvalid("assert.propEqual(actual, --foo);", "--foo"),
+        createInvalid("assert.propEqual(actual, foo--);", "foo--"),
+        createInvalid("assert.propEqual(actual, <JSX />)", "<JSX />")
+
+        // eslint-disable-next-line no-warning-comments
+        // TODO: Uncomment when support for ESLint 5 is dropped
+        // createInvalid("assert.propEqual(actual, 0n);", "0n"),
+    ]
+});