diff --git a/README.md b/README.md index c0afb51..ce67426 100644 --- a/README.md +++ b/README.md @@ -117,49 +117,50 @@ CLI option\ 💡 Manually fixable by [editor suggestions](https://eslint.org/docs/latest/developer-guide/working-with-rules#providing-suggestions) -| Rule | Description | ✅ | 🔧 | 💡 | -| --------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------- | :-: | :-: | :-: | -| [expect-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/expect-expect.md) | Enforce assertion to be made in a test body | ✅ | | | -| [max-expects](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/max-expects.md) | Enforces a maximum number assertion calls in a test body | ✅ | | | -| [max-nested-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/max-nested-describe.md) | Enforces a maximum depth to nested describe calls | ✅ | | | -| [missing-playwright-await](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/missing-playwright-await.md) | Enforce Playwright APIs to be awaited | ✅ | 🔧 | | -| [no-commented-out-tests](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-commented-out-tests.md) | Disallow commented out tests | | | | -| [no-conditional-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-conditional-expect.md) | Disallow calling `expect` conditionally | ✅ | | | -| [no-conditional-in-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-conditional-in-test.md) | Disallow conditional logic in tests | ✅ | | | -| [no-duplicate-hooks](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-duplicate-hooks.md) | Disallow duplicate setup and teardown hooks | | | | -| [no-element-handle](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-element-handle.md) | Disallow usage of element handles | ✅ | | 💡 | -| [no-eval](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-eval.md) | Disallow usage of `page.$eval()` and `page.$$eval()` | ✅ | | | -| [no-focused-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-focused-test.md) | Disallow usage of `.only` annotation | ✅ | | 💡 | -| [no-force-option](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-force-option.md) | Disallow usage of the `{ force: true }` option | ✅ | | | -| [no-hooks](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-hooks.md) | Disallow setup and teardown hooks | | | | -| [no-nested-step](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-nested-step.md) | Disallow nested `test.step()` methods | ✅ | | | -| [no-networkidle](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-networkidle.md) | Disallow usage of the `networkidle` option | ✅ | | | -| [no-nth-methods](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-nth-methods.md) | Disallow usage of `first()`, `last()`, and `nth()` methods | | | | -| [no-page-pause](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-page-pause.md) | Disallow using `page.pause()` | ✅ | | | -| [no-unsafe-references](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-unsafe-references.md) | Prevent unsafe variable references in `page.evaluate()` | ✅ | 🔧 | | -| [no-get-by-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-get-by-title.md) | Disallow using `getByTitle()` | | 🔧 | | -| [no-raw-locators](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-raw-locators.md) | Disallow using raw locators | | | | -| [no-useless-await](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-await.md) | Disallow unnecessary `await`s for Playwright methods | ✅ | 🔧 | | -| [no-restricted-matchers](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers | | | | -| [no-skipped-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-skipped-test.md) | Disallow usage of the `.skip` annotation | ✅ | | 💡 | -| [no-standalone-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-standalone-expect.md) | Disallow using expect outside of `test` blocks | ✅ | | | -| [no-useless-not](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-not.md) | Disallow usage of `not` matchers when a specific matcher exists | ✅ | 🔧 | | -| [no-wait-for-selector](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-selector.md) | Disallow usage of `page.waitForSelector()` | ✅ | | 💡 | -| [no-wait-for-timeout](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-timeout.md) | Disallow usage of `page.waitForTimeout()` | ✅ | | 💡 | -| [prefer-comparison-matcher](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-comparison-matcher.md) | Suggest using the built-in comparison matchers | | 🔧 | | -| [prefer-equality-matcher](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-equality-matcher.md) | Suggest using the built-in equality matchers | | | 💡 | -| [prefer-hooks-in-order](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-in-order.md) | Prefer having hooks in a consistent order | | | | -| [prefer-hooks-on-top](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | | | -| [prefer-strict-equal](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | | 💡 | -| [prefer-lowercase-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md) | Enforce lowercase test names | | 🔧 | | -| [prefer-to-be](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-be.md) | Suggest using `toBe()` | | 🔧 | | -| [prefer-to-contain](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | | 🔧 | | -| [prefer-to-have-count](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-have-count.md) | Suggest using `toHaveCount()` | | 🔧 | | -| [prefer-to-have-length](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-have-length.md) | Suggest using `toHaveLength()` | | 🔧 | | -| [prefer-web-first-assertions](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-web-first-assertions.md) | Suggest using web first assertions | ✅ | 🔧 | | -| [require-hook](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-hook.md) | Require setup and teardown code to be within a hook | | | | -| [require-top-level-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `test.describe` block | | | | -| [require-soft-assertions](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-soft-assertions.md) | Require assertions to use `expect.soft()` | | 🔧 | | -| [valid-describe-callback](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-describe-callback.md) | Enforce valid `describe()` callback | ✅ | | | -| [valid-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect.md) | Enforce valid `expect()` usage | ✅ | | | -| [valid-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-title.md) | Enforce valid titles | ✅ | 🔧 | | +| Rule | Description | ✅ | 🔧 | 💡 | +| --------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------ | :-: | :-: | :-: | +| [expect-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/expect-expect.md) | Enforce assertion to be made in a test body | ✅ | | | +| [max-expects](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/max-expects.md) | Enforces a maximum number assertion calls in a test body | ✅ | | | +| [max-nested-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/max-nested-describe.md) | Enforces a maximum depth to nested describe calls | ✅ | | | +| [missing-playwright-await](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/missing-playwright-await.md) | Enforce Playwright APIs to be awaited | ✅ | 🔧 | | +| [no-commented-out-tests](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-commented-out-tests.md) | Disallow commented out tests | | | | +| [no-conditional-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-conditional-expect.md) | Disallow calling `expect` conditionally | ✅ | | | +| [no-conditional-in-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-conditional-in-test.md) | Disallow conditional logic in tests | ✅ | | | +| [no-duplicate-hooks](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-duplicate-hooks.md) | Disallow duplicate setup and teardown hooks | | | | +| [no-element-handle](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-element-handle.md) | Disallow usage of element handles | ✅ | | 💡 | +| [no-eval](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-eval.md) | Disallow usage of `page.$eval()` and `page.$$eval()` | ✅ | | | +| [no-focused-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-focused-test.md) | Disallow usage of `.only` annotation | ✅ | | 💡 | +| [no-force-option](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-force-option.md) | Disallow usage of the `{ force: true }` option | ✅ | | | +| [no-hooks](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-hooks.md) | Disallow setup and teardown hooks | | | | +| [no-nested-step](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-nested-step.md) | Disallow nested `test.step()` methods | ✅ | | | +| [no-networkidle](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-networkidle.md) | Disallow usage of the `networkidle` option | ✅ | | | +| [no-nth-methods](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-nth-methods.md) | Disallow usage of `first()`, `last()`, and `nth()` methods | | | | +| [no-page-pause](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-page-pause.md) | Disallow using `page.pause()` | ✅ | | | +| [no-unsafe-references](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-unsafe-references.md) | Prevent unsafe variable references in `page.evaluate()` | ✅ | 🔧 | | +| [no-get-by-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-get-by-title.md) | Disallow using `getByTitle()` | | 🔧 | | +| [no-raw-locators](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-raw-locators.md) | Disallow using raw locators | | | | +| [no-useless-await](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-await.md) | Disallow unnecessary `await`s for Playwright methods | ✅ | 🔧 | | +| [no-restricted-matchers](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers | | | | +| [no-skipped-test](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-skipped-test.md) | Disallow usage of the `.skip` annotation | ✅ | | 💡 | +| [no-standalone-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-standalone-expect.md) | Disallow using expect outside of `test` blocks | ✅ | | | +| [no-useless-not](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-not.md) | Disallow usage of `not` matchers when a specific matcher exists | ✅ | 🔧 | | +| [no-wait-for-selector](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-selector.md) | Disallow usage of `page.waitForSelector()` | ✅ | | 💡 | +| [no-wait-for-timeout](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-timeout.md) | Disallow usage of `page.waitForTimeout()` | ✅ | | 💡 | +| [prefer-comparison-matcher](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-comparison-matcher.md) | Suggest using the built-in comparison matchers | | 🔧 | | +| [prefer-equality-matcher](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-equality-matcher.md) | Suggest using the built-in equality matchers | | | 💡 | +| [prefer-hooks-in-order](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-in-order.md) | Prefer having hooks in a consistent order | | | | +| [prefer-hooks-on-top](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | | | +| [prefer-strict-equal](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | | 💡 | +| [prefer-lowercase-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md) | Enforce lowercase test names | | 🔧 | | +| [prefer-to-be](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-be.md) | Suggest using `toBe()` | | 🔧 | | +| [prefer-to-contain](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | | 🔧 | | +| [prefer-to-have-count](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-have-count.md) | Suggest using `toHaveCount()` | | 🔧 | | +| [prefer-to-have-length](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-have-length.md) | Suggest using `toHaveLength()` | | 🔧 | | +| [prefer-web-first-assertions](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-web-first-assertions.md) | Suggest using web first assertions | ✅ | 🔧 | | +| [require-hook](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-hook.md) | Require setup and teardown code to be within a hook | | | | +| [require-top-level-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `test.describe` block | | | | +| [require-soft-assertions](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-soft-assertions.md) | Require assertions to use `expect.soft()` | | 🔧 | | +| [valid-describe-callback](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-describe-callback.md) | Enforce valid `describe()` callback | ✅ | | | +| [valid-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect.md) | Enforce valid `expect()` usage | ✅ | | | +| [valid-expect-in-promise](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect-in-promise.md) | Require promises that have expectations in their chain to be valid | ✅ | | | +| [valid-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-title.md) | Enforce valid titles | ✅ | 🔧 | | diff --git a/docs/rules/valid-expect-in-promise.md b/docs/rules/valid-expect-in-promise.md new file mode 100644 index 0000000..b407929 --- /dev/null +++ b/docs/rules/valid-expect-in-promise.md @@ -0,0 +1,72 @@ +# Require promises that have expectations in their chain to be valid (`valid-expect-in-promise`) + +Ensure promises that include expectations are returned or awaited. + +## Rule details + +This rule flags any promises within the body of a test that include expectations +that have either not been returned or awaited. + +The following patterns are considered warnings: + +```js +test('promises a person', () => { + api.getPersonByName('bob').then((person) => { + expect(person).toHaveProperty('name', 'Bob'); + }); +}); + +test('promises a counted person', () => { + const promise = api.getPersonByName('bob').then((person) => { + expect(person).toHaveProperty('name', 'Bob'); + }); + + promise.then(() => { + expect(analytics.gottenPeopleCount).toBe(1); + }); +}); + +test('promises multiple people', () => { + const firstPromise = api.getPersonByName('bob').then((person) => { + expect(person).toHaveProperty('name', 'Bob'); + }); + const secondPromise = api.getPersonByName('alice').then((person) => { + expect(person).toHaveProperty('name', 'Alice'); + }); + + return Promise.any([firstPromise, secondPromise]); +}); +``` + +The following pattern is not a warning: + +```js +test('promises a person', async () => { + await api.getPersonByName('bob').then((person) => { + expect(person).toHaveProperty('name', 'Bob'); + }); +}); + +test('promises a counted person', () => { + let promise = api.getPersonByName('bob').then((person) => { + expect(person).toHaveProperty('name', 'Bob'); + }); + + promise = promise.then(() => { + expect(analytics.gottenPeopleCount).toBe(1); + }); + + return promise; +}); + +test('promises multiple people', () => { + const firstPromise = api.getPersonByName('bob').then((person) => { + expect(person).toHaveProperty('name', 'Bob'); + }); + const secondPromise = api.getPersonByName('alice').then((person) => { + expect(person).toHaveProperty('name', 'Alice'); + }); + + return Promise.allSettled([firstPromise, secondPromise]); +}); +``` diff --git a/src/index.ts b/src/index.ts index f701ec5..a8e729f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -42,6 +42,7 @@ import requireSoftAssertions from './rules/require-soft-assertions'; import requireTopLevelDescribe from './rules/require-top-level-describe'; import validDescribeCallback from './rules/valid-describe-callback'; import validExpect from './rules/valid-expect'; +import validExpectInPromise from './rules/valid-expect-in-promise'; import validTitle from './rules/valid-title'; const index = { @@ -90,6 +91,7 @@ const index = { 'require-top-level-describe': requireTopLevelDescribe, 'valid-describe-callback': validDescribeCallback, 'valid-expect': validExpect, + 'valid-expect-in-promise': validExpectInPromise, 'valid-title': validTitle, }, }; @@ -119,6 +121,7 @@ const sharedConfig = { 'playwright/prefer-web-first-assertions': 'error', 'playwright/valid-describe-callback': 'error', 'playwright/valid-expect': 'error', + 'playwright/valid-expect-in-promise': 'error', 'playwright/valid-title': 'error', }, }; diff --git a/src/rules/valid-expect-in-promise.test.ts b/src/rules/valid-expect-in-promise.test.ts new file mode 100644 index 0000000..516d9a8 --- /dev/null +++ b/src/rules/valid-expect-in-promise.test.ts @@ -0,0 +1,1626 @@ +import dedent from 'dedent'; +import { runRuleTester } from '../utils/rule-tester'; +import rule from './valid-expect-in-promise'; + +runRuleTester('valid-expect-in-promise', rule, { + invalid: [ + { + code: dedent` + const myFn = () => { + Promise.resolve().then(() => { + expect(true).toBe(false); + }); + }; + + test('foo', () => { + somePromise.then(() => { + expect(someThing).toEqual(true); + }); + }); + `, + errors: [ + { + column: 3, + endColumn: 6, + line: 8, + messageId: 'expectInFloatingPromise', + }, + ], + }, + { + code: dedent` + test('foo', () => { + somePromise.then(() => { + expect(someThing).toEqual(true); + }); + }); + `, + errors: [ + { column: 3, endColumn: 6, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('foo', () => { + somePromise.finally(() => { + expect(someThing).toEqual(true); + }); + }); + `, + errors: [ + { column: 3, endColumn: 6, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: ` + test('foo', () => { + somePromise['then'](() => { + expect(someThing).toEqual(true); + }); + }); + `, + errors: [ + { column: 10, endColumn: 13, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('foo', function() { + getSomeThing().getPromise().then(function() { + expect(someThing).toEqual(true); + }); + }); + `, + errors: [ + { column: 3, endColumn: 6, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('foo', function() { + Promise.resolve().then(function() { + expect(someThing).toEqual(true); + }); + }); + `, + errors: [ + { column: 3, endColumn: 6, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('foo', function() { + somePromise.catch(function() { + expect(someThing).toEqual(true) + }) + }) + `, + errors: [ + { column: 3, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('foo', function() { + somePromise.then(function() { + expect(someThing).toEqual(true) + }) + }) + `, + errors: [ + { column: 3, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('foo', function () { + Promise.resolve().then(/*fulfillment*/ function () { + expect(someThing).toEqual(true); + }, /*rejection*/ function () { + expect(someThing).toEqual(true); + }) + }) + `, + errors: [ + { column: 3, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('foo', function () { + Promise.resolve().then(/*fulfillment*/ function () { + }, /*rejection*/ function () { + expect(someThing).toEqual(true) + }) + }); + `, + errors: [ + { column: 3, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('test function', () => { + Builder.getPromiseBuilder() + .get() + .build() + .then(data => expect(data).toEqual('Hi')); + }); + `, + errors: [ + { column: 3, endColumn: 47, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: ` + test('test function', async () => { + Builder.getPromiseBuilder() + .get() + .build() + .then(data => expect(data).toEqual('Hi')); + }); + `, + errors: [ + { column: 11, endColumn: 55, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('foo', () => { + somePromise.then(() => { + doSomeOperation(); + expect(someThing).toEqual(true); + }) + }); + `, + errors: [ + { column: 3, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('is a test', () => { + somePromise + .then(() => {}) + .then(() => expect(someThing).toEqual(value)) + }); + `, + errors: [ + { column: 3, endColumn: 50, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('is a test', () => { + somePromise + .then(() => expect(someThing).toEqual(value)) + .then(() => {}) + }); + `, + errors: [ + { column: 3, endColumn: 20, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('is a test', () => { + somePromise.then(() => { + return value; + }) + .then(value => { + expect(someThing).toEqual(value); + }) + }); + `, + errors: [ + { column: 3, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('is a test', () => { + somePromise.then(() => { + expect(someThing).toEqual(true); + }) + .then(() => { + console.log('this is silly'); + }) + }); + `, + errors: [ + { column: 3, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('is a test', () => { + somePromise.then(() => { + // return value; + }) + .then(value => { + expect(someThing).toEqual(value); + }) + }); + `, + errors: [ + { column: 3, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('is a test', () => { + somePromise.then(() => { + return value; + }) + .then(value => { + expect(someThing).toEqual(value); + }) + + return anotherPromise.then(() => expect(x).toBe(y)); + }); + `, + errors: [ + { column: 3, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('is a test', () => { + somePromise + .then(() => 1) + .then(x => x + 1) + .catch(() => -1) + .then(v => expect(v).toBe(2)); + + return anotherPromise.then(() => expect(x).toBe(y)); + }); + `, + errors: [ + { column: 3, endColumn: 35, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('is a test', () => { + somePromise + .then(() => 1) + .then(v => expect(v).toBe(2)) + .then(x => x + 1) + .catch(() => -1); + + return anotherPromise.then(() => expect(x).toBe(y)); + }); + `, + errors: [ + { column: 3, endColumn: 22, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('foo', () => { + somePromise.finally(() => { + doSomeOperation(); + expect(someThing).toEqual(true); + }) + }); + `, + errors: [ + { column: 3, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('invalid return', () => { + const promise = something().then(value => { + const foo = "foo"; + return expect(value).toBe('red'); + }); + }); + `, + errors: [ + { column: 9, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test.skip('foo', () => { + somePromise.then(() => { + doSomeOperation(); + expect(someThing).toEqual(true); + }) + }); + `, + errors: [ + { column: 3, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + promise; + }); + `, + errors: [ + { column: 9, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + return; + + await promise; + }); + `, + errors: [ + { column: 9, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + return 1; + + await promise; + }); + `, + errors: [ + { column: 9, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + return []; + + await promise; + }); + `, + errors: [ + { column: 9, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + return Promise.all([anotherPromise]); + + await promise; + }); + `, + errors: [ + { column: 9, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + return {}; + + await promise; + }); + `, + errors: [ + { column: 9, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + return Promise.all([]); + + await promise; + }); + `, + errors: [ + { column: 9, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + await 1; + }); + `, + errors: [ + { column: 9, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + await []; + }); + `, + errors: [ + { column: 9, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + await Promise.all([anotherPromise]); + }); + `, + errors: [ + { column: 9, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + await {}; + }); + `, + errors: [ + { column: 9, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + await Promise.all([]); + }); + `, + errors: [ + { column: 9, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }), x = 1; + }); + `, + errors: [ + { column: 9, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('later return', async () => { + const x = 1, promise = something().then(value => { + expect(value).toBe('red'); + }); + }); + `, + errors: [ + { column: 16, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + import { test } from '@playwright/test'; + + test('later return', async () => { + const x = 1, promise = something().then(value => { + expect(value).toBe('red'); + }); + }); + `, + errors: [ + { column: 16, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('promise test', () => { + const somePromise = getThatPromise(); + somePromise.then((data) => { + expect(data).toEqual('foo'); + }); + expect(somePromise).toBeDefined(); + return somePromise; + }); + `, + errors: [ + { column: 3, endColumn: 6, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('promise test', function () { + let somePromise = getThatPromise(); + somePromise.then((data) => { + expect(data).toEqual('foo'); + }); + expect(somePromise).toBeDefined(); + return somePromise; + }); + `, + errors: [ + { column: 3, endColumn: 6, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('promise test', async function () { + let somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + somePromise = null; + + await somePromise; + }); + `, + errors: [ + { column: 7, endColumn: 5, messageId: 'expectInFloatingPromise' }, + ], + }, + { + code: dedent` + test('promise test', async function () { + let somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + await somePromise; + }); + `, + errors: [ + { + column: 7, + endColumn: 5, + line: 2, + messageId: 'expectInFloatingPromise', + }, + ], + }, + { + code: dedent` + test('promise test', async function () { + let somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + ({ somePromise } = {}) + }); + `, + errors: [ + { + column: 7, + endColumn: 5, + line: 2, + messageId: 'expectInFloatingPromise', + }, + ], + }, + { + code: dedent` + test('promise test', async function () { + let somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + { + somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + await somePromise; + } + }); + `, + errors: [ + { + column: 7, + endColumn: 5, + line: 2, + messageId: 'expectInFloatingPromise', + }, + ], + }, + { + code: dedent` + test('that we error on this destructuring', async () => { + [promise] = something().then(value => { + expect(value).toBe('red'); + }); + }); + `, + errors: [ + { + column: 3, + endColumn: 5, + line: 2, + messageId: 'expectInFloatingPromise', + }, + ], + }, + { + code: dedent` + test('that we error on this', () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + log(promise); + }); + `, + errors: [ + { + column: 9, + line: 2, + messageId: 'expectInFloatingPromise', + }, + ], + }, + { + code: dedent` + test('is valid', async () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + expect(promise).toBeInstanceOf(Promise); + }); + `, + errors: [ + { + column: 9, + line: 2, + messageId: 'expectInFloatingPromise', + }, + ], + }, + { + code: dedent` + test('is valid', async () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + expect(anotherPromise).resolves.toBe(1); + }); + `, + errors: [ + { + column: 9, + line: 2, + messageId: 'expectInFloatingPromise', + }, + ], + }, + // TODO: + // { + // code: dedent` + // import { test as promiseThatThis } from '@playwright/test'; + // + // promiseThatThis('is valid', async () => { + // const promise = loadNumber().then(number => { + // expect(typeof number).toBe('number'); + // + // return number + 1; + // }); + // + // expect(anotherPromise).resolves.toBe(1); + // }); + // `, + // errors: [ + // { + // column: 9, + // line: 4, + // messageId: 'expectInFloatingPromise', + // }, + // ], + // }, + // Global aliases + { + code: dedent` + test('is valid', async () => { + const promise = loadNumber().then(number => { + assert(typeof number).toBe('number'); + + return number + 1; + }); + + assert(anotherPromise).resolves.toBe(1); + }); + `, + errors: [ + { + column: 9, + line: 2, + messageId: 'expectInFloatingPromise', + }, + ], + settings: { + playwright: { + globalAliases: { expect: ['assert'] }, + }, + }, + }, + { + code: dedent` + it('is valid', async () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + expect(anotherPromise).resolves.toBe(1); + }); + `, + errors: [ + { + column: 9, + line: 2, + messageId: 'expectInFloatingPromise', + }, + ], + settings: { + playwright: { + globalAliases: { test: ['it'] }, + }, + }, + }, + ], + valid: [ + "test('something', () => Promise.resolve().then(() => expect(1).toBe(2)));", + 'Promise.resolve().then(() => expect(1).toBe(2))', + 'const x = Promise.resolve().then(() => expect(1).toBe(2))', + dedent` + test('is valid', () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + expect(promise).resolves.toBe(1); + }); + `, + dedent` + test('is valid', () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + expect(promise).resolves.not.toBe(2); + }); + `, + dedent` + test('is valid', () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + expect(promise).rejects.toBe(1); + }); + `, + dedent` + test('is valid', () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + expect(promise).rejects.not.toBe(2); + }); + `, + dedent` + test('is valid', async () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + expect(await promise).toBeGreaterThan(1); + }); + `, + dedent` + test('is valid', async () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + expect(await promise).resolves.toBeGreaterThan(1); + }); + `, + dedent` + test('is valid', async () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + expect(1).toBeGreaterThan(await promise); + }); + `, + dedent` + test('is valid', async () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + expect.this.that.is(await promise); + }); + `, + dedent` + test('is valid', async () => { + expect(await loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + })).toBeGreaterThan(1); + }); + `, + dedent` + test('is valid', async () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + expect([await promise]).toHaveLength(1); + }); + `, + dedent` + test('is valid', async () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + expect([,,await promise,,]).toHaveLength(1); + }); + `, + dedent` + test('is valid', async () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + expect([[await promise]]).toHaveLength(1); + }); + `, + dedent` + test('is valid', async () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return number + 1; + }); + + logValue(await promise); + }); + `, + dedent` + test('is valid', async () => { + const promise = loadNumber().then(number => { + expect(typeof number).toBe('number'); + + return 1; + }); + + expect.assertions(await promise); + }); + `, + dedent` + test('is valid', async () => { + await loadNumber().then(number => { + expect(typeof number).toBe('number'); + }); + }); + `, + dedent` + test('foo', () => new Promise((done) => { + test() + .then(() => { + expect(someThing).toEqual(true); + done(); + }); + })); + `, + dedent` + test('foo', () => { + return new Promise(done => { + test().then(() => { + expect(someThing).toEqual(true); + done(); + }); + }); + }); + `, + dedent` + test('passes', () => { + Promise.resolve().then(() => { + grabber.grabSomething(); + }); + }); + `, + dedent` + test('passes', async () => { + const grabbing = Promise.resolve().then(() => { + grabber.grabSomething(); + }); + + await grabbing; + + expect(grabber.grabbedItems).toHaveLength(1); + }); + `, + dedent` + const myFn = () => { + Promise.resolve().then(() => { + expect(true).toBe(false); + }); + }; + `, + dedent` + const myFn = () => { + Promise.resolve().then(() => { + subject.invokeMethod(); + }); + }; + `, + dedent` + const myFn = () => { + Promise.resolve().then(() => { + expect(true).toBe(false); + }); + }; + + test('foo', () => { + return somePromise.then(() => { + expect(someThing).toEqual(true); + }); + }); + `, + dedent` + test('foo', () => new Promise((done) => { + test() + .finally(() => { + expect(someThing).toEqual(true); + done(); + }); + })); + `, + dedent` + test('foo', () => { + return somePromise.then(() => { + expect(someThing).toEqual(true); + }); + }); + `, + dedent` + test('foo', () => { + return somePromise.finally(() => { + expect(someThing).toEqual(true); + }); + }); + `, + dedent` + test('foo', function() { + return somePromise.catch(function() { + expect(someThing).toEqual(true); + }); + }); + `, + dedent` + test('foo', function() { + return somePromise.then(function() { + doSomeThingButNotExpect(); + }); + }); + `, + dedent` + test('foo', function() { + return getSomeThing().getPromise().then(function() { + expect(someThing).toEqual(true); + }); + }); + `, + dedent` + test('foo', function() { + return Promise.resolve().then(function() { + expect(someThing).toEqual(true); + }); + }); + `, + dedent` + test('foo', function () { + return Promise.resolve().then(function () { + /*fulfillment*/ + expect(someThing).toEqual(true); + }, function () { + /*rejection*/ + expect(someThing).toEqual(true); + }); + }); + `, + dedent` + test('foo', function () { + Promise.resolve().then(/*fulfillment*/ function () { + }, undefined, /*rejection*/ function () { + expect(someThing).toEqual(true) + }) + }); + `, + dedent` + test('foo', function () { + return Promise.resolve().then(function () { + /*fulfillment*/ + }, function () { + /*rejection*/ + expect(someThing).toEqual(true); + }); + }); + `, + dedent` + test('foo', function () { + return somePromise.then() + }); + `, + dedent` + test('foo', async () => { + await Promise.resolve().then(function () { + expect(someThing).toEqual(true) + }); + }); + `, + dedent` + test('foo', async () => { + await somePromise.then(() => { + expect(someThing).toEqual(true) + }); + }); + `, + dedent` + test('foo', async () => { + await getSomeThing().getPromise().then(function () { + expect(someThing).toEqual(true) + }); + }); + `, + dedent` + test('foo', () => { + return somePromise.then(() => { + expect(someThing).toEqual(true); + }) + .then(() => { + expect(someThing).toEqual(true); + }) + }); + `, + dedent` + test('foo', () => { + return somePromise.then(() => { + return value; + }) + .then(value => { + expect(someThing).toEqual(value); + }) + }); + `, + dedent` + test('foo', () => { + return somePromise.then(() => { + expect(someThing).toEqual(true); + }) + .then(() => { + console.log('this is silly'); + }) + }); + `, + dedent` + test('foo', () => { + return somePromise.then(() => { + expect(someThing).toEqual(true); + }) + .catch(() => { + expect(someThing).toEqual(false); + }) + }); + `, + dedent` + test('later return', () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + return promise; + }); + `, + dedent` + test('later return', async () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + await promise; + }); + `, + dedent` + test.only('later return', () => { + const promise = something().then(value => { + expect(value).toBe('red'); + }); + + return promise; + }); + `, + dedent` + test('that we bailout if destructuring is used', () => { + const [promise] = something().then(value => { + expect(value).toBe('red'); + }); + }); + `, + dedent` + test('that we bailout if destructuring is used', async () => { + const [promise] = await something().then(value => { + expect(value).toBe('red'); + }); + }); + `, + dedent` + test('that we bailout if destructuring is used', () => { + const [promise] = [ + something().then(value => { + expect(value).toBe('red'); + }) + ]; + }); + `, + dedent` + test('that we bailout if destructuring is used', () => { + const {promise} = { + promise: something().then(value => { + expect(value).toBe('red'); + }) + }; + }); + `, + dedent` + test('that we bailout in complex cases', () => { + promiseSomething({ + timeout: 500, + promise: something().then(value => { + expect(value).toBe('red'); + }) + }); + }); + `, + dedent` + test('shorthand arrow', () => + something().then(value => { + expect(() => { + value(); + }).toThrow(); + }) + ); + `, + dedent` + test('crawls for files based on patterns', () => { + const promise = nodeCrawl({}).then(data => { + expect(childProcess.spawn).lastCalledWith('find'); + }); + return promise; + }); + `, + dedent` + test('is a test', async () => { + const value = await somePromise().then(response => { + expect(response).toHaveProperty('data'); + + return response.data; + }); + + expect(value).toBe('hello world'); + }); + `, + dedent` + test('is a test', async () => { + return await somePromise().then(response => { + expect(response).toHaveProperty('data'); + + return response.data; + }); + }); + `, + dedent` + test('is a test', async () => { + return somePromise().then(response => { + expect(response).toHaveProperty('data'); + + return response.data; + }); + }); + `, + dedent` + test('is a test', async () => { + await somePromise().then(response => { + expect(response).toHaveProperty('data'); + + return response.data; + }); + }); + `, + dedent` + test( + 'test function', + () => { + return Builder + .getPromiseBuilder() + .get().build() + .then((data) => { + expect(data).toEqual('Hi'); + }); + } + ); + `, + dedent` + notATestFunction( + 'not a test function', + () => { + Builder + .getPromiseBuilder() + .get() + .build() + .then((data) => { + expect(data).toEqual('Hi'); + }); + } + ); + `, + dedent` + test('is valid', async () => { + const promiseOne = loadNumber().then(number => { + expect(typeof number).toBe('number'); + }); + const promiseTwo = loadNumber().then(number => { + expect(typeof number).toBe('number'); + }); + + await promiseTwo; + await promiseOne; + }); + `, + dedent` + test("foo", () => somePromise.then(() => { + expect(someThing).toEqual(true) + })) + `, + 'test("foo", () => somePromise.then(() => expect(someThing).toEqual(true)))', + dedent` + test('promise test with done', (done) => { + const promise = getPromise(); + promise.then(() => expect(someThing).toEqual(true)); + }); + `, + dedent` + test('name of done param does not matter', (nameDoesNotMatter) => { + const promise = getPromise(); + promise.then(() => expect(someThing).toEqual(true)); + }); + `, + dedent` + test('valid-expect-in-promise', async () => { + const text = await fetch('url') + .then(res => res.text()) + .then(text => text); + + expect(text).toBe('text'); + }); + `, + dedent` + test('promise test', async function () { + let somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }), x = 1; + + await somePromise; + }); + `, + dedent` + test('promise test', async function () { + let x = 1, somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + await somePromise; + }); + `, + dedent` + test('promise test', async function () { + let somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + await somePromise; + + somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + await somePromise; + }); + `, + dedent` + test('promise test', async function () { + let somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + await somePromise; + + somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + return somePromise; + }); + `, + dedent` + test('promise test', async function () { + let somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + {} + + await somePromise; + }); + `, + dedent` + test('promise test', async function () { + const somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + { + await somePromise; + } + }); + `, + dedent` + test('promise test', async function () { + let somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + { + await somePromise; + + somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + await somePromise; + } + }); + `, + dedent` + test('promise test', async function () { + let somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + await somePromise; + + { + somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + await somePromise; + } + }); + `, + dedent` + test('promise test', async function () { + let somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + somePromise = somePromise.then((data) => { + expect(data).toEqual('foo'); + }); + + await somePromise; + }); + `, + dedent` + test('promise test', async function () { + let somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + somePromise = somePromise + .then((data) => data) + .then((data) => data) + .then((data) => { + expect(data).toEqual('foo'); + }); + + await somePromise; + }); + `, + dedent` + test('promise test', async function () { + let somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + somePromise = somePromise + .then((data) => data) + .then((data) => data) + + await somePromise; + }); + `, + dedent` + test('promise test', async function () { + let somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + await somePromise; + + { + somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + { + await somePromise; + } + } + }); + `, + dedent` + test('promise test', async function () { + const somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + await Promise.all([somePromise]); + }); + `, + dedent` + test('promise test', async function () { + const somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + return Promise.all([somePromise]); + }); + `, + dedent` + test('promise test', async function () { + const somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + return Promise.resolve(somePromise); + }); + `, + dedent` + test('promise test', async function () { + const somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + return Promise.reject(somePromise); + }); + `, + dedent` + test('promise test', async function () { + const somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + await Promise.resolve(somePromise); + }); + `, + dedent` + test('promise test', async function () { + const somePromise = getPromise().then((data) => { + expect(data).toEqual('foo'); + }); + + await Promise.reject(somePromise); + }); + `, + dedent` + test('later return', async () => { + const onePromise = something().then(value => { + console.log(value); + }); + const twoPromise = something().then(value => { + expect(value).toBe('red'); + }); + + return Promise.all([onePromise, twoPromise]); + }); + `, + dedent` + test('later return', async () => { + const onePromise = something().then(value => { + console.log(value); + }); + const twoPromise = something().then(value => { + expect(value).toBe('red'); + }); + + return Promise.allSettled([onePromise, twoPromise]); + }); + `, + ], +}); diff --git a/src/rules/valid-expect-in-promise.ts b/src/rules/valid-expect-in-promise.ts new file mode 100644 index 0000000..487ebda --- /dev/null +++ b/src/rules/valid-expect-in-promise.ts @@ -0,0 +1,456 @@ +import { Rule } from 'eslint'; +import * as ESTree from 'estree'; +import { + getNodeName, + getParent, + getStringValue, + isFunction, + isIdentifier, +} from '../utils/ast'; +import { + findTopMostCallExpression, + isSupportedAccessor, + isTypeOfFnCall, + parseFnCall, +} from '../utils/parseFnCall'; +import { KnownCallExpression } from '../utils/types'; + +const isPromiseChainCall = (node: ESTree.Node): node is KnownCallExpression => { + if ( + node.type === 'CallExpression' && + node.callee.type === 'MemberExpression' && + isSupportedAccessor(node.callee.property) + ) { + // promise methods should have at least 1 argument + if (node.arguments.length === 0) { + return false; + } + + switch (getStringValue(node.callee.property)) { + case 'then': + return node.arguments.length < 3; + case 'catch': + case 'finally': + return node.arguments.length < 2; + } + } + + return false; +}; + +const isTestCaseCallWithCallbackArg = ( + context: Rule.RuleContext, + node: ESTree.CallExpression, +): boolean => { + const jestCallFn = parseFnCall(context, node); + + if (jestCallFn?.type !== 'test') { + return false; + } + + const isJestEach = jestCallFn.members.some( + (s) => getStringValue(s) === 'each', + ); + + if (isJestEach && node.callee.type !== 'TaggedTemplateExpression') { + // isJestEach but not a TaggedTemplateExpression, so this must be + // the `jest.each([])()` syntax which this rule doesn't support due + // to its complexity (see jest-community/eslint-plugin-jest#710) + // so we return true to trigger bailout + return true; + } + + const [, callback] = node.arguments; + + const callbackArgIndex = Number(isJestEach); + + return ( + callback && + isFunction(callback) && + callback.params.length === 1 + callbackArgIndex + ); +}; + +const isPromiseMethodThatUsesValue = ( + node: ESTree.AwaitExpression | ESTree.ReturnStatement, + identifier: ESTree.Identifier | ESTree.Pattern, +): boolean => { + const name = getStringValue(identifier); + if (node.argument == null) return false; + + if ( + node.argument.type === 'CallExpression' && + node.argument.arguments.length > 0 + ) { + const nodeName = getNodeName(node.argument); + + if (['Promise.all', 'Promise.allSettled'].includes(nodeName as string)) { + const [firstArg] = node.argument.arguments; + + if ( + firstArg.type === 'ArrayExpression' && + firstArg.elements.some((nod) => nod && isIdentifier(nod, name)) + ) { + return true; + } + } + + if ( + ['Promise.resolve', 'Promise.reject'].includes(nodeName as string) && + node.argument.arguments.length === 1 + ) { + return isIdentifier(node.argument.arguments[0], name); + } + } + + return isIdentifier(node.argument, name); +}; + +/** + * Attempts to determine if the runtime value represented by the given + * `identifier` is `await`ed within the given array of elements + */ +const isValueAwaitedInElements = ( + name: string, + elements: + | ESTree.ArrayExpression['elements'] + | ESTree.CallExpression['arguments'], +): boolean => { + for (const element of elements) { + if ( + element?.type === 'AwaitExpression' && + isIdentifier(element.argument, name) + ) { + return true; + } + + if ( + element?.type === 'ArrayExpression' && + isValueAwaitedInElements(name, element.elements) + ) { + return true; + } + } + + return false; +}; + +/** + * Attempts to determine if the runtime value represented by the given + * `identifier` is `await`ed as an argument along the given call expression + */ +const isValueAwaitedInArguments = ( + name: string, + call: ESTree.CallExpression, +): boolean => { + let node: ESTree.Node = call; + + while (node) { + if (node.type === 'CallExpression') { + if (isValueAwaitedInElements(name, node.arguments)) { + return true; + } + + node = node.callee; + } + + if (node.type !== 'MemberExpression') { + break; + } + + node = node.object; + } + + return false; +}; + +const getLeftMostCallExpression = ( + call: ESTree.CallExpression, +): ESTree.CallExpression => { + let leftMostCallExpression: ESTree.CallExpression = call; + let node: ESTree.Node = call; + + while (node) { + if (node.type === 'CallExpression') { + leftMostCallExpression = node; + node = node.callee; + } + + if (node.type !== 'MemberExpression') { + break; + } + + node = node.object; + } + + return leftMostCallExpression; +}; + +/** + * Attempts to determine if the runtime value represented by the given + * `identifier` is `await`ed or `return`ed within the given `body` of + * statements + */ +const isValueAwaitedOrReturned = ( + context: Rule.RuleContext, + identifier: ESTree.Identifier | ESTree.Pattern, + body: ESTree.Statement[], +): boolean => { + const name = getStringValue(identifier); + + for (const node of body) { + // skip all nodes that are before this identifier, because they'd probably + // be affecting a different runtime value (e.g. due to reassignment) + if (node.range![0] <= identifier.range![0]) { + continue; + } + + if (node.type === 'ReturnStatement') { + return isPromiseMethodThatUsesValue(node, identifier); + } + + if (node.type === 'ExpressionStatement') { + // it's possible that we're awaiting the value as an argument + if (node.expression.type === 'CallExpression') { + if (isValueAwaitedInArguments(name, node.expression)) { + return true; + } + + const leftMostCall = getLeftMostCallExpression(node.expression); + const call = parseFnCall(context, node.expression); + + if ( + call?.type === 'expect' && + leftMostCall.arguments.length > 0 && + isIdentifier(leftMostCall.arguments[0], name) + ) { + if ( + call.members.some((m) => { + const v = getStringValue(m); + + return v === 'resolves' || v === 'rejects'; + }) + ) { + return true; + } + } + } + + if ( + node.expression.type === 'AwaitExpression' && + isPromiseMethodThatUsesValue(node.expression, identifier) + ) { + return true; + } + + // (re)assignment changes the runtime value, so if we've not found an + // await or return already we act as if we've reached the end of the body + if (node.expression.type === 'AssignmentExpression') { + // unless we're assigning to the same identifier, in which case + // we might be chaining off the existing promise value + if ( + isIdentifier(node.expression.left, name) && + getNodeName(node.expression.right)?.startsWith(`${name}.`) && + isPromiseChainCall(node.expression.right) + ) { + continue; + } + + break; + } + } + + if ( + node.type === 'BlockStatement' && + isValueAwaitedOrReturned(context, identifier, node.body) + ) { + return true; + } + } + + return false; +}; + +const findFirstBlockBodyUp = ( + node: ESTree.Node, +): ESTree.BlockStatement['body'] => { + let parent: ESTree.Node = node; + + while (parent) { + if (parent.type === 'BlockStatement') { + return parent.body; + } + + parent = getParent(parent); + } + + throw new Error( + `Could not find BlockStatement - please file a github issue at https://github.com/playwright-community/eslint-plugin-playwright`, + ); +}; + +const isDirectlyWithinTestCaseCall = ( + context: Rule.RuleContext, + node: ESTree.Node, +): boolean => { + let parent: ESTree.Node = node; + + while (parent) { + if (isFunction(parent)) { + parent = parent.parent; + + return ( + parent?.type === 'CallExpression' && + isTypeOfFnCall(context, parent, ['test']) + ); + } + + parent = getParent(parent); + } + + return false; +}; + +const isVariableAwaitedOrReturned = ( + context: Rule.RuleContext, + variable: ESTree.VariableDeclarator, +): boolean => { + const body = findFirstBlockBodyUp(variable); + + // it's pretty much impossible for us to track destructuring assignments, + // so we return true to bailout gracefully + if (!isIdentifier(variable.id)) { + return true; + } + + return isValueAwaitedOrReturned(context, variable.id, body); +}; + +export default { + create(context) { + let inTestCaseWithDoneCallback = false; + // an array of booleans representing each promise chain we enter, with the + // boolean value representing if we think a given chain contains an expect + // in it's body. + // + // since we only care about the inner-most chain, we represent the state in + // reverse with the inner-most being the first item, as that makes it + // slightly less code to assign to by not needing to know the length + const chains: boolean[] = []; + + return { + CallExpression(node: ESTree.CallExpression) { + // there are too many ways that the done argument could be used with + // promises that contain expect that would make the promise safe for us + if (isTestCaseCallWithCallbackArg(context, node)) { + inTestCaseWithDoneCallback = true; + + return; + } + + // if this call expression is a promise chain, add it to the stack with + // value of "false", as we assume there are no expect calls initially + if (isPromiseChainCall(node)) { + chains.unshift(false); + + return; + } + + // if we're within a promise chain, and this call expression looks like + // an expect call, mark the deepest chain as having an expect call + if (chains.length > 0 && isTypeOfFnCall(context, node, ['expect'])) { + chains[0] = true; + } + }, + 'CallExpression:exit'(node: ESTree.CallExpression) { + // there are too many ways that the "done" argument could be used to + // make promises containing expects safe in a test for us to be able to + // accurately check, so we just bail out completely if it's present + if (inTestCaseWithDoneCallback) { + if (isTypeOfFnCall(context, node, ['test'])) { + inTestCaseWithDoneCallback = false; + } + + return; + } + + if (!isPromiseChainCall(node)) { + return; + } + + // since we're exiting this call expression (which is a promise chain) + // we remove it from the stack of chains, since we're unwinding + const hasExpectCall = chains.shift(); + + // if the promise chain we're exiting doesn't contain an expect, + // then we don't need to check it for anything + if (!hasExpectCall) { + return; + } + + const { parent } = findTopMostCallExpression(node); + + // if we don't have a parent (which is technically impossible at runtime) + // or our parent is not directly within the test case, we stop checking + // because we're most likely in the body of a function being defined + // within the test, which we can't track + if (!parent || !isDirectlyWithinTestCaseCall(context, parent)) { + return; + } + + switch (parent.type) { + case 'VariableDeclarator': { + if (isVariableAwaitedOrReturned(context, parent)) { + return; + } + + break; + } + + case 'AssignmentExpression': { + if ( + parent.left.type === 'Identifier' && + isValueAwaitedOrReturned( + context, + parent.left, + findFirstBlockBodyUp(parent), + ) + ) { + return; + } + + break; + } + + case 'ExpressionStatement': + break; + + case 'ReturnStatement': + case 'AwaitExpression': + default: + return; + } + + context.report({ + messageId: 'expectInFloatingPromise', + node: parent, + }); + }, + }; + }, + meta: { + docs: { + category: 'Best Practices', + description: + 'Require promises that have expectations in their chain to be valid', + recommended: true, + url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect-in-promise.md', + }, + messages: { + expectInFloatingPromise: + 'This promise should either be returned or awaited to ensure the expects in its chain are called', + }, + schema: [], + type: 'suggestion', + }, +} as Rule.RuleModule; diff --git a/src/utils/ast.ts b/src/utils/ast.ts index f4a3f3a..e4ce05a 100644 --- a/src/utils/ast.ts +++ b/src/utils/ast.ts @@ -1,5 +1,6 @@ import { Rule } from 'eslint'; import ESTree from 'estree'; +import { isSupportedAccessor } from './parseFnCall'; import { NodeWithParent, TypedNodeWithParent } from './types'; export function getStringValue(node: ESTree.Node | undefined) { @@ -18,10 +19,7 @@ export function getRawValue(node: ESTree.Node) { return node.type === 'Literal' ? node.raw : undefined; } -export function isIdentifier( - node: ESTree.Node, - name: string | RegExp | undefined, -) { +export function isIdentifier(node: ESTree.Node, name?: string | RegExp) { return ( node.type === 'Identifier' && (!name || @@ -74,7 +72,7 @@ export function isPropertyAccessor( export function getParent( node: ESTree.Node, -): (ESTree.Node & Rule.NodeParentExtension) | undefined { +): ESTree.Node & Rule.NodeParentExtension { return (node as NodeWithParent).parent; } @@ -129,3 +127,24 @@ export function isFunction( } export const equalityMatchers = new Set(['toBe', 'toEqual', 'toStrictEqual']); + +const joinNames = (a: string | null, b: string | null): string | null => + a && b ? `${a}.${b}` : null; + +export function getNodeName(node: ESTree.Node): string | null { + if (isSupportedAccessor(node)) { + return getStringValue(node); + } + + switch (node.type) { + case 'TaggedTemplateExpression': + return getNodeName(node.tag); + case 'MemberExpression': + return joinNames(getNodeName(node.object), getNodeName(node.property)); + case 'NewExpression': + case 'CallExpression': + return getNodeName(node.callee); + } + + return null; +} diff --git a/src/utils/parseFnCall.ts b/src/utils/parseFnCall.ts index 3c31093..01fd077 100644 --- a/src/utils/parseFnCall.ts +++ b/src/utils/parseFnCall.ts @@ -277,7 +277,7 @@ const parseExpectCall = ( export const findTopMostCallExpression = ( node: ESTree.CallExpression, -): ESTree.CallExpression => { +): ESTree.CallExpression & Rule.NodeParentExtension => { let top = node; let parent = getParent(node); let child: ESTree.Node = node; @@ -303,7 +303,7 @@ export const findTopMostCallExpression = ( parent = getParent(parent); } - return top; + return top as ESTree.CallExpression & Rule.NodeParentExtension; }; function parse(context: Rule.RuleContext, node: ESTree.CallExpression) {