Clarify text for unnecessary_final lint

[timsneath]

The unnecessary_final lint contains a controversial explanation of its rationale.

Specifically:

var is shorter, and final does not change the meaning of the code.

The historical context is here: dart-archive/linter#1827

While there are clearly divergent preferences around the use of final, it does change the meaning of the code (as the prefer_final_locals lint explains). And while var is shorter by two characters, that seems like a poor rationale for a lint. Readability is preferred over terseness, anyway.

Lastly, Effective Dart has settled case law on the subject:

State that is not mutable—that does not change over time—is easier for programmers to reason about. Classes and libraries that minimize the amount of mutable state they work with tend to be easier to maintain. Of course, it is often useful to have mutable data. But, if you don’t need it, your default should be to make fields and top-level variables final when you can.

While we don't need to remove the rule, we should have explanatory text that is more accurate. For example, "some people prefer the simplicity of using var for all variables."

[pq]

/cc @davidmorgan

[MelbourneDeveloper]

Thanks @timsneath.

There is nothing wrong with the lint itself. I understand why some teams may want to embrace this. The issue is the documentation, as you say.

var is shorter, and final does not change the meaning of the code.

I looked at this lint and was very confused. This isn't a reason to prefer var over final but it's also just plain wrong.

Some teams may want to reassign variables as a matter of preference. For the record, I think that something like this would be better.

You should make assignments to existing variables where possible to avoid creating multiple variables for the same logical value

and perhaps

var allows for reassignments while final means your code may end up with multiple variable names for the same logical value.

[davidmorgan]

Hi Tim,

Thanks :) this is a fun one.

The Dart team--particularly Bob and Lasse, and they persuaded me, which is why I added this lint--prefer and recommend using varfor locals. Nobody else on the team has a strong opinion as far as I'm aware.

The prefer_final_locals lint description is wrong: the compilers definitely know whether local variables are reassigned, final does not help.

The Effective Dart rule you point to is talking about instance/global state not variables. It says exactly "fields and top-level variables final" ... locals should not be. Again, according to Bob ;)

Unfortunately there is a tendency for people to pick up the "prefer final" lints with a sense that they are recommended and better--they are not. The recommended way is this lint. Unfortunately, the other lints got there first, and are far better established as a result. We might actually have to go with 'final' in google3 just for consistency--and against style recommendations.

I don't think there's any nice resolution possible here; given the strength of feeling from Bob and Lasse I'm pretty sure we will never recommend "final", and given how much better established those lints are I don't think we can ever recommend "var", either.

And in the end it just doesn't matter too much: what matters for the code is instance mutability, as called out in Effective Dart.

Thanks

Morgan

[MelbourneDeveloper]

@davidmorgan the issue is not the linter. The issue is the mistake in the documentation

[MelbourneDeveloper]

I will make a PR for the doco in the next couple of days

[davidmorgan]

Hi Christian,

Those suggestions don't make sense I'm afraid; we definitely don't want to encourage var as any kind of optimization.

Using var is simply the default: it's how the language was intended to be used. This is direct from the language designers ;)

We have repeatedly discussed whether to recommend final for locals and the answer is no.

Thanks

Morgan

[MelbourneDeveloper]

@davidmorgan I wouldn't use this linter if you paid me, but you're not getting it.

The current doco is completely incorrect and confusing

var is shorter, and final does not change the meaning of the code.

final does change the meaning of the code. You can frame the linter however you like. The point of the matter is that it reads like var is just as good as final and that's why this issue is here.

[davidmorgan]

Perhaps "change the meaning of the code" could be clarified.

What was meant was, "does not change the meaning of the code from the compiler's point of view". The code compiles and runs exactly the same with var or final.

It could be argued that it changes the intent of the code for a human reader, and perhaps this can be confused with "meaning"?

Thanks.

[MelbourneDeveloper]

@davidmorgan the reader of this document is not concerned with how the compiler interprets it.

What the reader takes away from this is that var and final mean the same thing, but they don't.

'var' means that the reference can be set multiple times while a 'final' declaration can only be set once.

The documentation is very confusing for people reading the documentation. It is especially confusing for people new to Dart that may not have a full understanding of what the two keywords mean.

[davidmorgan]

Yes, the wording could be improved; suggestions welcome.

[MelbourneDeveloper]

@davidmorgan

I think there's a broader question of how to document the intent of linters here.

