From 0fa3e181a680094b113adad5f0238e805512d8d5 Mon Sep 17 00:00:00 2001 From: KinectTheUnknown <34723558+KinectTheUnknown@users.noreply.github.com> Date: Tue, 28 Jul 2020 14:30:19 -1000 Subject: [PATCH 01/12] Created docs and Added to index.js --- docs/rules/no-invalid-modifier-chain.md | 45 +++++++++++++++++++++++++ index.js | 1 + readme.md | 2 ++ 3 files changed, 48 insertions(+) create mode 100644 docs/rules/no-invalid-modifier-chain.md diff --git a/docs/rules/no-invalid-modifier-chain.md b/docs/rules/no-invalid-modifier-chain.md new file mode 100644 index 00000000..a70751af --- /dev/null +++ b/docs/rules/no-invalid-modifier-chain.md @@ -0,0 +1,45 @@ +# Ensure valid modifier chains in tests and hooks + +Translations: [Français](https://github.com/avajs/ava-docs/blob/master/fr_FR/related/eslint-plugin-ava/docs/rules/no-invalid-modifier-chain.md) + +Disallow the invalid combinations, order, and names of modifiers in tests and hooks. + + +## Fail + +```js +const test = require('ava'); + +// Mispelled .todo +test.todu(t => {}) +// Modifiers can't be repeated +test.failing.failing(t => {}) +// .todo can't be chained (except after .serial) +test.todo.only(t => {}) +// .failing can only be suceeded by .only and .skip +test.failing.serial(t => {}) +// .only must be last +test.only.skip(t => {}) +// .skip must be last +test.skip.only(t => {}) +// .always is only available in .after* hooks +test.before.always(t => {}) +// .always must suceed .after +test.always.after(t => {}) +// .only is not available in hooks +test.before.only(t => {}) +``` + + +## Pass + +```js +const test = require('ava'); + +test(t => {}); +test.serial(t => {}) +test.serial.todo(t => {}) +test.failing.only(t => {}) +test.serial.cb.skip(t => {}) +test.after.always.cb.skip(t => {}) +``` diff --git a/index.js b/index.js index a935f051..b7460fff 100644 --- a/index.js +++ b/index.js @@ -32,6 +32,7 @@ module.exports = { 'ava/no-incorrect-deep-equal': 'error', 'ava/no-inline-assertions': 'error', 'ava/no-invalid-end': 'error', + 'ava/no-invalid-modifier-chain': 'error', 'ava/no-nested-tests': 'error', 'ava/no-only-test': 'error', 'ava/no-skip-assert': 'error', diff --git a/readme.md b/readme.md index 23914cbc..4df5f420 100644 --- a/readme.md +++ b/readme.md @@ -50,6 +50,7 @@ Configure it in `package.json`. "ava/no-incorrect-deep-equal": "error", "ava/no-inline-assertions": "error", "ava/no-invalid-end": "error", + "ava/no-invalid-modifier-chain": "error", "ava/no-nested-tests": "error", "ava/no-only-test": "error", "ava/no-skip-assert": "error", @@ -91,6 +92,7 @@ The rules will only activate in test files. - [no-incorrect-deep-equal](docs/rules/no-incorrect-deep-equal.md) - Avoid using `deepEqual` with primitives. *(fixable)* - [no-inline-assertions](docs/rules/no-inline-assertions.md) - Ensure assertions are not called from inline arrow functions. *(fixable)* - [no-invalid-end](docs/rules/no-invalid-end.md) - Ensure `t.end()` is only called inside `test.cb()`. +- [no-invalid-modifier-chain](docs/rules/no-invalid-modifier-chain.md) - Ensure valid modifier chains in tests and hooks - [no-nested-tests](docs/rules/no-nested-tests.md) - Ensure no tests are nested. - [no-only-test](docs/rules/no-only-test.md) - Ensure no `test.only()` are present. *(fixable)* - [no-skip-assert](docs/rules/no-skip-assert.md) - Ensure no assertions are skipped. From f43c2ead3b0d551ca8f28ed2daa4aae336ecaac6 Mon Sep 17 00:00:00 2001 From: KinectTheUnknown <34723558+KinectTheUnknown@users.noreply.github.com> Date: Tue, 28 Jul 2020 14:30:51 -1000 Subject: [PATCH 02/12] Created Rule --- rules/no-invalid-modifier-chain.js | 134 +++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 rules/no-invalid-modifier-chain.js diff --git a/rules/no-invalid-modifier-chain.js b/rules/no-invalid-modifier-chain.js new file mode 100644 index 00000000..906ad313 --- /dev/null +++ b/rules/no-invalid-modifier-chain.js @@ -0,0 +1,134 @@ +'use strict'; +const {visitIf} = require('enhance-visitors'); +const createAvaRule = require('../create-ava-rule'); +const util = require('../util'); +const validModifiers = new Set([ + 'after', + 'afterEach', + 'always', + 'before', + 'beforeEach', + 'cb', + 'only', + 'serial', + 'skip', + 'todo', + 'failing' +]); +const TestInterface = {}; +{ + const CbOnlyInterface = null; + const CbSkipInterface = null; + const OnlyInterface = null; + const SkipInterface = null; + const TodoDeclaration = null; + + const FailingInterface = { + only: OnlyInterface, + skip: SkipInterface + }; + + const CbFailingInterface = { + only: CbOnlyInterface, + skip: CbSkipInterface + }; + + const CbInterface = { + ...CbFailingInterface, + failing: CbFailingInterface + }; + + const SerialInterface = { + ...FailingInterface, + cb: CbInterface, + failing: FailingInterface, + todo: TodoDeclaration + }; + + const HookCbInterface = { + ...CbFailingInterface, + failing: CbFailingInterface + }; + + const AlwaysInterface = { + cb: HookCbInterface, + skip: SkipInterface + }; + + const AfterInterface = { + ...AlwaysInterface, + always: AlwaysInterface + }; + + const BeforeInterface = { + ...AlwaysInterface + }; + + Object.assign(TestInterface, SerialInterface); + /* eslint-disable no-multi-assign */ + TestInterface.after = TestInterface.afterEach = AfterInterface; + TestInterface.before = TestInterface.beforeEach = BeforeInterface; + /* eslint-enable no-multi-assign */ + TestInterface.serial = SerialInterface; +} + +const create = context => { + const ava = createAvaRule(); + + return ava.merge({ + CallExpression: visitIf([ + ava.isInTestFile, + ava.isTestNode + ])(node => { + const modifiers = util.getTestModifiers(node); + let currentInterface = TestInterface; + for (const modifier of modifiers) { + if (currentInterface === null) { + return context.report({ + node: modifier, + messageId: 'last', + data: { + name: modifier.name + } + }); + } + + if (!validModifiers.has(modifier.name)) { + return context.report({ + node: modifier, + messageId: 'unknown', + data: { + name: modifier.name + } + }); + } + + currentInterface = currentInterface[modifier.name]; + if (currentInterface === undefined) { + return context.report({ + node, + messageId: 'invalid', + data: { + name: modifier.name + } + }); + } + } + }) + }); +}; + +module.exports = { + create, + meta: { + messages: { + unknown: 'Unknown Test Modifier `{{ name }}`', + invalid: 'Invalid Test Modifier `{{ name }}`', + last: 'Unexpected Test Modifier `{{ name }}` after a terminal modifier' + }, + docs: { + url: util.getDocsUrl(__filename) + }, + type: 'problem' + } +}; From d41a5c88cdd2867b91af22cfad6cb46e7d9d8c92 Mon Sep 17 00:00:00 2001 From: KinectTheUnknown <34723558+KinectTheUnknown@users.noreply.github.com> Date: Tue, 28 Jul 2020 15:14:50 -1000 Subject: [PATCH 03/12] Created Rule --- test/no-invalid-modifier-chain.js | 65 +++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 test/no-invalid-modifier-chain.js diff --git a/test/no-invalid-modifier-chain.js b/test/no-invalid-modifier-chain.js new file mode 100644 index 00000000..f420df86 --- /dev/null +++ b/test/no-invalid-modifier-chain.js @@ -0,0 +1,65 @@ +const test = require('ava'); +const avaRuleTester = require('eslint-ava-rule-tester'); +const rule = require('../rules/no-invalid-modifier-chain'); + +const ruleTester = avaRuleTester(test, { + env: { + es6: true + } +}); + +const errors = [{ruleId: 'no-invalid-modifier-chain'}]; +const header = 'const test = require(\'ava\');\n'; + +ruleTester.run('no-invalid-modifier-chain', rule, { + valid: [ + header + 'test(t => {})', + header + 'test.only(t => {});', + // Not using the test variable + header + 'notTest.skap(t => {});', + // Not a test file + 'test.skap(t => {});' + ], + invalid: [ + { + code: header + 'test.skap(t => {})', + errors + }, + { + code: header + 'test.serial.skap(t => {})', + errors + }, + { + code: header + 'test.failing.failing(t => {})', + errors + }, + { + code: header + 'test.todo.only(t => {})', + errors + }, + { + code: header + 'test.failing.serial(t => {})', + errors + }, + { + code: header + 'test.only.skip(t => {})', + errors + }, + { + code: header + 'test.skip.only(t => {})', + errors + }, + { + code: header + 'test.before.always(t => {})', + errors + }, + { + code: header + 'test.always.after(t => {})', + errors + }, + { + code: header + 'test.before.only(t => {})', + errors + } + ] +}); From 176b1abbf25b1a581310f185cbe8fadf676c37d5 Mon Sep 17 00:00:00 2001 From: KinectTheUnknown <34723558+KinectTheUnknown@users.noreply.github.com> Date: Tue, 28 Jul 2020 15:17:03 -1000 Subject: [PATCH 04/12] Deprecated `no-unknown-modifiers` rule --- rules/no-unknown-modifiers.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rules/no-unknown-modifiers.js b/rules/no-unknown-modifiers.js index 9dae2b91..602a517d 100644 --- a/rules/no-unknown-modifiers.js +++ b/rules/no-unknown-modifiers.js @@ -42,6 +42,8 @@ const create = context => { module.exports = { create, + deprecated: true, + replacedBy: ['no-invalid-modifier-chain'], meta: { docs: { url: util.getDocsUrl(__filename) From 3a7297521fe91c8a57ccc6d2f345ec5b69d39904 Mon Sep 17 00:00:00 2001 From: KinectTheUnknown <34723558+KinectTheUnknown@users.noreply.github.com> Date: Tue, 28 Jul 2020 18:21:15 -1000 Subject: [PATCH 05/12] Move test modifier names to util.js --- rules/no-invalid-modifier-chain.js | 16 ++-------------- util.js | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/rules/no-invalid-modifier-chain.js b/rules/no-invalid-modifier-chain.js index 906ad313..a444a10d 100644 --- a/rules/no-invalid-modifier-chain.js +++ b/rules/no-invalid-modifier-chain.js @@ -2,19 +2,7 @@ const {visitIf} = require('enhance-visitors'); const createAvaRule = require('../create-ava-rule'); const util = require('../util'); -const validModifiers = new Set([ - 'after', - 'afterEach', - 'always', - 'before', - 'beforeEach', - 'cb', - 'only', - 'serial', - 'skip', - 'todo', - 'failing' -]); + const TestInterface = {}; { const CbOnlyInterface = null; @@ -93,7 +81,7 @@ const create = context => { }); } - if (!validModifiers.has(modifier.name)) { + if (!util.testModifierNames.has(modifier.name)) { return context.report({ node: modifier, messageId: 'unknown', diff --git a/util.js b/util.js index 9b54e551..0a0df707 100644 --- a/util.js +++ b/util.js @@ -126,3 +126,17 @@ const assertionMethodNames = [...assertionMethodsNumberArguments.keys()]; exports.assertionMethodsNumArguments = assertionMethodsNumberArguments; exports.assertionMethods = new Set(assertionMethodNames); exports.executionMethods = new Set(assertionMethodNames.concat(['end', 'plan', 'log', 'teardown', 'timeout'])); + +exports.testModifierNames = new Set([ + 'after', + 'afterEach', + 'always', + 'before', + 'beforeEach', + 'cb', + 'only', + 'serial', + 'skip', + 'todo', + 'failing' +]); \ No newline at end of file From e18bf038b137493c43fd8ee330d99f3bc93cf138 Mon Sep 17 00:00:00 2001 From: KinectTheUnknown <34723558+KinectTheUnknown@users.noreply.github.com> Date: Wed, 29 Jul 2020 00:46:01 -1000 Subject: [PATCH 06/12] Add newline in util.js --- util.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util.js b/util.js index 0a0df707..8cc6c9b5 100644 --- a/util.js +++ b/util.js @@ -139,4 +139,4 @@ exports.testModifierNames = new Set([ 'skip', 'todo', 'failing' -]); \ No newline at end of file +]); From 6931301a1a371e9e6edfa934c4e7d99407e6b825 Mon Sep 17 00:00:00 2001 From: KinectTheUnknown <34723558+KinectTheUnknown@users.noreply.github.com> Date: Wed, 29 Jul 2020 12:23:24 -1000 Subject: [PATCH 07/12] Converted mock TestInterface to an IIFE --- rules/no-invalid-modifier-chain.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/rules/no-invalid-modifier-chain.js b/rules/no-invalid-modifier-chain.js index a444a10d..be4b6dbe 100644 --- a/rules/no-invalid-modifier-chain.js +++ b/rules/no-invalid-modifier-chain.js @@ -3,8 +3,7 @@ const {visitIf} = require('enhance-visitors'); const createAvaRule = require('../create-ava-rule'); const util = require('../util'); -const TestInterface = {}; -{ +const TestInterface = (() => { const CbOnlyInterface = null; const CbSkipInterface = null; const OnlyInterface = null; @@ -52,13 +51,15 @@ const TestInterface = {}; ...AlwaysInterface }; - Object.assign(TestInterface, SerialInterface); - /* eslint-disable no-multi-assign */ - TestInterface.after = TestInterface.afterEach = AfterInterface; - TestInterface.before = TestInterface.beforeEach = BeforeInterface; - /* eslint-enable no-multi-assign */ - TestInterface.serial = SerialInterface; -} + return { + ...SerialInterface, + after: AfterInterface, + afterEach: AfterInterface, + before: BeforeInterface, + beforeEach: BeforeInterface, + serial: SerialInterface + } +})(); const create = context => { const ava = createAvaRule(); From 4611ecad227f11bc4640f55475aab2e75aa4c4cd Mon Sep 17 00:00:00 2001 From: KinectTheUnknown <34723558+KinectTheUnknown@users.noreply.github.com> Date: Wed, 29 Jul 2020 12:29:33 -1000 Subject: [PATCH 08/12] Fixed Indentation in readme.md --- readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readme.md b/readme.md index 4df5f420..63ef9019 100644 --- a/readme.md +++ b/readme.md @@ -50,7 +50,7 @@ Configure it in `package.json`. "ava/no-incorrect-deep-equal": "error", "ava/no-inline-assertions": "error", "ava/no-invalid-end": "error", - "ava/no-invalid-modifier-chain": "error", + "ava/no-invalid-modifier-chain": "error", "ava/no-nested-tests": "error", "ava/no-only-test": "error", "ava/no-skip-assert": "error", From cb89d3d3241bfd79b355a62cdf59f967463ef9b3 Mon Sep 17 00:00:00 2001 From: KinectTheUnknown <34723558+KinectTheUnknown@users.noreply.github.com> Date: Wed, 29 Jul 2020 12:31:24 -1000 Subject: [PATCH 09/12] Fixed linting issues --- rules/no-invalid-modifier-chain.js | 34 ++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/rules/no-invalid-modifier-chain.js b/rules/no-invalid-modifier-chain.js index be4b6dbe..6c939a98 100644 --- a/rules/no-invalid-modifier-chain.js +++ b/rules/no-invalid-modifier-chain.js @@ -2,13 +2,33 @@ const {visitIf} = require('enhance-visitors'); const createAvaRule = require('../create-ava-rule'); const util = require('../util'); - +const InterfaceData = Symbol('Data for the Mock Interfaces'); const TestInterface = (() => { - const CbOnlyInterface = null; - const CbSkipInterface = null; - const OnlyInterface = null; - const SkipInterface = null; - const TodoDeclaration = null; + const CbOnlyInterface = { + [InterfaceData]: { + isNull: true + } + }; + const CbSkipInterface = { + [InterfaceData]: { + isNull: true + } + }; + const OnlyInterface = { + [InterfaceData]: { + isNull: true + } + }; + const SkipInterface = { + [InterfaceData]: { + isNull: true + } + }; + const TodoDeclaration = { + [InterfaceData]: { + isNull: true + } + }; const FailingInterface = { only: OnlyInterface, @@ -58,7 +78,7 @@ const TestInterface = (() => { before: BeforeInterface, beforeEach: BeforeInterface, serial: SerialInterface - } + }; })(); const create = context => { From ce82abe478c7a9c80f2d3ab04454c8a5072aaa8d Mon Sep 17 00:00:00 2001 From: KinectTheUnknown <34723558+KinectTheUnknown@users.noreply.github.com> Date: Wed, 29 Jul 2020 23:10:29 -1000 Subject: [PATCH 10/12] Specified Error Message ID in "invalid" tests --- test/no-invalid-modifier-chain.js | 51 ++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/test/no-invalid-modifier-chain.js b/test/no-invalid-modifier-chain.js index f420df86..2550753c 100644 --- a/test/no-invalid-modifier-chain.js +++ b/test/no-invalid-modifier-chain.js @@ -8,7 +8,6 @@ const ruleTester = avaRuleTester(test, { } }); -const errors = [{ruleId: 'no-invalid-modifier-chain'}]; const header = 'const test = require(\'ava\');\n'; ruleTester.run('no-invalid-modifier-chain', rule, { @@ -23,43 +22,73 @@ ruleTester.run('no-invalid-modifier-chain', rule, { invalid: [ { code: header + 'test.skap(t => {})', - errors + errors: [{ + ruleId: 'no-invalid-modifier-chain', + messageId: 'unknown' + }] }, { code: header + 'test.serial.skap(t => {})', - errors + errors: [{ + ruleId: 'no-invalid-modifier-chain', + messageId: 'unknown' + }] }, { code: header + 'test.failing.failing(t => {})', - errors + errors: [{ + ruleId: 'no-invalid-modifier-chain', + messageId: 'invalid' + }] }, { code: header + 'test.todo.only(t => {})', - errors + errors: [{ + ruleId: 'no-invalid-modifier-chain', + messageId: 'last' + }] }, { code: header + 'test.failing.serial(t => {})', - errors + errors: [{ + ruleId: 'no-invalid-modifier-chain', + messageId: 'invalid' + }] }, { code: header + 'test.only.skip(t => {})', - errors + errors: [{ + ruleId: 'no-invalid-modifier-chain', + messageId: 'last' + }] }, { code: header + 'test.skip.only(t => {})', - errors + errors: [{ + ruleId: 'no-invalid-modifier-chain', + messageId: 'last' + }] }, { code: header + 'test.before.always(t => {})', - errors + errors: [{ + ruleId: 'no-invalid-modifier-chain', + messageId: 'invalid' + }] }, { code: header + 'test.always.after(t => {})', - errors + errors: [{ + ruleId: 'no-invalid-modifier-chain', + messageId: 'invalid' + }] }, { code: header + 'test.before.only(t => {})', - errors + errors: [{ + ruleId: 'no-invalid-modifier-chain', + messageId: 'invalid' + }] } ] }); From e4801309d7c68943211c5af55fc97384363f2e9b Mon Sep 17 00:00:00 2001 From: KinectTheUnknown <34723558+KinectTheUnknown@users.noreply.github.com> Date: Fri, 2 Oct 2020 23:17:51 -1000 Subject: [PATCH 11/12] First Implementation --- rules/no-invalid-modifier-chain.js | 558 ++++++++++++++++++++++++----- test/no-invalid-modifier-chain.js | 123 +++++-- 2 files changed, 572 insertions(+), 109 deletions(-) diff --git a/rules/no-invalid-modifier-chain.js b/rules/no-invalid-modifier-chain.js index 6c939a98..9be125c1 100644 --- a/rules/no-invalid-modifier-chain.js +++ b/rules/no-invalid-modifier-chain.js @@ -2,84 +2,304 @@ const {visitIf} = require('enhance-visitors'); const createAvaRule = require('../create-ava-rule'); const util = require('../util'); -const InterfaceData = Symbol('Data for the Mock Interfaces'); -const TestInterface = (() => { - const CbOnlyInterface = { - [InterfaceData]: { - isNull: true +/** + * @name TypedefData + */ +const TypedefData = Symbol('Data for the Mock Typedefs'); +/** + * @enum {number} + */ +const TypedefTypes = { + HOOK: 1, + TEST: 2, + ANY: 3 +}; +/** @typedef typedef + * @type {{ + * [TypedefData]: { + * depth: number, + * isTerminal?: boolean, + * type: TypedefTypes, + * time?: Map + * }, + * [s: string]: typedef + * }} + */ +/** + * @type {Object} + */ +const typedefs = ( + /** + * @returns {Object} + */ + () => { + const CbOnly = { + [TypedefData]: { + depth: 5, + isTerminal: true, + type: TypedefTypes.ANY + } + }; + const CbSkip = { + [TypedefData]: { + depth: 5, + isTerminal: true, + type: TypedefTypes.ANY + } + }; + const Only = { + [TypedefData]: { + depth: 3, + isTerminal: true, + type: TypedefTypes.TEST + } + }; + const Skip = { + [TypedefData]: { + depth: 3, + isTerminal: true, + type: TypedefTypes.ANY + } + }; + const Todo = { + [TypedefData]: { + depth: 2, + isTerminal: true, + type: TypedefTypes.TEST + } + }; + + const Failing = { + only: Only, + skip: Skip, + [TypedefData]: { + depth: 2, + type: TypedefTypes.TEST + } + }; + + const CbFailing = { + only: CbOnly, + skip: CbSkip, + [TypedefData]: { + depth: 4, + type: TypedefTypes.ANY + } + }; + + const Cb = { + ...CbFailing, + failing: CbFailing, + [TypedefData]: { + depth: 2, + type: TypedefTypes.TEST + } + }; + + const Serial = { + ...Failing, + cb: Cb, + failing: Failing, + todo: Todo, + [TypedefData]: { + depth: 1, + type: TypedefTypes.ANY + } + }; + + const HookCb = { + ...CbFailing, + failing: CbFailing, + [TypedefData]: { + depth: 3, + type: TypedefTypes.HOOK + } + }; + + const Always = { + cb: HookCb, + skip: Skip, + [TypedefData]: { + depth: 2, + type: TypedefTypes.HOOK + } + }; + + const After = { + ...Always, + always: Always, + [TypedefData]: { + depth: 1, + type: TypedefTypes.HOOK + } + }; + + const Before = { + ...Always, + [TypedefData]: { + depth: 1, + type: TypedefTypes.HOOK + } + }; + + const Test = { + ...Serial, + after: After, + afterEach: After, + before: Before, + beforeEach: Before, + serial: Serial, + [TypedefData]: { + depth: 0, + // Test by default, but may become a hook with .after or .before + type: TypedefTypes.ANY + } + }; + + return { + Test, + After, + Always, + Before, + Cb, + CbFailing, + CbOnly, + CbSkip, + Failing, + HookCb, + Only, + Serial, + Skip, + Todo + }; + })(); +{ // Calculate In Time and Out Times of each node + let timer = 0; + /** + * @param {typedef} typedef + */ + (function iterate(typedef) { + if (!typedef[TypedefData].time) { + typedef[TypedefData].time = new Map(); } - }; - const CbSkipInterface = { - [InterfaceData]: { - isNull: true + + const inTime = timer++; + Object.values(typedef).forEach(value => iterate(value)); + + const outTime = timer++; + typedef[TypedefData].time.set(inTime, outTime); + })(typedefs.Test); +} + +/** + * @param {typedef} typdefA + * @param {typedef} typedefB + */ +function typedefsAreCompatible(typdefA, typedefB) { + const dataA = typdefA[TypedefData]; + const dataB = typedefB[TypedefData]; + + if (dataA.depth === dataB.depth) { + return false; + } + + if (dataA.isTerminal && dataB.isTerminal) { + return false; + } + + if ((dataA.type & dataB.type) === 0) { + return false; + } + + for (const [inA, outA] of dataA.time) { + for (const [inB, outB] of dataB.time) { + // For any given node, all of its children must have an in and out time value that is exclusively between its own in and out time + if (inA > inB ? outA < outB : outA > outB) { + return true; + } } - }; - const OnlyInterface = { - [InterfaceData]: { - isNull: true + } + + return false; +} + +/** + * @type {Map>} + */ +const ModifierNames = new Map(); + +for (const typedef of Object.values(typedefs)) { + for (const [name, type] of Object.entries(typedef)) { + if (ModifierNames.has(name)) { + ModifierNames.get(name).add(type); + } else { + ModifierNames.set(name, new Set().add(type)); } - }; - const SkipInterface = { - [InterfaceData]: { - isNull: true + } +} + +/** + * @param {string} name + */ +function inferTypedef(name) { + /* eslint-ignore-next-line capitalized-comment */ + /* c8 ignore next 3 */ + if (!ModifierNames.has(name)) { + throw new Error('No typedefs found'); + } + + return [...ModifierNames.get(name)]; +} + +/** + * @typedef Tree + * @type {{[x: string]: Tree}} + */ +/** Searches tree for the props using Breadth-first search + * @param {Tree} object + * @param {{name: string, typedef: Tree?}[]?} props + */ +function findProps(object, props) { + /** @type {Set} */ + const propNames = new Set(); + /** @type {Set} */ + const propTypedefs = new Set(); + for (const {name, typedef} of props) { + if (typedef) { + propTypedefs.add(typedef); + } else { + propNames.add(name); } - }; - const TodoDeclaration = { - [InterfaceData]: { - isNull: true + } + + const queue = [{ + /** @type {{name: string?, object: Tree}[]} */ + path: [{name: 'test', object}], + remaining: propNames.size + propTypedefs.size + }]; + while (queue.length > 0) { + const data = queue.shift(); + + for (const [key, value] of Object.entries(data.path[data.path.length - 1].object)) { + const result = { + path: data.path.concat({name: key, object: value}), + remaining: data.remaining, + object: value + }; + if (propNames.has(key) || propTypedefs.has(value)) { + result.remaining--; + } + + if (result.remaining === 0) { + return result; + } + + queue.push(result); } - }; - - const FailingInterface = { - only: OnlyInterface, - skip: SkipInterface - }; - - const CbFailingInterface = { - only: CbOnlyInterface, - skip: CbSkipInterface - }; - - const CbInterface = { - ...CbFailingInterface, - failing: CbFailingInterface - }; - - const SerialInterface = { - ...FailingInterface, - cb: CbInterface, - failing: FailingInterface, - todo: TodoDeclaration - }; - - const HookCbInterface = { - ...CbFailingInterface, - failing: CbFailingInterface - }; - - const AlwaysInterface = { - cb: HookCbInterface, - skip: SkipInterface - }; - - const AfterInterface = { - ...AlwaysInterface, - always: AlwaysInterface - }; - - const BeforeInterface = { - ...AlwaysInterface - }; - - return { - ...SerialInterface, - after: AfterInterface, - afterEach: AfterInterface, - before: BeforeInterface, - beforeEach: BeforeInterface, - serial: SerialInterface - }; -})(); + } + + return null; +} const create = context => { const ava = createAvaRule(); @@ -89,39 +309,182 @@ const create = context => { ava.isInTestFile, ava.isTestNode ])(node => { + /** @type {Object[]} */ const modifiers = util.getTestModifiers(node); - let currentInterface = TestInterface; - for (const modifier of modifiers) { - if (currentInterface === null) { - return context.report({ + + // Pass 1: Validate modifier names and check for duplicate names + const usedNames = new Set(); + let allValidNames = true; + for (const modifier of modifiers.values()) { + if (!ModifierNames.has(modifier.name)) { + allValidNames = false; + context.report({ + node: modifier, + messageId: 'unknown', + data: { + name: modifier.name + } + }); + } else if (usedNames.has(modifier.name)) { + allValidNames = false; + context.report({ node: modifier, - messageId: 'last', + messageId: 'duplicate', data: { name: modifier.name } }); + } else { + usedNames.add(modifier.name); + } + } + + if (!allValidNames) { + return; + } + + // Pass 2: Check that modifier names don't conflict with each other + // Add modifiers that are inherited from its parent + /** @type {{modifier: Object?, typedef: typedef}[]} */ + const knownTypedefs = [{modifier: null, typedef: typedefs.Test}]; + for (const [i, modifier] of modifiers.entries()) { + const parentTypedef = knownTypedefs[i].typedef; + const typedef = parentTypedef[modifier.name]; + + if (!typedef) { + break; } - if (!util.testModifierNames.has(modifier.name)) { + knownTypedefs.push({ + modifier, + typedef + }); + } + + if (knownTypedefs.length - 1 === modifiers.length) { + return; + } + + // If modifier name can only refer to one typdef, add it to the chain + const unknownModifiers = []; + for (let i = knownTypedefs.length - 1; i < modifiers.length; i++) { + const modifier = modifiers[i]; + const inferences = inferTypedef(modifier.name); + + if (inferences.length > 1) { + for (let j = inferences.length; j--;) { + const inference = inferences[j]; + // Skip checking compatibility with 'test' since it's always true + for (let k = 1; k < knownTypedefs.length; k++) { + const typedef = knownTypedefs[k]; + if (!typedefsAreCompatible(inference, typedef.typedef)) { + inferences.splice(j, 1); + + if (inferences.length === 0) { + return context.report({ + node: modifier, + messageId: 'conflict', + data: { + nameB: typedef.modifier.name + } + }); + } + } + } + } + + if (inferences.length > 1) { + unknownModifiers.push(modifier.name); + + continue; + } + } + + const [typedef] = inferences; + const indexB = findLastIndex( + knownTypedefs, + ({typedef: tdef}) => tdef[TypedefData].depth <= typedef[TypedefData].depth + ) + 1; + if (!typedefsAreCompatible(typedef, knownTypedefs[indexB - 1].typedef)) { return context.report({ node: modifier, - messageId: 'unknown', + messageId: 'conflict', data: { - name: modifier.name + nameB: knownTypedefs[indexB - 1].modifier.name } }); } - currentInterface = currentInterface[modifier.name]; - if (currentInterface === undefined) { + if (indexB < knownTypedefs.length && !typedefsAreCompatible(typedef, knownTypedefs[indexB].typedef)) { return context.report({ - node, - messageId: 'invalid', + node: modifier, + messageId: 'conflict', data: { - name: modifier.name + nameB: knownTypedefs[indexB].modifier.name } }); } + + knownTypedefs.splice(indexB, 0, {modifier, typedef}); + } + + // Pass 3: Make sure all modifier are in a valid order + /** @type {string[]} */ + let expectedOrder; + if (unknownModifiers.length === 0) { + expectedOrder = knownTypedefs.slice(1).map(t => t.modifier.name); + } else { + const props = knownTypedefs.slice(1).map(t => ({typedef: t.typedef, name: null})) + .concat(unknownModifiers.map(name => ({typedef: null, name}))); + + const match = findProps(typedefs.Test, props); + // + /* c8 ignore next 3 */ + if (!match) { + throw new Error('Should never happen'); + } + + expectedOrder = match.path.map(item => item.name); + } + + /** @type {Map} */ + const incorrect = new Map(); + for (let i = 0; i < expectedOrder.length;) { + const name = expectedOrder[i]; + + if (name !== modifiers[i].name) { + incorrect.set(name, i - incorrect.size); + } + + i++; + } + + for (let i = 0; i < modifiers.length && incorrect.size > 0; i++) { + const modifier = modifiers[i]; + const {name} = modifier; + + if (incorrect.has(name)) { + context.report({ + node: modifier, + messageId: 'position', + data: { + name, + position: incorrect.get(name) + } + }); + incorrect.delete(name); + } + } + + for (const [name, position] of incorrect) { + context.report({ + node, + messageId: 'missing', + data: { + name, + position + } + }); } }) }); @@ -131,9 +494,11 @@ module.exports = { create, meta: { messages: { - unknown: 'Unknown Test Modifier `{{ name }}`', - invalid: 'Invalid Test Modifier `{{ name }}`', - last: 'Unexpected Test Modifier `{{ name }}` after a terminal modifier' + conflict: 'This modifier conflicts with another modifier `{{ nameB }}`', + duplicate: 'This modifier already exists in the modifier chain `{{ name }}', + missing: 'Expected Test Modifier `{{ name }}` at position {{ position }}', + position: 'Test Modifier {{ name }} is in the wrong position', + unknown: 'Unknown Test Modifier `{{ name }}`' }, docs: { url: util.getDocsUrl(__filename) @@ -141,3 +506,18 @@ module.exports = { type: 'problem' } }; + +/** + * @param {any[]} array + * @param {(item: any, i: number, array: any[]) => boolean} cb + */ +/* c8 ignore next 9 */ +function findLastIndex(array, cb) { + for (let i = array.length; i--;) { + if (cb(array[i], i, array)) { + return i; + } + } + + return -1; +} diff --git a/test/no-invalid-modifier-chain.js b/test/no-invalid-modifier-chain.js index 2550753c..55ed8475 100644 --- a/test/no-invalid-modifier-chain.js +++ b/test/no-invalid-modifier-chain.js @@ -5,6 +5,9 @@ const rule = require('../rules/no-invalid-modifier-chain'); const ruleTester = avaRuleTester(test, { env: { es6: true + }, + parserOptions: { + ecmaVersion: 2021 } }); @@ -16,6 +19,7 @@ ruleTester.run('no-invalid-modifier-chain', rule, { header + 'test.only(t => {});', // Not using the test variable header + 'notTest.skap(t => {});', + header + 'serial.test(t => {})', // Not a test file 'test.skap(t => {});' ], @@ -23,71 +27,150 @@ ruleTester.run('no-invalid-modifier-chain', rule, { { code: header + 'test.skap(t => {})', errors: [{ - ruleId: 'no-invalid-modifier-chain', - messageId: 'unknown' + messageId: 'unknown', + data: { + name: 'skap' + } }] }, { code: header + 'test.serial.skap(t => {})', errors: [{ - ruleId: 'no-invalid-modifier-chain', - messageId: 'unknown' + messageId: 'unknown', + data: { + name: 'skap' + } }] }, { code: header + 'test.failing.failing(t => {})', errors: [{ - ruleId: 'no-invalid-modifier-chain', - messageId: 'invalid' + messageId: 'duplicate', + data: { + name: 'failing' + } }] }, { code: header + 'test.todo.only(t => {})', errors: [{ - ruleId: 'no-invalid-modifier-chain', - messageId: 'last' + messageId: 'conflict', + data: { + nameB: 'todo' + } + }] + }, + { + code: header + 'test.only.todo(t => {})', + errors: [{ + messageId: 'conflict', + data: { + nameB: 'only' + } }] }, { code: header + 'test.failing.serial(t => {})', errors: [{ - ruleId: 'no-invalid-modifier-chain', - messageId: 'invalid' + messageId: 'position', + data: { + name: 'failing' + // Position: '2' + } + }, { + messageId: 'position', + data: { + name: 'serial' + // Position: '3' + } }] }, { code: header + 'test.only.skip(t => {})', errors: [{ - ruleId: 'no-invalid-modifier-chain', - messageId: 'last' + messageId: 'conflict', + data: { + nameB: 'only' + } }] }, { code: header + 'test.skip.only(t => {})', errors: [{ - ruleId: 'no-invalid-modifier-chain', - messageId: 'last' + messageId: 'conflict', + data: { + nameB: 'skip' + } }] }, { code: header + 'test.before.always(t => {})', errors: [{ - ruleId: 'no-invalid-modifier-chain', - messageId: 'invalid' + messageId: 'conflict', + data: { + nameB: 'before' + } }] }, { code: header + 'test.always.after(t => {})', errors: [{ - ruleId: 'no-invalid-modifier-chain', - messageId: 'invalid' + messageId: 'position', + data: { + name: 'always' + } + }, { + messageId: 'position', + data: { + name: 'after' + } }] }, { code: header + 'test.before.only(t => {})', errors: [{ - ruleId: 'no-invalid-modifier-chain', - messageId: 'invalid' + messageId: 'missing', + data: { + name: 'cb', + position: '2' + } + }] + }, + { + code: header + 'test.skip.failing.serial(t => {})', + errors: [{ + messageId: 'position', + data: { + name: 'skip' + } + }, { + messageId: 'position', + data: { + name: 'serial' + } + }] + }, + { + code: header + 'test.failing.cb(t => {})', + errors: [{ + messageId: 'position', + data: { + name: 'failing' + } + }, { + messageId: 'position', + data: { + name: 'cb' + } + }] + }, + { + code: header + 'test.after.todo(t => {})', + errors: [{ + messageId: 'conflict', + data: { + nameB: 'after' + } }] } ] From e7a5e0bf7883201e5708d0836087a2894fd2b3f7 Mon Sep 17 00:00:00 2001 From: KinectTheUnknown <34723558+KinectTheUnknown@users.noreply.github.com> Date: Mon, 12 Oct 2020 22:56:31 -1000 Subject: [PATCH 12/12] Fixed linting error --- rules/no-invalid-modifier-chain.js | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/rules/no-invalid-modifier-chain.js b/rules/no-invalid-modifier-chain.js index 9be125c1..8d35302d 100644 --- a/rules/no-invalid-modifier-chain.js +++ b/rules/no-invalid-modifier-chain.js @@ -377,18 +377,20 @@ const create = context => { // Skip checking compatibility with 'test' since it's always true for (let k = 1; k < knownTypedefs.length; k++) { const typedef = knownTypedefs[k]; - if (!typedefsAreCompatible(inference, typedef.typedef)) { - inferences.splice(j, 1); - - if (inferences.length === 0) { - return context.report({ - node: modifier, - messageId: 'conflict', - data: { - nameB: typedef.modifier.name - } - }); - } + if (typedefsAreCompatible(inference, typedef.typedef)) { + continue; + } + + inferences.splice(j, 1); + + if (inferences.length === 0) { + return context.report({ + node: modifier, + messageId: 'conflict', + data: { + nameB: typedef.modifier.name + } + }); } } }