Skip to content

Commit

Permalink
feat: add new enforce option to lines-between-class-members (#17462)
Browse files Browse the repository at this point in the history
* feat: add new \`enforce\` option (`lines-between-class-members`)

* test: add cases for enforce with exceptAfterSingleLine option

* docs: add `enforce` option

* docs: fix example

* fix: update schema to make enforce option required

* refactor: remove redundant if condition

* refactor: remove redundant if condition

* docs: add suggestions

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* test: add cases where multiple config objects match a pair

---------

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
  • Loading branch information
snitin315 and mdjermanovic committed Sep 2, 2023
1 parent 032c4b1 commit acb7df3
Show file tree
Hide file tree
Showing 3 changed files with 2,714 additions and 41 deletions.
185 changes: 182 additions & 3 deletions docs/src/rules/lines-between-class-members.md
Expand Up @@ -69,14 +69,19 @@ class MyClass {

### Options

This rule has a string option and an object option.
This rule has two options, first option can be string or object, second option is object.

String option:
First option can be string `"always"` or `"never"` or an object with a property named `enforce`:

* `"always"`(default) require an empty line after class members
* `"never"` disallows an empty line after class members
* `Object`: An object with a property named `enforce`. The enforce property should be an array of objects, each specifying the configuration for enforcing empty lines between specific pairs of class members.
* **enforce**: You can supply any number of configurations. If a member pair matches multiple configurations, the last matched configuration will be used. If a member pair does not match any configurations, it will be ignored. Each object should have the following properties:
* **blankLine**: Can be set to either `"always"` or `"never"`, indicating whether a blank line should be required or disallowed between the specified members.
* **prev**: Specifies the type of the preceding class member. It can be `"method"` for class methods, `"field"` for class fields, or `"*"` for any class member.
* **next**: Specifies the type of the following class member. It follows the same options as `prev`.

Object option:
Second option is an object with a property named `exceptAfterSingleLine`:

* `"exceptAfterSingleLine": false`(default) **do not** skip checking empty lines after single-line class members
* `"exceptAfterSingleLine": true` skip checking empty lines after single-line class members
Expand Down Expand Up @@ -129,6 +134,146 @@ class Foo{

:::

Examples of **incorrect** code for this rule with the array of configurations option:

::: incorrect

```js
// disallows blank lines between methods
/*eslint lines-between-class-members: [
"error",
{
enforce: [
{ blankLine: "never", prev: "method", next: "method" }
]
},
]*/

class MyClass {
constructor(height, width) {
this.height = height;
this.width = width;
}

fieldA = 'Field A';
#fieldB = 'Field B';

method1() {}

get area() {
return this.method1();
}

method2() {}
}
```

:::

::: incorrect

```js
// requires blank lines around fields, disallows blank lines between methods
/*eslint lines-between-class-members: [
"error",
{
enforce: [
{ blankLine: "always", prev: "*", next: "field" },
{ blankLine: "always", prev: "field", next: "*" },
{ blankLine: "never", prev: "method", next: "method" }
]
},
]*/

class MyClass {
constructor(height, width) {
this.height = height;
this.width = width;
}
fieldA = 'Field A';
#fieldB = 'Field B';
method1() {}

get area() {
return this.method1();
}

method2() {}
}
```

:::

Examples of **correct** code for this rule with the array of configurations option:

::: correct

```js
// disallows blank lines between methods
/*eslint lines-between-class-members: [
"error",
{
enforce: [
{ blankLine: "never", prev: "method", next: "method" }
]
},
]*/

class MyClass {
constructor(height, width) {
this.height = height;
this.width = width;
}

fieldA = 'Field A';

#fieldB = 'Field B';

method1() {}
get area() {
return this.method1();
}
method2() {}
}
```

:::

::: correct

```js
// requires blank lines around fields, disallows blank lines between methods
/*eslint lines-between-class-members: [
"error",
{
enforce: [
{ blankLine: "always", prev: "*", next: "field" },
{ blankLine: "always", prev: "field", next: "*" },
{ blankLine: "never", prev: "method", next: "method" }
]
},
]*/

class MyClass {
constructor(height, width) {
this.height = height;
this.width = width;
}

fieldA = 'Field A';

#fieldB = 'Field B';

method1() {}
get area() {
return this.method1();
}
method2() {}
}
```

:::

Examples of **correct** code for this rule with the object option:

::: correct
Expand All @@ -148,6 +293,40 @@ class Foo{

:::

::: correct

```js
/*eslint lines-between-class-members: [
"error",
{
enforce: [
{ blankLine: "always", prev: "*", next: "method" },
{ blankLine: "always", prev: "method", next: "*" },
{ blankLine: "always", prev: "field", next: "field" }
]
},
{ exceptAfterSingleLine: true }
]*/

class MyClass {
constructor(height, width) {
this.height = height;
this.width = width;
}

fieldA = 'Field A';
#fieldB = 'Field B';
method1() {}
get area() {
return this.method1();
}

method2() {}
}
```

:::

## When Not To Use It

If you don't want to enforce empty lines between class members, you can disable this rule.
Expand Down
99 changes: 92 additions & 7 deletions lib/rules/lines-between-class-members.js
Expand Up @@ -10,6 +10,21 @@

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

/**
* Types of class members.
* Those have `test` method to check it matches to the given class member.
* @private
*/
const ClassMemberTypes = {
"*": { test: () => true },
field: { test: node => node.type === "PropertyDefinition" },
method: { test: node => node.type === "MethodDefinition" }
};

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand All @@ -29,7 +44,32 @@ module.exports = {

schema: [
{
enum: ["always", "never"]
anyOf: [
{
type: "object",
properties: {
enforce: {
type: "array",
items: {
type: "object",
properties: {
blankLine: { enum: ["always", "never"] },
prev: { enum: ["method", "field", "*"] },
next: { enum: ["method", "field", "*"] }
},
additionalProperties: false,
required: ["blankLine", "prev", "next"]
},
minItems: 1
}
},
additionalProperties: false,
required: ["enforce"]
},
{
enum: ["always", "never"]
}
]
},
{
type: "object",
Expand All @@ -55,6 +95,7 @@ module.exports = {
options[0] = context.options[0] || "always";
options[1] = context.options[1] || { exceptAfterSingleLine: false };

const configureList = typeof options[0] === "object" ? options[0].enforce : [{ blankLine: options[0], prev: "*", next: "*" }];
const sourceCode = context.sourceCode;

/**
Expand Down Expand Up @@ -144,6 +185,38 @@ module.exports = {
return sourceCode.getTokensBetween(before, after, { includeComments: true }).length !== 0;
}

/**
* Checks whether the given node matches the given type.
* @param {ASTNode} node The class member node to check.
* @param {string} type The class member type to check.
* @returns {boolean} `true` if the class member node matched the type.
* @private
*/
function match(node, type) {
return ClassMemberTypes[type].test(node);
}

/**
* Finds the last matched configuration from the configureList.
* @param {ASTNode} prevNode The previous node to match.
* @param {ASTNode} nextNode The current node to match.
* @returns {string|null} Padding type or `null` if no matches were found.
* @private
*/
function getPaddingType(prevNode, nextNode) {
for (let i = configureList.length - 1; i >= 0; --i) {
const configure = configureList[i];
const matched =
match(prevNode, configure.prev) &&
match(nextNode, configure.next);

if (matched) {
return configure.blankLine;
}
}
return null;
}

return {
ClassBody(node) {
const body = node.body;
Expand All @@ -158,22 +231,34 @@ module.exports = {
const isPadded = afterPadding.loc.start.line - beforePadding.loc.end.line > 1;
const hasTokenInPadding = hasTokenOrCommentBetween(beforePadding, afterPadding);
const curLineLastToken = findLastConsecutiveTokenAfter(curLast, nextFirst, 0);
const paddingType = getPaddingType(body[i], body[i + 1]);

if (paddingType === "never" && isPadded) {
context.report({
node: body[i + 1],
messageId: "never",

if ((options[0] === "always" && !skip && !isPadded) ||
(options[0] === "never" && isPadded)) {
fix(fixer) {
if (hasTokenInPadding) {
return null;
}
return fixer.replaceTextRange([beforePadding.range[1], afterPadding.range[0]], "\n");
}
});
} else if (paddingType === "always" && !skip && !isPadded) {
context.report({
node: body[i + 1],
messageId: isPadded ? "never" : "always",
messageId: "always",

fix(fixer) {
if (hasTokenInPadding) {
return null;
}
return isPadded
? fixer.replaceTextRange([beforePadding.range[1], afterPadding.range[0]], "\n")
: fixer.insertTextAfter(curLineLastToken, "\n");
return fixer.insertTextAfter(curLineLastToken, "\n");
}
});
}

}
}
};
Expand Down

0 comments on commit acb7df3

Please sign in to comment.