Remove the annoying const lints from flutter_lints

[goderbauer]

In theory, const-ness should give apps a performance boost, but during development the lints enforcing const are constantly nagging developers: "Make this const" or "This cannot be const anymore". To evaluate whether we are making the right trade-offs here between annoyingness and performance we ran some benchmarks (see flutter/flutter#149932). The benchmarks have not shown sufficient evidence to suggest that there is a statistically significant difference in performance between const and nonconst for real world apps¹. Given these results, the lints that constantly nag developers during development are not carrying their weight and I suggest we remove them from the flutter_lints set. People can still manually enable them for their app, if they like.

Lints suggested for removal from the flutter_lints set (they can still be enabled manually):

• prefer_const_constructors
• prefer_const_literals_to_create_immutables
• prefer_const_declarations

I suggest we keep the forth const lint (prefer_const_constructors_in_immutables) because it is very narrow in scope and generally doesn't cause any pain during development. It also enables downstream consumers to make their own choice of whether they want to create a const instance of a widget or not.

Footnotes

1. In a contrived benchmark that did not represent real world usage in Flutter apps we were able to show that const in general can give you a small performance advantage. ↩

[pq]

/fyi @dart-lang/language-team

[eernstg]

The language team has had discussions about the churn caused by a small modification that changes which maximal enclosing expression can be const. E.g., const [e1, e2, e3] may become [e1, const e2, const e3] if e1 is modified such that it isn't a constant expression any more. The converse is similar: [e1, const e2, const e3] may become const [e1, e2, e3] if e1 becomes a constant expression—except that we may also be able to "move the const keyword out" to an even bigger enclosing expression.

We tried to introduce an "auto-const" feature a long time ago, but we removed the feature again because there were too many difficulties with it. In particular, it's a very bad idea to define a language mechanism in terms of "do a specific thing unless it produces a compile-time error".

Another difficulty is that an expression like <int>[] has a different semantics than const <int>[], which means that a number of expressions may allow for the addition of const, but it introduces a bug in the program. We can't remove const because it disrupts the outcome of identical, and we can't add it because it prevents otherwise available run-time behaviors (like adding new elements to a list).

This means that it is difficult to create automatic and smooth support for "keeping expressions as const as possible". Hence the irritation that motivates this issue.

I think another implication of this is that it is not a very good idea for the maintainers of a package to say "let's turn these lints off for now, never write const anywhere, and then we'll constify everything just before we create a new release of this package". We can't really introduce massive changes in this area on a short notice, no matter whether it's done automatically or manually. It's too error prone, and the potential bugs can be very subtle.

Does this matter when it comes to these lints? Would it be fine if we just end up having a lot fewer occurrences of const? Do we expect the removal of these lints from flutter_lints to cause the constness changes to happen in big chunks rather than continuously, and how much of a problem would that be?

[eernstg]

I created dart-lang/language#4084 in order to push on the possible introduction of a mechanism that would (largely) eliminate the churn that motivates this issue.

[lrhn]

In theory, const-ness should give apps a performance boost

Yeah, I'd be very wary about any blanket statement like that. It's a tool. If used precisely, I'm sure it can improve some performance measures.

It will likely reduce the number of allocations, if equivalent objects would otherwise be created multiple times, but it may also increase the total memory pressure by having a bunch of objects that are never GC'ed. If the objects are pre-created, it could hurt start-up time too. Native AoT has options. Web code compiled to JavaScript needs code to create the object either way, and then needs extra state to remember the canonicalized constants.

Being able to use identical for comparisons instead of a slower property-by-property == implementation can be a benefit. But again, if it doesn't short-cut often enough, then it's an extra step for all the other comparisons.

(A very optimizing compiler could potentially recognize that a class is immutable, its arguments are immutable, and nobody ever uses identical on it, and choose to canonicalize instance of the class created in different places. But I guess if the idea is to use identical instead of ==, then that won't help.)

I'm a little surprised that it has no effect, but I'm not so surprised that the effect isn't big.

[mit-mit]

Please see recent updates in flutter/flutter#149932 (comment)

[caseycrogers]

I give the proposal to remove the const lints a huge +1.

I find using const everywhere where it is applicable to be one of the biggest if not the biggest annoyance in writing day-to-day Flutter. It's especially annoying in the scenario where I have a const children list that I want to add a non-const child to:

return const Column(
  children: [
    Foo(),
    Bar(),
    Baz(baz: getBa...), // <- `getBaz` is not constant so autocomplete refuses to suggest it.
  ],
);

I think efforts to make const lints not annoying/less annoying (eg the aforementioned const? keyword) are admirable, but feel like a very hard circle to square. IMO, the best place to solve dev-ex around const would be improving IDE tooling to make the IDE const/un-const expressions at will as I type so I don't ever have to think about it. But of course still leaving me the ability to opt-out of this behavior at the project, file and expression level for the narrow set of circumstances where const is undesirable.

All the above said, I think the burden of proof should be on the const lints to justify their existence and if they can't demonstrate a strong benefit, they should be opt-in lints, not opt out. We can remove the lints and then if we do find a way to make them not tedious, add them back in. That could be a good bit of thrash, but the odds that the tediousness gets resolved feels very low given the subtle edge cases involved.

[lucavenir]

Isn't it possible to "relax" the analysis constraints, such that an autofix simply removes the const where it isn't meant to be there?

DX wise that would imply that, on save, the consts are auto removed when erroneous, and added when an optimization can be set.

The whole "problem" would disappear at that point, no more going back and forth looking for consts that may or may not be actually consts.

EDIT. I just recognized I basically proposed what @caseycrogers just said 😆 But I disagree on one thing: why should there be a burden of proof on lints? There's nothing to prove: a const expression has its optimizations (no heap reallocation, identical, etc.). Whether or not such optimization suits your use case is situational. But when possible it should be done.

[alefwd]

@goderbauer just out of curiosity, why you'd rather remove the lint rules, than simply disable them? 🤔

[caseycrogers]

...There's nothing to prove: a const expression has its optimizations (no heap reallocation, identical, etc.). Whether or not such optimization suits your use case is situational. But when possible it should be done.

Better in theory or even in synthetic benchmark doesn't necessarily translate to measurably better in practice. The benchmarks at the top of this issue show no measurable perf benefit in real world apps.

Someone should have to prove that const is providing measurable benefit to many projects for the lints to stay opt out. Otherwise, they should be opt in. The more I think about it, I'm not even sure why we're bothering trying to improve dev ex around const if attempts to measure its real world impact show no measurable benefit. If we can't demonstrate benefit in real programs, let's just turn off the lints and move on? It's a bad use of dev resources. Can anyone make a compelling practical argument otherwise?

Fwiw, we only have one production use case benchmarked. If we tested more and found that a lot of real world projects do benefit then that'd change things, but shouldn't we do those benchmarks before even bothering trying to improve const dev ex?

[mit-mit]

@goderbauer just out of curiosity, why you'd rather remove the lint rules, than simply disable them? 🤔

@alefwd: I believe that what Goderbauer means here is to "remove them from our Flutter recommended lints", which is essentially no longer enabling them by default, as those lints are what flutter create uses.

Developers can still enable them if they want to by simply adding the lints to the analysis_options.yaml file.

[gregslu]

'constantly nagging developers: "Make this const"'
In vscode for example, this can be done automatically on save:

"editor.codeActionsOnSave": { "source.fixAll": "explicit", "source.organizeImports": "explicit" },

[lukepighetti]

'constantly nagging developers: "Make this const"' In vscode for example, this can be done automatically on save:

"editor.codeActionsOnSave": { "source.fixAll": "explicit", "source.organizeImports": "explicit" },

So I'm obviously against removing it

why do you want them to begin with?

also, adding const is a tiny fraction of the pain caused by this lint. and a solution that works for your workflow in your IDE is not universal

and furthermore i dont recommend autofix to solve this problem because the linter will recommend something be const, then in subsequent iteration youre stuck with an invalid const that breaks autocomplete in your widget tree. its much better to iterate ignoring the lint and then running a fix all on the codebase before committing

[gregslu]

'constantly nagging developers: "Make this const"' In vscode for example, this can be done automatically on save:
"editor.codeActionsOnSave": { "source.fixAll": "explicit", "source.organizeImports": "explicit" },
So I'm obviously against removing it

why do you want them to begin with?

also, adding const is a tiny fraction of the pain caused by this lint. and a solution that works for your workflow in your IDE is not universal

I like the idea that if something can be const, it will be automatically (in the context of this lint rule, and vscode automatic action on save) because without this rule, you will forget to make stuff const, and if for example main listview item, that will be drawn possibly in hundreds, is not const, and could be, it's huge performance lost. But I agree that mostly it doesn't matter, for performance, for example if simple text widget is const or not, and that for other IDE's it can piss off ppl. For me it works :)