denoland / deno_lint

Blazing fast linter for JavaScript and TypeScript written in Rust
https://lint.deno.land/
MIT License
1.53k stars 172 forks source link

The require-await rule does more harm than good #1220

Open dbrockman opened 9 months ago

dbrockman commented 9 months ago

The require-await rule tells the user to remove the async keyword when await is not used in the function. This is bad advise and will change the behaviour of the function. If you remove the async keyword then any error that is thrown in the function will no longer result in a rejected promise. A function that returns a Promise should always return a Promise, even in case of errors.

I think a better way to implement this rule would be to invert it. If the return type is Promise<...> then require the use of the async keyword.

Lint Name

require-await

Code Snippet

Simple example

// Linter tells me to remove the `async` keyword here
async function fail(): Promise<void> {
  throw new Error("Failed");
}

Slightly more realistic example

function assertString(value: unknown): asserts value is string {
  if (typeof value !== "string") {
    throw new Error("Expected a string");
  }
}

// Linter tells me to remove async here
async function sha256(input: unknown): Promise<ArrayBuffer | null> {
  // Assert that input is a string
  assertString(input);

  // Call a method that would fail if the input is not a string
  const utf8 = new TextEncoder().encode(input);

  // Return the result of an async function without await
  // It would be better if the linter told me to add an extra await here instead of removing async.
  return globalThis.crypto.subtle.digest("SHA-256", utf8);
}

async function main() {
  const hash = await sha256(123).catch(() => null);
  console.log(hash);
}

Expected Result

The linter should not warn about the async keyword.

Actual Result

The lister tells the user to remove the async keyword.

If you follow the advice of the linter and remove the async keywords, you will get an uncaught error.

Additional Info

Version

deno 1.39.1 (release, x86_64-apple-darwin) v8 12.0.267.8 typescript 5.3.3

karfau commented 4 months ago

Similar but with a different proposed solution: #1170