Add a new lint to require types on variables but not collections

[Levi-Lesches]

Per Effective Dart:

AVOID redundant type arguments on generic invocations.
A type argument is redundant if inference would fill in the same type. If the invocation is the initializer for a type-annotated variable, or is an argument to a function, then inference usually fills in the type

Set<String> things = Set(); (good)
Set<String> things = Set<String>(); (bad)

Here, the type annotation on the variable is used to infer the type argument of constructor call in the initializer.

In other contexts, there isn’t enough information to infer the type and then you should write the type argument:

var things = Set<String>(); (good)
var things = Set(); (bad)

Here, since the variable has no type annotation, there isn’t enough context to determine what kind of Set to create, so the type argument should be provided explicitly.

But with the following code:

const Set<String> temp = {};

I get the following:
INFO|LINT|... Specify type annotations.

This comes from the rule always_specify_types.

[natebosch]

The linter includes some rules that are explicitly against the effective Dart style guide. In particular, always_specify_types is a lint that was requested by the flutter team for the flutter SDK. It is not something the Dart team recommends using.

[Levi-Lesches]

Yes I get that. I also prefer to always specify types. But specifying the type is only needed for the final or var part. Like I do this:
final List<String> temp = [];
instead of what other people do:
final temp = [];

But the linter shouldn't make me do:
final List<String> temp = <String>[];

That's what I was pointing out.

If there is another rule that makes me specify the type but not in the literal itself, please point me to it, I couldn't find one.

[natebosch]

I also prefer to always specify types. But specifying the type is only needed for the final or var part.

That goes against Effective Dart. https://dart.dev/guides/language/effective-dart/design#avoid-type-annotating-initialized-local-variables

So what you are asking for is neither Effective Dart style, nor Flutter SDK style.

If there is another rule that makes me specify the type but not in the literal itself, please point me to it, I couldn't find one.

There isn't one. I guess we could reopen this as a request for a new lint like specify_variable_and_field_types... but things get tricky. Would you want required types on collection literals in other places? Like if they are passed to a function argument and not assigned to a variable with a specified type?

In any case I worry that there might not be enough value in a lint that is neither Dart nor Flutter SDK style...

[Levi-Lesches]

That goes against Effective Dart.

Yeah, I know. But the reason I still do it is beacuse I don't like including type arguments on literals (basically what this issue is about), so the type is not at all obvious. Besides, it lets me understand code by looking at it quicker.

So my code would be:
final List<String> temp = [];

And since I don't like putting generics on literals:
final temp = [];
and Dart wouldn't be able to figure that out, as opposed to Effective Dart:
final temp = <String>[];

I just want always_specify_types to not enforce generics on literals, ESPECIALLY when the type has been specified on the variable itself.

[natebosch]

I just want always_specify_types to not enforce generics on literals, ESPECIALLY when the type has been specified on the variable itself.

As above, this would require a new lint which follows neither Dart nor Flutter SDK style. The current lint is behaving as requested by the Flutter team and we can't change its behavior without their permission. Specifically it looks like @Hixie drove the request in #144

[Levi-Lesches]

I don't see anything about generics on literals in that issue, nor any issue linked there, but I had a look through some Flutter source code and found that there are type args on literals, so I guess that's something they do.

I see your point about this not really being needed. Would you be able to point me to any resources (probably involving the analyzer package) that I could use to help me write a custom lint replacement? I know it sounds overkill but I've always wanted to get into the lint system someday and this is a good excuse.

[natebosch]

I don't see anything about generics on literals in that issue

Specifically it was here: #144 (comment)
"specialising generics like List (whether in constructors or literals)"

Would you be able to point me to any resources (probably involving the analyzer package) that I could use to help me write a custom lint replacement?

This package is really the only place we have to write lints, and they are centralized. We don't have good infrastructure for plugging in distributed lints. Perhaps @bwilkerson knows of some existing feature requests for that type of thing.

[natebosch]

As an aside, you might consider trying to work for a while without all the redundant type annotations. We see a general trend where authors who are new to Dart are initially resistant to letting inference fill in the gaps, but then grow to prefer it when they give it a chance. Usually the reluctance comes from being comfortable seeing redundant types because it's what we're used to. Given the change to become accustomed to var it can feel really nice.

