dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
636 stars 170 forks source link

[Linter] `use_padding` lint rule #5122

Open ricardodalarme opened 2 weeks ago

ricardodalarme commented 2 weeks ago

Proposal for New Dart Lint Rule

TLDR

Add lint rule to avoid using Container with only a margin property.

Motivation

Using Container with only a margin property introduces an unnecessary widget in the widget tree, as Container adds a range of unused functionalities (decoration, padding, alignment) in this case. Replacing Container with Padding improves performance and makes the widget tree more readable. Padding is also a constant widget, which tends to be more performative.

Alternatives

Instead of creating a new rule, this could be added as an enhancement to the existing avoid_unnecessary_containers rule.

Lint Rule Implementation

Code Examples

Bad
Container(
  margin: EdgeInsets.all(8),
  child: Text("Hello, World!"),
);
Good
Padding(
  padding: EdgeInsets.all(8),
  child: Text("Hello, World!"),
);
dart-github-bot commented 2 weeks ago

Summary: This issue proposes a new Dart lint rule, prefer_padding_over_margin_container, to warn developers when using Container with only a margin property, suggesting the use of Padding instead. This is because Padding is more performant and results in a cleaner widget tree.

srawlins commented 1 week ago

CC @gspencergoog what do you think of this suggested lint rule? Would flutter recommend this rule?

gspencergoog commented 1 week ago

It's just another type of unnecessary Container, so my first inclination was to add it to avoid_unnecessary_containers.

However, it looks like we already have one for Containers that only have a color (ColoredBox) (use_colored_box) or only have a decoration (replace with DecoratedBox)(use_decorated_box), so maybe the granularity of the lint messages is finer than I thought. In that case, we should probably add one for each of these cases too:

ricardodalarme commented 1 week ago

We could also extend this behavior to other props as well:

gspencergoog commented 1 week ago

We also have sized_box_for_whitespace, but we should probably also recommend SizedBox if the container only has width and/or height set. (called use_sized_box?)