Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent "await Task.FromResult()" #513

Open
omsmith opened this issue Jun 20, 2019 · 0 comments
Open

Prevent "await Task.FromResult()" #513

omsmith opened this issue Jun 20, 2019 · 0 comments

Comments

@omsmith
Copy link
Contributor

omsmith commented Jun 20, 2019

A developer working on an async method may end up following a series of steps resulting in a completely unnecessary return await Task.FromResult( foo ); expression, instead of the much simpler return foo;.

This analyzer would not be about correctness, but cleanliness.

async Task<bool> Foo() {
  bool knownResult = false;

  return await Task.FromResult( knownResult ); // D2LXXXX: Do not await a Task.FromResult. Simply use the result value instead.
}

Writing this analyzer becomes somewhat easier due to D2L0055, which requires that all awaited tasks are configured, and so we can limit the syntax we expect somewhat

async Task<int> Foo() => await Task.FromResult( 1 ).ConfigureAwait( false );
async Task<int> Bar() => await Task.FromResult( 2 ).SafeAsync();

Breaking this down:

  • Analyze IAwaitOperations.
  • Look at the syntax's Operand (Task.FromResult( 1 ).ConfigureAwait( false ))
  • Expect an InvocationExpression and access it's Expression (Task.FromResult( 1 ).ConfigureAwait)
  • Expect a MemberAccessExpression and access it's Expression (Task.FromResult( 1 ))
  • Expect an IncovationExpression and access it's Expression
  • Expect a MemberAccessExpression
  • If the member is a static FromResult and the Expression is of type Task, raise a diagnostic

Clearly there are other scenarios this would not cover, but I would not expect an initial implementation to.

async Task<int> Foo() {
  ConfiguredTaskAwaitable<int> t = Task.FromResult( 1 ).ConfigureAwait( false );
  return await t;
}
async Task<int> Foo() {
  Task<int> t = Task.FromResult( 1 );
  return await t.ConfigureAwait( false );
}
async Task<int> Foo() {
  Task<int> t = Task.FromResult( 1 );
  ConfiguredTaskAwaitable<int> a = t.ConfigureAwait( false );
  return await a;
}

Additionally, a codefix could be provided to remove the await expression.

async Task<bool> Foo() {
  bool knownResult = false;

-  return await Task.FromResult( knownResult );
+  return knownResult;
}

It's worth noting that applying this codefix could easily end up with warning CS1998, but that's probably reasonable to let the developer deal with converting it to a non-async method.

// warning CS1998: This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
async Task<bool> Foo() {
  bool knownResult = false;

  return knownResult;
}
@omsmith omsmith added this to New analyzers in General Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
General
New analyzers
Development

No branches or pull requests

1 participant