FP S4123 (no-invalid-await): TypeScript Pick type

[codyebberson]

Creating new issue as requested by @vilchik-elena in #2636

The following TypeScript code snippet creates a “Refactor this redundant ‘await’ on a non-promise.” warning, which is incorrect.

interface MyQuery<T> extends Pick<Promise<T>, keyof Promise<T>> {
  toQuery(): string;
}

export async function demo(query: MyQuery<string>) {
  const result = await query;
  console.log(result);
}

I see the error both in the SonarLint VS Code plugin (v2.0.0) and the current version of SonarCloud.

The Pick type is a Typescript "utility" type. It creates a dynamic interface. For example Pick<Todo, "title" | "completed"> would be an interface with only the "title" and "completed" fields in the "Todo" interface.

The keyof operator returns all of the keys/members of a type.

As far as I can tell, these should be equivalent:

interface MyQuery<T> extends Pick<Promise<T>, keyof Promise<T>> {
  toQuery(): string;
}

interface MyQuery<T> extends Promise<T> {
  toQuery(): string;
}

To be candid, I don't use either of these features myself. This is just my understanding.

My project uses Knex, which is a popular SQL builder. Knex uses this pattern extensively, and then creates many false positives.

Here is the root declaration in Knex:

https://github.com/knex/knex/blob/dfbc76b915fff9f40e806ac2e0aafe8e4e5b6965/types/index.d.ts#L1742

interface ChainableInterface<T = any> extends Pick<Promise<T>, keyof Promise<T> & ExposedPromiseKeys>, StringTagSupport {
  generateDdlCommands(): Promise<{ pre: string[], sql: string[], post: string[] }>;
  toQuery(): string;

Originally posted by @codyebberson in #2636 (comment)

[codyebberson]

I looked into this today. The critical lines are in no-invalid-await.ts:

function hasThenMethod(type: ts.Type) {
  const thenProperty = type.getProperty('then');
  return Boolean(thenProperty && thenProperty.flags & ts.SymbolFlags.Method);
}

For the code in the bug (using Pick), thenProperty.flags = 33554436, which would be ts.SymbolFlags.Property | ts.SymbolFlags.Transient.

Ok, easy peasy, add that as a valid case:

function hasThenMethod(type: ts.Type) {
  const thenProperty = type.getProperty('then');
  return Boolean(
    thenProperty &&
      (thenProperty.flags & ts.SymbolFlags.Method || thenProperty.flags & ts.SymbolFlags.Transient),
  );
}

Unfortunately, that leads to failure on this test case:

async function foo() {
  await {then: 42};
}

Which I found quite surprising, that TypeScript could not differentiate between those two cases. They truly have exactly identical flags of 33554436.

With all of that said, in my opinion, a false positive is worse than a false negative.

[sunmorgus]

We're seeing similar behavior with the new fs/promises module in node 14. The following causes the rule to trigger on the await stat(dirUrl); below, but isn't necessary as stat here from fs/promises is an async function:

import { stat, mkdir } from 'fs/promises';

/**
 *
 * @param {URL} dirUrl
 */
export const ensureDirExists = async (dirUrl) => {
    let exists = true;
    try {
        await stat(dirUrl);
    } catch (err) {
        exists = false;
    }
    if (!exists) {
        await mkdir(dirUrl, { recursive: true });
    }
};

[spartanatreyu]

I'm having this issue just inside standard JS without imports.

Here's my testcase:

const getLoginStatus = async () => {
    return true;
};

const startApp = async () => {
    if (await getLoginStatus() === true){ // Refactor this redundant 'await' on a non-promise.
        console.log('go to dashboard route');
    }
    else {
        console.log('go to login route');
    }
};

startApp();

However, this seems to work just fine, no linting errors appearing:

const startApp = async () => {
    if (await Promise.resolve(true) === true){
        console.log('go to dashboard route');
    }
    else {
        console.log('go to login route');
    }
};

startApp();