Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Breaking: Remove fixToNotOk and checkBooleanAssertions rule options #197

Merged
merged 1 commit into from
Aug 28, 2021
Merged

Breaking: Remove fixToNotOk and checkBooleanAssertions rule options #197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions docs/rules/no-compare-relation-boolean.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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