tslint-clean-code icon indicating copy to clipboard operation
tslint-clean-code copied to clipboard

newspaper-order ignores member-ordering rule (e.g. private after public) mutually excluding themselves.

Open adlh opened this issue 7 years ago • 3 comments

adlh avatar Dec 02 '17 09:12 adlh

Please provide a quick example.

Even better would be adding a test to https://github.com/Glavin001/tslint-clean-code/blob/master/src/tests/NewspaperOrderRuleTests.ts .

Thanks in advance!

Glavin001 avatar Dec 02 '17 14:12 Glavin001

@Glavin001 ok, I added a test that demonstrates the problem. Hier is the commit

It took me a while to understand the problem and write an isolated test :-) ... So the problem is, when the first 2 public methods (get and set use a protected method getField) and then between those public methods and the protected getField, there is another public method. The rule then wants to move getField before that 3d public method:

  1) newspaperOrderRule ClassDeclaration should respect member-order when defining order:
     AssertionError: Wrong # of failures: 
[
  {
    "failure": "The class does not read like a Newspaper. Reorder the methods of the class: HasProtectedFieldsClass\n\nMethods order:\n1. ✓ get\n2. ✓ set\n3. x getField\n4. x anotherPublicMethod",
    "name": "file.ts",
    "ruleName": "newspaper-order",
    "ruleSeverity": "ERROR",
    "startPosition": {
      "character": 17,
      "line": 15
    }
  }
]: expected 1 to equal 0

adlh avatar Dec 06 '17 12:12 adlh

🎉 This is great, thank you!

I looked at TSLint and could not figure out how to have one rule acknowledge another rule being enabled.

I think the best route may be to add configuration options for newspaper-order rule. Looking at https://palantir.github.io/tslint/rules/member-ordering/ there are a lot of options / possible configurations to support.

The applicable place to make a change would be around closestList: https://github.com/Glavin001/tslint-clean-code/blob/master/src/newspaperOrderRule.ts#L358-L377 . Instead of only trying to find the list of methods which results in minimal changes, also consider the member ordering configuration to filter out undesirable orders. This would also mean we need a function which can look up the access modifiers (e.g. public, private) for a method name.

I won't have time to investigate this in great detail until the weekend. If you'd like to continue working on a fix in a Pull Request, let me know! Pull Requests are very welcome! 😃

Glavin001 avatar Dec 06 '17 14:12 Glavin001