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

New: rule lines-between-class-members(fixes #5949). #9141

Merged
merged 25 commits into from Oct 2, 2017
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions conf/eslint-recommended.js
Expand Up @@ -63,6 +63,7 @@ module.exports = {
"linebreak-style": "off",
"lines-around-comment": "off",
"lines-around-directive": "off",
"lines-between-class-members": "off",
"max-depth": "off",
"max-len": "off",
"max-lines": "off",
Expand Down
103 changes: 103 additions & 0 deletions docs/rules/lines-between-class-members.md
@@ -0,0 +1,103 @@
# require or disallow an empty line between class members (lines-between-class-members)

This rule improves readability by enforcing lines between class members. It will not check empty lines before the first member and after the last member, since that is already taken care of by padded-blocks.

## Rule Details

Examples of **incorrect** code for this rule:

```js
/* eslint lines-between-class-members: ["error", "always"]*/
class MyClass {
foo() {
//...
}
bar() {
//...
}
}
```

Examples of **correct** code for this rule:

```js
/* eslint lines-between-class-members: ["error", "always"]*/
class MyClass {
foo() {
//...
}

bar() {
//...
}
}
```

### Options

This rule has a string option and an object option.

String option:

* `"always"`(default) require an empty line after after class members
* `"never"` disallows an empty line after after class members

Object option:

* `"exceptAfterSingleLine": "false"`(default) **do not** skip checking empty lines after singleline class members
* `"exceptAfterSingleLine": "true"` skip checking empty lines after singleline class members

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

```js
/* eslint lines-between-class-members: ["error", "always"]*/
class Foo{
bar(){}
baz(){}
}

/* eslint lines-between-class-members: ["error", "never"]*/
class Foo{
bar(){}

baz(){}
}
```

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

```js
/* eslint lines-between-class-members: ["error", "always"]*/
class Foo{
bar(){}

baz(){}
}

/* eslint lines-between-class-members: ["error", "never"]*/
class Foo{
bar(){}
baz(){}
}
```

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

```js
/* eslint lines-between-class-members: ["error", "always", { exceptAfterSingleLine: true }]*/
class Foo{
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this example more clear? It's not obvious how it's different from the default option.

Maybe something like:

class Foo{
  bar(){} // single line class member
  baz(){
    // multi line class member
  }

  qux(){}
}

bar(){}
baz(){}
}
```

## When Not To Use It

If you don't want enforce empty lines between class members, you can disable this rule.

## Related Rules

* [padded-blocks](padded-blocks.md)
* [padding-line-between-statement](padding-line-between-statement.md)
* [requirePaddingNewLinesAfterBlocks](http://jscs.info/rule/requirePaddingNewLinesAfterBlocks)
* [disallowPaddingNewLinesAfterBlocks](http://jscs.info/rule/disallowPaddingNewLinesAfterBlocks)
5 changes: 5 additions & 0 deletions docs/rules/padded-blocks.md
Expand Up @@ -354,3 +354,8 @@ if (a) {
## When Not To Use It

You can turn this rule off if you are not concerned with the consistency of padding within blocks.

## Related Rules

* [lines-between-class-members](lines-between-class-members.md)
* [padding-line-between-statement](padding-line-between-statement.md)
104 changes: 104 additions & 0 deletions lib/rules/lines-between-class-members.js
@@ -0,0 +1,104 @@
/**
* @fileoverview Rule to check empty newline between class members
* @author 薛定谔的猫<hh_2013@foxmail.com>
*/
"use strict";

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

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: "require or disallow an empty line between class members",
category: "Stylistic Issues",
recommended: false
},

fixable: "whitespace",

schema: [
{
enum: ["always", "never"]
},
{
type: "object",
properties: {
exceptAfterSingleLine: {
type: "boolean"
}
},
additionalProperties: false
}
]
},

create(context) {

const options = [];

options[0] = context.options[0] || "always";
options[1] = context.options[1] || { exceptAfterSingleLine: false };

const ALWAYS_MESSAGE = "Expected blank line between class members.";
const NEVER_MESSAGE = "Unexpected blank line between class members.";

const sourceCode = context.getSourceCode();

/**
* Checks if there is padding between two tokens
* @param {Token} first The first token
* @param {Token} second The second token
* @returns {boolean} True if there is at least a line between the tokens
*/
function isPaddingBetweenTokens(first, second) {
return second.loc.start.line - first.loc.end.line >= 2;
}

/**
* Checks the given MethodDefinition node to be padded.
* @param {ASTNode} node The AST node of a MethodDefinition.
* @returns {void} undefined.
*/
function checkPadding(node) {
const body = node.body;

for (let i = 0; i < body.length - 1; i++) {

// only check padding lines after class members(skip empty).
if (body[i]) {

const curFirst = sourceCode.getFirstToken(body[i]);
const curLast = sourceCode.getLastToken(body[i]);
const comments = sourceCode.getCommentsBefore(body[i + 1]);
const nextFirst = comments.length ? comments[0] : sourceCode.getFirstToken(body[i + 1]);
const isPadded = isPaddingBetweenTokens(curLast, nextFirst);
const isMulti = !astUtils.isTokenOnSameLine(curFirst, curLast);
const skip = !isMulti && options[1].exceptAfterSingleLine;


if ((options[0] === "always" && !skip && !isPadded) ||
(options[0] === "never" && isPadded)) {
context.report({
node: body[i + 1],
message: isPadded ? NEVER_MESSAGE : ALWAYS_MESSAGE,
fix(fixer) {
return isPadded
? fixer.replaceTextRange([curLast.range[1], nextFirst.range[0]], "\n")
: fixer.insertTextAfter(curLast, "\n");
}
});
}
}
}
}

return {
ClassBody: checkPadding
};

}
};
68 changes: 68 additions & 0 deletions tests/lib/rules/lines-between-class-members.js
@@ -0,0 +1,68 @@
/**
* @fileoverview Tests for lines-between-class-members rule.
* @author 薛定谔的猫<hh_2013@foxmail.com>
*/

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const rule = require("../../../lib/rules/lines-between-class-members");
const RuleTester = require("../../../lib/testers/rule-tester");

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

const ALWAYS_MESSAGE = "Expected blank line between class members.";
const NEVER_MESSAGE = "Unexpected blank line between class members.";

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });

