From 2f9ed805a9cbf1b47cf598f95ff76ba129bdc36b Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Mon, 12 Jul 2021 22:40:18 -0400 Subject: [PATCH] Breaking: Remove `fixToNotOk` and `checkBooleanAssertions` rule options * 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 --- docs/rules/no-compare-relation-boolean.md | 6 -- docs/rules/no-negated-ok.md | 7 -- docs/rules/no-ok-equality.md | 1 - lib/rules/no-compare-relation-boolean.js | 21 +--- lib/rules/no-negated-ok.js | 33 +------ lib/rules/no-ok-equality.js | 18 ++-- .../lib/rules/no-compare-relation-boolean.js | 29 ------ tests/lib/rules/no-negated-ok.js | 98 +------------------ tests/lib/rules/no-ok-equality.js | 40 +------- 9 files changed, 22 insertions(+), 231 deletions(-) diff --git a/docs/rules/no-compare-relation-boolean.md b/docs/rules/no-compare-relation-boolean.md index 37185f1a..b7c6d0d3 100644 --- a/docs/rules/no-compare-relation-boolean.md +++ b/docs/rules/no-compare-relation-boolean.md @@ -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. diff --git a/docs/rules/no-negated-ok.md b/docs/rules/no-negated-ok.md index b3a1baaa..43c61b39 100644 --- a/docs/rules/no-negated-ok.md +++ b/docs/rules/no-negated-ok.md @@ -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 diff --git a/docs/rules/no-ok-equality.md b/docs/rules/no-ok-equality.md index ac37732c..96933ce8 100644 --- a/docs/rules/no-ok-equality.md +++ b/docs/rules/no-ok-equality.md @@ -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 diff --git a/lib/rules/no-compare-relation-boolean.js b/lib/rules/no-compare-relation-boolean.js index ce8b09cf..33e3ff05 100644 --- a/lib/rules/no-compare-relation-boolean.js +++ b/lib/rules/no-compare-relation-boolean.js @@ -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([ "==", "!=", "===", "!==", "<", "<=", ">", ">=", @@ -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})`); diff --git a/lib/rules/no-negated-ok.js b/lib/rules/no-negated-ok.js index 9f0d10e5..5b174e4d 100644 --- a/lib/rules/no-negated-ok.js +++ b/lib/rules/no-negated-ok.js @@ -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). @@ -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})`); } }); } diff --git a/lib/rules/no-ok-equality.js b/lib/rules/no-ok-equality.js index fac4d461..bfc5d3cb 100644 --- a/lib/rules/no-ok-equality.js +++ b/lib/rules/no-ok-equality.js @@ -28,9 +28,6 @@ module.exports = { properties: { allowGlobal: { type: "boolean" - }, - checkBooleanAssertions: { - type: "boolean" } }, additionalProperties: false @@ -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]; @@ -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; @@ -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; diff --git a/tests/lib/rules/no-compare-relation-boolean.js b/tests/lib/rules/no-compare-relation-boolean.js index 40315dce..db76a081 100644 --- a/tests/lib/rules/no-compare-relation-boolean.js +++ b/tests/lib/rules/no-compare-relation-boolean.js @@ -69,20 +69,8 @@ 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 @@ -90,7 +78,6 @@ ruleTester.run("no-compare-relation-boolean", rule, { }, { code: "assert.equal(a === b, false, 'message');", // With message - options: [{ fixToNotOk: true }], output: "assert.notOk(a === b, 'message');" }, @@ -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);" }, @@ -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);" }, @@ -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);" }, { @@ -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);" }, { @@ -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);" }, { @@ -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);" }, { @@ -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);" }, @@ -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');" }, @@ -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);" }, @@ -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);" }, @@ -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);" }, { @@ -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);" }, { @@ -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);" }, { @@ -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);" }, { diff --git a/tests/lib/rules/no-negated-ok.js b/tests/lib/rules/no-negated-ok.js index 97419084..998e6956 100644 --- a/tests/lib/rules/no-negated-ok.js +++ b/tests/lib/rules/no-negated-ok.js @@ -94,64 +94,15 @@ ruleTester.run("no-negated-ok", rule, { wrap("foo.assert.bar.ok.baz(!a)"), // Boolean assertions, no negation - { - code: wrap("assert.true(foo)"), - options: [{ checkBooleanAssertions: true }] - }, - { - code: wrap("assert.true(foo, 'message')"), - options: [{ checkBooleanAssertions: true }] - }, - { - code: wrap("assert.false(foo)"), - options: [{ checkBooleanAssertions: true }] - }, - { - code: wrap("assert.false(foo, 'message')"), - options: [{ checkBooleanAssertions: true }] - }, - - // Boolean assertions, negation, checkBooleanAssertions = false (implicitly) - wrap("assert.true(!foo)"), - wrap("assert.true(!foo, 'message')"), - wrap("assert.false(!foo)"), - wrap("assert.false(!foo, 'message')"), - - // Boolean assertions, negation, checkBooleanAssertions = false (explicitly) - { - code: wrap("assert.true(!foo)"), - options: [{ checkBooleanAssertions: false }] - }, - { - code: wrap("assert.true(!foo, 'message')"), - options: [{ checkBooleanAssertions: false }] - }, - { - code: wrap("assert.false(!foo)"), - options: [{ checkBooleanAssertions: false }] - }, - { - code: wrap("assert.false(!foo, 'message')"), - options: [{ checkBooleanAssertions: false }] - } + wrap("assert.true(foo)"), + wrap("assert.true(foo, 'message')"), + wrap("assert.false(foo)"), + wrap("assert.false(foo, 'message')") ], invalid: [ // ok { - code: wrap("assert.ok(!foo)"), - output: wrap("assert.equal(foo, false)"), - options: [{ fixToNotOk: false }], - errors: [createError("assert.ok")] - }, - { - code: wrap("assert.ok(!foo)"), - output: wrap("assert.notOk(foo)"), - options: [{ fixToNotOk: true }], - errors: [createError("assert.ok")] - }, - { - // fixToNotOk = true (implicit since this is the option's default) code: wrap("assert.ok(!foo)"), output: wrap("assert.notOk(foo)"), errors: [createError("assert.ok")] @@ -159,19 +110,6 @@ ruleTester.run("no-negated-ok", rule, { // ok (with message) { - code: wrap("assert.ok(!foo, 'message')"), - output: wrap("assert.equal(foo, false, 'message')"), - options: [{ fixToNotOk: false }], - errors: [createError("assert.ok")] - }, - { - code: wrap("assert.ok(!foo, 'message')"), - output: wrap("assert.notOk(foo, 'message')"), - options: [{ fixToNotOk: true }], - errors: [createError("assert.ok")] - }, - { - // fixToNotOk = true (implicit since this is the option's default) code: wrap("assert.ok(!foo, 'message')"), output: wrap("assert.notOk(foo, 'message')"), errors: [createError("assert.ok")] @@ -190,20 +128,7 @@ ruleTester.run("no-negated-ok", rule, { }, // triple negation is not allowed - { - code: wrap("assert.ok(!!!foo)"), - output: wrap("assert.equal(foo, false)"), - options: [{ fixToNotOk: false }], - errors: [createError("assert.ok")] - }, { - code: wrap("assert.ok(!!!foo)"), - output: wrap("assert.notOk(foo)"), - options: [{ fixToNotOk: true }], - errors: [createError("assert.ok")] - }, - { - // fixToNotOk = true (implicit since this is the option's default) code: wrap("assert.ok(!!!foo)"), output: wrap("assert.notOk(foo)"), errors: [createError("assert.ok")] @@ -211,19 +136,6 @@ ruleTester.run("no-negated-ok", rule, { // triple negation is not allowed (with message) { - code: wrap("assert.ok(!!!foo, 'message')"), - output: wrap("assert.equal(foo, false, 'message')"), - options: [{ fixToNotOk: false }], - errors: [createError("assert.ok")] - }, - { - code: wrap("assert.ok(!!!foo, 'message')"), - output: wrap("assert.notOk(foo, 'message')"), - options: [{ fixToNotOk: true }], - errors: [createError("assert.ok")] - }, - { - // fixToNotOk = true (implicit since this is the option's default) code: wrap("assert.notOk(!!!foo)"), output: wrap("assert.ok(foo)"), errors: [createError("assert.notOk")] @@ -240,7 +152,6 @@ ruleTester.run("no-negated-ok", rule, { { code: wrap("assert.true(!foo)"), output: wrap("assert.false(foo)"), - options: [{ checkBooleanAssertions: true }], errors: [createError("assert.true")] }, @@ -248,7 +159,6 @@ ruleTester.run("no-negated-ok", rule, { { code: wrap("assert.false(!foo)"), output: wrap("assert.true(foo)"), - options: [{ checkBooleanAssertions: true }], errors: [createError("assert.false")] } ] diff --git a/tests/lib/rules/no-ok-equality.js b/tests/lib/rules/no-ok-equality.js index bcf3b8f1..dad0fdc5 100644 --- a/tests/lib/rules/no-ok-equality.js +++ b/tests/lib/rules/no-ok-equality.js @@ -82,36 +82,10 @@ ruleTester.run("no-ok-equality", rule, { }, // 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 }] - } + "test('Name', function (assert) { assert.true(x); });", + "test('Name', function (assert) { assert.true(x, 'message'); });", + "test('Name', function (assert) { assert.false(x); });", + "test('Name', function (assert) { assert.false(x, 'message'); });" ], invalid: [ @@ -244,12 +218,11 @@ ruleTester.run("no-ok-equality", rule, { ] }, - // Boolean assertions with equality checks (checkBooleanAssertions = true, explicitly) + // Boolean assertions with equality checks { // 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") ] @@ -258,7 +231,6 @@ ruleTester.run("no-ok-equality", rule, { // 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") ] @@ -267,7 +239,6 @@ ruleTester.run("no-ok-equality", rule, { // 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") ] @@ -276,7 +247,6 @@ ruleTester.run("no-ok-equality", rule, { // 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") ]