Skip to content

Commit

Permalink
Update: add checkBooleanAssertions option to no-ok-equality rule (#173)
Browse files Browse the repository at this point in the history
  • Loading branch information
bmish authored Apr 22, 2021
1 parent e8403d5 commit 291a6c1
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 5 deletions.
7 changes: 7 additions & 0 deletions docs/rules/no-ok-equality.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ QUnit.test("Name", function (assert) { assert.ok(x instanceof Number); });

```

## Configuration

This rule takes an optional object containing:

* `allowGlobals` (boolean, default: true): Whether the rule should check global assertions
* `checkBooleanAssertions` (boolean, default: false): Whether the rule should check the [true](https://api.qunitjs.com/assert/true/) and [false](https://api.qunitjs.com/assert/false/) boolean assertions

## When Not to Use It

It is best to enable this rule, but since it is possible to test a codebase
Expand Down
17 changes: 12 additions & 5 deletions lib/rules/no-ok-equality.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ module.exports = {
properties: {
allowGlobal: {
type: "boolean"
},
checkBooleanAssertions: {
type: "boolean"
}
},
additionalProperties: false
Expand All @@ -40,11 +43,15 @@ module.exports = {
// in QUnit).
const asyncStateStack = [],
DEFAULT_OPTIONS = {
allowGlobal: true
allowGlobal: true,
checkBooleanAssertions: false
},
options = context.options[0] || DEFAULT_OPTIONS,
sourceCode = context.getSourceCode();

const POSITIVE_ASSERTIONS = options.checkBooleanAssertions ? ["ok", "true"] : ["ok"];
const NEGATIVE_ASSERTIONS = options.checkBooleanAssertions ? ["notOk", "false"] : ["notOk"];

function getAssertContextVar() {
const state = asyncStateStack[asyncStateStack.length - 1];
return state && state.assertContextVar;
Expand All @@ -53,13 +60,13 @@ module.exports = {
function isOk(calleeNode) {
const assertContextVar = getAssertContextVar();

const isOk = calleeNode.type === "Identifier" && calleeNode.name === "ok";
const isOk = calleeNode.type === "Identifier" && POSITIVE_ASSERTIONS.includes(calleeNode.name);

const isAssertOk = calleeNode.type === "MemberExpression" &&
calleeNode.object.type === "Identifier" &&
calleeNode.object.name === assertContextVar &&
calleeNode.property.type === "Identifier" &&
calleeNode.property.name === "ok";
POSITIVE_ASSERTIONS.includes(calleeNode.property.name);

if (options.allowGlobal) {
return isOk || isAssertOk;
Expand All @@ -71,13 +78,13 @@ module.exports = {
function isNotOk(calleeNode) {
const assertContextVar = getAssertContextVar();

const isNotOk = calleeNode.type === "Identifier" && calleeNode.name === "notOk";
const isNotOk = calleeNode.type === "Identifier" && NEGATIVE_ASSERTIONS.includes(calleeNode.name);

const isAssertNotOk = calleeNode.type === "MemberExpression" &&
calleeNode.object.type === "Identifier" &&
calleeNode.object.name === assertContextVar &&
calleeNode.property.type === "Identifier" &&
calleeNode.property.name === "notOk";
NEGATIVE_ASSERTIONS.includes(calleeNode.property.name);

if (options.allowGlobal) {
return isNotOk || isAssertNotOk;
Expand Down
70 changes: 70 additions & 0 deletions tests/lib/rules/no-ok-equality.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,38 @@ ruleTester.run("no-ok-equality", rule, {
{
code: "test('Name', function () { notOk(x != 1); });",
options: [{ allowGlobal: false }]
},

// Boolean assertions with no equality checks:
{
code: "test('Name', function (assert) { assert.true(x); });",
options: [{ checkBooleanAssertions: true }]
},
{
code: "test('Name', function (assert) { assert.true(x, 'message'); });",
options: [{ checkBooleanAssertions: true }]
},
{
code: "test('Name', function (assert) { assert.false(x); });",
options: [{ checkBooleanAssertions: true }]
},
{
code: "test('Name', function (assert) { assert.false(x, 'message'); });",
options: [{ checkBooleanAssertions: true }]
},

// Boolean assertions with equality checks (checkBooleanAssertions = false, implicitly)
"test('Name', function (assert) { assert.true(x === 1); });",
"test('Name', function (assert) { assert.false(x === 1); });",

// Boolean assertions with equality checks (checkBooleanAssertions = false, explicitly)
{
code: "test('Name', function (assert) { assert.true(x === 1); });",
options: [{ checkBooleanAssertions: false }]
},
{
code: "test('Name', function (assert) { assert.false(x === 1); });",
options: [{ checkBooleanAssertions: false }]
}
],

Expand Down Expand Up @@ -210,6 +242,44 @@ ruleTester.run("no-ok-equality", rule, {
errors: [
createError("notOk", "equal", "x", "1")
]
},

// Boolean assertions with equality checks (checkBooleanAssertions = true, explicitly)
{
// true
code: "test('Name', function (assert) { assert.true(x === 1); });",
output: "test('Name', function (assert) { assert.strictEqual(x, 1); });",
options: [{ checkBooleanAssertions: true }],
errors: [
createError("assert.true", "assert.strictEqual", "x", "1")
]
},
{
// true, with message
code: "test('Name', function (assert) { assert.true(x === 1, 'message'); });",
output: "test('Name', function (assert) { assert.strictEqual(x, 1, 'message'); });",
options: [{ checkBooleanAssertions: true }],
errors: [
createError("assert.true", "assert.strictEqual", "x", "1")
]
},
{
// false
code: "test('Name', function (assert) { assert.false(x === 1); });",
output: "test('Name', function (assert) { assert.notStrictEqual(x, 1); });",
options: [{ checkBooleanAssertions: true }],
errors: [
createError("assert.false", "assert.notStrictEqual", "x", "1")
]
},
{
// false, with message
code: "test('Name', function (assert) { assert.false(x === 1, 'message'); });",
output: "test('Name', function (assert) { assert.notStrictEqual(x, 1, 'message'); });",
options: [{ checkBooleanAssertions: true }],
errors: [
createError("assert.false", "assert.notStrictEqual", "x", "1")
]
}
]

Expand Down

0 comments on commit 291a6c1

Please sign in to comment.