diff --git a/babel.config.js b/babel.config.js index 65965928f..d167ade1f 100644 --- a/babel.config.js +++ b/babel.config.js @@ -1,5 +1,6 @@ 'use strict'; +// todo: https://github.com/babel/babel/issues/8529 :'( module.exports = { plugins: ['replace-ts-export-assignment'], presets: [ diff --git a/src/rules/__tests__/no-alias-methods.test.js b/src/rules/__tests__/no-alias-methods.test.ts similarity index 97% rename from src/rules/__tests__/no-alias-methods.test.js rename to src/rules/__tests__/no-alias-methods.test.ts index 326844cc4..ea21c9499 100644 --- a/src/rules/__tests__/no-alias-methods.test.js +++ b/src/rules/__tests__/no-alias-methods.test.ts @@ -1,7 +1,7 @@ -import { RuleTester } from 'eslint'; +import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../no-alias-methods'; -const ruleTester = new RuleTester(); +const ruleTester = new TSESLint.RuleTester(); ruleTester.run('no-alias-methods', rule, { valid: [ diff --git a/src/rules/__tests__/no-truthy-falsy.test.js b/src/rules/__tests__/no-truthy-falsy.test.ts similarity index 87% rename from src/rules/__tests__/no-truthy-falsy.test.js rename to src/rules/__tests__/no-truthy-falsy.test.ts index e0b124449..46c05ed53 100644 --- a/src/rules/__tests__/no-truthy-falsy.test.js +++ b/src/rules/__tests__/no-truthy-falsy.test.ts @@ -1,7 +1,7 @@ -import { RuleTester } from 'eslint'; +import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../no-truthy-falsy'; -const ruleTester = new RuleTester(); +const ruleTester = new TSESLint.RuleTester(); ruleTester.run('no-truthy-falsy', rule, { valid: [ @@ -28,6 +28,17 @@ ruleTester.run('no-truthy-falsy', rule, { }, ], }, + { + code: 'expect(true)["toBeTruthy"]();', + errors: [ + { + messageId: 'avoidMessage', + data: { methodName: 'toBeTruthy' }, + column: 14, + line: 1, + }, + ], + }, { code: 'expect(false).not.toBeTruthy();', errors: [ diff --git a/src/rules/__tests__/prefer-called-with.test.js b/src/rules/__tests__/prefer-called-with.test.ts similarity index 90% rename from src/rules/__tests__/prefer-called-with.test.js rename to src/rules/__tests__/prefer-called-with.test.ts index 5a641a648..78c130d6d 100644 --- a/src/rules/__tests__/prefer-called-with.test.js +++ b/src/rules/__tests__/prefer-called-with.test.ts @@ -1,7 +1,7 @@ -import { RuleTester } from 'eslint'; +import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../prefer-called-with'; -const ruleTester = new RuleTester(); +const ruleTester = new TSESLint.RuleTester(); ruleTester.run('prefer-called-with', rule, { valid: [ diff --git a/src/rules/__tests__/prefer-to-be-null.test.js b/src/rules/__tests__/prefer-to-be-null.test.ts similarity index 92% rename from src/rules/__tests__/prefer-to-be-null.test.js rename to src/rules/__tests__/prefer-to-be-null.test.ts index 91631e8b0..4798181b1 100644 --- a/src/rules/__tests__/prefer-to-be-null.test.js +++ b/src/rules/__tests__/prefer-to-be-null.test.ts @@ -1,7 +1,7 @@ -import { RuleTester } from 'eslint'; +import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../prefer-to-be-null'; -const ruleTester = new RuleTester(); +const ruleTester = new TSESLint.RuleTester(); ruleTester.run('prefer-to-be-null', rule, { valid: [ diff --git a/src/rules/__tests__/prefer-to-be-undefined.test.js b/src/rules/__tests__/prefer-to-be-undefined.test.ts similarity index 92% rename from src/rules/__tests__/prefer-to-be-undefined.test.js rename to src/rules/__tests__/prefer-to-be-undefined.test.ts index f75767a29..0749bdce3 100644 --- a/src/rules/__tests__/prefer-to-be-undefined.test.js +++ b/src/rules/__tests__/prefer-to-be-undefined.test.ts @@ -1,7 +1,7 @@ -import { RuleTester } from 'eslint'; +import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../prefer-to-be-undefined'; -const ruleTester = new RuleTester(); +const ruleTester = new TSESLint.RuleTester(); ruleTester.run('prefer-to-be-undefined', rule, { valid: [ diff --git a/src/rules/__tests__/prefer-to-contain.test.js b/src/rules/__tests__/prefer-to-contain.test.ts similarity index 91% rename from src/rules/__tests__/prefer-to-contain.test.js rename to src/rules/__tests__/prefer-to-contain.test.ts index 54e20a4c7..85d8c68d4 100644 --- a/src/rules/__tests__/prefer-to-contain.test.js +++ b/src/rules/__tests__/prefer-to-contain.test.ts @@ -1,7 +1,7 @@ -import { RuleTester } from 'eslint'; +import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../prefer-to-contain'; -const ruleTester = new RuleTester(); +const ruleTester = new TSESLint.RuleTester(); ruleTester.run('prefer-to-contain', rule, { valid: [ @@ -31,6 +31,12 @@ ruleTester.run('prefer-to-contain', rule, { errors: [{ messageId: 'useToContain', column: 23, line: 1 }], output: 'expect(a).toContain(b);', }, + // todo: support this, as it's counted by isSupportedAccessor + // { + // code: "expect(a['includes'](b)).toEqual(true);", + // errors: [{ messageId: 'useToContain', column: 23, line: 1 }], + // output: 'expect(a).toContain(b);', + // }, { code: 'expect(a.includes(b)).toEqual(false);', errors: [{ messageId: 'useToContain', column: 23, line: 1 }], diff --git a/src/rules/__tests__/prefer-to-have-length.test.js b/src/rules/__tests__/prefer-to-have-length.test.ts similarity index 61% rename from src/rules/__tests__/prefer-to-have-length.test.js rename to src/rules/__tests__/prefer-to-have-length.test.ts index a5981e7ff..e3ec820cc 100644 --- a/src/rules/__tests__/prefer-to-have-length.test.js +++ b/src/rules/__tests__/prefer-to-have-length.test.ts @@ -1,19 +1,31 @@ -import { RuleTester } from 'eslint'; +import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../prefer-to-have-length'; -const ruleTester = new RuleTester(); +const ruleTester = new TSESLint.RuleTester({ + parserOptions: { + ecmaVersion: 2015, + }, +}); ruleTester.run('prefer-to-have-length', rule, { valid: [ 'expect(files).toHaveLength(1);', "expect(files.name).toBe('file');", + "expect(files[`name`]).toBe('file');", 'expect(result).toBe(true);', `expect(user.getUserName(5)).resolves.toEqual('Paul')`, `expect(user.getUserName(5)).rejects.toEqual('Paul')`, 'expect(a);', + 'expect(files["length"]).toBe(1);', ], invalid: [ + // todo: support this + // { + // code: 'expect(files["length"]).toBe(1);', + // errors: [{ messageId: 'useToHaveLength', column: 22, line: 1 }], + // output: 'expect(files).toHaveLength(1);', + // }, { code: 'expect(files.length).toBe(1);', errors: [{ messageId: 'useToHaveLength', column: 22, line: 1 }], diff --git a/src/rules/__tests__/require-tothrow-message.test.js b/src/rules/__tests__/require-tothrow-message.test.ts similarity index 70% rename from src/rules/__tests__/require-tothrow-message.test.js rename to src/rules/__tests__/require-tothrow-message.test.ts index 4a3a6e445..c750efef4 100644 --- a/src/rules/__tests__/require-tothrow-message.test.js +++ b/src/rules/__tests__/require-tothrow-message.test.ts @@ -1,7 +1,7 @@ -import { RuleTester } from 'eslint'; +import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../require-tothrow-message'; -const ruleTester = new RuleTester({ +const ruleTester = new TSESLint.RuleTester({ parserOptions: { ecmaVersion: 8, }, @@ -12,55 +12,74 @@ ruleTester.run('require-tothrow-message', rule, { // String "expect(() => { throw new Error('a'); }).toThrow('a');", "expect(() => { throw new Error('a'); }).toThrowError('a');", - `test('string', async () => { + ` + test('string', async () => { const throwErrorAsync = async () => { throw new Error('a') }; await expect(throwErrorAsync()).rejects.toThrow('a'); await expect(throwErrorAsync()).rejects.toThrowError('a'); - })`, + }) + `, // Template literal "const a = 'a'; expect(() => { throw new Error('a'); }).toThrow(`${a}`);", "const a = 'a'; expect(() => { throw new Error('a'); }).toThrowError(`${a}`);", - `test('Template literal', async () => { + ` + test('Template literal', async () => { const a = 'a'; const throwErrorAsync = async () => { throw new Error('a') }; await expect(throwErrorAsync()).rejects.toThrow(\`\${a}\`); await expect(throwErrorAsync()).rejects.toThrowError(\`\${a}\`); - })`, + }) + `, // Regex "expect(() => { throw new Error('a'); }).toThrow(/^a$/);", "expect(() => { throw new Error('a'); }).toThrowError(/^a$/);", - `test('Regex', async () => { + ` + test('Regex', async () => { const throwErrorAsync = async () => { throw new Error('a') }; await expect(throwErrorAsync()).rejects.toThrow(/^a$/); await expect(throwErrorAsync()).rejects.toThrowError(/^a$/); - })`, + }) + `, // Function - "expect(() => { throw new Error('a'); })" + - ".toThrow((() => { return 'a'; })());", - "expect(() => { throw new Error('a'); })" + - ".toThrowError((() => { return 'a'; })());", - `test('Function', async () => { + "expect(() => { throw new Error('a'); }).toThrow((() => { return 'a'; })());", + "expect(() => { throw new Error('a'); }).toThrowError((() => { return 'a'; })());", + ` + test('Function', async () => { const throwErrorAsync = async () => { throw new Error('a') }; const fn = () => { return 'a'; }; await expect(throwErrorAsync()).rejects.toThrow(fn()); await expect(throwErrorAsync()).rejects.toThrowError(fn()); - })`, + }) + `, // Allow no message for `not`. "expect(() => { throw new Error('a'); }).not.toThrow();", "expect(() => { throw new Error('a'); }).not.toThrowError();", - `test('Allow no message for "not"', async () => { + ` + test('Allow no message for "not"', async () => { const throwErrorAsync = async () => { throw new Error('a') }; await expect(throwErrorAsync()).resolves.not.toThrow(); await expect(throwErrorAsync()).resolves.not.toThrowError(); - })`, + }) + `, 'expect(a);', ], invalid: [ + { + code: "expect(() => { throw new Error('a'); })[`toThrow`]();", + errors: [ + { + messageId: 'requireRethrow', + data: { propertyName: 'toThrow' }, + column: 41, + line: 1, + }, + ], + }, // Empty toThrow { code: "expect(() => { throw new Error('a'); }).toThrow();", @@ -88,23 +107,25 @@ ruleTester.run('require-tothrow-message', rule, { // Empty rejects.toThrow / rejects.toThrowError { - code: `test('empty rejects.toThrow', async () => { - const throwErrorAsync = async () => { throw new Error('a') }; - await expect(throwErrorAsync()).rejects.toThrow(); - await expect(throwErrorAsync()).rejects.toThrowError(); - })`, + code: ` + test('empty rejects.toThrow', async () => { + const throwErrorAsync = async () => { throw new Error('a') }; + await expect(throwErrorAsync()).rejects.toThrow(); + await expect(throwErrorAsync()).rejects.toThrowError(); + }) + `, errors: [ { messageId: 'requireRethrow', data: { propertyName: 'toThrow' }, - column: 49, - line: 3, + column: 51, + line: 4, }, { messageId: 'requireRethrow', data: { propertyName: 'toThrowError' }, - column: 49, - line: 4, + column: 51, + line: 5, }, ], }, diff --git a/src/rules/__tests__/valid-expect.test.js b/src/rules/__tests__/valid-expect.test.ts similarity index 71% rename from src/rules/__tests__/valid-expect.test.js rename to src/rules/__tests__/valid-expect.test.ts index a10caea02..eff761e31 100644 --- a/src/rules/__tests__/valid-expect.test.js +++ b/src/rules/__tests__/valid-expect.test.ts @@ -1,7 +1,7 @@ -import { RuleTester } from 'eslint'; +import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../valid-expect'; -const ruleTester = new RuleTester({ +const ruleTester = new TSESLint.RuleTester({ parserOptions: { ecmaVersion: 8, }, @@ -15,12 +15,12 @@ ruleTester.run('valid-expect', rule, { 'expect(undefined).not.toBeDefined();', 'test("valid-expect", () => { return expect(Promise.resolve(2)).resolves.toBeDefined(); });', 'test("valid-expect", () => { return expect(Promise.reject(2)).rejects.toBeDefined(); });', - 'test("valid-expect", () => { return expect(Promise.resolve(2)).not.resolves.toBeDefined(); });', - 'test("valid-expect", () => { return expect(Promise.resolve(2)).not.rejects.toBeDefined(); });', - 'test("valid-expect", function () { return expect(Promise.resolve(2)).not.resolves.toBeDefined(); });', - 'test("valid-expect", function () { return expect(Promise.resolve(2)).not.rejects.toBeDefined(); });', - 'test("valid-expect", function () { return Promise.resolve(expect(Promise.resolve(2)).not.resolves.toBeDefined()); });', - 'test("valid-expect", function () { return Promise.resolve(expect(Promise.resolve(2)).not.rejects.toBeDefined()); });', + 'test("valid-expect", () => { return expect(Promise.resolve(2)).resolves.not.toBeDefined(); });', + 'test("valid-expect", () => { return expect(Promise.resolve(2)).rejects.not.toBeDefined(); });', + 'test("valid-expect", function () { return expect(Promise.resolve(2)).resolves.not.toBeDefined(); });', + 'test("valid-expect", function () { return expect(Promise.resolve(2)).rejects.not.toBeDefined(); });', + 'test("valid-expect", function () { return Promise.resolve(expect(Promise.resolve(2)).resolves.not.toBeDefined()); });', + 'test("valid-expect", function () { return Promise.resolve(expect(Promise.resolve(2)).rejects.not.toBeDefined()); });', { code: 'test("valid-expect", () => expect(Promise.resolve(2)).resolves.toBeDefined());', @@ -28,10 +28,52 @@ ruleTester.run('valid-expect', rule, { }, 'test("valid-expect", () => expect(Promise.resolve(2)).resolves.toBeDefined());', 'test("valid-expect", () => expect(Promise.reject(2)).rejects.toBeDefined());', - 'test("valid-expect", () => expect(Promise.reject(2)).not.resolves.toBeDefined());', - 'test("valid-expect", () => expect(Promise.reject(2)).not.rejects.toBeDefined());', 'test("valid-expect", () => expect(Promise.reject(2)).resolves.not.toBeDefined());', 'test("valid-expect", () => expect(Promise.reject(2)).rejects.not.toBeDefined());', + // 'test("valid-expect", () => expect(Promise.reject(2)).not.resolves.toBeDefined());', + // 'test("valid-expect", () => expect(Promise.reject(2)).not.rejects.toBeDefined());', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).resolves.not.toBeDefined(); });', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).rejects.not.toBeDefined(); });', + 'test("valid-expect", async function () { await expect(Promise.reject(2)).resolves.not.toBeDefined(); });', + 'test("valid-expect", async function () { await expect(Promise.reject(2)).rejects.not.toBeDefined(); });', + 'test("valid-expect", async () => { await Promise.resolve(expect(Promise.reject(2)).rejects.not.toBeDefined()); });', + 'test("valid-expect", async () => { await Promise.reject(expect(Promise.reject(2)).rejects.not.toBeDefined()); });', + 'test("valid-expect", async () => { await Promise.all([expect(Promise.reject(2)).rejects.not.toBeDefined(), expect(Promise.reject(2)).rejects.not.toBeDefined()]); });', + 'test("valid-expect", async () => { await Promise.race([expect(Promise.reject(2)).rejects.not.toBeDefined(), expect(Promise.reject(2)).rejects.not.toBeDefined()]); });', + 'test("valid-expect", async () => { await Promise.allSettled([expect(Promise.reject(2)).rejects.not.toBeDefined(), expect(Promise.reject(2)).rejects.not.toBeDefined()]); });', + 'test("valid-expect", async () => { await Promise.any([expect(Promise.reject(2)).rejects.not.toBeDefined(), expect(Promise.reject(2)).rejects.not.toBeDefined()]); });', + 'test("valid-expect", async () => { return expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => console.log("valid-case")); });', + 'test("valid-expect", async () => { return expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => console.log("valid-case")).then(() => console.log("another valid case")); });', + 'test("valid-expect", async () => { return expect(Promise.reject(2)).resolves.not.toBeDefined().catch(() => console.log("valid-case")); });', + 'test("valid-expect", async () => { return expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => console.log("valid-case")).catch(() => console.log("another valid case")); });', + 'test("valid-expect", async () => { return expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => { expect(someMock).toHaveBeenCalledTimes(1); }); });', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => console.log("valid-case")); });', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => console.log("valid-case")).then(() => console.log("another valid case")); });', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).resolves.not.toBeDefined().catch(() => console.log("valid-case")); });', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => console.log("valid-case")).catch(() => console.log("another valid case")); });', + 'test("valid-expect", async () => { await expect(Promise.reject(2)).resolves.not.toBeDefined().then(() => { expect(someMock).toHaveBeenCalledTimes(1); }); });', + { + code: `test("valid-expect", () => { + return expect(functionReturningAPromise()).resolves.toEqual(1).then(() => { + return expect(Promise.resolve(2)).resolves.toBe(1); + }); + });`, + }, + { + code: `test("valid-expect", () => { + return expect(functionReturningAPromise()).resolves.toEqual(1).then(async () => { + await expect(Promise.resolve(2)).resolves.toBe(1); + }); + });`, + }, + { + code: `test("valid-expect", () => { + return expect(functionReturningAPromise()).resolves.toEqual(1).then(() => expect(Promise.resolve(2)).resolves.toBe(1)); + });`, + }, + ], + invalid: [ + /* 'test("valid-expect", async () => { await expect(Promise.reject(2)).not.resolves.toBeDefined(); });', 'test("valid-expect", async () => { await expect(Promise.reject(2)).not.rejects.toBeDefined(); });', 'test("valid-expect", async function () { await expect(Promise.reject(2)).not.resolves.toBeDefined(); });', @@ -52,28 +94,7 @@ ruleTester.run('valid-expect', rule, { 'test("valid-expect", async () => { await expect(Promise.reject(2)).not.resolves.toBeDefined().catch(() => console.log("valid-case")); });', 'test("valid-expect", async () => { await expect(Promise.reject(2)).not.resolves.toBeDefined().then(() => console.log("valid-case")).catch(() => console.log("another valid case")); });', 'test("valid-expect", async () => { await expect(Promise.reject(2)).not.resolves.toBeDefined().then(() => { expect(someMock).toHaveBeenCalledTimes(1); }); });', - { - code: `test("valid-expect", () => { - return expect(functionReturningAPromise()).resolves.toEqual(1).then(() => { - return expect(Promise.resolve(2)).resolves.toBe(1); - }); - });`, - }, - { - code: `test("valid-expect", () => { - return expect(functionReturningAPromise()).resolves.toEqual(1).then(async () => { - await expect(Promise.resolve(2)).resolves.toBe(1); - }); - });`, - }, - { - code: `test("valid-expect", () => { - return expect(functionReturningAPromise()).resolves.toEqual(1).then(() => expect(Promise.resolve(2)).resolves.toBe(1)); - });`, - }, - ], - - invalid: [ + */ { code: 'expect().toBe(true);', errors: [{ endColumn: 8, column: 7, messageId: 'noArgs' }], @@ -215,10 +236,10 @@ ruleTester.run('valid-expect', rule, { }, ], }, - // expect().not.resolves + // expect().resolves.not { code: - 'test("valid-expect", () => { expect(Promise.resolve(2)).not.resolves.toBeDefined(); });', + 'test("valid-expect", () => { expect(Promise.resolve(2)).resolves.not.toBeDefined(); });', errors: [ { column: 30, @@ -241,10 +262,10 @@ ruleTester.run('valid-expect', rule, { }, ], }, - // expect().not.rejects + // expect().rejects.not { code: - 'test("valid-expect", () => { expect(Promise.resolve(2)).not.rejects.toBeDefined(); });', + 'test("valid-expect", () => { expect(Promise.resolve(2)).rejects.not.toBeDefined(); });', errors: [ { column: 30, @@ -269,7 +290,7 @@ ruleTester.run('valid-expect', rule, { }, { code: - 'test("valid-expect", async () => { expect(Promise.resolve(2)).not.resolves.toBeDefined(); });', + 'test("valid-expect", async () => { expect(Promise.resolve(2)).resolves.not.toBeDefined(); });', errors: [ { column: 36, @@ -281,9 +302,9 @@ ruleTester.run('valid-expect', rule, { }, // alwaysAwait:false, one not awaited { - code: `test("valid-expect", async () => { - expect(Promise.resolve(2)).not.resolves.toBeDefined(); - expect(Promise.resolve(1)).rejects.toBeDefined(); + code: `test("valid-expect", async () => { + expect(Promise.resolve(2)).resolves.not.toBeDefined(); + expect(Promise.resolve(1)).rejects.toBeDefined(); });`, errors: [ { @@ -304,9 +325,9 @@ ruleTester.run('valid-expect', rule, { }, // alwaysAwait: true, one returned { - code: `test("valid-expect", async () => { - await expect(Promise.resolve(2)).not.resolves.toBeDefined(); - expect(Promise.resolve(1)).rejects.toBeDefined(); + code: `test("valid-expect", async () => { + await expect(Promise.resolve(2)).resolves.not.toBeDefined(); + expect(Promise.resolve(1)).rejects.toBeDefined(); });`, errors: [ { @@ -323,9 +344,9 @@ ruleTester.run('valid-expect', rule, { */ // both not awaited { - code: `test("valid-expect", async () => { - expect(Promise.resolve(2)).not.resolves.toBeDefined(); - return expect(Promise.resolve(1)).rejects.toBeDefined(); + code: `test("valid-expect", async () => { + expect(Promise.resolve(2)).resolves.not.toBeDefined(); + return expect(Promise.resolve(1)).rejects.toBeDefined(); });`, options: [{ alwaysAwait: true }], errors: [ @@ -345,9 +366,9 @@ ruleTester.run('valid-expect', rule, { }, // alwaysAwait:true, one not awaited, one returned { - code: `test("valid-expect", async () => { - expect(Promise.resolve(2)).not.resolves.toBeDefined(); - return expect(Promise.resolve(1)).rejects.toBeDefined(); + code: `test("valid-expect", async () => { + expect(Promise.resolve(2)).resolves.not.toBeDefined(); + return expect(Promise.resolve(1)).rejects.toBeDefined(); });`, errors: [ { @@ -361,9 +382,9 @@ ruleTester.run('valid-expect', rule, { }, // one not awaited { - code: `test("valid-expect", async () => { - await expect(Promise.resolve(2)).not.resolves.toBeDefined(); - return expect(Promise.resolve(1)).rejects.toBeDefined(); + code: `test("valid-expect", async () => { + await expect(Promise.resolve(2)).resolves.not.toBeDefined(); + return expect(Promise.resolve(1)).rejects.toBeDefined(); });`, options: [{ alwaysAwait: true }], errors: [ @@ -380,8 +401,8 @@ ruleTester.run('valid-expect', rule, { * Promise.x(expect()) usages */ { - code: `test("valid-expect", () => { - Promise.resolve(expect(Promise.resolve(2)).not.resolves.toBeDefined()); + code: `test("valid-expect", () => { + Promise.resolve(expect(Promise.resolve(2)).resolves.not.toBeDefined()); });`, errors: [ { @@ -394,8 +415,8 @@ ruleTester.run('valid-expect', rule, { ], }, { - code: `test("valid-expect", () => { - Promise.reject(expect(Promise.resolve(2)).not.resolves.toBeDefined()); + code: `test("valid-expect", () => { + Promise.reject(expect(Promise.resolve(2)).resolves.not.toBeDefined()); });`, errors: [ { @@ -408,8 +429,8 @@ ruleTester.run('valid-expect', rule, { ], }, { - code: `test("valid-expect", () => { - Promise.x(expect(Promise.resolve(2)).not.resolves.toBeDefined()); + code: `test("valid-expect", () => { + Promise.x(expect(Promise.resolve(2)).resolves.not.toBeDefined()); });`, errors: [ { @@ -423,8 +444,8 @@ ruleTester.run('valid-expect', rule, { }, // alwaysAwait option changes error message { - code: `test("valid-expect", () => { - Promise.resolve(expect(Promise.resolve(2)).not.resolves.toBeDefined()); + code: `test("valid-expect", () => { + Promise.resolve(expect(Promise.resolve(2)).resolves.not.toBeDefined()); });`, options: [{ alwaysAwait: true }], errors: [ @@ -438,11 +459,11 @@ ruleTester.run('valid-expect', rule, { }, // Promise method accepts arrays and returns 1 error { - code: `test("valid-expect", () => { + code: `test("valid-expect", () => { Promise.all([ - expect(Promise.resolve(2)).not.resolves.toBeDefined(), - expect(Promise.resolve(3)).not.resolves.toBeDefined(), - ]); + expect(Promise.resolve(2)).resolves.not.toBeDefined(), + expect(Promise.resolve(3)).resolves.not.toBeDefined(), + ]); });`, errors: [ { @@ -457,11 +478,11 @@ ruleTester.run('valid-expect', rule, { }, // Promise.any([expect1, expect2]) returns one error { - code: `test("valid-expect", () => { + code: `test("valid-expect", () => { Promise.x([ - expect(Promise.resolve(2)).not.resolves.toBeDefined(), - expect(Promise.resolve(3)).not.resolves.toBeDefined(), - ]); + expect(Promise.resolve(2)).resolves.not.toBeDefined(), + expect(Promise.resolve(3)).resolves.not.toBeDefined(), + ]); });`, errors: [ { @@ -476,10 +497,10 @@ ruleTester.run('valid-expect', rule, { }, // { - code: `test("valid-expect", () => { + code: `test("valid-expect", () => { const assertions = [ - expect(Promise.resolve(2)).not.resolves.toBeDefined(), - expect(Promise.resolve(3)).not.resolves.toBeDefined(), + expect(Promise.resolve(2)).resolves.not.toBeDefined(), + expect(Promise.resolve(3)).resolves.not.toBeDefined(), ] });`, errors: [ @@ -505,6 +526,13 @@ ruleTester.run('valid-expect', rule, { { code: 'expect(Promise.resolve(2)).resolves.toBe;', errors: [ + { + line: 1, + column: 1, + endLine: 1, + endColumn: 42, + messageId: 'asyncMustBeAwaited', + }, { column: 37, endColumn: 41, @@ -514,7 +542,7 @@ ruleTester.run('valid-expect', rule, { ], }, { - code: `test("valid-expect", () => { + code: `test("valid-expect", () => { return expect(functionReturningAPromise()).resolves.toEqual(1).then(() => { expect(Promise.resolve(2)).resolves.toBe(1); }); @@ -531,7 +559,7 @@ ruleTester.run('valid-expect', rule, { ], }, { - code: `test("valid-expect", () => { + code: `test("valid-expect", () => { return expect(functionReturningAPromise()).resolves.toEqual(1).then(async () => { await expect(Promise.resolve(2)).resolves.toBe(1); expect(Promise.resolve(4)).resolves.toBe(4); diff --git a/src/rules/no-alias-methods.js b/src/rules/no-alias-methods.ts similarity index 66% rename from src/rules/no-alias-methods.js rename to src/rules/no-alias-methods.ts index 80a5cc0a3..158254b6e 100644 --- a/src/rules/no-alias-methods.js +++ b/src/rules/no-alias-methods.ts @@ -1,19 +1,25 @@ -import { expectCaseWithParent, getDocsUrl, method } from './util'; +import { createRule, isExpectCall, parseExpectCall } from './tsUtils'; -export default { +export default createRule({ + name: __filename, meta: { docs: { - url: getDocsUrl(__filename), + category: 'Best Practices', + description: 'Disallow alias methods', + recommended: 'warn', }, messages: { replaceAlias: `Replace {{ replace }}() with its canonical name of {{ canonical }}()`, }, fixable: 'code', + type: 'suggestion', schema: [], }, + defaultOptions: [], create(context) { // The Jest methods which have aliases. The canonical name is the first // index of each item. + // todo: replace w/ Map const methodNames = [ ['toHaveBeenCalled', 'toBeCalled'], ['toHaveBeenCalledTimes', 'toBeCalledTimes'], @@ -30,27 +36,18 @@ export default { return { CallExpression(node) { - if (!expectCaseWithParent(node)) { + if (!isExpectCall(node)) { return; } - let targetNode = method(node); - if ( - targetNode.name === 'resolves' || - targetNode.name === 'rejects' || - targetNode.name === 'not' - ) { - targetNode = method(node.parent); - } + const { matcher } = parseExpectCall(node); - if (!targetNode) { + if (!matcher) { return; } // Check if the method used matches any of ours - const methodItem = methodNames.find( - item => item[1] === targetNode.name, - ); + const methodItem = methodNames.find(item => item[1] === matcher.name); if (methodItem) { context.report({ @@ -59,11 +56,13 @@ export default { replace: methodItem[1], canonical: methodItem[0], }, - node: targetNode, - fix: fixer => [fixer.replaceText(targetNode, methodItem[0])], + node: matcher.node.property, + fix: fixer => [ + fixer.replaceText(matcher.node.property, methodItem[0]), + ], }); } }, }; }, -}; +}); diff --git a/src/rules/no-truthy-falsy.js b/src/rules/no-truthy-falsy.js deleted file mode 100644 index 27cdf715f..000000000 --- a/src/rules/no-truthy-falsy.js +++ /dev/null @@ -1,46 +0,0 @@ -import { - expectCaseWithParent, - expectNotCase, - expectRejectsCase, - expectResolvesCase, - getDocsUrl, - method, -} from './util'; - -export default { - meta: { - docs: { - url: getDocsUrl(__filename), - }, - messages: { - avoidMessage: 'Avoid {{methodName}}', - }, - schema: [], - }, - create(context) { - return { - CallExpression(node) { - if ( - expectCaseWithParent(node) || - expectNotCase(node) || - expectResolvesCase(node) || - expectRejectsCase(node) - ) { - const targetNode = - node.parent.parent.type === 'MemberExpression' ? node.parent : node; - - const methodNode = method(targetNode); - const { name: methodName } = methodNode; - - if (methodName === 'toBeTruthy' || methodName === 'toBeFalsy') { - context.report({ - data: { methodName }, - messageId: 'avoidMessage', - node: methodNode, - }); - } - } - }, - }; - }, -}; diff --git a/src/rules/no-truthy-falsy.ts b/src/rules/no-truthy-falsy.ts new file mode 100644 index 000000000..53e32cd23 --- /dev/null +++ b/src/rules/no-truthy-falsy.ts @@ -0,0 +1,39 @@ +import { createRule, isExpectCall, parseExpectCall } from './tsUtils'; + +// todo: refactor into "ban-matchers" +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Disallow using `toBeTruthy()` & `toBeFalsy()`', + recommended: false, + }, + messages: { + avoidMessage: 'Avoid {{ methodName }}', + }, + type: 'suggestion', + schema: [], + }, + defaultOptions: [], + create(context) { + return { + CallExpression(node) { + if (!isExpectCall(node)) { + return; + } + + const { matcher } = parseExpectCall(node); + if (!matcher || !['toBeTruthy', 'toBeFalsy'].includes(matcher.name)) { + return; + } + + context.report({ + data: { methodName: matcher.name }, // todo: rename to 'matcherName' + messageId: 'avoidMessage', // todo: rename to 'avoidMatcher' + node: matcher.node.property, + }); + }, + }; + }, +}); diff --git a/src/rules/prefer-called-with.js b/src/rules/prefer-called-with.js deleted file mode 100644 index 0e4f99554..000000000 --- a/src/rules/prefer-called-with.js +++ /dev/null @@ -1,36 +0,0 @@ -import { - expectCaseWithParent, - expectNotCase, - getDocsUrl, - method, -} from './util'; - -export default { - meta: { - docs: { - url: getDocsUrl(__filename), - }, - messages: { - preferCalledWith: 'Prefer {{name}}With(/* expected args */)', - }, - schema: [], - }, - create(context) { - return { - CallExpression(node) { - // Could check resolves/rejects here but not a likely idiom. - if (expectCaseWithParent(node) && !expectNotCase(node)) { - const methodNode = method(node); - const { name } = methodNode; - if (name === 'toBeCalled' || name === 'toHaveBeenCalled') { - context.report({ - data: { name }, - messageId: 'preferCalledWith', - node: methodNode, - }); - } - } - }, - }; - }, -}; diff --git a/src/rules/prefer-called-with.ts b/src/rules/prefer-called-with.ts new file mode 100644 index 000000000..57324f38a --- /dev/null +++ b/src/rules/prefer-called-with.ts @@ -0,0 +1,41 @@ +import { createRule, isExpectCall, parseExpectCall } from './tsUtils'; + +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: + 'Suggest using `toBeCalledWith()` OR `toHaveBeenCalledWith()`', + recommended: false, + }, + messages: { + preferCalledWith: 'Prefer {{name}}With(/* expected args */)', + }, + type: 'suggestion', + schema: [], + }, + defaultOptions: [], + create(context) { + return { + CallExpression(node) { + if (!isExpectCall(node)) { + return; + } + + const { modifier, matcher } = parseExpectCall(node); + + // Could check resolves/rejects here but not a likely idiom. + if (matcher && !modifier) { + if (['toBeCalled', 'toHaveBeenCalled'].includes(matcher.name)) { + context.report({ + data: { name: matcher.name }, // todo: rename to 'matcherName' + messageId: 'preferCalledWith', + node: matcher.node.property, + }); + } + } + }, + }; + }, +}); diff --git a/src/rules/prefer-strict-equal.ts b/src/rules/prefer-strict-equal.ts index c5d115102..671650c14 100644 --- a/src/rules/prefer-strict-equal.ts +++ b/src/rules/prefer-strict-equal.ts @@ -1,4 +1,4 @@ -import { createRule, isExpectCallWithParent } from './tsUtils'; +import { createRule, isExpectCall, parseExpectCall } from './tsUtils'; export default createRule({ name: __filename, @@ -12,26 +12,26 @@ export default createRule({ useToStrictEqual: 'Use toStrictEqual() instead', }, fixable: 'code', - schema: [], type: 'suggestion', + schema: [], }, defaultOptions: [], create(context) { return { CallExpression(node) { - if (!isExpectCallWithParent(node)) { + if (!isExpectCall(node)) { return; } - const methodNode = node.parent.property; + const { matcher } = parseExpectCall(node); - if (methodNode && methodNode.name === 'toEqual') { + if (matcher && matcher.name === 'toEqual') { context.report({ - fix(fixer) { - return [fixer.replaceText(methodNode, 'toStrictEqual')]; - }, + fix: fixer => [ + fixer.replaceText(matcher.node.property, 'toStrictEqual'), + ], messageId: 'useToStrictEqual', - node: methodNode, + node: matcher.node.property, }); } }, diff --git a/src/rules/prefer-to-be-null.js b/src/rules/prefer-to-be-null.js deleted file mode 100644 index 23e453aa7..000000000 --- a/src/rules/prefer-to-be-null.js +++ /dev/null @@ -1,52 +0,0 @@ -import { - argument, - argument2, - expectNotToBeCase, - expectNotToEqualCase, - expectToBeCase, - expectToEqualCase, - getDocsUrl, - method, - method2, -} from './util'; - -export default { - meta: { - docs: { - url: getDocsUrl(__filename), - }, - messages: { - useToBeNull: 'Use toBeNull() instead', - }, - fixable: 'code', - schema: [], - }, - create(context) { - return { - CallExpression(node) { - const is = expectToBeCase(node, null) || expectToEqualCase(node, null); - const isNot = - expectNotToEqualCase(node, null) || expectNotToBeCase(node, null); - - if (is || isNot) { - context.report({ - fix(fixer) { - if (is) { - return [ - fixer.replaceText(method(node), 'toBeNull'), - fixer.remove(argument(node)), - ]; - } - return [ - fixer.replaceText(method2(node), 'toBeNull'), - fixer.remove(argument2(node)), - ]; - }, - messageId: 'useToBeNull', - node: is ? method(node) : method2(node), - }); - } - }, - }; - }, -}; diff --git a/src/rules/prefer-to-be-null.ts b/src/rules/prefer-to-be-null.ts new file mode 100644 index 000000000..7b8626801 --- /dev/null +++ b/src/rules/prefer-to-be-null.ts @@ -0,0 +1,72 @@ +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import { + ParsedEqualityMatcherCall, + ParsedExpectMatcher, + createRule, + isExpectCall, + isParsedEqualityMatcherCall, + parseExpectCall, +} from './tsUtils'; + +interface NullLiteral extends TSESTree.Literal { + value: null; +} + +const isNullLiteral = (node: TSESTree.Node): node is NullLiteral => + node.type === AST_NODE_TYPES.Literal && node.value === null; + +/** + * Checks if the given `ParsedExpectMatcher` is a call to one of the equality matchers, + * with a `null` literal as the sole argument. + * + * @param {ParsedExpectMatcher} matcher + * + * @return {matcher is ParsedNullEqualityMatcher} + */ +const isNullEqualityMatcher = ( + matcher: ParsedExpectMatcher, +): matcher is ParsedEqualityMatcherCall => + isParsedEqualityMatcherCall(matcher) && isNullLiteral(matcher.arguments[0]); + +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Suggest using `toBeNull()`', + recommended: false, + }, + messages: { + useToBeNull: 'Use toBeNull() instead', + }, + fixable: 'code', + type: 'suggestion', + schema: [], + }, + defaultOptions: [], + create(context) { + return { + CallExpression(node) { + if (!isExpectCall(node)) { + return; + } + + const { matcher } = parseExpectCall(node); + + if (matcher && isNullEqualityMatcher(matcher)) { + context.report({ + fix: fixer => [ + fixer.replaceText(matcher.node.property, 'toBeNull'), + fixer.remove(matcher.arguments[0]), + ], + messageId: 'useToBeNull', + node: matcher.node.property, + }); + } + }, + }; + }, +}); diff --git a/src/rules/prefer-to-be-undefined.js b/src/rules/prefer-to-be-undefined.js deleted file mode 100644 index 1aff3f98e..000000000 --- a/src/rules/prefer-to-be-undefined.js +++ /dev/null @@ -1,54 +0,0 @@ -import { - argument, - argument2, - expectNotToBeCase, - expectNotToEqualCase, - expectToBeCase, - expectToEqualCase, - getDocsUrl, - method, - method2, -} from './util'; - -export default { - meta: { - docs: { - url: getDocsUrl(__filename), - }, - messages: { - useToBeUndefined: 'Use toBeUndefined() instead', - }, - fixable: 'code', - schema: [], - }, - create(context) { - return { - CallExpression(node) { - const is = - expectToBeCase(node, undefined) || expectToEqualCase(node, undefined); - const isNot = - expectNotToEqualCase(node, undefined) || - expectNotToBeCase(node, undefined); - - if (is || isNot) { - context.report({ - fix(fixer) { - if (is) { - return [ - fixer.replaceText(method(node), 'toBeUndefined'), - fixer.remove(argument(node)), - ]; - } - return [ - fixer.replaceText(method2(node), 'toBeUndefined'), - fixer.remove(argument2(node)), - ]; - }, - messageId: 'useToBeUndefined', - node: is ? method(node) : method2(node), - }); - } - }, - }; - }, -}; diff --git a/src/rules/prefer-to-be-undefined.ts b/src/rules/prefer-to-be-undefined.ts new file mode 100644 index 000000000..7f3c7938e --- /dev/null +++ b/src/rules/prefer-to-be-undefined.ts @@ -0,0 +1,75 @@ +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import { + ParsedEqualityMatcherCall, + ParsedExpectMatcher, + createRule, + isExpectCall, + isParsedEqualityMatcherCall, + parseExpectCall, +} from './tsUtils'; + +interface UndefinedIdentifier extends TSESTree.Identifier { + name: 'undefined'; +} + +const isUndefinedIdentifier = ( + node: TSESTree.Node, +): node is UndefinedIdentifier => + node.type === AST_NODE_TYPES.Identifier && node.name === `${undefined}`; + +/** + * Checks if the given `ParsedExpectMatcher` is a call to one of the equality matchers, + * with a `undefined` identifier as the sole argument. + * + * @param {ParsedExpectMatcher} matcher + * + * @return {matcher is ParsedUndefinedEqualityMatcher} + */ +const isUndefinedEqualityMatcher = ( + matcher: ParsedExpectMatcher, +): matcher is ParsedEqualityMatcherCall => + isParsedEqualityMatcherCall(matcher) && + isUndefinedIdentifier(matcher.arguments[0]); + +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Suggest using `toBeUndefined()`', + recommended: false, + }, + messages: { + useToBeUndefined: 'Use toBeUndefined() instead', + }, + fixable: 'code', + type: 'suggestion', + schema: [], + }, + defaultOptions: [], + create(context) { + return { + CallExpression(node) { + if (!isExpectCall(node)) { + return; + } + + const { matcher } = parseExpectCall(node); + + if (matcher && isUndefinedEqualityMatcher(matcher)) { + context.report({ + fix: fixer => [ + fixer.replaceText(matcher.node.property, 'toBeUndefined'), + fixer.remove(matcher.arguments[0]), + ], + messageId: 'useToBeUndefined', + node: matcher.node.property, + }); + } + }, + }; + }, +}); diff --git a/src/rules/prefer-to-contain.js b/src/rules/prefer-to-contain.js deleted file mode 100644 index ba59f6c5b..000000000 --- a/src/rules/prefer-to-contain.js +++ /dev/null @@ -1,131 +0,0 @@ -import { - argument, - expectCaseWithParent, - expectRejectsCase, - expectResolvesCase, - getDocsUrl, - method, -} from './util'; - -const isEqualityCheck = node => - method(node) && - (method(node).name === 'toBe' || method(node).name === 'toEqual'); - -const isArgumentValid = node => - argument(node).value === true || argument(node).value === false; - -const hasOneArgument = node => node.arguments && node.arguments.length === 1; - -const isValidEqualityCheck = node => - isEqualityCheck(node) && - hasOneArgument(node.parent.parent) && - isArgumentValid(node); - -const isEqualityNegation = node => - method(node).name === 'not' && isValidEqualityCheck(node.parent); - -const hasIncludesMethod = node => - node.arguments[0] && - node.arguments[0].callee && - node.arguments[0].callee.property && - node.arguments[0].callee.property.name === 'includes'; - -const isValidIncludesMethod = node => - hasIncludesMethod(node) && hasOneArgument(node.arguments[0]); - -const getNegationFixes = (node, sourceCode, fixer) => { - const negationPropertyDot = sourceCode.getFirstTokenBetween( - node.parent.object, - node.parent.property, - token => token.value === '.', - ); - const toContainFunc = - isEqualityNegation(node) && argument(node.parent).value - ? 'not.toContain' - : 'toContain'; - - //.includes function argument - const [containArg] = node.arguments[0].arguments; - return [ - fixer.remove(negationPropertyDot), - fixer.remove(method(node)), - fixer.replaceText(method(node.parent), toContainFunc), - fixer.replaceText(argument(node.parent), sourceCode.getText(containArg)), - ]; -}; - -const getCommonFixes = (node, sourceCode, fixer) => { - const [containArg] = node.arguments[0].arguments; - const includesCaller = node.arguments[0].callee; - - const propertyDot = sourceCode.getFirstTokenBetween( - includesCaller.object, - includesCaller.property, - token => token.value === '.', - ); - - const closingParenthesis = sourceCode.getTokenAfter(containArg); - const openParenthesis = sourceCode.getTokenBefore(containArg); - - return [ - fixer.remove(containArg), - fixer.remove(includesCaller.property), - fixer.remove(propertyDot), - fixer.remove(closingParenthesis), - fixer.remove(openParenthesis), - ]; -}; - -export default { - meta: { - docs: { - url: getDocsUrl(__filename), - }, - messages: { - useToContain: 'Use toContain() instead', - }, - fixable: 'code', - schema: [], - }, - create(context) { - return { - CallExpression(node) { - if ( - !(expectResolvesCase(node) || expectRejectsCase(node)) && - expectCaseWithParent(node) && - (isEqualityNegation(node) || isValidEqualityCheck(node)) && - isValidIncludesMethod(node) - ) { - context.report({ - fix(fixer) { - const sourceCode = context.getSourceCode(); - - let fixArr = getCommonFixes(node, sourceCode, fixer); - if (isEqualityNegation(node)) { - return getNegationFixes(node, sourceCode, fixer).concat(fixArr); - } - - const toContainFunc = argument(node).value - ? 'toContain' - : 'not.toContain'; - - //.includes function argument - const [containArg] = node.arguments[0].arguments; - - fixArr.push(fixer.replaceText(method(node), toContainFunc)); - fixArr.push( - fixer.replaceText( - argument(node), - sourceCode.getText(containArg), - ), - ); - return fixArr; - }, - messageId: 'useToContain', - node: method(node), - }); - } - }, - }; - }, -}; diff --git a/src/rules/prefer-to-contain.ts b/src/rules/prefer-to-contain.ts new file mode 100644 index 000000000..4e7641ca9 --- /dev/null +++ b/src/rules/prefer-to-contain.ts @@ -0,0 +1,248 @@ +import { + AST_NODE_TYPES, + TSESLint, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import { + CallExpressionWithSingleArgument, + KnownCallExpression, + ModifierName, + NotNegatableParsedModifier, + ParsedEqualityMatcherCall, + ParsedExpectMatcher, + createRule, + hasOnlyOneArgument, + isExpectCall, + isParsedEqualityMatcherCall, + isSupportedAccessor, + parseExpectCall, +} from './tsUtils'; + +interface BooleanLiteral extends TSESTree.Literal { + value: boolean; +} + +const isBooleanLiteral = (node: TSESTree.Node): node is BooleanLiteral => + node.type === AST_NODE_TYPES.Literal && typeof node.value === 'boolean'; + +type ParsedBooleanEqualityMatcherCall = ParsedEqualityMatcherCall< + BooleanLiteral +>; + +/** + * Checks if the given `ParsedExpectMatcher` is a call to one of the equality matchers, + * with a boolean literal as the sole argument. + * + * @example javascript + * toBe(true); + * toEqual(false); + * + * @param {ParsedExpectMatcher} matcher + * + * @return {matcher is ParsedBooleanEqualityMatcher} + */ +const isBooleanEqualityMatcher = ( + matcher: ParsedExpectMatcher, +): matcher is ParsedBooleanEqualityMatcherCall => + isParsedEqualityMatcherCall(matcher) && + isBooleanLiteral(matcher.arguments[0]); + +type FixableIncludesCallExpression = KnownCallExpression<'includes'> & + CallExpressionWithSingleArgument; + +/** + * Checks if the given `node` is a `CallExpression` representing the calling + * of an `includes`-like method that can be 'fixed' (using `toContain`). + * + * @param {CallExpression} node + * + * @return {node is FixableIncludesCallExpression} + * + * @todo support `['includes']()` syntax (remove last property.type check to begin) + * @todo break out into `isMethodCall(node: TSESTree.Node, method: Name)` util-fn + */ +const isFixableIncludesCallExpression = ( + node: TSESTree.Node, +): node is FixableIncludesCallExpression => + node.type === AST_NODE_TYPES.CallExpression && + node.callee.type === AST_NODE_TYPES.MemberExpression && + isSupportedAccessor(node.callee.property, 'includes') && + node.callee.property.type === AST_NODE_TYPES.Identifier && + hasOnlyOneArgument(node); + +const buildToContainFuncExpectation = (negated: boolean) => + negated ? `${ModifierName.not}.toContain` : 'toContain'; + +/** + * Finds the first `.` character token between the `object` & `property` of the given `member` expression. + * + * @param {TSESTree.MemberExpression} member + * @param {SourceCode} sourceCode + * + * @return {Token | null} + */ +const findPropertyDotToken = ( + member: TSESTree.MemberExpression, + sourceCode: TSESLint.SourceCode, +) => + sourceCode.getFirstTokenBetween( + member.object, + member.property, + token => token.value === '.', + ); + +const getNegationFixes = ( + node: FixableIncludesCallExpression, + modifier: NotNegatableParsedModifier, + matcher: ParsedBooleanEqualityMatcherCall, + sourceCode: TSESLint.SourceCode, + fixer: TSESLint.RuleFixer, + fileName: string, +) => { + //.includes function argument + const [containArg] = node.arguments; + const negationPropertyDot = findPropertyDotToken(modifier.node, sourceCode); + + const toContainFunc = buildToContainFuncExpectation( + matcher.arguments[0].value, + ); + // ? 'not.toContain' + // : 'toContain'; + + /* istanbul ignore next */ + if (negationPropertyDot === null) { + throw new Error( + `Unexpected null when attempting to fix ${fileName} - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`, + ); + } + + return [ + fixer.remove(negationPropertyDot), + fixer.remove(modifier.node.property), + fixer.replaceText(matcher.node.property, toContainFunc), + fixer.replaceText(matcher.arguments[0], sourceCode.getText(containArg)), + ]; +}; + +const getCommonFixes = ( + node: FixableIncludesCallExpression, + sourceCode: TSESLint.SourceCode, + fixer: TSESLint.RuleFixer, + fileName: string, +) => { + const [containArg] = node.arguments; + const includesCallee = node.callee; + + const propertyDot = findPropertyDotToken(includesCallee, sourceCode); + + const closingParenthesis = sourceCode.getTokenAfter(containArg); + const openParenthesis = sourceCode.getTokenBefore(containArg); + + /* istanbul ignore next */ + if ( + propertyDot === null || + closingParenthesis === null || + openParenthesis === null + ) { + throw new Error( + `Unexpected null when attempting to fix ${fileName} - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`, + ); + } + + return [ + fixer.remove(containArg), + fixer.remove(includesCallee.property), + fixer.remove(propertyDot), + fixer.remove(closingParenthesis), + fixer.remove(openParenthesis), + ]; +}; +// expect(array.includes()[not.]{toBe,toEqual}() +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Suggest using `toContain()`', + recommended: false, + }, + messages: { + useToContain: 'Use toContain() instead', + }, + fixable: 'code', + type: 'suggestion', + schema: [], + }, + defaultOptions: [], + create(context) { + return { + CallExpression(node) { + if (!isExpectCall(node)) { + return; + } + + const { + expect: { + arguments: [includesCall], + }, + matcher, + modifier, + } = parseExpectCall(node); + + if ( + !matcher || + (modifier && modifier.name !== ModifierName.not) || + !isBooleanEqualityMatcher(matcher) || + !isFixableIncludesCallExpression(includesCall) + ) { + return; + } + + context.report({ + fix(fixer) { + const sourceCode = context.getSourceCode(); + const fileName = context.getFilename(); + + const fixArr = getCommonFixes( + includesCall, + sourceCode, + fixer, + fileName, + ); + + if (modifier && modifier.name === ModifierName.not) { + return getNegationFixes( + includesCall, + modifier, + matcher, + sourceCode, + fixer, + fileName, + ).concat(fixArr); + } + + const toContainFunc = buildToContainFuncExpectation( + !matcher.arguments[0].value, + ); + + //.includes function argument + const [containArg] = includesCall.arguments; + + fixArr.push( + fixer.replaceText(matcher.node.property, toContainFunc), + ); + fixArr.push( + fixer.replaceText( + matcher.arguments[0], + sourceCode.getText(containArg), + ), + ); + return fixArr; + }, + messageId: 'useToContain', + node: (modifier || matcher).node.property, + }); + }, + }; + }, +}); diff --git a/src/rules/prefer-to-have-length.js b/src/rules/prefer-to-have-length.js deleted file mode 100644 index bc8792602..000000000 --- a/src/rules/prefer-to-have-length.js +++ /dev/null @@ -1,57 +0,0 @@ -import { - expectCaseWithParent, - expectNotCase, - expectRejectsCase, - expectResolvesCase, - getDocsUrl, - method, -} from './util'; - -export default { - meta: { - docs: { - url: getDocsUrl(__filename), - }, - messages: { - useToHaveLength: 'Use toHaveLength() instead', - }, - fixable: 'code', - schema: [], - }, - create(context) { - return { - CallExpression(node) { - if ( - !( - expectNotCase(node) || - expectResolvesCase(node) || - expectRejectsCase(node) - ) && - expectCaseWithParent(node) && - (method(node).name === 'toBe' || method(node).name === 'toEqual') && - node.arguments[0].property && - node.arguments[0].property.name === 'length' - ) { - const propertyDot = context - .getSourceCode() - .getFirstTokenBetween( - node.arguments[0].object, - node.arguments[0].property, - token => token.value === '.', - ); - context.report({ - fix(fixer) { - return [ - fixer.remove(propertyDot), - fixer.remove(node.arguments[0].property), - fixer.replaceText(method(node), 'toHaveLength'), - ]; - }, - messageId: 'useToHaveLength', - node: method(node), - }); - } - }, - }; - }, -}; diff --git a/src/rules/prefer-to-have-length.ts b/src/rules/prefer-to-have-length.ts new file mode 100644 index 000000000..f44ea3e47 --- /dev/null +++ b/src/rules/prefer-to-have-length.ts @@ -0,0 +1,80 @@ +import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; +import { + createRule, + isExpectCall, + isParsedEqualityMatcherCall, + isSupportedAccessor, + parseExpectCall, +} from './tsUtils'; + +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Suggest using `toHaveLength()`', + recommended: false, + }, + messages: { + useToHaveLength: 'Use toHaveLength() instead', + }, + fixable: 'code', + type: 'suggestion', + schema: [], + }, + defaultOptions: [], + create(context) { + return { + CallExpression(node) { + if (!isExpectCall(node)) { + return; + } + + const { + expect: { + arguments: [argument], + }, + matcher, + } = parseExpectCall(node); + + if ( + !matcher || + !isParsedEqualityMatcherCall(matcher) || + !argument || + argument.type !== AST_NODE_TYPES.MemberExpression || + !isSupportedAccessor(argument.property, 'length') || + argument.property.type !== AST_NODE_TYPES.Identifier + ) { + return; + } + + context.report({ + fix(fixer) { + const propertyDot = context + .getSourceCode() + .getFirstTokenBetween( + argument.object, + argument.property, + token => token.value === '.', + ); + + /* istanbul ignore next */ + if (propertyDot === null) { + throw new Error( + `Unexpected null when attempting to fix ${context.getFilename()} - please file a github issue at https://github.com/jest-community/eslint-plugin-jest`, + ); + } + + return [ + fixer.remove(propertyDot), + fixer.remove(argument.property), + fixer.replaceText(matcher.node.property, 'toHaveLength'), + ]; + }, + messageId: 'useToHaveLength', + node: matcher.node.property, + }); + }, + }; + }, +}); diff --git a/src/rules/require-tothrow-message.js b/src/rules/require-tothrow-message.js deleted file mode 100644 index 5e609fe5d..000000000 --- a/src/rules/require-tothrow-message.js +++ /dev/null @@ -1,41 +0,0 @@ -import { argument, expectCaseWithParent, getDocsUrl, method } from './util'; - -export default { - meta: { - docs: { - url: getDocsUrl(__filename), - }, - messages: { - requireRethrow: 'Add an error message to {{ propertyName }}()', - }, - schema: [], - }, - create(context) { - return { - CallExpression(node) { - if (!expectCaseWithParent(node)) { - return; - } - - let targetNode = method(node); - if (targetNode.name === 'rejects') { - targetNode = method(node.parent); - } - - const propertyName = method(targetNode) && method(targetNode).name; - - // Look for `toThrow` calls with no arguments. - if ( - ['toThrow', 'toThrowError'].includes(propertyName) && - !argument(targetNode) - ) { - context.report({ - messageId: 'requireRethrow', - data: { propertyName }, - node: targetNode, - }); - } - }, - }; - }, -}; diff --git a/src/rules/require-tothrow-message.ts b/src/rules/require-tothrow-message.ts new file mode 100644 index 000000000..6f3644dd9 --- /dev/null +++ b/src/rules/require-tothrow-message.ts @@ -0,0 +1,50 @@ +import { + ModifierName, + createRule, + isExpectCall, + parseExpectCall, +} from './tsUtils'; + +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Require a message for `toThrow()`', + recommended: false, + }, + messages: { + requireRethrow: 'Add an error message to {{ propertyName }}()', + }, + type: 'suggestion', + schema: [], + }, + defaultOptions: [], + create(context) { + return { + CallExpression(node) { + if (!isExpectCall(node)) { + return; + } + + const { matcher, modifier } = parseExpectCall(node); + + if ( + matcher && + matcher.arguments && + matcher.arguments.length === 0 && + ['toThrow', 'toThrowError'].includes(matcher.name) && + (!modifier || + !(modifier.name === ModifierName.not || modifier.negation)) + ) { + // Look for `toThrow` calls with no arguments. + context.report({ + messageId: 'requireRethrow', // todo: rename to 'addErrorMessage' + data: { propertyName: matcher.name }, // todo: rename to 'matcherName' + node: matcher.node.property, + }); + } + }, + }; + }, +}); diff --git a/src/rules/tsUtils.ts b/src/rules/tsUtils.ts index fe0d4afd0..9ff3f7b49 100644 --- a/src/rules/tsUtils.ts +++ b/src/rules/tsUtils.ts @@ -225,7 +225,6 @@ export const isSupportedAccessor = ( export const getAccessorValue = ( accessor: AccessorNode, ): S => - /* istanbul ignore next */ accessor.type === AST_NODE_TYPES.Identifier ? accessor.name : getStringValue(accessor); @@ -239,6 +238,23 @@ interface ExpectCall extends TSESTree.CallExpression { parent: TSESTree.Node; } +/** + * Checks if the given `node` is a valid `ExpectCall`. + * + * In order to be an `ExpectCall`, the `node` must: + * * be a `CallExpression`, + * * have an accessor named 'expect', + * * have a `parent`. + * + * @param {Node} node + * + * @return {node is ExpectCall} + */ +export const isExpectCall = (node: TSESTree.Node): node is ExpectCall => + node.type === AST_NODE_TYPES.CallExpression && + isSupportedAccessor(node.callee, 'expect') && + node.parent !== undefined; + /** * Represents a `MemberExpression` that comes after an `ExpectCall`. */ @@ -257,7 +273,8 @@ export const isExpectMember = < name?: Name, ): node is ExpectMember => node.type === AST_NODE_TYPES.MemberExpression && - isSupportedAccessor(node.property, name); + isSupportedAccessor(node.property) && + (name === undefined || getAccessorValue(node.property) === name); /** * Represents all the jest matchers. @@ -265,60 +282,207 @@ export const isExpectMember = < type MatcherName = string /* & not ModifierName */; type ExpectPropertyName = ModifierName | MatcherName; +export type ParsedEqualityMatcherCall< + Argument extends TSESTree.Expression = TSESTree.Expression +> = Omit, 'arguments'> & { + // todo: probs should also type node parent as CallExpression + arguments: [Argument]; +}; + +enum EqualityMatcher { + toBe = 'toBe', + toEqual = 'toEqual', +} + +export const isParsedEqualityMatcherCall = ( + matcher: ParsedExpectMatcher, +): matcher is ParsedEqualityMatcherCall => + EqualityMatcher.hasOwnProperty(matcher.name) && + matcher.arguments !== null && + matcher.arguments.length === 1; + export enum ModifierName { not = 'not', rejects = 'rejects', resolves = 'resolves', } -interface JestExpectIdentifier extends TSESTree.Identifier { - name: 'expect'; +interface ParsedExpectMember< + Name extends ExpectPropertyName = ExpectPropertyName, + Node extends ExpectMember = ExpectMember +> { + name: Name; + node: Node; } -// represents "expect()" specifically -interface JestExpectCallExpression extends TSESTree.CallExpression { - callee: JestExpectIdentifier; +/** + * Represents a parsed expect matcher, such as `toBe`, `toContain`, and so on. + */ +export interface ParsedExpectMatcher< + Matcher extends MatcherName = MatcherName, + Node extends ExpectMember = ExpectMember +> extends ParsedExpectMember { + arguments: TSESTree.CallExpression['arguments'] | null; + // isNamed( + // name: PossibleName, + // ...or: PossibleName[] + // ): this is ParsedExpectMatcher; + // isNamed( + // // name: PossibleName, + // ...names: [PossibleName, ...PossibleName[]] + // ): this is ParsedExpectMatcher; } -// represents expect usage like "expect().toBe" & "expect().not.toBe" -interface JestExpectCallMemberExpression extends TSESTree.MemberExpression { - object: JestExpectCallMemberExpression | JestExpectCallExpression; - property: TSESTree.Identifier; +type BaseParsedModifier< + Modifier extends ModifierName = ModifierName +> = ParsedExpectMember; + +type NegatableModifierName = ModifierName.rejects | ModifierName.resolves; +type NotNegatableModifierName = ModifierName.not; + +/** + * Represents a parsed modifier that can be followed by a `not` negation modifier. + */ +interface NegatableParsedModifier< + Modifier extends NegatableModifierName = NegatableModifierName +> extends BaseParsedModifier { + negation?: ExpectMember; } -interface ExpectCall extends TSESTree.CallExpression { - callee: AccessorNode<'expect'>; - parent: TSESTree.Node; +/** + * Represents a parsed modifier that cannot be followed by a `not` negation modifier. + */ +export interface NotNegatableParsedModifier< + Modifier extends NotNegatableModifierName = NotNegatableModifierName +> extends BaseParsedModifier { + negation?: never; +} + +type ParsedExpectModifier = + | NotNegatableParsedModifier + | NegatableParsedModifier; + +interface Expectation { + expect: ExpectNode; + modifier?: ParsedExpectModifier; + matcher?: ParsedExpectMatcher; } +const parseExpectMember = ( + expectMember: ExpectMember, +): ParsedExpectMember => ({ + name: getAccessorValue(expectMember.property), + node: expectMember, +}); + +const reparseAsMatcher = ( + parsedMember: ParsedExpectMember, +): ParsedExpectMatcher => ({ + ...parsedMember, + /** + * The arguments being passed to this `Matcher`, if any. + * + * If this matcher isn't called, this will be `null`. + */ + arguments: + parsedMember.node.parent && + parsedMember.node.parent.type === AST_NODE_TYPES.CallExpression + ? parsedMember.node.parent.arguments + : null, + // isNamed(...names: string[]) { + // return names.includes(this.name); + // }, +}); + /** - * Checks if the given `node` is a valid `ExpectCall`. + * Re-parses the given `parsedMember` as a `ParsedExpectModifier`. * - * In order to be an `ExpectCall`, the `node` must: - * * be a `CallExpression`, - * * have an accessor named 'expect', - * * have a `parent`. + * If the given `parsedMember` does not have a `name` of a valid `Modifier`, + * an exception will be thrown. * - * @param {Node} node + * @param {ParsedExpectMember} parsedMember * - * @return {node is ExpectCall} + * @return {ParsedExpectModifier} */ -export const isExpectCall = (node: TSESTree.Node): node is ExpectCall => - node.type === AST_NODE_TYPES.CallExpression && - isSupportedAccessor(node.callee, 'expect') && - node.parent !== undefined; +const reparseMemberAsModifier = ( + parsedMember: ParsedExpectMember, +): ParsedExpectModifier => { + if (isSpecificMember(parsedMember, ModifierName.not)) { + return parsedMember; + } -interface JestExpectCallWithParent extends JestExpectCallExpression { - parent: JestExpectCallMemberExpression; -} + /* istanbul ignore if */ + if ( + !isSpecificMember(parsedMember, ModifierName.resolves) && + !isSpecificMember(parsedMember, ModifierName.rejects) + ) { + throw new Error( + `modifier name must be either "${ModifierName.resolves}" or "${ModifierName.rejects}" (got "${parsedMember.name}")`, + ); // ts doesn't think that the ModifierName.not check is the direct inverse as the above two checks + } // todo: impossible at runtime, but can't be typed w/o negation support + + const negation = + parsedMember.node.parent && + isExpectMember(parsedMember.node.parent, ModifierName.not) + ? parsedMember.node.parent + : undefined; + + return { + ...parsedMember, + negation, + }; +}; -export const isExpectCallWithParent = ( - node: TSESTree.Node, -): node is JestExpectCallWithParent => - isExpectCall(node) && - node.parent !== undefined && - node.parent.type === AST_NODE_TYPES.MemberExpression && - node.parent.property.type === AST_NODE_TYPES.Identifier; +const isSpecificMember = ( + member: ParsedExpectMember, + specific: Name, +): member is ParsedExpectMember => member.name === specific; + +/** + * Checks if the given `ParsedExpectMember` should be re-parsed as an `ParsedExpectModifier`. + * + * @param {ParsedExpectMember} member + * + * @return {member is ParsedExpectMember} + */ +const shouldBeParsedExpectModifier = ( + member: ParsedExpectMember, +): member is ParsedExpectMember => + ModifierName.hasOwnProperty(member.name); + +export const parseExpectCall = ( + expect: ExpectNode, +): Expectation => { + const expectation: Expectation = { + expect, + }; + + if (!isExpectMember(expect.parent)) { + return expectation; + } + + const parsedMember = parseExpectMember(expect.parent); + if (!shouldBeParsedExpectModifier(parsedMember)) { + expectation.matcher = reparseAsMatcher(parsedMember); + + return expectation; + } + + const modifier = (expectation.modifier = reparseMemberAsModifier( + parsedMember, + )); + + const memberNode = + ('negation' in modifier && modifier.negation) || modifier.node; + + if (!memberNode.parent || !isExpectMember(memberNode.parent)) { + return expectation; + } + + expectation.matcher = reparseAsMatcher(parseExpectMember(memberNode.parent)); + + return expectation; +}; export enum DescribeAlias { 'describe' = 'describe', diff --git a/src/rules/valid-expect.js b/src/rules/valid-expect.js deleted file mode 100644 index 84e6f2ffa..000000000 --- a/src/rules/valid-expect.js +++ /dev/null @@ -1,314 +0,0 @@ -/* - * This implementation is ported from from eslint-plugin-jasmine. - * MIT license, Tom Vincent. - */ - -import { - expectCase, - expectNotRejectsCase, - expectNotResolvesCase, - expectRejectsCase, - expectResolvesCase, - getDocsUrl, -} from './util'; - -const expectProperties = ['not', 'resolves', 'rejects']; -const promiseArgumentTypes = ['CallExpression', 'ArrayExpression']; - -/** - * expect statement can have chained matchers. - * We are looking for the closest CallExpression - * to find `expect().x.y.z.t()` usage. - * - * @Returns CallExpressionNode - */ -const getClosestParentCallExpressionNode = node => { - if (!node || !node.parent || !node.parent.parent) { - return null; - } - - if (node.parent.type === 'CallExpression') { - return node.parent; - } - return getClosestParentCallExpressionNode(node.parent); -}; - -/** - * Async assertions might be called in Promise - * methods like `Promise.x(expect1)` or `Promise.x([expect1, expect2])`. - * If that's the case, Promise node have to be awaited or returned. - * - * @Returns CallExpressionNode - */ -const getPromiseCallExpressionNode = node => { - if ( - node && - node.type === 'ArrayExpression' && - node.parent && - node.parent.type === 'CallExpression' - ) { - node = node.parent; - } - - if ( - node.type === 'CallExpression' && - node.callee && - node.callee.type === 'MemberExpression' && - node.callee.object.name === 'Promise' && - node.parent - ) { - return node; - } - - return null; -}; - -const getParentIfThenified = node => { - const grandParentNode = node.parent && node.parent.parent; - - if ( - grandParentNode && - grandParentNode.type === 'CallExpression' && - grandParentNode.callee && - grandParentNode.callee.type === 'MemberExpression' && - ['then', 'catch'].includes(grandParentNode.callee.property.name) && - grandParentNode.parent - ) { - // Just in case `then`s are chained look one above. - return getParentIfThenified(grandParentNode); - } - - return node; -}; - -const checkIfValidReturn = (parentCallExpressionNode, allowReturn) => { - const validParentNodeTypes = ['ArrowFunctionExpression', 'AwaitExpression']; - if (allowReturn) { - validParentNodeTypes.push('ReturnStatement'); - } - - return validParentNodeTypes.includes(parentCallExpressionNode.type); -}; - -const promiseArrayExceptionKey = ({ start, end }) => - `${start.line}:${start.column}-${end.line}:${end.column}`; - -export default { - meta: { - docs: { - url: getDocsUrl(__filename), - }, - messages: { - multipleArgs: 'More than one argument was passed to expect().', - noArgs: 'No arguments were passed to expect().', - noAssertions: 'No assertion was called on expect().', - invalidProperty: - '"{{ propertyName }}" is not a valid property of expect.', - propertyWithoutMatcher: '"{{ propertyName }}" needs to call a matcher.', - matcherOnPropertyNotCalled: '"{{ propertyName }}" was not called.', - asyncMustBeAwaited: 'Async assertions must be awaited{{ orReturned }}.', - promisesWithAsyncAssertionsMustBeAwaited: - 'Promises which return async assertions must be awaited{{ orReturned }}.', - }, - schema: [ - { - type: 'object', - properties: { - alwaysAwait: { - type: 'boolean', - default: false, - }, - }, - additionalProperties: false, - }, - ], - }, - create(context) { - // Context state - const arrayExceptions = {}; - - const pushPromiseArrayException = loc => { - const key = promiseArrayExceptionKey(loc); - arrayExceptions[key] = true; - }; - - /** - * Promise method that accepts an array of promises, - * ( eg. Promise.all), will throw warnings for the each - * unawaited or non-returned promise. To avoid throwing - * multiple warnings, we check if there is a warning in - * the given location. - */ - const promiseArrayExceptionExists = loc => { - const key = promiseArrayExceptionKey(loc); - return !!arrayExceptions[key]; - }; - - return { - CallExpression(node) { - // checking "expect()" arguments - if (expectCase(node)) { - if (node.arguments.length > 1) { - const secondArgumentLocStart = node.arguments[1].loc.start; - const lastArgumentLocEnd = - node.arguments[node.arguments.length - 1].loc.end; - - context.report({ - loc: { - end: { - column: lastArgumentLocEnd.column - 1, - line: lastArgumentLocEnd.line, - }, - start: secondArgumentLocStart, - }, - messageId: 'multipleArgs', - node, - }); - } else if (node.arguments.length === 0) { - const expectLength = node.callee.name.length; - context.report({ - loc: { - end: { - column: node.loc.start.column + expectLength + 1, - line: node.loc.start.line, - }, - start: { - column: node.loc.start.column + expectLength, - line: node.loc.start.line, - }, - }, - messageId: 'noArgs', - node, - }); - } - - // something was called on `expect()` - if ( - node.parent && - node.parent.type === 'MemberExpression' && - node.parent.parent - ) { - let parentNode = node.parent; - let parentProperty = parentNode.property; - let propertyName = parentProperty.name; - let grandParent = parentNode.parent; - - // a property is accessed, get the next node - if (grandParent.type === 'MemberExpression') { - // a modifier is used, just get the next one - if (expectProperties.includes(propertyName)) { - grandParent = grandParent.parent; - } else { - // only a few properties are allowed - context.report({ - // For some reason `endColumn` isn't set in tests if `loc` is - // not added - loc: parentProperty.loc, - messageId: 'invalidProperty', - data: { propertyName }, - node: parentProperty, - }); - } - - // this next one should be the matcher - parentNode = parentNode.parent; - parentProperty = parentNode.property; - propertyName = parentProperty.name; - } - - // matcher was not called - if (grandParent.type === 'ExpressionStatement') { - context.report({ - // For some reason `endColumn` isn't set in tests if `loc` is not - // added - loc: parentProperty.loc, - data: { propertyName }, - messageId: expectProperties.includes(propertyName) - ? 'propertyWithoutMatcher' - : 'matcherOnPropertyNotCalled', - node: parentProperty, - }); - } - } - } - - if ( - expectResolvesCase(node) || - expectRejectsCase(node) || - expectNotResolvesCase(node) || - expectNotRejectsCase(node) - ) { - let parentNode = getClosestParentCallExpressionNode(node); - if (parentNode) { - /** - * If parent node is an array expression, we'll report the warning, - * for the array object, not for each individual assertion. - */ - const isParentArrayExpression = - parentNode.parent.type === 'ArrayExpression'; - - const { options } = context; - const allowReturn = !options[0] || !options[0].alwaysAwait; - const orReturned = allowReturn ? ' or returned' : ''; - let messageId = 'asyncMustBeAwaited'; - - /** - * An async assertion can be chained with `then` or `catch` statements. - * In that case our target CallExpression node is the one with - * the last `then` or `catch` statement. - */ - parentNode = getParentIfThenified(parentNode); - - // Promise.x([expect()]) || Promise.x(expect()) - if (promiseArgumentTypes.includes(parentNode.parent.type)) { - const promiseNode = getPromiseCallExpressionNode( - parentNode.parent, - ); - - if (promiseNode) { - parentNode = promiseNode; - messageId = 'promisesWithAsyncAssertionsMustBeAwaited'; - } - } - - if ( - // If node is not awaited or returned - !checkIfValidReturn(parentNode.parent, allowReturn) && - // if we didn't warn user already - !promiseArrayExceptionExists(parentNode.loc) - ) { - context.report({ - loc: parentNode.loc, - data: { - orReturned, - }, - messageId, - node, - }); - - if (isParentArrayExpression) { - pushPromiseArrayException(parentNode.loc); - } - } - } - } - }, - - // nothing called on "expect()" - 'CallExpression:exit'(node) { - if ( - node.callee.name === 'expect' && - node.parent.type === 'ExpressionStatement' - ) { - context.report({ - // For some reason `endColumn` isn't set in tests if `loc` is not - // added - loc: node.loc, - messageId: 'noAssertions', - node, - }); - } - }, - }; - }, -}; diff --git a/src/rules/valid-expect.ts b/src/rules/valid-expect.ts new file mode 100644 index 000000000..d602aaa89 --- /dev/null +++ b/src/rules/valid-expect.ts @@ -0,0 +1,292 @@ +/* + * This implementation is ported from from eslint-plugin-jasmine. + * MIT license, Tom Vincent. + */ + +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import { + ModifierName, + createRule, + getAccessorValue, + isExpectCall, + isExpectMember, + isSupportedAccessor, + parseExpectCall, +} from './tsUtils'; + +/** + * Async assertions might be called in Promise + * methods like `Promise.x(expect1)` or `Promise.x([expect1, expect2])`. + * If that's the case, Promise node have to be awaited or returned. + * + * @Returns CallExpressionNode + */ +const getPromiseCallExpressionNode = (node: TSESTree.Node) => { + if ( + node && + node.type === AST_NODE_TYPES.ArrayExpression && + node.parent && + node.parent.type === AST_NODE_TYPES.CallExpression + ) { + node = node.parent; + } + + if ( + node.type === AST_NODE_TYPES.CallExpression && + node.callee && + node.callee.type === AST_NODE_TYPES.MemberExpression && + isSupportedAccessor(node.callee.object) && + getAccessorValue(node.callee.object) === 'Promise' && + node.parent + ) { + return node; + } + + return null; +}; + +const findPromiseCallExpressionNode = (node: TSESTree.Node) => + node.parent && + node.parent.parent && + [AST_NODE_TYPES.CallExpression, AST_NODE_TYPES.ArrayExpression].includes( + node.parent.type, + ) + ? getPromiseCallExpressionNode(node.parent) + : null; + +const getParentIfThenified = (node: TSESTree.Node): TSESTree.Node => { + const grandParentNode = node.parent && node.parent.parent; + + if ( + grandParentNode && + grandParentNode.type === AST_NODE_TYPES.CallExpression && + grandParentNode.callee && + isExpectMember(grandParentNode.callee) && + ['then', 'catch'].includes( + getAccessorValue(grandParentNode.callee.property), + ) && + grandParentNode.parent + ) { + // Just in case `then`s are chained look one above. + return getParentIfThenified(grandParentNode); + } + + return node; +}; + +const isAcceptableReturnNode = ( + node: TSESTree.Node, + allowReturn: boolean, +): node is + | TSESTree.ArrowFunctionExpression + | TSESTree.AwaitExpression + | TSESTree.ReturnStatement => + (allowReturn && node.type === AST_NODE_TYPES.ReturnStatement) || + [ + AST_NODE_TYPES.ArrowFunctionExpression, + AST_NODE_TYPES.AwaitExpression, + ].includes(node.type); + +const promiseArrayExceptionKey = ({ start, end }: TSESTree.SourceLocation) => + `${start.line}:${start.column}-${end.line}:${end.column}`; + +type MessageIds = + | 'multipleArgs' + | 'noArgs' + | 'noAssertions' + | 'invalidProperty' + | 'propertyWithoutMatcher' + | 'matcherOnPropertyNotCalled' + | 'asyncMustBeAwaited' + | 'promisesWithAsyncAssertionsMustBeAwaited'; + +export default createRule<[{ alwaysAwait?: boolean }], MessageIds>({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Enforce valid `expect()` usage', + recommended: 'error', + }, + messages: { + multipleArgs: 'More than one argument was passed to expect().', + noArgs: 'No arguments were passed to expect().', + noAssertions: 'No assertion was called on expect().', + invalidProperty: + '"{{ propertyName }}" is not a valid property of expect.', + propertyWithoutMatcher: '"{{ propertyName }}" needs to call a matcher.', + matcherOnPropertyNotCalled: '"{{ propertyName }}" was not called.', + asyncMustBeAwaited: 'Async assertions must be awaited{{ orReturned }}.', + promisesWithAsyncAssertionsMustBeAwaited: + 'Promises which return async assertions must be awaited{{ orReturned }}.', + }, + type: 'suggestion', + schema: [ + { + type: 'object', + properties: { + alwaysAwait: { + type: 'boolean', + default: false, + }, + }, + additionalProperties: false, + }, + ], + }, + defaultOptions: [{ alwaysAwait: false }], + create(context, [{ alwaysAwait }]) { + // Context state + const arrayExceptions = new Set(); + + const pushPromiseArrayException = (loc: TSESTree.SourceLocation) => + arrayExceptions.add(promiseArrayExceptionKey(loc)); + + /** + * Promise method that accepts an array of promises, + * (eg. Promise.all), will throw warnings for the each + * unawaited or non-returned promise. To avoid throwing + * multiple warnings, we check if there is a warning in + * the given location. + */ + const promiseArrayExceptionExists = (loc: TSESTree.SourceLocation) => + arrayExceptions.has(promiseArrayExceptionKey(loc)); + + return { + CallExpression(node) { + if (!isExpectCall(node)) { + return; + } + + const { expect, modifier, matcher } = parseExpectCall(node); + + if (expect.arguments.length > 1) { + const secondArgumentLocStart = expect.arguments[1].loc.start; + const lastArgumentLocEnd = + expect.arguments[node.arguments.length - 1].loc.end; + + context.report({ + loc: { + end: { + column: lastArgumentLocEnd.column - 1, + line: lastArgumentLocEnd.line, + }, + start: secondArgumentLocStart, + }, + messageId: 'multipleArgs', + node, + }); + } else if (expect.arguments.length === 0) { + const expectLength = getAccessorValue(expect.callee).length; + context.report({ + loc: { + end: { + column: node.loc.start.column + expectLength + 1, + line: node.loc.start.line, + }, + start: { + column: node.loc.start.column + expectLength, + line: node.loc.start.line, + }, + }, + messageId: 'noArgs', + node, + }); + } + + // something was called on `expect()` + if (!matcher) { + if (modifier) { + context.report({ + data: { propertyName: modifier.name }, // todo: rename to 'modifierName' + messageId: 'propertyWithoutMatcher', // todo: rename to 'modifierWithoutMatcher' + node: modifier.node.property, + }); + } + + return; + } + + if (matcher.node.parent && isExpectMember(matcher.node.parent)) { + context.report({ + messageId: 'invalidProperty', // todo: rename to 'invalidModifier' + data: { propertyName: matcher.name }, // todo: rename to 'matcherName' (or modifierName?) + node: matcher.node.property, + }); + + return; + } + + if (!matcher.arguments) { + context.report({ + data: { propertyName: matcher.name }, // todo: rename to 'matcherName' + messageId: 'matcherOnPropertyNotCalled', // todo: rename to 'matcherNotCalled' + node: matcher.node.property, + }); + } + + const parentNode = matcher.node.parent; + + if ( + !modifier || + !parentNode || + !parentNode.parent || + modifier.name === ModifierName.not + ) { + return; + } + /** + * If parent node is an array expression, we'll report the warning, + * for the array object, not for each individual assertion. + */ + const isParentArrayExpression = + parentNode.parent.type === AST_NODE_TYPES.ArrayExpression; + const orReturned = alwaysAwait ? '' : ' or returned'; + /** + * An async assertion can be chained with `then` or `catch` statements. + * In that case our target CallExpression node is the one with + * the last `then` or `catch` statement. + */ + const targetNode = getParentIfThenified(parentNode); + const finalNode = + findPromiseCallExpressionNode(targetNode) || targetNode; + if ( + finalNode.parent && + // If node is not awaited or returned + !isAcceptableReturnNode(finalNode.parent, !alwaysAwait) && + // if we didn't warn user already + !promiseArrayExceptionExists(finalNode.loc) + ) { + context.report({ + loc: finalNode.loc, + data: { + orReturned, + }, + messageId: + finalNode === targetNode + ? 'asyncMustBeAwaited' + : 'promisesWithAsyncAssertionsMustBeAwaited', + node, + }); + + if (isParentArrayExpression) { + pushPromiseArrayException(finalNode.loc); + } + } + }, + + // nothing called on "expect()" + 'CallExpression:exit'(node: TSESTree.CallExpression) { + if ( + isExpectCall(node) && + node.parent.type === AST_NODE_TYPES.ExpressionStatement + ) { + context.report({ messageId: 'noAssertions', node }); + } + }, + }; + }, +});