@mdjermanovic
oh..Yes. this is how padding-line-between-statements handles comments - demo(no fix).
I changed it. thanks! :)

Thank for the change! Looks like the comments are completely safe now.

There is another thing to note. In some extremely rare cases, this fix can change the behavior of this rule to report more or fewer warnings. The actual master version looks for comments before the next member (using sourceCode.getCommentsBefore, which returns comments after the first preceding non-comment token), while this fix searches from both sides.

This will almost always produce the same result at the end, but a class body can also contain useless empty members (semicolons), which don't exist in AST.

An example where the actual version doesn't report warning, but the version from this PR does:

/* eslint lines-between-class-members: ["error", "always"] */

class A {
  foo() {}
  /* comment */;
  bar() {}
}

This rule in general at the moment doesn't work well with these semicolons. I think it's okay to merge this PR as is (just mark with Update because it can produce more warnings) and maybe later analyze and fix handling of semicolons in a separate PR.

@mdjermanovic
Thanks for the elaborate review. I changed the pr message(fix -> update).

TIL! Nice catch, @mdjermanovic 👍

I'm personally not sure we should defer the fix until later. Would it be possible to check for semicolon tokens in addition to comments in this PR?

@kaicataldo @mdjermanovic

May I ask some questions?
Before I move on, I would like to double-check whether I understand correctly. :)

The actual version doesn't report an error in case1. But I assume that this is a bug. ...right?

• case1

/* eslint lines-between-class-members: ["error", "always"] */

class A {
  foo() {}
  /* comment */; // No error, but here is no blank line.
  bar() {}
}

The PR version report an error in case1, but no error in case2.

• case2

/* eslint lines-between-class-members: ["error", "always"] */

class A {
  foo() {}
  /* comment */;
  ;  // No error, but here is no blank line.
  bar() {}
}

So I need to add check for semicolon tokens. Do I get this right?

The actual version doesn't report an error in case1. But I assume that this is a bug. ...right?

In my opinion - yes, this is a bug, It seems that the actual master version doesn't handle semicolons well. In some cases, this rule can even remove them:

/* eslint lines-between-class-members: ["error", "never"] */

class A {
  foo() {}
  ;
  bar() {}
}

This is fixed to:

/* eslint lines-between-class-members: ["error", "never"] */

class A {
  foo() {}
bar() {}
}

This doesn't look like something this rule should do, no-extra-semi removes these semicolons.

So I need to add check for semicolon tokens. Do I get this right?

While these semicolons are technically class members (I think), this rule should probably treat them as it treats comments - don't require/disallow empty lines around semicolons, but a line with a semicolon shouldn't be seen as an empty line.

Maybe getLastConsecutiveTokenAfter/getFirstConsecutiveTokenBefore shouldn't explicitly check for comments and semicolons, but use first/last token of the following/previous class members as a stop token instead.

@mdjermanovic @kaicataldo
there is one more bug(in my opinion) in the actual version. - demo

/* eslint lines-between-class-members: ["error", "always"] */

class foo{ 
  bar(){} //comment
  baz(){}
}

This is fixed to:

/* eslint lines-between-class-members: ["error", "always"] */

class foo{ 
  bar(){}

  //comment
  baz(){}
}

It's because the actual version inserts newline only after the last member node without considering trailing comments or semicolon.
I pushed the commit(7ec30b2) includes fixing this problem also. thanks.

@mdjermanovic
thanks for the reviews. I'm all done :)

@mdjermanovic
Oh.. sorry for my mistake. I fixed typo.

Thanks for contributing!