Consider adding always_declare_return_types

[vincevargadev]

Describe the rule you'd like to see added and to what rule set

I'd like to see always_declare_return_types included in the rules.

To what rule set? Ideally core (but I'll take whatever I can get 😸).

If there is a good reason not to include this rule, it would be great to document that, I couldn't find why it isn't included.

Additional context

• It is included in the "community" passsy/dart-lint (lint)
• It was included in google/pedantic

I've seen this rule mentioned in the following issues, but I didn't see any of these issues actually addressing why this rule is no longer included, it looks to me that it was simply dropped without any explanation

• https://github.com/dart-lang/lints/issues?q=is%3Aissue+always_declare_return_types

[lrhn]

I don't think it's something we should add.

We do have the "type public API" lint in the recommended set (I think, or is it just the flutter lints?), so this is only really about local and private functions.

At that point it's no longer a recommendation about public API, so it would be because omitting types of particularly error prone.

I don't think it is. Local inference is more efficient than top -level inference, and if getting dynamic is a problem, that's what "strict inference" is for.

It's a perfectly good lint for enforcing a particular style, which some people may prefer, but it's not the official Dart style, so we shouldn't be enforcing it in the official lints.

[devoncarew]

cc'ing in @natebosch and @goderbauer for thoughts as well

[natebosch]

I don't think this applies to local functions does it? Edit: it does apply.

I think I'm in favor to help avoid the implicit dynamic until we nail down the strict dynamic lints, but I'd be on board for waiting until that happens instead.

Edit: There is one place where this doesn't involve an implicit dynamic which is then even more of a style question - overrides can safely omit return types.

I'd still be OK with adding this to recommended.

[devoncarew]

It looks we do recommend this in effective dart:

"DO annotate return types on function declarations"
https://dart.dev/effective-dart/design#do-annotate-return-types-on-function-declarations

(in addition to the recommendation, "DO annotate parameter types on function declarations")

For some other data points, this is enabled in package:very_good_analysis (a popular lint set), has been enabled in the dart_flutter team set for a while, is in the flame lint set, and as the original poster mentions, was in the pedantic set.

[lrhn]

From the Effective Dart entry:

Dart doesn't generally infer the return type of a function declaration from its body, unlike some other languages.

That's not true for local functions (any longer, and for quite some time), the language actually does infer return types.

There is a difference between local and top-level inference (mainly because top-level declarations can be mutually dependent and can introduce types). Local inference is more powerful than top-level inference, and this is one of the places where it's visible.

topfoo() => 42;
topbar(bool x) {
  if (x) return 3.14;
  return 42;
}

void main() {
  foo() => 42;
  bar(bool x) {
    if (x) return 3.14;
    return 42;
  }
  
  print(foo.staticType); // () => int
  print(foo.runtimeType); // () => int
  print(bar.staticType); // () => num
  print(bar.runtimeType); // () => num

  print(topfoo.staticType); // () => dynamic
  print(topfoo.runtimeType); // () => dynamic
  print(topbar.staticType); // () => dynamic
  print(topbar.runtimeType); // () => dynamic
}

extension <T> on T {
  Type get staticType => T;
}

That's why I think it's an unnecessary lint for local functions, and therefore it's too strict to be included.

The no_implicit_dynamic lint is an adequate guard for the top-level functions. It goes directly to the point of why you should write a return type: Because if you don't, you get dynamic.

You can also declare instance methods without types, which are then inherited from the superinterface.

abstract class I {
  int _foo(int x);
}
class C implements I {
  _foo(x) => x + 1;
}
void main() [
  print(C().foo.runtimeType); // (int) => int
}

Again the lint is asking you to write something that the language doesn't require, only because the style guide says so, and the style guide's argument doesn't agree with reality. And again, no-implicit-dynamic should catch any instance methods that do not inherit a type, and if it's for readability, type_annotate_public_apis are directly about that.
(I'd vote for type_annotate_public_apis because that increases the quality of the user-experience for a package.)

The always_declare_return_types lint, and the style guide rule it's based on, is not up to date with the current language.

I don't think we should enable this lint.
The only reason for inclusion that it satisfies is the "is in the style guide", and I think the style guide is outdated here.
That makes the lint too strict, it requires types in places where the language doesn't, and where the program will work fine without them.
It's also only about return types, which is only half the public API. For ensuring public top-level API is typed, using type_annotate_public_apis is better.

The (aka. my) reasons for including a lint is help authors:

• Write code that is easy for your clients to use. (Follow standards and idiomatic patterns, good documented public APIs.)
  • Follow the style guide. (But only when the style guide makes sense).
  • Choose the better alternative when there are multiple ways to do the same thing (use .isEmpty, etc).
  • Don't write code that looks like a mistake. (Even if it isn't.)
• Avoid pit-falls (for easy-to-make errors, the only kind of lint that's allowed to have significant false positives, because it's worth it to avoid the errors).

If you follow that, we should be happy. What you do inside the bodies of your functions is not our business, as long as it does what it looks like it does. And id(int x) => x; does look like it returns an int.

TL;DR: Not this lint. Maybe type_annotate_public_apis instead. That actually improves the user experience for users of the code. This lint applies in places where the return type isn't necessary.

[natebosch]

I didn't know that local functions get an inferred return type. That does change my opinion significantly. I agree with Lasse - it would serve our users better to steer them away from implicit dynamic, and allow implicit not-dynamic if the author prefers.

[devoncarew]

For clarity, when you refer to the no_implicit_dynamic lint above, do you mean the strict-inference mode?

analyzer:
  language:
    strict-inference: true

[lrhn]

I probably mean strict-inference, yes. I can't remember the difference between the different strict modes.
What I really want is a global "no implicit dynamic" language change, which also has suitable workarounds built in. Until then, the strict modes are an alternative, but not ones I'd want in the recommended set as-is.
(And I also think they should be lints.)

[devoncarew]

@vincevargadev, thanks for the proposal. Some notes from a discussion:

• wrt always_declare_return_types, we care mostly about avoiding the use of unintended dynamic
• always_declare_return_types address part of this but doesn't speak to things like method param types (though type_annotate_public_apis covers this a bit)
• for type_annotate_public_apis, we don't enforce it because it requires explicit types for fields, even initialized fields where the inferred type is clear

Briefly, we think we want a new lint, which warns for omitted return types or parameter types, where inference would default those omitted types to dynamic; @lrhn is going to follow up with a more specific description.

[devoncarew]

Opened here: dart-lang/linter#5115.