Enforce consistent order of class members

[jibiel]

Consistently structured class declarations are often regarded as not important but they sure improve code readability and a definite must-have for teams.

The most comprehensive guide on this topic is probably Style guide for Flutter repo. The suggested default order is quite detailed and potentially could be implemented as a conventional Flutter Style rule.

However not everyone may find such a structure appropriate for their project. For example, I find it odd that private methods used in build should be listed before it. I'm new to Dart and honestly don't know if the Linter supports configurable rules, but in the ideal world it could look something like this:

 # Uses flutter repo style by default

- class_members_ordering

# Custom order that potentially could be specified separately for different paths,
# e.g. `lib/models`, `lib/widgets` etc.

class_members_ordering:
  - constructors
  - factories
  - instance_variables
  # etc.

The rule is not specific to any Dart framework.

[bwilkerson]

Thanks for the request!

The linter doesn't support configurable rules. We've talked about adding support before but haven't yet done so.

One of the reasons we don't have a lint to enforce ordering is the lack of consensus as to what the ordering ought to be. Without some way for different teams to define their own preference and a lack of consensus within the community it would be difficult to know what order to support.

Within IntelliJ (I'm not sure about VS Code) there is a "Sort Members in Dart File" command. The team that works on the Dart analyzer and analysis server use that command to enforce an order on the code. It's a bit simpler, grouping declarations by kind and then by name. I mention it because there is at least some level of tooling support for it which might be useful for you.

[pq]

Thanks @jibiel for the request and Brian for your response! FWIW, here's the issue tracking configurability, in general, as a feature: https://github.com/dart-lang/linter/issues/883. There's some additional context there. Feel free to chime in! (You may also find this long conversation about support for custom rules interesting: https://github.com/dart-lang/linter/issues/697.)

As for the request in general, I'd personally very much like a sort-order lint. For my (personal) purposes, one that simply agreed with the order enforced by the "Sort Members in Dart File" command would be a great step. (It's interesting to note that we have an extra step of analysis to enforce this in the analyzer codebase. See: verify_sorted_test. With a lint we could make this go away and it would be easier for others to benefit.)

Aside: I just yesterday introduced some unwanted churn in the linter codebase that would have been avoided if we were enforcing sorting on commit (see: #2362).

[jibiel]

@bwilkerson Turns out we also have "Dart: Sort Members" within VS Code via Dart extension. But it produces strange results:

• Placing static constants/variables and final fields before a constructor (that's actually how I've would've organized it before I was introduced to the Flutter repo style guide) but with what looks like random blank lines that mess grouped static/final lines.
• Removing blank lines between constructors and factories.
• Removing blank lines between import groups (I like to separate external and internal imports).
• Placing initState after build.

@pq On it. :neckbeard:

[incendial]

@jibiel Hi, we are developing additional linting tool for dart https://dcm.dev/ and have configurable member ordering rule https://dcm.dev/docs/rules/common/member-ordering/. It would be great if u could take a look. Any feedback is welcome

[NatoBoram]

Similar to directives_ordering, there should be a linter rule to sort members according to the Dart Style Guide and one according to the Flutter Style Guide.

It doesn't need any configuration since that would go against the reasons why linter rules aren't configurable in the first place. Sure, I disagree with lots of the reasoning that both styles have for their chosen ordering, but their goal isn't to satisfy me personally so that's fine. There's no consensus on spaces vs tabs but dartfmt still picks one and forces it on everyone and that's fine.

The current patchwork of using an external extension (even if it's first-party) to do it isn't viable if it's something that a team wants to be enforced via CI.

I think this issue should be focused specifically on member_ordering & flutter_member_ordering without considering potential configuration :

linter:
  rules:
    member_ordering: true

linter:
  rules:
    flutter_member_ordering: true

[rumax]

Are there any updates or plans regarding the implementation of sorting? For further inspiration, you can refer to the documentation on sorting components in the eslint-plugin-react repository on GitHub: eslint-plugin-react - sort-comp.md or TypeScript member-ordering

[srawlins]

No updates and no plans that I know of.

[pq]

No updates but this is still something I'd very much like to see happen. As I mentioned in https://github.com/dart-lang/linter/issues/2367#issuecomment-741784173, the absence of this in lint form leads to ad hoc solutions and lots of needless churn. I do wish we could converge on one agreed ordering (and admit that's part of why I haven't pushed harder on this) but could get over that I guess.

@goderbauer: I'm curious, from the flutter perspective how valued this enforcement would be?

[goderbauer]

So far, this hasn't been a huge problem in the Flutter repository. I can't remember the last time I had to ask in a code review to change the order to match Flutter's style. So, while this would be nice to have - and it would be interesting to see where we are currently breaking our own rules - I wouldn't consider it very high priority for us.

[rumax]

I would like to bring to your attention that according to the Flutter Style Guide, it is recommended to have the mentioned rule in place.

If there’s a clear lifecycle, then the order in which methods get invoked would be useful, for example an initState method coming before dispose. This helps readers because the code is in chronological order, so they can see variables get initialized before they are used, for instance. Fields should come before the methods that manipulate them, if they are specific to a particular group of methods.

If there are no current priorities or plans to incorporate this rule, it would be helpful to officially close the issue with a clear explanation. This will ensure that there are no false expectations or lingering anticipation regarding the implementation of the rule.

[pq]

I'm not in a rush to close this one. I do think it will happen, I just don't know when. It may get easier to implement soon too as we're looking at re-structuring the linter sources to live closer to the analyzer bits they're built on.

That said, I do know it's a drag. I'm with you!

[lrhn]

Worth noting that dart-style doesn't have a recommend ordering of members. The only thing it specifies ordering for is imports.

And there is no real consensus either. (

The only ordering I go by is:

1. Public static final/const variables. Possibly constants before non-constants, but it's rare to have public non-constants.
2. Instance variables. Semantic ordering (related variables grouped).
3. Constructors. Public before private.

The rest is below (and maybe some more statics above).

[pq]

Worth noting that dart-style doesn't have a recommend ordering of members.

Agreed. This makes green-lighting an implementation harder.

Practically speaking, it's worth noting that we do kind of a notion of a canonical ordering as implemented in the "sort members" action.

The only ordering I go by is:

If I'm not mistaken, the "sort members" ordering is consistent with this.

@scheglov?

[scheglov]

Yes, "Sort Members" has more details, but for the listed declarations it does mostly as described above. I fully agree with semantic ordering of instance variables. By similar logic we also don't sort constants.

See https://github.com/dart-lang/sdk/blob/463c7cb5a2fdedf1022e3e71db9b79aa5bc8849e/pkg/analysis_server/lib/src/services/correction/sort_members.dart#L15 for details.

[jakemac53]

I would really love this lint so we could replace the verify_sorted_test in the analyzer try jobs with it. I don't work in that codebase often so I don't know about the requirement to sort (and always forget). This leads to a large amount of churn (and many hours of delay) in landing cls because I don't see the failure until the bots run this test. It doesn't help that this bot seems to be the slowest one, so the failure shows up very late.

So something that would show up in the IDE would be a large improvement here in the workflow and reduce the tech debt a lot for casual contributors 👍 .