[Levi-Lesches]

That's actually pretty funny seeing as the language I was most involved in before Dart was Python, and I originally hated all the types!

So yeah, like you said, I do feel a lot better being able to see the types (and for final List<String> temp = []; I need to include types if I'm not going to include the type args for the [] itself), but I think one of the main issues of this is (also like you said) the type system. I feel like not letting Dart struggle and just specifying the type can save some debugging time later down the line.

But I get your point. Most times when I sit down to code I ask myself when I'll finally stop typing everything. And it's things like Map<int, Map<int, List<String>>> that make me hope it's one day soon.

[Levi-Lesches]

Hang on, I was looking through the code in this package (I didn't realize linter was separate from analyzer) and I found this little snippet:

@override
  void registerNodeProcessors(
      NodeLintRegistry registry, LinterContext context) {
    final visitor = _Visitor(this);
    registry.addDeclaredIdentifier(this, visitor);
    registry.addListLiteral(this, visitor);
    registry.addSetOrMapLiteral(this, visitor);
    registry.addSimpleFormalParameter(this, visitor);
    registry.addTypeName(this, visitor);
    registry.addVariableDeclarationList(this, visitor);
  }
}

Which I presume relates to this:

@override
  void visitDeclaredIdentifier(DeclaredIdentifier node) {
    if (node.type == null) {
      rule.reportLintForToken(node.keyword);
    }
  }

this:

@override
void checkLiteral(TypedLiteral literal) {
    if (literal.typeArguments == null) {
      rule.reportLintForToken(literal.beginToken);
    }
  }

and this:

@override
  void visitSetOrMapLiteral(SetOrMapLiteral literal) {
    checkLiteral(literal);
  }

If I customize the logic in these two methods to only react if the main identifier doesn't already have a type, would that achieve the effect I'm looking for?

[Levi-Lesches]

And I just love that Dart-parsing code is written in Dart which makes this process of finding fixes so straightforward.

[natebosch]

If I customize the logic in these two methods to only react if the main identifier doesn't already have a type, would that achieve the effect I'm looking for?

It might. We'd need to get buy-in before we can make such a change and I think we're unlikely to get agreement.

which makes this process of finding fixes so straightforward.

If you want to try it out you'll need to be able to build a local SDK, so not quite as straightforward as editing Dart code in your own projects... See https://github.com/dart-lang/sdk/wiki/Building

[bwilkerson]

... registerNodeProcessors ...

The registerNodeProcessors method registers the classes of AST nodes that this visitor is interested in visiting. It's an optimization technique, but if a class of node isn't registered for then the corresponding visit method won't be invoked. It has nothing to do with an methods other than visit methods. In particular, it has nothing to do with checkLiteral.

... a custom lint replacement?

There is a mechanism that exists that would allow you to add new custom lint rules, but unfortunately it has some serious drawbacks. As a result, we discourage users from trying to use it. We had hoped to make it a feature in our product, but we have not yet been able to put that on our roadmap.

First, it's fairly heavy-weight in that you have to create and publish a package that includes the new lints, duplicate most of the linter support, and register the package as a plugin you want to use in every package that wants to use it.

Second, we have not been able to do any performance profiling for it, so we can't guarantee that using this mechanism won't negatively impact the performance of the analyzer enough to make it unusable.

Third, it's only supported in the analysis server, so you can't use it from the command line or in a continuous build environment.

[Levi-Lesches]

it has nothing to do with checkLiteral.

Yes, I know, I was just using that to justify my interest in visitSetOrMapLiteral and visitDeclaredIdentifier, since they were both "registered" in registerNodeProcessors. I can see (as I commented above), that by removing the logic in visitSetOrMapLiteral and visitListLiteral, I can acheive the desired effect (of only checking the declared identifier).

we discourage users from trying to use it

So I had a look around, and I found a couple of issues of interest, namely: #697, #142, #144 (closed), and #1722 (also closed). I see that this will probably take a while (understandably). I personally would like to explore @natebosch's idea of rebuilding the SDK with my own logic in place of the old, but obviously this is not ideal and just a temporary solution if at all.