Skip to content

Commit

Permalink
Breaking: Remove fixToNotOk and checkBooleanAssertions rule optio…
Browse files Browse the repository at this point in the history 
…ns (#197)

* The [notOk](https://api.qunitjs.com/assert/notOk/) assertion was added in QUnit 1.18, and we only support QUnit 2+, so we can enable/remove the `fixToNotOk` option so that rules can assume `notOk` is available
* The [true](https://api.qunitjs.com/assert/true/) and [false](https://api.qunitjs.com/assert/false/) boolean assertions were added in QUnit 2.11, and enabling/removing the `checkBooleanAssertions` option will subject these assertions to additional linting
  • Loading branch information
@bmish
bmish authored Aug 28, 2021
1 parent 32fa473 commit 9882dd2
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 231 deletions.
6 changes: 0 additions & 6 deletions docs/rules/no-compare-relation-boolean.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,6 @@ assert.ok(a > b);

```

## Configuration

This rule takes an optional object containing:

* `fixToNotOk` (boolean, default: true): Whether the rule should autofix examples like `assert.equal(a === b, false)` to `assert.notOk(a === b)` ([notOk](https://api.qunitjs.com/assert/notOk/) was added in QUnit 1.18)

## When Not To Use It

If you are not concerned with the formatting of assertions in any QUnit reporter, you can safely disable this rule.
7 changes: 0 additions & 7 deletions docs/rules/no-negated-ok.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,6 @@ QUnit.test('test', function (assert) {

```

## Configuration

This rule takes an optional object containing:

* `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
* `fixToNotOk` (boolean, default: true): Whether the rule should autofix `assert.ok(!foo)` to `assert.notOk(foo)` ([notOk](https://api.qunitjs.com/assert/notOk/) was added in QUnit 1.18)

## When Not to Use It

Since `assert.notOk()` was only introduced in QUnit 1.18.0, this rule can be
Expand Down
1 change: 0 additions & 1 deletion docs/rules/no-ok-equality.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ QUnit.test("Name", function (assert) { assert.ok(x instanceof Number); });
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

Expand Down
21 changes: 1 addition & 20 deletions lib/rules/no-compare-relation-boolean.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,10 @@ module.exports = {
messages: {
redundantComparison: "Redundant comparison of relational expression to boolean literal."
},
schema: [
{
type: "object",
properties: {
fixToNotOk: {
type: "boolean",
default: true
}
},
additionalProperties: false
}
]
schema: []
},

create: function (context) {
const fixToNotOk = !context.options[0] || context.options[0].fixToNotOk;

const testStack = [],
RELATIONAL_OPS = new Set([
"==", "!=", "===", "!==", "<", "<=", ">", ">=",
Expand Down Expand Up @@ -88,12 +75,6 @@ module.exports = {
countNegations++;
}
const newAssertionFunctionName = countNegations % 2 === 0 ? "ok" : "notOk";

if (newAssertionFunctionName === "notOk" && !fixToNotOk) {
// No autofix in this situation if the rule option is off.
return null;
}

const newArgsTextArray = [binaryExprNode, ...callExprNode.arguments.slice(2)].map(arg => sourceCode.getText(arg));
const newArgsTextJoined = newArgsTextArray.join(", ");
return fixer.replaceText(callExprNode, `${assertionVariableName}.${newAssertionFunctionName}(${newArgsTextJoined})`);
Expand Down
33 changes: 5 additions & 28 deletions lib/rules/no-negated-ok.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,30 +30,12 @@ module.exports = {
messages: {
noNegationInOk: "Unexpected negation in {{callee}}() assertion."
},
schema: [
{
type: "object",
properties: {
checkBooleanAssertions: {
type: "boolean",
default: false
},
fixToNotOk: {
type: "boolean",
default: true
}
},
additionalProperties: false
}
]
schema: []
},

create: function (context) {
const checkBooleanAssertions = context.options[0] && context.options[0].checkBooleanAssertions;
const fixToNotOk = !context.options[0] || context.options[0].fixToNotOk;

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

// Declare a stack in case of nested test cases (not currently supported
// in QUnit).
Expand Down Expand Up @@ -131,18 +113,13 @@ module.exports = {
fix(fixer) {
// Conversions:
// * assert.notOk(!foo) => assert.ok(foo)
// * assert.ok(!foo) => assert.equal(foo, false) -- when `fixToNotOk` option disabled
// * assert.ok(!foo) => assert.notOk(foo) -- when `fixToNotOk` option enabled
// * assert.ok(!foo) => assert.notOk(foo)

const assertionVariableName = callExprNode.callee.object.name;
const oppositeAssertionFunctionName = ASSERTION_OPPOSITES[callExprNode.callee.property.name];
const newAssertionFunctionName = !fixToNotOk && oppositeAssertionFunctionName === "notOk" ? "equal" : oppositeAssertionFunctionName;
const newArgsTextArray = [unwrapNegation(firstArgNode), ...callExprNode.arguments.slice(1)].map(arg => sourceCode.getText(arg));
if (newAssertionFunctionName === "equal") {
newArgsTextArray.splice(1, 0, "false");
}
const newArgsTextJoined = newArgsTextArray.join(", ");
return fixer.replaceText(callExprNode, `${assertionVariableName}.${newAssertionFunctionName}(${newArgsTextJoined})`);
return fixer.replaceText(callExprNode, `${assertionVariableName}.${oppositeAssertionFunctionName}(${newArgsTextJoined})`);
}
});
}
Expand Down
18 changes: 7 additions & 11 deletions lib/rules/no-ok-equality.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ module.exports = {
properties: {
allowGlobal: {
type: "boolean"
},
checkBooleanAssertions: {
type: "boolean"
}
},
additionalProperties: false
Expand All @@ -43,14 +40,13 @@ module.exports = {
// in QUnit).
const asyncStateStack = [],
DEFAULT_OPTIONS = {
allowGlobal: true,
checkBooleanAssertions: false
allowGlobal: true
},
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"];
const POSITIVE_ASSERTIONS = new Set(["ok", "true"]);
const NEGATIVE_ASSERTIONS = new Set(["notOk", "false"]);

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

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

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

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

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

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

if (options.allowGlobal) {
return isNotOk || isAssertNotOk;
Expand Down
29 changes: 0 additions & 29 deletions tests/lib/rules/no-compare-relation-boolean.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,28 +69,15 @@ ruleTester.run("no-compare-relation-boolean", rule, {
},
{
code: "assert.equal(a === b, false);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},
{
// fixToNotOk = true (implicit since this is the option's default)
code: "assert.equal(a === b, false);",
output: "assert.notOk(a === b);"
},
{
// No autofix when rule option `fixToNotOk` is off explicitly.
code: "assert.equal(a === b, false);",
options: [{ fixToNotOk: false }],
output: null
},

{
code: "assert.equal(a === b, true, 'message');", // With message
output: "assert.ok(a === b, 'message');"
},
{
code: "assert.equal(a === b, false, 'message');", // With message
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b, 'message');"
},

Expand All @@ -100,7 +87,6 @@ ruleTester.run("no-compare-relation-boolean", rule, {
},
{
code: "assert.strictEqual(a === b, false);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},

Expand All @@ -110,7 +96,6 @@ ruleTester.run("no-compare-relation-boolean", rule, {
},
{
code: "assert.deepEqual(a === b, false);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},

Expand All @@ -120,13 +105,11 @@ ruleTester.run("no-compare-relation-boolean", rule, {
},
{
code: "assert.propEqual(a === b, false);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},

{
code: "assert.notEqual(a === b, true);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},
{
Expand All @@ -136,7 +119,6 @@ ruleTester.run("no-compare-relation-boolean", rule, {

{
code: "assert.notStrictEqual(a === b, true);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},
{
Expand All @@ -146,7 +128,6 @@ ruleTester.run("no-compare-relation-boolean", rule, {

{
code: "assert.notDeepEqual(a === b, true);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},
{
Expand All @@ -156,7 +137,6 @@ ruleTester.run("no-compare-relation-boolean", rule, {

{
code: "assert.notPropEqual(a === b, true);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},
{
Expand All @@ -171,7 +151,6 @@ ruleTester.run("no-compare-relation-boolean", rule, {
},
{
code: "assert.equal(false, a === b);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},

Expand All @@ -181,7 +160,6 @@ ruleTester.run("no-compare-relation-boolean", rule, {
},
{
code: "assert.equal(false, a === b, 'message');", // With message
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b, 'message');"
},

Expand All @@ -191,7 +169,6 @@ ruleTester.run("no-compare-relation-boolean", rule, {
},
{
code: "assert.strictEqual(false, a === b);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},

Expand All @@ -201,7 +178,6 @@ ruleTester.run("no-compare-relation-boolean", rule, {
},
{
code: "assert.deepEqual(false, a === b);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},

Expand All @@ -211,13 +187,11 @@ ruleTester.run("no-compare-relation-boolean", rule, {
},
{
code: "assert.propEqual(false, a === b);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},

{
code: "assert.notEqual(true, a === b);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},
{
Expand All @@ -227,7 +201,6 @@ ruleTester.run("no-compare-relation-boolean", rule, {

{
code: "assert.notStrictEqual(true, a === b);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},
{
Expand All @@ -237,7 +210,6 @@ ruleTester.run("no-compare-relation-boolean", rule, {

{
code: "assert.notDeepEqual(true, a === b);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},
{
Expand All @@ -247,7 +219,6 @@ ruleTester.run("no-compare-relation-boolean", rule, {

{
code: "assert.notPropEqual(true, a === b);",
options: [{ fixToNotOk: true }],
output: "assert.notOk(a === b);"
},
{
Expand Down
Loading

0 comments on commit 9882dd2

Please sign in to comment.