Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: sort_properties_then_constructors #58923

Open
5 tasks
bsutton opened this issue Nov 9, 2022 · 8 comments
Open
5 tasks

proposal: sort_properties_then_constructors #58923

bsutton opened this issue Nov 9, 2022 · 8 comments
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-lint-proposal linter-status-pending P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@bsutton
Copy link

bsutton commented Nov 9, 2022

sort_properties_then_constructors

Description

Sort properties then constructors then other members

Details

My understanding is that the sort_constructors_first and the sort_unnamed_constructors_first rules are motivated by the flutter style guide:

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

This helps readers determine whether the class has a default implied constructor or not at a glance. If it was possible for a constructor to be anywhere in the class, then the reader would have to examine every line of the class to determine whether or not there was an implicit constructor or not.

The problem with the style guide is that it is trying to solve a problem that doesn't exist.

If I'm constructing a instance I'm using the IDE's autocomplete feature which you can see from the following vscode screen grab clearly highlights the set of available constructors.

image

The statement that the user 'have to examine every line' is also untrue given that they know the name of the constructor and a simple search will find each constructor.

As such I never open a library to look for constructors nor any other method.

If I'm opening a source file its because I don't understand what the code is doing.

I would typically ctrl-click through a method to look at a specific method and from their I'm probably going to be looking at the state of the class and hence the set of fields.

As such I've always viewed the set of fields as the most important piece in a source file.

The fields give a user a quick idea of what the class can hold which is far more informative then the list of ctors.

The result is that I've always sorted my code as:

consts
fields
unamed ctors
name ctors
methods

The current sorting method is particularity problematic as it makes finding fields visually hard as they are typically in the middle of the two blocks of code (ctors and methods).
I'm also not aware of any lint that tries to ensure the fields are grouped (which probably should be something we discuss) with the result that they can be scattered across the class making them particualary hard to find and there is no easy to search for the field (or set of fields) if you don't know its name.

Kind

Enforces style advise.

Good Examples

class A {
const defaultName = "A";
String a;
int b;
A() ;
A.from();
void doSomething();
}

Bad Examples

class A {
A() ;
A.from();
void doSomething();
String a;
int b;

}

class A {
A() ;
A.from();
String a;
void doSomething();
int b;
}

Discussion

sort_constructors_first rule: accessors proposal #57651
This issue takes about putting properties first.

Discussion checklist

  • List any existing rules this proposal modifies, complements, overlaps or conflicts with.
  • List any relevant issues (reported here, the [SDK Tracker], or elsewhere).
  • If there's any prior art (e.g., in other linters), please add references here.
  • If this proposal corresponds to [Effective Dart] or [Flutter Style Guide] advice, please call it out. (If there isn't any corresponding advice, should there be?)
  • If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.
@bsutton
Copy link
Author

bsutton commented Nov 9, 2022

Just a follow up thought.
My suggested lint still delivers on the objective of the flutter style guide in that the ctors are still easy to find as they are in the first block of code after the fields and will typically be visible when you first open the source file at the worst a short scroll down.

@srawlins
Copy link
Member

srawlins commented Nov 9, 2022

The problem with the style guide is that it is trying to solve a problem that doesn't exist.

Lol, this is exactly what I was thinking as I read what you quoted. I don't get it.

In any case, we cannot support any new style-oriented rules in the linter which are not backed by either Effective Dart or the Flutter style guide. I think you'll have to make your case there before we land a rule.

@bsutton
Copy link
Author

bsutton commented Nov 9, 2022

I think you'll have to make your case there before we land a rule.
So how does one go about that?

Can we create a lint that isn't in the effective or flutter style guide?
I personally use neither of these and I note that there are a no. of alternate third party lint packages available.

@srawlins
Copy link
Member

srawlins commented Nov 9, 2022

Can we create a lint that isn't in the effective or flutter style guide?

Not a style-based lint rule, no. It becomes too much maintenance burden, both in terms of new language features (for "enhanced enums", the static analysis team would be responsible for updating the rule to work with enums; if constructors become allowed on mixins..., etc.), and in terms of bugs filed and feature requests. The static analysis team can't field questions or false positive or false negative bugs for a style that isn't backed by any consistent authoritative source.

@incendial
Copy link
Contributor

@bsutton please take a look at https://dartcodemetrics.dev/docs/rules/common/member-ordering - this might be exactly what are you looking for

@bsutton
Copy link
Author

bsutton commented Nov 10, 2022

@incendial thanks for the link, that does look like it would do the job.

It would still be nice to see the style guide change because it actually makes libraries harder to explore.

@bsutton
Copy link
Author

bsutton commented Nov 10, 2022

@srawlins so what forum is used to lobby for a change in the style guides.

@srawlins
Copy link
Member

I'm not sure for the Flutter style guide. Maybe a chat or an issue on flutter/flutter?

And then for the Dart style guide, Effective Dart, I think an issue at site-www is typically how we hold these conversations.

But I don't think we enforce any ordering in Effective Dart. Flutter might be your best bet.

@pq pq added the P3 A lower priority bug or feature request label May 22, 2023
@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Mar 27, 2024
@devoncarew devoncarew added analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-lang/linter Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-lint-proposal linter-status-pending P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants