Proposed Rule: Enforce lines between class methods

[LinusU]

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:

class Test1 {
  constructor () {
    // ...
  }
  otherFunc () {
    // ...
  }
}

class Test2 {
  constructor () {
    // ...
  }
  // comment
  otherFunc () {
    // ...
  }
}

The following pattern would be considered valid:

class Test3 {
  constructor () {
    // ...
  }

  otherFunc () {
    // ...
  }
}

class Test4 {
  constructor () {
    // ...
  }



  otherFunc () {
    // ...
  }
}

class Test5 {
  constructor () {
    // ...
  }

  // comment
  otherFunc () {
    // ...
  }
}

class Test6 {
  constructor () {
    // ...
  }

  // Comment

  otherFunc () {
    // ...
  }
}

[LinusU]

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 () {
    // ...
  }
}

[LinusU]

Come to think about it, I think that always should allow multiple empty lines since there is another rule that can take care of that: no-multiple-empty-lines. Furthermore, it might be nice to allow the following:

class Test5 {
  constructor () {
    // ...
  }

  // Comment

  otherFunc () {
    // ...
  }
}

[gyandeeps]

Sounds like a reasonable request. @eslint/eslint-team thoughts?

[alberto]

There may be some overlapping with #3092

[nzakas]

@LinusU can you please spell out your complete proposal? It's a bit hard to follow when separated across multiple comments.

[LinusU]

Absolutely, sorry about that...

(edited the first post)

[nzakas]

Thanks. Per our guidelines, we need someone on the team to champion the rule and three 👍s.

[LinusU]

Sounds good, I'd be happy to assist if there is anything needed from me 👌

[mikesherov]

I'd say this belongs as a configuration option of #3092

[LinusU]

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 padded-blocks to never and lines-between-class-methods to always. So that it looks like this:

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?

[ilyavolodin]

@eslint/eslint-team Anyone willing to champion this and support it?

[mysticatea]

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.

• 2 options are needed for this proposed rule to reproduce the behavior of JSCS rule.
  • "never" option.
  • An option to separate settings of single line methods.
• We should consider our rules for requirePaddingNewLinesAfterBlocks and disallowPaddingNewLinesAfterBlocks in total. Maybe lines-between-class-members, lines-between-props, and lines-around-block? Or just lines-after-blocks?
• What about lines-between-class-members instead of lines-between-class-methods? Class property declarations might arrive stage 4 in future. The name of no-dupe-class-members rule was considered for that.

[LinusU]

2 options are needed for this proposed rule to reproduce the behavior of JSCS rule.

I'd be happy to add that, are you thinking something like "always", "always-excpet-single-line-methods", "never", or something like { "single-line": "never", "multi-line": "always" }?

What about lines-between-class-members instead of lines-between-class-methods?

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 () {
    // ...
  }
}

[mysticatea]

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 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...

I agreed.
But,

• what about between a method and a field?

• what about if there is a function expression in a field? (hmm, maybe this is other rule's responsibility?)

  class A {
      onChanged = (event) => {
          doSomething();
      }
  
      constructor() {
      }
  }

  We don't need to consider options for that now, but I'm afraid lines-between-class-methods might be a limited name. I'd like to hear other member's opinions also.

[mikesherov]

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.

[nzakas]

Sounds good. 👍

[LinusU]

@mikesherov feel free to tell me how you want me to update my pull request to get this landed 👍

[mysticatea]

Hmm, I'm OK that this name is kept, also.

👍

[RobbieTheWagner]

@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.

[platinumazure]

@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!

[aladdin-add]

I‘ll work on this, perhaps this weekend.:(

[RobbieTheWagner]

@aladdin-add if you have time, that would be great! If you need an extra hand, please reach out.

[RobbieTheWagner]

@aladdin-add how is this going? Need any help?

[aladdin-add]

@rwwagner90 so sorry for the delay -- I have been so busy these days.

it's almost finished, but for the docs~

[RobbieTheWagner]

@aladdin-add no problem at all! I just wanted to see if you needed help with anything.

[RobbieTheWagner]

@aladdin-add would you like help with docs or anything?

[aladdin-add]

@rwwagner90 sure, could you help to improve the docs? my English is so poor.. 😂

[helgeh]

I thought this was being worked on... Anything new?

[LinusU]

@helgeh it's landed on master 👉 ee99876

[helgeh]

@LinusU well that is some good news! Thanks! 👍

[ghost]

Was there any issue created to track the comment made by @kaicataldo about requiring padding lines between object members?

[aladdin-add]

do you mean #9048 ?

[ghost]

@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

[RobbieTheWagner]

Does this only work for classes and not objects?

[ghost]

Yes, #9048 is the issue about adding this for objects

[RobbieTheWagner]

@bwatson3 that seems to be padded objects, not lines between methods in objects, but it may work.

[kaicataldo]

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 lines-beween-class-members, but for objects.