linter icon indicating copy to clipboard operation
linter copied to clipboard

Implement PREFER async/await over using raw futures.

Open JPaulsen opened this issue 8 years ago • 2 comments

From [Effective Dart] (https://www.dartlang.org/guides/language/effective-dart/usage#prefer-asyncawait-over-using-raw-futures)

JPaulsen avatar Mar 23 '17 19:03 JPaulsen

I am not sure about how to implement this rule, by now I am linting every FunctionExpression that doesn't have a keyword (async, sync, etc) and every ReturnStament inside (at least 1) is a then, onError, timeout or whenComplete method invocation of a target that is a Future.

These are my tests by now:

// Copyright (c) 2017, the Dart project authors.  Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

// test w/ `pub run test -N prefer_async_await_over_raw_futures`

import 'dart:async';

Future<bool> bad1() { // LINT
  return longRunningCalculation().then((result) {
    return verifyResult(result.summary);
  }).catchError((e) {
    return new Future.value(false);
  });
}

Future<bool> bad2() { // LINT
  return longRunningCalculation().then((result) {
    return verifyResult(result.summary);
  });
}

Future bad3() { // LINT
  return longRunningCalculation().whenComplete(() {
    return verifyResult(null);
  });
}

Future bad4() { // LINT
  return longRunningCalculation().timeout(null, onTimeout: () {
    return verifyResult(null);
  });
}

Future<bool> verifyResult(summary) => null;

Future longRunningCalculation() => null;

Is this correct @bwilkerson?

JPaulsen avatar Mar 23 '17 19:03 JPaulsen

I haven't thought about this enough to be convinced that I know the right answer here, but I'm fairly sure you can't turn an invocation of timeout into an async/await form, so invocations of that method should disable the lint.

As for the rest, some questions to consider:

  • If the method doesn't already return a Future, should we exempt it?
  • Do we want to lint functions that invoke one or more of these methods even if they're not in a return statement?

But in general, I think that for a lint like this it's perfectly reasonable to start with a narrow definition that covers the most common cases and to expand the definition later based on user feedback. So I'm fine with most of what you said, though I'd probably require that the return type is already expected to be either Future or dynamic (not specified).

If you're going to tackle closures then the expected return type is a little harder to compute because it depends on context. If the closure is an argument to a function, then we'd need to look at the type of the parameter. If it's being assigned to a variable we'd need to look at the variable's type. It might be better to not handle closures for the first pass.

bwilkerson avatar Mar 23 '17 20:03 bwilkerson