Overall I think this is an excellent brevity win.

However, I dislike how this removes types when using a named constructor. From a call site, you cannot distinguish a named constructor from a static method. In my opinion, the type from a named constructor invocation is not obvious at the call site. For example:

class Foo {
  Foo.fromEntries(Map<String, dynamic> entries);
  
  static int fromEntry(dynamic entry) => 5;
}

// Before - types are obvious
final Foo one = Foo.fromEntries({'name': 'example', 'value': 42});
final int two = Foo.fromEntry('example');

// After - what's the type of `one`?
// It's not obvious `Foo.fromEntries` is a named constructor
final one = Foo.fromEntries({'name': 'example', 'value': 42});
final int two = Foo.fromEntry('example');

Overall I think this is an excellent brevity win.

However, I dislike how this removes types when using a named constructor. From a call site, you cannot distinguish a named constructor from a static method. In my opinion, the type from a named constructor invocation is not obvious at the call site. For example:

class Foo {
  Foo.fromEntries(Map<String, dynamic> entries);
  
  static int fromEntry(dynamic entry) => 5;
}

// Before - types are obvious
final Foo one = Foo.fromEntries({'name': 'example', 'value': 42});
final int two = Foo.fromEntry('example');

// After - what's the type of `one`?
// It's not obvious `Foo.fromEntries` is a named constructor
final one = Foo.fromEntries({'name': 'example', 'value': 42});
final int two = Foo.fromEntry('example');

I would argue that most of the time as a reader/reviewer I don't really care what the exact type of a variable is since the typechcker / analyzer would catch any potential typing error, and the variable name is (or at least should be) much more helpful for understanding what it is for. Sometimes I do find it convenient that when the expression returns a collection, the type annotation tells me right away it's a List or a Map (which is usually not reflected in variable names), but I would also have to look at how the variable is used to see if the choice of data structure makes sense so that part of review is not local anyway.

@loic-sharma wrote:

I dislike how this removes types when using a named constructor. ...
In my opinion, the type from a named constructor invocation is not obvious at the call site.

That's a good point, and I have had discussions about this previously. However, here's a reason why it could be justified, especially in the case where both omit_obvious_local_variable_types and specify_nonobvious_local_variable_types are used together:

If we encounter a declaration of the form var v = C.m(...); then the fact that both lints are enabled shows that the right hand side C.m(...) does indeed have an obvious type (otherwise it would have been T v = C.m(...) for some type T). But this implies that it is a constructor invocation because the static method invocation never has an obvious type.

(Of course, if it's OK to have some local variable declarations with a type annotation whose initializing expression actually calls a constructor then it's enough to have specify_nonobvious_local_variable_types. But it seems nice to me that a reader can think "if and only if".)

@justinmc wrote:

Is there any way to do this change in pieces? This PR is so big that my browser can barely handle navigating the Files tab here.

We could temporarily relax the lints: Disable always_specify_types and enable type_annotate_public_apis. Then we could transform all the libraries similarly to the changes in this PR, but applied to a smaller set of files in each of a series of PRs. Finally we could enable omit_obvious_local_variable_types and specify_nonobvious_local_variable_types, and fix up anything that might have gone off track in the meantime.

@LongCatIsLooong wrote:

I would argue that most of the time as a reader/reviewer I don't really care what the exact type of a variable is since the typechcker / analyzer would catch any potential typing error, and the variable name is (or at least should be) much more helpful for understanding what it is for.

The underlying goal with the lint pair omit_obvious_local_variable_types is actually a bit more strict than this: It is always supposed to be easy to discern the declared type of a local variable, either because the initializing expression has an obvious type, or because the declaration has an explicit type annotation. The fact that we're asking for type annotations to be added as well as removed implies that local variables will get a consistent treatment (so there's no need to fight about having or not having a type annotation during reviewing etc. ;-).

If you want to go further and eliminate some type annotations, even in a situation where the type isn't obvious, you could choose to enable omit_obvious_local_variable_types, but not specify_nonobvious_local_variable_types.

If you want to go even further and discourage local variable type annotations entirely then you could forget all about the ...obvious... lints and enable omit_local_variable_types.

(In any case, declarations like Iterable<num> it = <int>[1, 2, 3]; will not be linted, because the declared type is not the one that would be inferred from the initializing expression.)

I think the starting point for this particular effort is that both lints will be enabled.

@loic-sharma wrote:

I dislike how this removes types when using a named constructor. ...
In my opinion, the type from a named constructor invocation is not obvious at the call site.

... here's a reason why it could be justified, especially in the case where both omit_obvious_local_variable_types and specify_nonobvious_local_variable_types are used together:

If we encounter a declaration of the form var v = C.m(...); then the fact that both lints are enabled shows that the right hand side C.m(...) does indeed have an obvious type (otherwise it would have been T v = C.m(...) for some type T). But this implies that it is a constructor invocation because the static method invocation never has an obvious type.

@eernstg This makes perfect sense for framework developers. However, in my mind the primary target audience for the framework's source code is not framework developers, but rather all Flutter app developers. That includes the next 1 million Flutter app developers, most of whom have never used Dart before and don't know which lints the framework has enabled. I'd prefer if we strove to make the framework's source code obvious to this target demographic, even though it makes the experience a little worse for framework developers.

@loic-sharma wrote, about considering instance creations of the form C.m(...) as obviously typed:

This makes perfect sense for framework developers. However, in my mind the primary target audience for the framework's source code is not framework developers, but rather all Flutter app developers.

I always had the impression that the framework (and indeed any core element of Flutter, including the engine) would use a style which implies more work and more verbosity in return for added correctness support, but application developers would not be expected to use the same approach. In short, code which is widely (re)used would need more scrutiny than code which is only used by other libraries in the same application.

With that, I'd expect Flutter app developers to already have a culture where code can be less heavily type annotated, compared to Flutter itself.

With that, I'd think it's OK to primarily think about the Flutter framework and engine, and then trust the community to make choices that are optimized for their purposes.