Disallow calls to async functions from async functions without using 'await'

[RyanCavanaugh]

Fixes #13376

[cevek]

@RyanCavanaugh I think it would very useful also to check calls without await in control flow condition statements

Because to catch errors like this I need tslint with slow "strict-boolean-expressions" rule

async function bar(){return false}

async function foo() {
  if (bar()) {}
  while (bar()) {}  
  do {} while (bar())
  var x = bar() ? 1 : 2;
  for (var x = 1; i++; bar()){}
  switch (bar()) {case bar(): break;}
}

[kpreisser]

Hi,
is there an ETA when this PR will be merged to master (e.g. after a 2.4 branch was created)?
Currently I'm merging this branch for a custom TS build and every time I have to fix the merge conflict in diagnosticMessages.json. 😉

Thanks!

[finnigantime]

Would love to have this! Currently relying on tslint rule no-floating-promises but having this guard in TypeScript itself seems preferable.

[leebenson]

This would be especially useful for VSCode users, where no-floating-promises doesn't work out-the-box with the TSLint plugin. Having code look fine but then throw when hitting tslint via the CLI or VSCode task is annoying.

[mhegazy]

in light of the discussion about errors vs warnings we should instead move this to tslint.

[leebenson]

It already is in tslint. This is problematic enough to be in TS proper. The tslint plugin in VS code doesn’t catch it and those without a separate linting step in their build pipeline could be shipping very broken code. I hit a MAJOR security issue with not awaiting a Promise— no way should this be downgraded to a warning.

[conordickinson]

The TSLint rule also requires type info to be on, which we do not do because it is so much slower (and causes tslint to run out of memory with our repo last I tried it).

[leebenson]

Merging this would prevent things like this:

try {
  await someImportantSecurityCheck(); // <-- ditching `await` is a MAJOR security flaw
  runSomePrivilegedFunction(); <-- drop `await` above, and this runs... yikes
} catch (e) {
  // deal with unauthorised...
}

// ...

Which incidentally, is exactly the issue I ran into.

Not awaiting promises can cause major, not easily caught security headaches, race conditions and non-deterministic code. Not to mention unhandled Promise rejections are deprecated, and will eventually terminated with a non-zero error code.

Again - this is not linting. It's a significant flaw in an otherwise decent type system.

Promises should be dealt with based on type, period. If we don't care about how or when they run, we stick void in front of them. But it should absolutely be explicit.

[RyanCavanaugh]

Will pick this up later