Skip to content

Commit

Permalink
Chore: avoid loose equality assertions (#9415)
Browse files Browse the repository at this point in the history
* Chore: avoid loose equality assertions

This updates the `no-restricted-syntax` configuration to disallow
`assert.equal` and `assert.notEqual` in favor of `assert.strictEqual`
and `assert.notStrictEqual`. It also fixes a few places where the tests
were relying on a loose equality check (e.g. comparing a single-element
    array to a string).

* Use assert.deepStrictEqual instead of assert.deepEqual

* Revert accidental fixture changes
  • Loading branch information
not-an-aardvark authored and kaicataldo committed Oct 14, 2017
1 parent 2247efa commit febb897
Show file tree
Hide file tree
Showing 54 changed files with 1,796 additions and 1,778 deletions.
22 changes: 18 additions & 4 deletions lib/testers/rule-tester.js
Expand Up @@ -369,6 +369,7 @@ class RuleTester {
if (!lodash.isEqual(beforeAST, afterAST)) {

// Not using directly to avoid performance problem in node 6.1.0. See #6111
// eslint-disable-next-line no-restricted-properties
assert.deepEqual(beforeAST, afterAST, "Rule should not modify AST.");
}
}
Expand All @@ -384,7 +385,7 @@ class RuleTester {
const result = runRuleForItem(item);
const messages = result.messages;

assert.equal(messages.length, 0, util.format("Should have no errors but had %d: %s",
assert.strictEqual(messages.length, 0, util.format("Should have no errors but had %d: %s",
messages.length, util.inspect(messages)));

assertASTDidntChange(result.beforeAST, result.afterAST);
Expand All @@ -408,7 +409,7 @@ class RuleTester {
`Expected '${actual}' to match ${expected}`
);
} else {
assert.equal(actual, expected);
assert.strictEqual(actual, expected);
}
}

Expand All @@ -428,10 +429,10 @@ class RuleTester {


if (typeof item.errors === "number") {
assert.equal(messages.length, item.errors, util.format("Should have %d error%s but had %d: %s",
assert.strictEqual(messages.length, item.errors, util.format("Should have %d error%s but had %d: %s",
item.errors, item.errors === 1 ? "" : "s", messages.length, util.inspect(messages)));
} else {
assert.equal(
assert.strictEqual(
messages.length, item.errors.length,
util.format(
"Should have %d error%s but had %d: %s",
Expand Down Expand Up @@ -460,23 +461,35 @@ class RuleTester {
assertMessageMatches(messages[i].message, item.errors[i].message);
}

// The following checks use loose equality assertions for backwards compatibility.

if (item.errors[i].type) {

// eslint-disable-next-line no-restricted-properties
assert.equal(messages[i].nodeType, item.errors[i].type, `Error type should be ${item.errors[i].type}, found ${messages[i].nodeType}`);
}

if (item.errors[i].hasOwnProperty("line")) {

// eslint-disable-next-line no-restricted-properties
assert.equal(messages[i].line, item.errors[i].line, `Error line should be ${item.errors[i].line}`);
}

if (item.errors[i].hasOwnProperty("column")) {

// eslint-disable-next-line no-restricted-properties
assert.equal(messages[i].column, item.errors[i].column, `Error column should be ${item.errors[i].column}`);
}

if (item.errors[i].hasOwnProperty("endLine")) {

// eslint-disable-next-line no-restricted-properties
assert.equal(messages[i].endLine, item.errors[i].endLine, `Error endLine should be ${item.errors[i].endLine}`);
}

if (item.errors[i].hasOwnProperty("endColumn")) {

// eslint-disable-next-line no-restricted-properties
assert.equal(messages[i].endColumn, item.errors[i].endColumn, `Error endColumn should be ${item.errors[i].endColumn}`);
}
} else {
Expand All @@ -497,6 +510,7 @@ class RuleTester {
} else {
const fixResult = SourceCodeFixer.applyFixes(item.code, messages);

// eslint-disable-next-line no-restricted-properties
assert.equal(fixResult.output, item.output, "Output is incorrect.");
}
}
Expand Down
6 changes: 5 additions & 1 deletion packages/eslint-config-eslint/default.yml
Expand Up @@ -85,7 +85,11 @@ rules:
no-restricted-properties: [
"error",
{ property: "substring", message: "Use .slice instead of .substring." },
{ property: "substr", message: "Use .slice instead of .substr." }
{ property: "substr", message: "Use .slice instead of .substr." },
{ object: "assert", property: "equal", message: "Use assert.strictEqual instead of assert.equal." },
{ object: "assert", property: "notEqual", message: "Use assert.notStrictEqual instead of assert.notEqual." },
{ object: "assert", property: "deepEqual", message: "Use assert.deepStrictEqual instead of assert.deepStrictEqual." },
{ object: "assert", property: "notDeepEqual", message: "Use assert.notDeepStrictEqual instead of assert.notDeepEqual." }
]
no-return-assign: "error"
no-script-url: "error"
Expand Down
2 changes: 1 addition & 1 deletion tests/conf/eslint-all.js
Expand Up @@ -34,6 +34,6 @@ describe("eslint-all", () => {
const ruleNames = Object.keys(rules);
const nonErrorRules = ruleNames.filter(ruleName => rules[ruleName] !== "error");

assert.equal(nonErrorRules.length, 0);
assert.strictEqual(nonErrorRules.length, 0);
});
});

0 comments on commit febb897

Please sign in to comment.