feat(eslint-plugin): sort members alphabetically

[timkraut]

Hi,

this PR adds a new rule to sort members in interfaces alphabetically. A use case for such a rule would be the following:

Use case

Sorting things can have certain benefits, e.g. fewer merge conflicts or easier manual comparison of implementations.

Currently, in ESLint, multiple things can be sorted alphabetically:

• sort-keys
• sort-vars
• sort-imports

Additionally, it is possible to sort members using member-ordering. Except if I didn't understand how to use the member-ordering rule correctly, currently, there is no option to sort members of interfaces alphabetically.

Let's say we have an interface which looks as follows:

interface Foo {
  B: string; // should come after `A`
  A: string;
  C: string;
}

The intended definition of this interface is:

interface Foo {
  A: string;
  B: string;
  C: string;
}

This PR adds a rule which detects these issues. This is probably something where we could also provide a fix. I just wanted to make sure this goes into the right direction before. Looks like there is an open issue to create a rule for this in TSLint: Feature Request: alphabetize interface members/fields. Let me know what you think!

[bradzacher]

what happens if you have this rule plus member ordering turned on?
They'll conflict, won't they?
I don't think we can put the plugin into a state where you can have two rules which can directly conflict. (I don't think we have that yet do we?)

It may be better to get this as an option in member-ordering.
That rule might need a revisit because it's pretty messy...

worth a discussion @typescript-eslint/core-team

[timkraut]

Thanks a lot for your ideas about how this might look like and your feedback for the code changes. This helped a lot!

I'll start with updating the documentation of the rule to be more clear how this all works and then I'll try to add the options to sort all members regardless of their type alphabetically or to sort them alphabetically inside of their groups.

[Fuzzyma]

@timkraut does a default value keeps me from writing a long list of types? Then yes! Often people dont have any particular order. They just want a consistent order. A default value can do that without any configuration overhead. Alternatively you can accept a string default instead of the array so the user has a choice

[timkraut]

@Fuzzyma Yes. The default config is the following:

export const defaultOrder = [
  // Index signature
  'signature',

  // Fields
  'public-static-field',
  'protected-static-field',
  'private-static-field',

  'public-instance-field',
  'protected-instance-field',
  'private-instance-field',

  'public-abstract-field',
  'protected-abstract-field',
  'private-abstract-field',

  'public-field',
  'protected-field',
  'private-field',

  'static-field',
  'instance-field',
  'abstract-field',

  'field',

  // Constructors
  'public-constructor',
  'protected-constructor',
  'private-constructor',

  'constructor',

  // Methods
  'public-static-method',
  'protected-static-method',
  'private-static-method',

  'public-instance-method',
  'protected-instance-method',
  'private-instance-method',

  'public-abstract-method',
  'protected-abstract-method',
  'private-abstract-method',

  'public-method',
  'protected-method',
  'private-method',

  'static-method',
  'instance-method',
  'abstract-method',

  'method',
];

[Fuzzyma]

I am all for it! Did I already say thank you that you continued working on this? :)

[codecov]

Codecov Report

Merging #263 into master will increase coverage by 0.09%.
The diff coverage is 99.00%.

@@            Coverage Diff             @@
##           master     #263      +/-   ##
==========================================
+ Coverage   94.64%   94.73%   +0.09%     
==========================================
  Files         160      160              
  Lines        7265     7300      +35     
  Branches     2082     2092      +10     
==========================================
+ Hits         6876     6916      +40     
+ Misses        167      164       -3     
+ Partials      222      220       -2     

Flag	Coverage Δ
#unittest	94.73% <99.00%> (+0.09%)	⬆️

Impacted Files	Coverage Δ
...ackages/eslint-plugin/src/rules/member-ordering.ts	99.14% <99.00%> (+6.46%)	⬆️

[bradzacher]

A few nit picks, but otherwise LGTM.
Thanks for seeing this through to the end.

[timkraut]

@bradzacher Thanks for going through all the code again! I've fixed the docs issues and rebased this branch against the current master

[bradzacher]

LGTM - thanks!

[timkraut]

Thanks for the helpful reviews!

[escape0707]

Hi! I would I like to know if there is a way to apply the rule like this:

"rules": {
  "@typescript-eslint/member-ordering": [
    "warn",
    {
      "default": { "memberTypes": "defaultOrder", "order": "alphabetically" }
    }
  ]
}

[bradzacher]

The docs do a good job of explaining how to configure the rule:
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/member-ordering.md#options

[escape0707]

@bradzacher Thanks for your reply, but I mean I checked and followed the doc but I still don't understand what's the <Default Order> part means. I think it's not a valid json notation, right?

Do I have to repeat the default orders again here?

[timkraut]

@escape0707 Yes, exactly. There is no magic string "use-default-order" or something like this. If you want to have the default order as written in the docs (this is copied directly from the source code btw.), you have to specify this manually (see https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/member-ordering.md#default-configuration for the default order)

[escape0707]

Okay! Thank you so much for your help!

…

On Mon, Apr 20, 2020 at 10:10 PM Tim Kraut ***@***.***> wrote: @escape0707 <https://github.com/escape0707> Yes, exactly. There is no magic string "use-default-order" or something like this. If you want to have the default order as written in the docs (this is copied directly from the source code btw.), you have to specify this manually (see https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/member-ordering.md#default-configuration for the default order) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#263 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACUBEWN7ROTXDFBXOC2ZSA3RNRJUTANCNFSM4GW5FUWA> .