passsy / dart-lint

An opinionated, community-driven set of lint rules for Dart and Flutter projects. Like pedantic but stricter
Apache License 2.0
277 stars 82 forks source link

Uses of `dynamic` as generic type are not flagged because `always_specify_types` is disabled #73

Closed navaronbracke closed 1 year ago

navaronbracke commented 1 year ago

In https://github.com/passsy/dart-lint/blob/master/lib/strict.yaml the lint always_specify_types is disabled, which is then forwarded to the lints for packages.

The comment indicates

# Since dart 2.0 dart is a sound language, specifying types is not required anymore.
# `var foo = 10;` is enough information for the compiler to make foo an int.
# Violates Effective Dart "AVOID type annotating initialized local variables".

This statement is not entirely true.

For example, I recently had to fix a bug in a package which had the following code:

ValueListenableBuilder(
 valueListenable: someListenable,
  builder: (context, value, child) {
     if (value == null) {
       return const SizedBox();
     }

     switch(value as Foo) {
       case Foo.a:
         return const Text('a');
       case Foo.b:
         return const Text('b');
     }
  },
)

Both the null check and the cast were unnecessary in this code. But, this is only surfaced after writing ValueListenableBuilder<Foo>, because ValueListenableBuilder() is actually ValueListenableBuilder<dynamic>() and dynamic disables type checks.

This bug is one of the reasons why flutter_lints does have this enabled.

Maybe we should have a different lint, i.e. always_specify_generic_type?

passsy commented 1 year ago

Please provide the type of someListenable. ValueListenableBuilder<T> should pick up the type of someListenable. But it can't when someListenable returns a dynamic type.

At best, provide a working dartpad with the issue ✌️

navaronbracke commented 1 year ago

@passsy Turns out when the lower constraint is Dart 2.17, the analyzer doesn't enforce the check. Bumping to Dart 2.18.0 does fix it. My bad.