feat(valid-expect-with-promise): add checkThenables option for custom promises (#1996)

* fix(valid-expect-with-promise): support promise types not declared in default libs

* refactor(valid-expect-with-promise): update rule to align with no-floating-promises

Author: christopher-buss

docs/rules/valid-expect-with-promise.md

@@ -41,3 +41,65 @@ expect('hello world').toBe('hello sunshine');
 
 expect(new Promise(r => r(0))).rejects.toThrow('oh noes!');
 ```
+
+## Options
+
+```json
+{
+  "jest/valid-expect-with-promise": [
+    "error",
+    {
+      "checkThenables": false
+    }
+  ]
+}
+```
+
+### `checkThenables`
+
+Default: `false`
+
+A
+["Thenable"](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise#thenables)
+value is an object which has a `then` method, such as a `Promise`. Other
+Thenables include TypeScript's built-in `PromiseLike` interface and any custom
+object that happens to have a `.then()`.
+
+The `checkThenables` option triggers `valid-expect-with-promise` to also
+consider all values that satisfy the Thenable shape (a `.then()` method that
+takes two callback parameters), not just Promises. This can be useful if your
+code works with older `Promise` polyfills instead of the native `Promise` class,
+or in environments where the global `Promise` is not declared by TypeScript's
+default libraries (such as with `noLib`).
+
+Examples of **incorrect** code when `checkThenables` is `true`:
+
+```ts
+declare function createPromiseLike(): PromiseLike<string>;
+
+expect(createPromiseLike()).toBe('hello sunshine');
+
+interface MyThenable {
+  then(onFulfilled: () => void, onRejected: () => void): MyThenable;
+}
+
+declare function createMyThenable(): MyThenable;
+
+expect(createMyThenable()).toBe('hello sunshine');
+```
+
+Examples of **correct** code when `checkThenables` is `true`:
+
+```ts
+declare function createPromiseLike(): PromiseLike<string>;
+
+await expect(createPromiseLike()).resolves.toBe('hello sunshine');
+
+interface MyThenable {
+  then(onFulfilled: () => void, onRejected: () => void): MyThenable;
+}
+
+declare function createMyThenable(): MyThenable;
+
+await expect(createMyThenable()).resolves.toBe('hello sunshine');
+```

src/rules/__tests__/valid-expect-with-promise.test.ts

@@ -36,6 +36,17 @@ describe('error handling', () => {
   });
 });
 
+const customPromiseDeclaration = dedent`
+  declare class CustomPromise<T> {
+    then<R>(
+      onFulfilled?: (value: T) => R,
+      onRejected?: (error: unknown) => R,
+    ): CustomPromise<R>;
+  }
+
+  declare const promised: CustomPromise<string>;
+`;
+
 ruleTester.run('valid-expect-with-promise', requireRule(false), {
   valid: withFixtureFilename([
     'expect',
@@ -74,6 +85,67 @@ ruleTester.run('valid-expect-with-promise', requireRule(false), {
     `,
     '<T extends Promise<unknown> = Promise<string>>(v: T) => expect(v).resolves.toThrow()',
     '<T = string>(v: T) => expect(v).toBe(1)',
+    {
+      code: dedent`
+        ${customPromiseDeclaration}
+
+        it('works', async () => {
+          await expect(promised).resolves.toBe('value');
+        });
+      `,
+      options: [{ checkThenables: true }],
+    },
+    // thenables are not treated as promises unless checkThenables is enabled
+    dedent`
+      ${customPromiseDeclaration}
+
+      expect(promised).toEqual(1);
+    `,
+    'expect({ then: 1 }).toBe(1)',
+    {
+      code: 'expect({ then: 1 }).toBe(1)',
+      options: [{ checkThenables: true }],
+    },
+    {
+      code: 'expect(1).toBe(1)',
+      options: [{ checkThenables: true }],
+    },
+    'expect().toBe(1)',
+    dedent`
+      declare class Chainable {
+        then(next: string): this;
+      }
+
+      declare const chain: Chainable;
+
+      expect(chain).toEqual(chain);
+    `,
+    {
+      code: dedent`
+        declare class Chainable {
+          then(next: string): this;
+        }
+
+        declare const chain: Chainable;
+
+        expect(chain).toEqual(chain);
+      `,
+      options: [{ checkThenables: true }],
+    },
+    // a `then` accepting only a fulfillment callback (like Cypress) is not
+    // enough to be considered thenable - a rejection callback is required too
+    {
+      code: dedent`
+        declare class Thenish<T> {
+          then<R>(onFulfilled?: (value: T) => R): Thenish<R>;
+        }
+
+        declare const thenish: Thenish<string>;
+
+        expect(thenish).toEqual(1);
+      `,
+      options: [{ checkThenables: true }],
+    },
   ]),
   invalid: withFixtureFilename([
     {
@@ -181,6 +253,38 @@ ruleTester.run('valid-expect-with-promise', requireRule(false), {
         },
       ],
     },
+    {
+      code: dedent`
+        ${customPromiseDeclaration}
+
+        expect(promised).toEqual(1);
+      `,
+      options: [{ checkThenables: true }],
+      errors: [
+        {
+          messageId: 'poorlyExpectedPromise',
+          line: 10,
+        },
+      ],
+    },
+    // without checkThenables, a thenable is not considered a promise, so
+    // using resolves or rejects on one is reported as unneeded
+    {
+      code: dedent`
+        ${customPromiseDeclaration}
+
+        it('works', async () => {
+          await expect(promised).resolves.toBe('value');
+        });
+      `,
+      errors: [
+        {
+          messageId: 'unneededRejectResolve',
+          data: { modifier: 'resolves' },
+          line: 11,
+        },
+      ],
+    },
     // technically this is valid, but we choose not to give it special treatment
     // as it should be very rare and doing so could give a lot of false negatives
     {

src/rules/utils/ts.ts

@@ -3,6 +3,11 @@ import type ts from 'typescript';
 let SymbolFlags: typeof ts.SymbolFlags;
 let TypeFlags: typeof ts.TypeFlags;
 
+function loadTypeScriptFlags(): void {
+  // eslint-disable-next-line @typescript-eslint/no-require-imports
+  ({ TypeFlags, SymbolFlags } = require('typescript'));
+}
+
 function isSymbolFromDefaultLibrary(
   program: ts.Program,
   symbol: ts.Symbol,
@@ -28,8 +33,7 @@ export function isBuiltinSymbolLike(
   type: ts.Type,
   symbolName: string,
 ): boolean {
-  // eslint-disable-next-line @typescript-eslint/no-require-imports
-  ({ TypeFlags, SymbolFlags } = require('typescript'));
+  loadTypeScriptFlags();
 
   return isBuiltinSymbolLikeRecurser(program, type, subType => {
     const symbol = subType.getSymbol();
@@ -51,6 +55,61 @@ export function isBuiltinSymbolLike(
   });
 }
 
+/**
+ * Checks if the given type is thenable: it has a `then` method that accepts
+ * both a fulfillment and a rejection callback, mirroring the check performed
+ * by typescript-eslint's `no-floating-promises` rule.
+ */
+export function isThenableType(
+  program: ts.Program,
+  node: ts.Node,
+  type: ts.Type,
+): boolean {
+  loadTypeScriptFlags();
+
+  const checker = program.getTypeChecker();
+
+  return isBuiltinSymbolLikeRecurser(program, type, subType => {
+    const then = subType.getProperty('then');
+
+    if (!then) {
+      return false;
+    }
+
+    return unionParts(checker.getTypeOfSymbolAtLocation(then, node)).some(
+      thenType =>
+        thenType.getCallSignatures().some(signature => {
+          const [onFulfilled, onRejected] = signature.getParameters();
+
+          return (
+            onFulfilled !== undefined &&
+            onRejected !== undefined &&
+            isFunctionParam(checker, node, onFulfilled) &&
+            isFunctionParam(checker, node, onRejected)
+          );
+        }),
+    );
+  });
+}
+
+function isFunctionParam(
+  checker: ts.TypeChecker,
+  node: ts.Node,
+  param: ts.Symbol,
+): boolean {
+  const paramType = checker.getApparentType(
+    checker.getTypeOfSymbolAtLocation(param, node),
+  );
+
+  return unionParts(paramType).some(
+    subType => subType.getCallSignatures().length > 0,
+  );
+}
+
+function unionParts(type: ts.Type): ts.Type[] {
+  return type.isUnion() ? type.types : [type];
+}
+
 function isBuiltinSymbolLikeRecurser(
   program: ts.Program,
   type: ts.Type,

src/rules/valid-expect-with-promise.ts

@@ -3,12 +3,17 @@ import {
   createRule,
   getAccessorValue,
   isBuiltinSymbolLike,
+  isThenableType,
   parseJestFnCall,
 } from './utils';
 
 export type MessageIds = 'poorlyExpectedPromise' | 'unneededRejectResolve';
 
-export type Options = [];
+export type Options = [
+  {
+    checkThenables?: boolean;
+  },
+];
 
 export default createRule<Options, MessageIds>({
   name: __filename,
@@ -25,10 +30,18 @@ export default createRule<Options, MessageIds>({
         'Subject is not a promise so {{ modifier }} is not needed',
     },
     type: 'suggestion',
-    schema: [],
+    schema: [
+      {
+        type: 'object',
+        properties: {
+          checkThenables: { type: 'boolean' },
+        },
+        additionalProperties: false,
+      },
+    ],
   },
-  defaultOptions: [],
-  create(context) {
+  defaultOptions: [{ checkThenables: false }],
+  create(context, [{ checkThenables }]) {
     const services = ESLintUtils.getParserServices(context);
 
     return {
@@ -44,11 +57,19 @@ export default createRule<Options, MessageIds>({
 
         const [argument] = jestFnCall.head.node.parent.arguments;
 
-        const isPromiseLike = isBuiltinSymbolLike(
-          services.program,
-          services.getTypeAtLocation(argument),
-          'Promise',
-        );
+        const argumentType = services.getTypeAtLocation(argument);
+
+        // `isBuiltinSymbolLike` only recognises the `Promise` declared in
+        // TypeScript's default libraries, so optionally check for any thenable
+        // (e.g. custom promise types in `noLib` environments, or polyfills)
+        const isPromiseLike =
+          isBuiltinSymbolLike(services.program, argumentType, 'Promise') ||
+          (checkThenables === true &&
+            isThenableType(
+              services.program,
+              services.esTreeNodeToTSNodeMap.get(argument),
+              argumentType,
+            ));
 
         const promiseModifier = jestFnCall.modifiers.find(
           nod => getAccessorValue(nod) !== 'not',