I don't like this linter, but I understand the motivation. Many people new to Dart will use var more often than final and actually decide that it's better than final. They may decide to enforce that because of some arbitrary team decision, and that's fine.

The question is how to phrase the intent of the linter without recommending it and possibly explaining that it goes against the Dart recommendations.

To be honest, the linter doco should be more opinionated in order to match the tone of the Effective Dart guide.

[davidmorgan]

Using var, as enforced by this lint, is not quite officially recommended, but it's as close as it can get without being recommended.

Using final for locals is definitely not officially recommended.

The Dart team--particularly Bob and Lasse, and they persuaded me, which is why I added this lint--prefer and recommend using var for locals. Nobody else on the team has a strong opinion as far as I'm aware.

Bob is the author and maintainer of Effective Dart, and he prefers var.

As I said, I'm not aware of anyone on the Dart team who wants to recommend final.

[MelbourneDeveloper]

@davidmorgan well that totally changes the game then.

That's certainly not clear in the linter.

If the Dart team prefers var over final then both the related linters should mention that and put the case forward.

[MelbourneDeveloper]

But the point still stands that var and final are different and the documentation needs to clarify that.

[davidmorgan]

The individual lint docs generally don't talk about whether the lint is recommended; that's left to packages like

https://github.com/dart-lang/lints

to recommend a set. These then get a badge at the top of their page like 'style recommended'.

FWIW I tend to agree; people link to lint documentation as if it's guidelines, when actually each lint documentation stands by itself. I think this has led to a lot of confusion over the years.

I wonder if it would be worth putting a big banner at the top of lints that are not in any recommended set, to make it clear that people should not follow/quote them.

Thanks.

[MelbourneDeveloper]

@davidmorgan

As a general comment, I think that the linter docs should give more context than they do. They often only give a line or two and they don't list the pros and cons of a given approach.

For the record, I think they should link to the Effective Dart document where they relate to that, they should explain the intention of why someone created them in the first place, and they should explain the consensus in the Dart team.

I don't treat them as gospel, but my assumption is that if the rule exists, and there is no opposite rule, the Dart team included the linter for a good reason.

[davidmorgan]

I 100% agree--I think many people assume that if a lint exists it's recommended; I see teams go through the whole list and try to enable as many as possible. Unfortunately there are lints that it is recommended to not enable. (vs being just not recommended, "optional").

That's why I was building a list of banned lints here

https://github.com/google/pedantic/blob/66f2f6c27581c7936482e83be80b27be2719901c/README.md#unused-lints

but with the transition from package:pedantic to package:lints that got dropped; it was anyway never surfaced in the linter docs. Note the "banned" list in pedantic was only ever for internal use at Google, they were not Dart team recommendations. But they show the idea.

Perhaps we should organize a "linter docs fixit" to raise the bar here.

[MelbourneDeveloper]

You will hate this then @davidmorgan

https://pub.dev/packages/austerity

I do hear where you're coming from. I also understand that some linters just directly contradict each other, and that some linters make code more verbose for some illusory idea of readability. But, I've combed through the linters and found that the vast majority are useful and help to guide people in the right direction.

Anyway, I will try to make some suggestions on the text for this particular linter, but I would suggest that the docs get more opinionated about what the Dart team recommends. This is something that I and many others take very seriously.

[davidmorgan]

;)

There is an important usability issue around the expectations for lints.

Some lints correctly point out all places where a style guideline might apply, but can't tell you whether it does apply. If you always follow the lint then you are not following the guideline: you are ignoring the intended exceptions. use_to_and_as_if_applicable is a good example, it was created to follow an Effective Dart rule but it does not implement the "if applicable" part.

If you intentionally enable these lints knowing that they should sometimes be ignored, that's fine.

But as far as I know people usually don't differentiate the lints with false positives from the ones that are always correct. (We don't actually mark the ones with false positives anywhere, so it's not exactly easy; the pedantic banned list is a partial list). So, enabling these lints directly causes violations of good style.

That's a shame, because they are pointing out something useful. I have toyed with the idea of a new way to enable a lint, maybe call it "transitory" or "FYI". It fires once as you are writing the code, to say "here is something you might think about", then goes silent forever. We could then use it for lints like us_to_and_as_if_applicable without leading people too strongly in the direction of a guideline that should only apply sometimes.

I like that idea, but have not had time to do anything with it :)

[eernstg]

@davidmorgan wrote

As I said, I'm not aware of anyone on the Dart team who wants to recommend final.

Here is one! ;-)

I understand that some teams/developers may prioritize properties like simplicity ("just use var every time"), brevity (final is 5 chars), and alignment (var at the beginning of every line at the beginning of a function body is easier to recognize than a mixture of var, final, types, and final plus types, which makes the declared names easier to spot).

However, the argument in favor of using final whenever possible on non-local declarations works exactly as stated for local variables as well:

State that is not mutable—that does not change over time—is easier for programmers to reason about.

A developer who is reading the code in a function body where final is used whenever possible can get an overview of the variables and immediately know which variables are just a name for a value (the final ones), and which ones are storage locations where the algorithm in the rest of the body will put different things at different times (all other variables).

The latter will require a substantially larger number of brain cells to process when we read the code (not to mention when we're changing it). This means that final is a provider of reading simplicity (whereas the consistent use of var will only provide writing simplicity, and actually makes it more complex to understand the code).

This effect may very well be overlooked, because we are so familiar with the situation where lots of variables are effectively final (but not final), and we're not necessarily aware of the clarity that we could have enjoyed if there had not been a need to skim the entire function body in order to single out the mutable variables and keep them in mind.

In summary, I'd certainly support anyone who is using a style where constraints like final are expressed explicitly whenever possible. Even a sporadic or inconsistent use of final can provide a certain amount of reading simplicity, one variable at a time. On the other hand, it's not surprising or even unreasonable that some organizations/developers think it isn't worth the effort.

final does not change the meaning of the code.

As mentioned several times in previous comments, this is simply untrue. For example, a final local variable is not promoted based on an initializing expression in some situations where it would be promoted if final were removed.

More importantly, the knowledge that a particular variable v is mutable changes the meaning of the code in the broader sense that any attempt to understand what a given snippet of code containing v is doing needs to include an overall understanding of how v changes over time. If v had been final then it's enough to look at the initialization of v (ok, we can have final int i; if (...) i = 2; else i = 3;, but initialization is still simpler than mutation).

I think @timsneath's original proposal is a very good description of the main reason I've seen in this thread for abolishing local final:

"some people prefer the simplicity of using var for all variables."

[davidmorgan]

Thanks Erik! I will update my future comment on the topic ;)

FWIW I started this whole journey using final for locals, tried to get it enforced everywhere, failed, switched to var myself, and found that it seems to make very little difference (to me).

I also looking at highlighting 'could be final' in the IDE, which would give the readability benefits for free--and had a local fork of the analyzer that did that. But, I was unable to land it.

Unfortunately I think discussion around this topic has by now far exceeded the importance of the topic ... but that there is no path to ending the discussion.

At this point I would be happy with us picking one of the two at random, recommending the corresponding lint, deleting the other, and auto updating all our code to match ;) ... more than anything else it's inconsistency that hurts us here, which is why we have https://dart.dev/guides/language/effective-dart/usage#do-follow-a-consistent-rule-for-var-and-final-on-local-variables ... but have been unable to apply that at a higher level ;)

"some people prefer the simplicity of using var for all local variables"

Sounds good to me :)

[eernstg]

I also looking at highlighting 'could be final' in the IDE

That's a very interesting way to get the same benefits in some reading situations!

There's still one difference, though: If a function body has a final T x = ...; then an update that assigns a new value to x is a compile-time error. This could be important because it's necessary to make changes here and there if x used to be immutable, but it's now mutable (for instance, if we have z = x.foo somewhere then we may need to update z whenever x is updated, and possibly to change z to be mutable in order to be able to do that).

In contrast, if x was just effectively final (and not final) then the code modification that mutates x is silently accepted, and there's no hint that we may need to reconsider other parts of the code (perhaps the background color in the declaration of x changes, but that might not be on screen at that point in time).

You could say that the rationale here is something like the following: "Immutability of a local variable x may justify certain decisions about how to write code elements that do not refer directly to x. A modification of the code that makes x mutable should be considered as relevant to, potentially, the entire function body. Hence, the status of being immutable/mutable should be explicitly stated in the declaration of x."

[davidmorgan]

Thanks Erik.

If a local scope is big enough that (re)assigment might be unclear, I prefer to split up the block. In this sense final locals could be considered detrimental to readability as it slightly discourages splitting large blocks of code ;)

Of course you can use final in small methods too:

  C computeC() {
    final a = computeA();
    return C(a, computeB(a));
  }

But then it really does seem like a distraction: it's carrying weight just in case the method grows big enough that reassignment isn't clear.

Anyway, I no longer think it's worth arguing this to any specific conclusion, but if there is a way to reach any conclusion, I'm in favour; at this point I think nothing short of putting everyone with an opinion in a room and only letting them out when they pick one will suffice ;)

[mit-mit]

Sorry, I having a hard time understanding the discussion here. What exactly is being discussed?

This bug started with a request to update the description text of the unnecessary_final. We should do that. It implies that there is no semantic difference between var and final. I don't think that is true: void foo() { final b = 42; b = 42; } will produce an error, void foo() { var b = 42; b = 42; } will not. Further I've confirmed with the native compiler team there are a few cases where the compilers do perform additional optimizations on final local variables (in addition to many cases on final fields).

So let's update that description. I suggest something like:

"var is shorter, and thus preferred to keep code less verbose"

[davidmorgan]

We got a bit off topic ;)

How about referencing Effective Dart?

---

Use var for all local variables, even ones that aren’t reassigned. Never use final for locals.

Per Effective Dart, there are two rules in wide use for var vs final; this lint enforces the second of the two. For the alternative style that prefers final, enable prefer_final_locals and prefer_final_in_for_each instead.

---

And then the corresponding update for those lints to point to the style guide and back to this lint?

Thanks.

[mit-mit]

Yeah, I really like the cross linking here.

Should we also clarify that the lint is only for local variables? The name unnecessary_final sounds a bit like it's for both local variables and fields.

[davidmorgan]

Yes, definitely include the word 'local' anywhere it's unclear :)

[bwilkerson]

Perhaps we should organize a "linter docs fixit" to raise the bar here.

There's a lot of interest in improving the linter documentation along several dimensions. We are in conversation with @mit-mit and the documentation team (@MaryaBelanger and @atsansone) to discuss how to raise the bar here. Some of the rewriting is already underway. So please coordinate with us before doing any significant amount of information in this area.

Yeah, I really like the cross linking here.

I agree. I think that should be included in our discussion about improving the lint documentation.

I have toyed with the idea of a new way to enable a lint, maybe call it "transitory" or "FYI".

We have talked about having such a mechanism. I'd love to talk with you about this more. (On a different thread.)

[MelbourneDeveloper]

All this discussion is important but it does take away from this doco just being wrong, confusing, and misleading

var is shorter, and final does not change the meaning of the code.

[mit-mit]

Draft PR here: dart-archive/linter#3832

[lrhn]

Being just nitpicky "var is shorter, and final does not change the meaning of the code." is correct for code that uses final, which this lint is about. Changing final to var does not change the meaning of the actual code, because it clearly doesn't assign to the variable.

It may be confusing and misleading, and we should change it to something else, but it's not wrong.

(If we had a three-letter final, I'd consider using it. It's not just the number of characters to type, it's also the different alignment it causes.)

[MelbourneDeveloper]

@lrhn var is different to final. If you declare a local variable with var, you can assign a value more than once, but final stops you from assigning it more than once.

[eernstg]

"var is shorter, and final does not change the meaning of the code." is correct
for code that uses final, which this lint is about.

As I mentioned here, this is not quite true:

extension on int? {
  void get foo => print('Final!');
}

extension on int {
  void get foo => print('Not final!');
}

void main() {
  final int? i1 = 1;
  i1.foo; // 'Final!'
  
  int? i2 = 1;
  i2.foo; // 'Not final!'.
}

[MelbourneDeveloper]

@davidmorgan

At this point I would be happy with us picking one of the two at random, recommending the corresponding lint, deleting the other, and auto updating all our code to match ;) ... more than anything else it's inconsistency that hurts us here, which is why we have https://dart.dev/guides/language/effective-dart/usage#do-follow-a-consistent-rule-for-var-and-final-on-local-variables ... but have been unable to apply that at a higher level ;)

At this point, it's probably worth pointing out that the two rules are not exact opposites of each other.

This rule only kicks in if the value is never reassigned.
https://dart-lang.github.io/linter/lints/prefer_final_locals.html

But this rule kicks in any time you use final.
https://dart-lang.github.io/linter/lints/unnecessary_final.html

It's not inconsistency. The first rule doesn't try to stop you from doing reassignments. You can reassign as much as you like if you switch to var. The second rule is much more opinionated. It's steering you away from final no matter what.

I doubt that either of these rules should be recommended. It's a team-by-team choice.