"Refactor this redundant 'await' on a non-promise." is not correct

SonarQube Cloud
,
1

In our company we’re using SonarCloud which reports the following error:

Refactor this redundant ‘await’ on a non-promise.

This does not work with Mongoose where they add a then method inside the prototype of function. This is already enough to create a Promise. My guess is that SonarCloud checks if the code is an instanceof Promise which is not the case with Mongoose. It’s a function named Query and when you await it will call the prototype.then method.

Here is an example of how Mongoose does it:

function Query() {}
Query.prototype.then = function(resolve, reject) {
  resolve(true);
};

async function main() {
  const query = new Query();
  console.log(query instanceof Promise); // false 

  const result = await query;
  console.log(); // true
}

main();

Can this be fixed in SonarCloud? Thanks.

1 Like
2

Hi @kkoomen,

Rule S4123 implementation checks if given object has a property called then so it should cover the case of Mongoose.

Can you share a reproducer of the false positive you encountered?

3

It’s not Mongoose specifically, there are more libraries who do this. The async keyword wraps the expression after it into a promise and resolves it. A valid promise can be by using new Promise() or by using an object that contains then in the prototype.

Can you share a reproducer of the false positive you encountered?

See the code example I added in my description. It’s a perfect reproducible scenario.

4

I’m experiencing this issue as well using React, Redux (Redux-Thunk) on actions that I am dispatching that clearly return promises

image
image889×101 21.2 KB

Code: await dispatch(getThings(someId, value));

1 Like
5

Hi guys, I’ve been seeing this yellow underline here for a while now for my async await function and it’s really annoying. At first I did think my code was wrong but that’s not true and it caused some issues. This is a bug really and should be fixed ASAP.

6

I’ve opened a GitHub issue for this bug.
If you can please get this issue noticed, give it some reactions and comments so that it gets bumped up with some attention from the Solar guys that’d be great. Thanks!

3 Likes
7

I opened a related GitHub issue:

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.

1 Like
8

I’m not sure if this applies to everyone’s case, but my team was able to solve our specific problem with dispatch. We ended up finding out that we needed to specify a type when using dispatch that ended up solving all of our problems specifically with using redux/redux-thunk.

Here is our hook that we made that solved our problems

// Based off of: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/45575
const useThunkDispatch = () => useDispatch<ThunkDispatch<AppStore, unknown, Action>>();

export default useThunkDispatch;

Instead of using useDispatch we now just useThunkDispatch instead.

Hopefully this helps someone

9

I’m seeing the same issue with fs/promises

import { readFile } from 'fs/promises';

const packageJson = await readFile(new URL('../package.json', import.meta.url));
11

Hi all,

Thank for reporting about FPs on this rule, I hope they are fixed now (see FP no-invalid-await on JS file in SonarLint · Issue #2636 · SonarSource/SonarJS · GitHub). It’s part of the 8.3 release done yesterday, so it will be soon deployed on SonarCloud and will be part of next SQ release (9.1).
If you continue having some FPs on this rule even after upgrade, please create a new thread, as this one is being pretty hard to follow already.

Regards
Elena

13

I’m still having this issue today. In the same .js file, I have an async function and below an await on that function that is being flagged as smell.

Other instances include the same error from .vue files that import the .js file.