Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add new enforce option to lines-between-class-members #17462

Merged
merged 9 commits into from Sep 2, 2023
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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some tests where multiple config objects match a pair, to confirm that the last match wins? I tried switching this to for (let i = 0; i < configureList.length; ++i) and all tests were still passing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in ea4fb7f

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