dart-lang / linter

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

proposal: `sort_child_method_arguements_last` #3317

Open SpencerC opened 2 years ago

SpencerC commented 2 years ago

sort_child_method_arguements_last

Description

Sort child arguments in widget construction helper methods.

Details

Similar to https://dart-lang.github.io/linter/lints/sort_child_properties_last.html, but for widget building helper methods.

Kind

style

Good Examples

Widget _buildCustomContainer({
  BuildContext context,
  Color? color,
  EdgeInsetsGeometry? padding,
  required Widget child,
}) {
  ...
}

@override
Widget build(BuildContext context) => _buildCustomContainer(
  context: context,
  color:  ColorName.backgroundBlack,
  padding:  const EdgeInsets.only(left: 8, top: 2),
  child: Container(
    <some long widget tree>
  ),
);

Bad Examples

Widget _buildCustomContainer({
  required Widget child,
  BuildContext context,
  Color? color,
  EdgeInsetsGeometry? padding,
}) {
  ...
}

@override
Widget build(BuildContext context) => _buildCustomContainer(
  child: Container(
    <some long widget tree>
  ),
  context: context,
  color:  ColorName.backgroundBlack,
  padding:  const EdgeInsets.only(left: 8, top: 2),
);

Discussion

In many instances in Flutter, it's more appropriate to define widgets rather than use builder helper methods. In these instances, projects can enable sort_child_properties_last to ensure that they're styled sensibly. However, in instances where helper methods are appropriate, there's not currently an option to ensure they are styled consistently with this rule.

While it's not out of the question that some developers might want to create non-rendering methods with child keyword arguments, I believe those picky enough to enable the sort_child_properties_last rule would probably also ask for different semantics in code reviews if they saw something like this on a Flutter project (speaking for myself here :innocent:). It might also make sense to enable this check just for methods that return Widgets.

Discussion checklist

pq commented 2 years ago

/fyi @goderbauer