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
Proposed Rule: Enforce lines between class methods #5949
Comments
I think that this rule should be able to handle comments as well, the following pattern should be considered valid: class Test4 {
constructor () {
// ...
}
// blah blah
otherFunc () {
// ...
}
} |
Come to think about it, I think that class Test5 {
constructor () {
// ...
}
// Comment
otherFunc () {
// ...
}
} |
Sounds like a reasonable request. @eslint/eslint-team thoughts? |
There may be some overlapping with #3092 |
@LinusU can you please spell out your complete proposal? It's a bit hard to follow when separated across multiple comments. |
Absolutely, sorry about that... (edited the first post) |
Thanks. Per our guidelines, we need someone on the team to champion the rule and three 👍s. |
Sounds good, I'd be happy to assist if there is anything needed from me 👌 |
I'd say this belongs as a configuration option of #3092 |
I'm not sure that I agree with that, it seems like it's going to be a very complex options object if we put everything into that rule. This might be desirable though, I'm not familiar enough with the project to tell. The thing I like by this rule is that it plays nicely with other rules already in place, including padded-blocks. class T {
// padded-blocks
constructor () {
// padded-blocks
this.x = 5
this.y = 10
// padded-blocks
}
// lines-between-class-methods
moveRight () {
// padded-blocks
this.x += 10
// padded-blocks
}
// lines-between-class-methods
moveDown () {
// padded-blocks
this.y += 10
// padded-blocks
}
// padded-blocks
} The style guide that we wanted to implement in standard would have class T {
constructor () {
this.x = 5
this.y = 10
}
moveRight () {
this.x += 10
}
moveDown () {
this.y += 10
}
} Which in my opinion looks very nice. For that to be supported with padded-blocks there would have to be a separate setting for spaces between class methods, which would be this exact rule but as an option. I personally think that it would clutter up the padded-blocks rule, but that's of course only my opinion. Thoughts? |
@eslint/eslint-team Anyone willing to champion this and support it? |
Maybe I'm willing. I think this is one of JSCS compatibility rules, for a part of requirePaddingNewLinesAfterBlocks and disallowPaddingNewLinesAfterBlocks. Those rules enforce blank line style after all blocks, so it checks blank line(s) between class methods also. I have some concerns.
|
I'd be happy to add that, are you thinking something like
I personally would not like to enforce a blank line between every property if that lands. And I don't think that it's common to want that... Consider the following example, it would look very spare if it required a newline between the props... class Dog {
x = 0
y = 0
name = 'Barkly'
mood = 'Happy'
breed = 'American Cocker Spaniel'
origin = 'United States'
constructor () {
// ...
}
bark () {
// ...
}
} |
Thank you for the fast reply! I'm thinking something like: {
"lines-between-class-methods": ["error", "always" or "never"]
// or
"lines-between-class-methods": ["error", {
"multiLine": "always" or "never",
"singleLine": "always" or "never"
}]
}
I agreed.
|
I'm willing to champion. I think this rule should just consider class methods. The analogy is that we have rules for functions and separate rules for variable declarations, and keep them separate. |
Sounds good. 👍 |
@mikesherov feel free to tell me how you want me to update my pull request to get this landed 👍 |
Hmm, I'm OK that this name is kept, also. 👍 |
@kaicataldo did this get the required thumbs up? I definitely wouldn't want to spend time on this, if it's not going to be merged in. |
@rwwagner90 This issue is accepted, so we should accept any PRs submitted (as long as the author is willing to work with us when we suggest changes or request tests, etc.). By the way, you can just look for the "accepted" label in future to know if an issue has the required thumbs up. Otherwise, we use the "evaluating" label for issues that haven't yet met that requirement. Hope this helps! |
I‘ll work on this, perhaps this weekend.:( |
@aladdin-add if you have time, that would be great! If you need an extra hand, please reach out. |
@aladdin-add how is this going? Need any help? |
@rwwagner90 so sorry for the delay -- I have been so busy these days. it's almost finished, but for the docs~ |
@aladdin-add no problem at all! I just wanted to see if you needed help with anything. |
@aladdin-add would you like help with docs or anything? |
@rwwagner90 sure, could you help to improve the docs? my English is so poor.. 😂 |
I thought this was being worked on... Anything new? |
@LinusU well that is some good news! Thanks! 👍 |
Was there any issue created to track the comment made by @kaicataldo about requiring padding lines between object members? |
do you mean #9048 ? |
@aladdin-add thanks. I'm not sure if that's about adding padding lines between object members or just padding at the start and end of objects but I'll check in that thread |
Does this only work for classes and not objects? |
Yes, #9048 is the issue about adding this for objects |
@bwatson3 that seems to be padded objects, not lines between methods in objects, but it may work. |
This rule is only for classes. It was decided in the discussion above (see #5949 (comment)) that we should keep these rules separate. @rwwagner90 #9048 is the analogous proposal to |
I would like to propose a rule that enforces empty lines between class functions. When enabled and set to "always" it should make sure that there is at least one empty line between every method in a class declaration. If set to "never" the opposite would be true, that is, there should not be any empty lines at all between class methods.
A line with a comment on should not count as an empty line.
This rule should not concern itself with the class padding (empty lines before the first method and after the last method) since that is already taken care of by
padded-blocks
.It also shouldn't concern itself with how many empty lines that are present, since that is a job for
no-multiple-empty-lines
.This is a stylistic rule.
The following patterns would be considered problems:
The following pattern would be considered valid:
The text was updated successfully, but these errors were encountered: