sort_constructors_first applies for all members

[a14n]

According to Flutter style guide: Constructors come first in a class.

The default (unnamed) constructor should come first, then the named constructors. They should come before anything else (including, e.g., constants or static methods).

[alexeieleusis]

I still prefer instance and static fields before that, not to mention that the analysis server sorts them that way. Even though there are some things I don't like in the way the tool sorts the code, I prefer using an automated tool than doing it manually.

Should this be a new rule specific to flutter? Should the two tools be consistent with one another?

[a14n]

Actually sort_constructors_first was done to enforce this flutter rule (see this comment from @pq). So this PR could be seen as a fix ;)

However I understand that this change could bring some bad feedback because it currently works quite differently. Perhaps it should be an additional rule.

And like many style rules it's a matter of taste.

[a14n]

btw regarding members ordering you can look at https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#order-other-class-members-in-a-way-that-makes-sense to see the recommanded default order ;)

[alexeieleusis]

You are right, I guess there is no reason to use this rule for people that use a tool to sort the code.

[aemino]

Perhaps we could have two rules, one for Flutter style, and one for people who still want to enforce a different kind of member sorting on their codebase?

[a14n]

I'm sympathetic to have a new rule to enforce Flutter style and to leave sort_constructors_first unchanged. Any idea for the name of this new rule?

[aemino]

Perhaps it could be called sort_members_by_lifecycle?

[a14n]

IMHO it should be sort_constructors_first (Flutter style) and sort_constructors_before_methods (the current sort_constructors_first) but it's breaking.

I don't think sort_members_by_lifecycle works because its name suggests lints on more members than only constructors.

[pq]

@a14n : do we have a way forward for this one? What's most pressing for Flutter?

[a14n]

Oops I forgot this pending PR. Thanks for the reminder.

We could land it and see later if we need to add a sort_constructors_before_methods if some people report problems with this change. WDYT ?

[pq]

Works for me but might put it past @Hixie first since he motivated the initial implementation, though I'm guessing he'd agree this is in the original spirit.

[Hixie]

Definitely would like a lint to push constructors before fields (static or otherwise). I could see an argument for constructors after fields if we required all fields to come before the constructors, but that makes it much harder to read the code (since then getters and setters are far from their fields, etc).

The constructor coming first makes it much, much easier to tell if there is an implied constructor, because you only have to look at the top and see if there's a constructor to know. If you have a ton of fields, you have to look through the code to spot where they end before you can tell, which makes it much easier for bugs to sneak in (e.g. because you intended for there to be a constructor but you named it wrong so it's actually a method, or whatever).

[bwilkerson]

@pq We really need to either land or close this PR.

[a14n]

ping

[pq]

nit: missing trailing newline

[pq]

Sorry! If this works for @Hixie, I'm totally good with it. (And in fact, would have saved me some time in a recent PR he reviewed 😬.)

[pq]

🚀

[a14n]

Thanks :)