-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
15eaef4
New: rule lines-between-class-methods(fixes #5949).
aladdin-add b7a73d5
add options: multiline & singleline.
3c969b0
add --fix
0b9fbb6
rename variable & add more tests.
f46a8d1
Docs: lines-between-class-methods
02c8523
multiLine => multiline.
04e3786
Update lines-between-class-methods.js
aladdin-add 979582f
rename rulename.
aladdin-add 0d86820
Fix: should consider comments before methods.
aladdin-add 7fa4d84
opt.
aladdin-add 71517ea
Update lines-between-class-members.md
aladdin-add 3f9d163
rename rulename.
aladdin-add bdc5da7
Fix: accept review suggestions.
aladdin-add 9696eb4
update reporting node.
aladdin-add 11414a0
rename to padding-after-class-members.
aladdin-add 6fb4dcf
update: only check padding lines after methods def.
aladdin-add 8a376d0
Docs: update rule doc related to padded-blocks.
aladdin-add 80cb47e
Update padding-line-after-class-members.md
aladdin-add 0f24f9a
rename rulename.
af58efc
update options.
42ea9ee
modify options & update docs.
8601163
update docs.
aladdin-add 286d1ae
finished.:)
aladdin-add 971e15d
Update lines-between-class-members.md
aladdin-add 870e2e5
Chore: rm unused check & add tests.
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
# 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{ | ||
bar(){} // single line class member | ||
baz(){ | ||
// multi line class member | ||
} | ||
|
||
qux(){} | ||
} | ||
``` | ||
|
||
## When Not To Use It | ||
|
||
If you don't want to 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) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
/** | ||
* @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; | ||
} | ||
|
||
return { | ||
ClassBody(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]) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a test that covers this check? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @btmills I found this check is unused -- it is always true. removed it and added some tests. |
||
|
||
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"); | ||
} | ||
}); | ||
} | ||
} | ||
} | ||
} | ||
}; | ||
} | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 }] | ||
} | ||
] | ||
}); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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: