Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: avoid_var_declarations #3136

Closed
5 tasks done
minhqdao opened this issue Jan 9, 2022 · 6 comments
Closed
5 tasks done

proposal: avoid_var_declarations #3136

minhqdao opened this issue Jan 9, 2022 · 6 comments

Comments

@minhqdao
Copy link
Contributor

minhqdao commented Jan 9, 2022

avoid_var_declarations

Description

Avoid using var altogether. Instead, declare a variable using its explicit type, final or const.

Details

Declaring a variable using its explicit type is slightly more verbose but improves readability, adds self-documentation (also in terms of whether the variable is nullable or not) and makes sure that you are not dependant on compiler-inferred types. Declaring variables as final is good practice because it helps avoiding accidental reassignments and allows the compiler to do optimization.

Kind

Style.

Good Examples

class Person {
  String name = 'Bella';
  int age = 64;
  static const ageAtBirth = 0;

  void celebratesBirthday() => age++;
  void addsSecondName(String secondName) => name = '$name $secondName';
}

Bad Examples

class Person {
  var name = 'Bella';
  var age = 64;
  var ageAtBirth = 0;

  void celebratesBirthday() => age++;
  void addsSecondName(String secondName) => name = '$name $secondName';
}

Discussion

I've personally never used var in any of my projects, not a single time. If there is a rule such as unnecessary_final (which I feel rather negative about), I think the opposite should also exist so it can be set as a project standard in a collaborative context.

To my knowledge, the proposal:

  • conflicts with unnecessary_final and omit_local_variable_types and
  • overlaps with always_specify_types, prefer_final_fields, prefer_final_locals and prefer_final_in_for_each.

Despite numerous overlaps with existing rules, I'm still missing the one that hits the nail on the head, especially since the other rules are somewhat differently nuanced. For example, if I'd be fine with not having always_specify_types activated (because I'm absolutely okay with final _controller = TextEditingController(); vs. final TextEditingController _controller = TextEditingController();), var i = 0 would also be allowed if it is reassigned while all the other rules are turned on.

I understand that var is very popular among Flutter/Dart devs that follow a more practical approach. I also wouldn't add it to the standard library of lint rules, especially not for beginners. But I would definitely count myself to those who'd always have it activated if it exists.

Further reads:

https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-using-var-and-dynamic
https://dart.dev/guides/language/effective-dart/usage#do-follow-a-consistent-rule-for-var-and-final-on-local-variables
https://stackoverflow.com/questions/61183907/should-i-give-type
https://stackoverflow.com/questions/66836046/difference-between-var-and-other-more-specific-types-in-dart

Discussion checklist

  • List any existing rules this proposal complements, overlaps or conflict with.
  • List any relevant issues (reported here, the SDK Tracker, or elsewhere).
  • If there's any prior art (e.g., in other linters), please add references here.
  • If this proposal corresponds to Effective Dart or Flutter Style Guide advice, please call it out. (If there isn’t any corresponding advice, should there be?)
  • If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.
@srawlins
Copy link
Member

Can you explain a bit more why this lint rule would allow final foo;, but ban var foo;? I don't understand what problem is solved by changing var foo to either <type> foo, final foo, or const foo. The examples you include are restricted to fields with initializers, but I'd be interested in seeing examples of local variables (with and without initializers) and parameters.

@minhqdao
Copy link
Contributor Author

Sure, I think it boils down to "intention". If you declare a field as final foo = 'bar';, you make clear that this variable is not going to be reassigned. Even more with const: A local declared as const foo = 'bar'; couldn't even be assigned at runtime. So you'd use var foo = 'bar'; only if you want to reassign it (e.g., later set foo = 'baaar';), but then you could just do String foo = 'bar'. Advantages of the latter are self-documentation, no need for compiler inference and arguably a higher readability. The latter is certainly is a matter of taste, but let me demonstrate what I mean:

var a = '5'; // String, not to be confused with an int
var b = 5 + 3 / 1; // double or int? It's a double
var c = 8;
double d = 8;
... EdgeInsets.all(c) // Error
... EdgeInsets.all(d) // Fine

Bottom-line is I could get rid of var altogether and the code quality would only improve (in my personal understanding), but there is no lint rule that enforces it.

If you dislike final foo; because there is no type information, you should include always_specify_types, so both rules complement each other. But if turn off always_specify_types, there's nothing that prevents you from going var foo; when foo gets reassigned. And that's what this rule is for. 😅

There are more examples in the lint description and lots of tested examples that cover also locals but I can add some of them for you above.

@srawlins
Copy link
Member

Thanks for the details.

I still don't see why the current lint rules (the overlaps you mention) do not cover your needs.

If you declare a field as final foo = 'bar';, you make clear that this variable is not going to be reassigned.

This is covered by prefer_final_* rules.

So you'd use var foo = 'bar'; only if you want to reassign it (e.g., later set foo = 'baaar';), but then you could just do String foo = 'bar'. Advantages of the latter are self-documentation

This is covered by always_specify_types. The idea of "prefer final for values which are not re-assigned" and "prefer explicit typing over inference" are orthogonal.

Are you saying you want explicit types, except in the case of final and const variables, in which case the preference for self-documentation and higher readability are no longer important? Why would a variable being final or const change whether you want readability, self-documentation, or inference?

@minhqdao
Copy link
Contributor Author

minhqdao commented Jan 10, 2022

Yes, that's all correct. Maybe we should call the rule specify_types_of_reassignables then and also reduce its coverage so it doesn't overlap with any of the prefer_final_* rules.

But would a subset of always_specify_types justify its existence? 🤔

The reason I find adding the type of final variables redundant is that most of the time, you create or reference class instances as:

final _foo = Bar();

and I don't want to add another Bar in front of the _foo because it's clear what type it is.

Maybe we should have a rule where types only need to get specified when the name isn't already part of the expression or equals the name of the constructor. 🤔

@eernstg
Copy link
Member

eernstg commented Jan 10, 2022

I think it would be great if we could unify two rather disjoint subcommunities in the Dart community: The ones who enable omit_local_variable_types, and the ones who enable always_specify_types and/or similar lints like avoid_var_declarations/specify_types_of_reassignables.

The point is, as mentioned already in this issue, that declarations like int i = 1; and Bar bar = Bar(); are redundant, and they may be verbose in the case where the declared type is complex, or just because it contains one or more long identifiers.

So it might be useful if we could make these lints less aggressive, such that everybody can have declared types just when they think it is useful (OK, nearly so ;-). We would need to do something like the following:

  • avoid_var_declarations/specify_types_of_reassignables would not flag a local variable declaration which has an initializing expression whose type is 'obvious' or 'trivial' (so they can omit the declared type and use var).
  • omit_local_variable_types would not flag a local variable declaration which has an initializing expression unless that expression has an trivial type (so they can include the declared type).

A rather strict style could be maintained by enabling both lints: The declared type would then be included, essentially, if and only if the type is not trivial.

The discussion about what makes a type trivial has been taken several times. For instance, I proposed some ideas here, and a broader discussion about this concept can be found in Avoid unnecessary type annotations.

I'm not so sure about the distinction between var and final/const: It does make sense to say that assignments to a local variable are a particularly good motivation for having an explicit declared type, but I tend to think that it is just as important to enter into a kind of contract with the dependencies about the type that arises as a result of some non-trivial computations, even in the case where there are no assignments (only an initialization), because the variable is final or constant:

import 'whatever needed';

void main() {
  final x = foo(bar()); // `foo` and `bar` could be generic.
  final y = baz(x); // Now we depend on two layers of implicit typing.
  ...
  y.qux('Hello!'); // Error, `TheTypeOfY<With,Several,Arguments>.qux` does not accept a string!
}

So perhaps it would be useful to just check for trivial types, and in particular to treat mutable and immutable variables the same?

@srawlins
Copy link
Member

srawlins commented Aug 3, 2022

This request seems to be for a very bespoke style. As noted, the suggested rule overlaps with always_specify_types, prefer_final_fields, prefer_final_locals and prefer_final_in_for_each. The allowance for inference to occur on final variables is arbitrary, since a final variable is no more likely to have a simple initializer than a non-final one (I have no evidence, just a hypothesis).

We cannot accept all possible lint rules into the linter codebase, since it is maintained by a small team, and each rule does add a maintenance effort. I think this rule is too far from typical coding styles to be considered for the linter codebase.

@srawlins srawlins closed this as completed Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants