Consider adding a lint for huge methods

[alexmarkov]

In order to compile extremely huge methods compiler may spend a lot of time and memory, and may eventually run out of memory. Also, compiler may compile such methods with certain optimizations disabled or reduced (such as inlining). Huge methods are also harder to reason about and maintain.

We can add a lint to detect huge methods and warn users about that. Unlike other lints, this lint should be also enabled for auto-generated code, as it is often a source of huge methods.

Related: #48495

[bwilkerson]

We can add a lint to detect huge methods ...

Do we have a definition of what "huge" means in this context? Are we talking about lines of code? Are there other factors that should be considered, or that mitigate the damage?

Unlike other lints, this lint should be also enabled for auto-generated code, as it is often a source of huge methods.

One of the arguments against enabling lints for auto-generated code is that users can't control the code generator and hence can't fix the problem. I think that applies here as well. So I have to ask: what value would it bring to users to know that the code generator is causing longer compile times and/or un-optimizable code?

[alexmarkov]

We can define huge method in terms of substantial code (number of AST nodes or tokens), ignoring comments and excluding nested closures (whose can be measured separately). The threshold could be quite arbitrary and maybe configurable (high enough to not trigger on reasonable code, but probably smaller than thresholds used by the compilers).

Users can control the input of the code generator (which may directly affect the size of the output). They can also consider replacing code generator with something else if they're aware that it is the generated code which causes the problem (such as too long compilation or OOM during compilation).

[bwilkerson]

The threshold could be quite arbitrary and maybe configurable ...

We don't currently have any mechanism to allow users to configure lints. Even if we did, I'd question whether users have the knowledge required to choose a good value.

... but probably smaller than thresholds used by the compilers ...

That sounds like a promising definition. I'm guessing, based on your use of plurals, that different compilers have different thresholds. We could presumably choose a reasonable threshold that is smaller than any used by existing compilers. We would then need to be careful to adjust the lint any time the thresholds used by the compilers are changed in a way that impacts the lint.

But this raises another question. Would it be better for this kind of analysis to be done by the compiler? It seems to me that having this analysis be done by the compiler would

• prevent the results from getting out of date,
• be more accurate, and consequently
• minimize the number of methods being flagged.

This all makes me think that it would be better for users to have the analysis be done by the compilers.

[alexmarkov]

If there is no mechanism to configure particular lints, then having a reasonable hardcoded threshold might be also fine.

See #48495 (comment) for some discussion.

I can also add that users run dartanalyzer much more often in the development cycle than certain compilers (such as AOT), and the particular thresholds are likely to be back-end specific.

I think this lint is not about reaching certain thresholds where compilers start to behave differently. This is more about showing users potential problems in their code which lead to large code size and poor maintainability, in addition to slow compilation and high memory usage during compilation. Analyzer might be better equipped to show such warnings. Users can also disable this lint or ignore it on a particular method.

[bwilkerson]

This sounds like functionality that could be useful for users, but given our current support levels I'm concerned about the number of false positives. There has recently been some discussion around defining "suggestion level lints" that would be displayed to users in a less intrusive manner. This might be a good candidate for such support. @jacob314

[srawlins]

I like this idea, protecting the users from crazy code. I imagine it often comes from code-generators? Which is good feedback for authors of code-generators.

@alexmarkov do you know the ballpark of a number-of-statements that has crashed the VM? Or that blows up compile time? I imagine it's huge like 10_000 statements? And if it really is huge like that, then for style and readability purposes, the dev should have quit much sooner, like at 1000 statements, so I'm not worried about false positives.

[bwilkerson]

I still think it would be better for the compilers to do this kind of check and to not have a lint.