object-literal-sort-keys: Allow grouping of object properties via additional blank lines

[Merott]

PR checklist

• Addresses an existing issue: object-literal-sort-keys: allow grouping with new lines #3190
• New feature, bugfix, or enhancement
  • Includes tests
• Documentation update

Overview of change:

Changes the behaviour of object-literal-sort-keys when using alphabetical ordering, and allows object keys to be grouped together separated with blank lines.

CHANGELOG.md entry:

[enhancement] object-literal-sort-keys: allow grouping of object properties via additional blank lines when using alphabetical ordering.

[palantirtech]

Thanks for your interest in palantir/tslint, @Merott! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

[ajafff]

This will also match for blank lines inside comments, for example:

let foo = {
    foo: 1,
    /* I need some space to express myself

       and here the comment continues */
    bar: 2,
};

You can have a look at newline-before-return to see how I would check for a blank line before, between or after comments (but not inside).
https://github.com/palantir/tslint/blob/master/src/rules/newlineBeforeReturnRule.ts#L62-L81

[ajafff]

please add one of these test cases to an existing test to check if the new behavior is not enabled without the option

[Merott]

Thank you @ajafff - I've pushed up an update based on your feedback.

[adidahiya]

This rule change makes sense to me. This would work similar to ordered-imports which allows import groups separated by newlines. In fact we might just want to make this the default behavior and not keep it behind a rule option, to be consistent with import statement linting behavior. Thoughts @ajafff @Merott?

[Merott]

It would make me personally happy to have this behaviour by default, similar toordered-imports as you mentioned. However, given that this behaviour could be considered a breaking change if it were to be applied by default, I thought about introducing it as an option instead.

I should also mention that ordered-imports doesn't support grouping with new lines within each import statement. Thet'd be closer in terms of syntax to what my proposed rule option here does, and is something that I'd also like to have.

[ajafff]

Making this the default behavior is not a breaking change. Code that passes now will also pass after this change.
I think this makes sense to be the default.

[Merott]

@ajafff OK - I will take out the option and update the PR accordingly.

[Merott]

Ready for a re-review 👍🏻.

[ajafff]

Code looks good with one suggestion regarding performance

[ajafff]

This can be pretty expensive when checked for each property. I'd prefer to only check this right before adding a failure

[Merott]

Done.

[ajafff]

Thanks @Merott