Rule proposal: No deepEqual for primitives values

[jfmengels]

I see a lot of of uses of t.deepEqual that compares to a promitive value when a simple t.is(a, b) or t.true(a === b) would work.

t.deepEqual(value, 'foo');

I'm not sure if there is a big perf hit or something, but I think it's better to use a simpler form of assertion whenever possible. I personally like to use t.is() or t.true(===) instead (frankly, most of my assertions now are t.true())

Invalid

t.deepEqual(expression, 'foo');
t.deepEqual(expression, 1);
t.deepEqual(expression, `foo${bar}`);
t.deepEqual(expression, null);
t.deepEqual(expression, undefined);
t.notDeepEqual(expression, undefined);
t.deepEqual(1, expression); // ?

Valid

t.deepEqual(expression, otherExpression);
t.deepEqual(expression, {});
t.deepEqual(expression, []);
t.notDeepEqual(expression, []);

As for the rule name, I think this only targets deepEqual and notDeepEqual, so I propose no-simple-deep-equal or something equivalent (but would love a better name for it).

---

IssueHunt Summary

mrhen has been rewarded.

Backers (Total: $60.00)

• issuehunt ($60.00)

Submitted pull Requests

• #264 Add no-incorrect-deep-equal rule

---

Tips

• Checkout the Issuehunt explorer to discover more funded issues.
• Need some help from other developers? Add your repositories on IssueHunt to raise funds.

---

IssueHunt has been backed by the following sponsors. Become a sponsor

[sindresorhus]

Sounds good. Using t.is()/t.not() instead also makes the intent clearer.

I couldn't think of a better name. Only thing that comes to mind is no-incorrect-deep-equal.

Could this be auto-fixable?

[jfmengels]

that sounds like a really easy fix, just change the name of the method) to is/not.

Rule name open to bikeshed for those interested :)

[revelt]

I wanted to add my 2p. with regards of t.true. It's is bad because wrong value does not get reported:

Having said that, it would be nice if t.deepEqual was automatically replaced with t.is and backwards — it is very important to consider the real user's workflow.

Namely. Users have gajillion of unit tests, it's already tedious to visually grep them. Now, if linter could do optimisation switching deepEqual to is and backwards, AUTOMATICALLY for me, I'd appreciate that. If it will just nag me, I refuse to enable such rule.

Like Sindre said, definitely, let's do auto-fixable. And very important, both ways. Detect comparison to "object-type", switch to deepEqual. Detect compared-to value being number or Bool, switch to t.is.

What do you think about backwards (is -> deepEqual) scenario?

[novemberborn]

It's is bad because wrong value does not get reported:

Your example doesn't use variables though, I think the values would be displayed, though to your point not as nicely as with t.is().

What do you think about backwards (is -> deepEqual) scenario?

👍

[IssueHuntBot]

@issuehunt has funded $60.00 to this issue.

---

• Submit pull request via IssueHunt to receive this reward.
• Want to contribute? Chip in to this issue via IssueHunt.
• Checkout the IssueHunt Issue Explorer to see more funded issues.
• Need help from developers? Add your repository on IssueHunt to raise funds.

[issuehunt-oss]

@sindresorhus has rewarded $54.00 to @MrHen. See it on IssueHunt

• 💰 Total deposit: $60.00
• 🎉 Repository reward(0%): $0.00
• 🔧 Service fee(10%): $6.00

[Airkro]

Comparing regular expressions with is() makes my tests fail.

const ext2regexp = require('ext-to-regexp');

const jsx = ext2regexp('js', 'jsx');

t.deepEqual(jsx, /\.(js|jsx)$/); // Pass

t.is(jsx, /\.(js|jsx)$/);
// Fail: Values are deeply equal to each other, but they are not the same

[revelt]

@Airkro but it's expected, jsx like you defined is one object, the regex you're comparing it with is another object. Just like ["z"] !== ["z"], same thing, is is strict === comparison.

[Airkro]

@revelt I know, but this rule keep telling me that I should use is().

[sindresorhus]

@Airkro Open a new issue about this. I guess we can add support for detecting t.deepEqual(jsx, /\.(js|jsx)$/); and t.is(jsx, /\.(js|jsx)$/); in the https://github.com/avajs/eslint-plugin-ava/blob/master/docs/rules/prefer-t-regex.md rule.

[novemberborn]

We shouldn't recommend t.is() when a regular expression is passed to t.deepEqual() though. That'd be a bug. @Airkro could you open a new issue for this?

[Airkro]

#270 sure