member-ordering rule, alphabetize not working on getters and setters

[t-cst]

Bug Report

member-ordering rule seems not to work properly on getters and setters.

• TSLint version: 5.10.0
• TypeScript version: 2.9.2
• Node version: v8.11.1
• Running TSLint via: CLI / VSCode

TypeScript code being linted

class Test {
  public static a;
  public static b;

  public method1(): any {
    //
  }

  public method2(): any {
    //
  }

  private get k() {  // no error here
    return 2;
  }

  public get e() { // no error here
      return 2;
  }

  public get c() { // no error here
      return 2;
  }

  public set f(value: any) {
    //
  }
}

with tslint.json configuration:

{
  "defaultSeverity": "error",
  "extends": [
    "tslint:recommended"
  ],
  "rules": {
    "member-ordering": [true, {
      "alphabetize": true,
      "order": "statics-first"
    }]
  }
}

Actual behavior

No error

$ node_modules/.bin/tslint Test.ts
$ echo $?
0

Expected behavior

tslint should report errors like:

• 'c' should come alphabetically before 'e'
• Declaration of public instance method not allowed after declaration of private instance method.

Maybe this issue is somehow related to:

• member-access rule not working on getters and setters member-access rule not working on getters and setters #750
• Fix member-access false positive for functions in ES6 object literals #612 Fix #612 #801

[NaridaL]

I'd be willing to fix this once #3935 is merged.

[NaridaL]

As far as I can tell, getters and setters are currently being completely ignored. I could add separate categories "publicStaticGetter", "privateInstanceSetter" etc, or just treat getters and setters as instance variables.

I think, in general, you'd want getter and setter for the same member to be together, but maybe want to enforce that getters come before setters. Unfortunately the current framework doesn't really allow this, as alphabetization is done last. So you'd have either:

// getters have lower rank than setters:
get a() {}
get b() {}
set a(x) {}
set b(x) {}

// OR: getters have same rank as setters, alphabetized:
// corresponding getters and setters are together, however:
get a() {}
set a(x) {}
set b(x) {}
get b() {}
// getter after setter, but no warning.

The above behavior could be acceptable... or I could just build in a hack and have "get a" be alphabetically be before "set a". Would anyone want setters before getters? Also, it might lead to weird errors messages...

Any thoughts?

[NaridaL]

Not exactly intuitive, but does work in 99% of cases...

[t-cst]

Many thanks for taking the time to look at this issue.

I think it would be interesting to have separate categories, as you comment. So they can be grouped separately from instance variables.

I like to have getters and setters together, but it is a personal preference, although I would say is the most common case.

I would prefer:

get a() {}
set a(x) {}
set b(x) {}
get b() {}

over:

get a() {}
get b() {}
set a(x) {}
set b(x) {}

It would be a significant improvement over current behavior. And I think it covers the scope of the current issue.

Ideally, a perfect result would be:

get a() {}
set a(x) {}
get b() {}
set b(x) {}

but if it's hacky to achieve, maybe it's better to leave it as it is. Similar behavior happens with function overloads, where no particular order guaranteed.

public foo(a: string, b: string): void;
public foo(a: number): void;
public foo(a: any, b?: string): void {
  return;
}
// or
public foo(a: number): void;
public foo(a: string, b: string): void;
public foo(a: any, b?: string): void {
  return;
}

Being able to take into account the method parameters when ordering would be cool. Mainly for overloading. But I think this is another enhancement/issue.

[NaridaL]

Sorry if I wasn't clear above^^. The two examples would both be possible if separate categories were added for the getters and setters, the result are with alphabetization enabled or disabled.

The order of overloads changes the semantics of the function, and as such should definitely not be changed by this rule.

[NoizyCr1cket]

Any news on this? I'm looking forward to having this feature.