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 :+1: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. :+1:

[LinusU]

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

[mysticatea]

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

:+1:

[nzakas]

Still looking for one more 👍 To move forward

[IanVS]

Instead of "always" and "never" (or maybe in addition to), how about an object with min and max properties? That might be more flexible. For instance, I would like to enforce exactly one line between class methods, but I already have no-multiple-empty-lines set with a max of two empty lines (which I use in some cases to separate larger code blocks).

So, "always" would be an alias to {min: 1}, and "never" is equivalent to {max: 0}, but it would also be possible to have {min: 1, max: 1} to specify exactly one line as I want.

Might an option like this have merit?

[LinusU]

@IanVS I think that if we do that we might be getting a bit of unwanted overlap with the no-multiple-empty-lines. I'm not saying I'm against your idea, just the if you specify e.g. max: 2 in the rule, and you have no-multiple-empty-lines configured to only allow one consecutive empty line, what should happen? I think that ESLint has a policy to not try and have overlap between rules, however that would be best answered by someone on the team. Just my two cents...

[hzoo]

I see lines-between-class-methods - is there already a similar rule for objects as well?

Also class properties aren't supported in the default parser anyway (through babel-eslint) class A { asdf = 1}

[nzakas]

@hzoo we don't have anything for objects.

@ianvs I think always and never follows our other rules better, like padded-blocks and avoids the clash with max-empty-lines that @LinusU mentioned.

We still need one more 👍 here.

[IanVS]

Sounds good, I'll give my 👍.

[kaicataldo]

One more :+1: for good measure.

[nzakas]

There's a discussion about whether this should be a standalone rule or part of a larger "spaces after blocks" rule: https://github.com/eslint/eslint/issues/3092

[caesarsol]

Hello, I'd like to join the discussion. Just a couple thoughts:

• The lines- in the name lines-between-class-methods has no precedent in other rules. I would (and I did) search for it in a way that is already expressed in similar already-existant rules, as:
  • http://eslint.org/docs/rules/newline-before-return
  • http://eslint.org/docs/rules/padded-blocks
• I'd mentally associate this rule to a possibly proposed rule about empty-line spacing of functions, or between classes.

[TheSavior]

What is the status of this issue?

I'm happy to help contribute to this to push it over the finish line. I wrote the JSCS rule this is discussing and this is one of the last rules left in my company's conversion from JSCS to eslint.

[nzakas]

I think there's still an open question around whether this should be a standalone rule or should be packaged as part of another rule (https://github.com/eslint/eslint/issues/3092). It looks like no one has expressed any strong opinion one way or another, so we should probably get that squared away.

[quinn]

superseded by https://github.com/eslint/eslint/issues/7356

[hoetmaaiers]

I would add this as part of padded blocks.

[helgeFox]

Could this issue be reopened again? It was never resolved by the other issues that closed this one...

[platinumazure]

@eslint/eslint-team I've reopened this since padding-line-between-statements does not include padding between method definitions.

Do we need a padding-line-between-methods rule?

[helgeFox]

Yes! 👍

[kaicataldo]

I'm honestly not sure about this, so just want to bring it up as a discussion point: should this rule also include objects? Maybe with a config option to only enforce between methods?

Example:

class Foo {
  a() {}

  b() {}

  c() {}
}

// with config option to always enforce
const foo = {
  a,

  b,

  c() {},

  d() {}
}

// with config option to only enforce between methods
const foo = {
  a,
  b,
  c() {},

  d() {}
}

[helgeFox]

Good question but I don't know. I guess it would be nice to have in one of the projects I work on, where we use Backbone. In Backbone, at least in this old version we use, classes are declared as objects.

[kaicataldo]

To be clearer, I think I have two discussion questions:

1) Should this include methods in objects? 2) If it should, should we also enforce padded newlines between non-method properties?

[platinumazure]

I'm leaning towards a separate rule for objects, myself. Classes and objects just have a different grammar to them. Even experimental ECMAScript changes that might make classes more like objects (e.g., static properties/initializers) are going in a different direction than how they are handled in objects.

More generally, I could see a sensible pattern of rules for different types of member-containing entities:

• Newline between open token and first member, or last member and closing token (e.g., array-bracket-newline, object-curly-newline)
• Newline between members (e.g., array-element-newline, object-property-newline)
• Padding at beginning/end of member list (like padded-blocks but I don't think we have anything in arrays/objects/classes)
• Padding between members (e.g., padding-line-between-statements)

And we would create one rule for each of those cases for each construct (object, array, class, block statement, etc.).

Might be thinking too far ahead, but in summary, I think we should handle classes and objects differently.

[RobbieTheWagner]

Seems like this has been open a really long time. Is this being worked on? I would like this to be able to be enforced in objects and classes.

[kaicataldo]

@rwwagner90 I don't believe it's being worked on, but please feel free to make a PR! Looks like the configuration schema that was accepted is https://github.com/eslint/eslint/issues/5949#issuecomment-223238513.

[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?