ruleTester.run("lines-between-class-members", rule, {
valid: [
"class foo{}",
"class foo{\n\n}",
"class foo{constructor(){}\n}",
"class foo{\nconstructor(){}}",

"class foo{ bar(){}\n\nbaz(){}}",
"class foo{ bar(){}\n\n/*comments*/baz(){}}",
"class foo{ bar(){}\n\n//comments\nbaz(){}}",

{ code: "class foo{ bar(){}\nbaz(){}}", options: ["never"] },
{ code: "class foo{ bar(){}\n/*comments*/baz(){}}", options: ["never"] },
{ code: "class foo{ bar(){}\n//comments\nbaz(){}}", options: ["never"] },

{ code: "class foo{ bar(){}\n\nbaz(){}}", options: ["always"] },
{ code: "class foo{ bar(){}\n\n/*comments*/baz(){}}", options: ["always"] },
{ code: "class foo{ bar(){}\n\n//comments\nbaz(){}}", options: ["always"] },

{ code: "class foo{ bar(){}\nbaz(){}}", options: ["always", { exceptAfterSingleLine: true }] },
{ code: "class foo{ bar(){\n}\n\nbaz(){}}", options: ["always", { exceptAfterSingleLine: true }] }
],
invalid: [
{
code: "class foo{ bar(){}\nbaz(){}}",
output: "class foo{ bar(){}\n\nbaz(){}}",
options: ["always"],
errors: [{ message: ALWAYS_MESSAGE }]
}, {
code: "class foo{ bar(){}\n\nbaz(){}}",
output: "class foo{ bar(){}\nbaz(){}}",
options: ["never"],
errors: [{ message: NEVER_MESSAGE }]
}, {
code: "class foo{ bar(){\n}\nbaz(){}}",
output: "class foo{ bar(){\n}\n\nbaz(){}}",
options: ["always", { exceptAfterSingleLine: true }],
errors: [{ message: ALWAYS_MESSAGE }]
}
]
});