New: rule lines-between-class-members(fixes #5949).

[aladdin-add]

What is the purpose of this pull request? (put an "X" next to item)

[x] New rule (template)

What changes did you make? (Give an overview)
fixes ##5949

Is there anything you'd like reviewers to focus on?

[kaicataldo]

Thanks for working on this! Anything we can do to help? Looks like it hasn't been touched in a little while.

[aladdin-add]

I was so busy these days, so sorry for the delay.

in current implement, to fix the problem, I just replace the text between methods, to one or two linebreak as needed. But seems like it cannot preserve comments. any suggestions?

[not-an-aardvark]

Instead of replacing the text with two linebreaks, would it work to just insert a single linebreak after the first method? That way, you wouldn't need to delete any existing text between the methods.

[aladdin-add]

almost finished, but for the docs. lol.

[kaicataldo]

@aladdin-add No need to apologize! Just checking in :)

[mysticatea]

I'd like to see more tests, especially for semicolons and comments.

class A {
    a() {}
    ;
    b() {}
}

class B {
    a() {}
    // comment
    b() {}
}

class C {
    a() {}
    /**
     * comment
     */
    b() {}
}

[aladdin-add]

class A {
    a() {}
    ;
    b() {}
}

should also add tests for semi? this is not a necessary and looks strange!

[mysticatea]

Yeah, it's strange, but it's valid syntax. This rule should work well even if semicolons exist.

In semantics, the semicolon is an empty member. So if this rule is checking blank lines between class members, this rule should check blank lines between a method and a semicolon, and between consecutive semicolons. But, this rule seems to check only between methods, so maybe it should ignore blank lines between a method and a semicolon, and between consecutive semicolons.

But I'm not sure if this rule should check only between methods. In this direction, we have to add rules as many as the combination of the number of class member types.

• lines-between-class-empty-members
• lines-between-class-methods
• lines-between-class-method-and-empty
• lines-between-class-fields (in future)
• lines-between-class-field-and-empty (in future)
• lines-between-class-field-and-method (in future)

I'm imaging a rule like padding-line-between-statements rule. Thought?

[aladdin-add]

👍 . a semicolon can be seen as a method? if not, I think the rulename lines-between-class-methods is a little misleading.

[not-an-aardvark]

I think empty members are rare and not very useful, and it doesn't seem like people have a lot of different preferences for enforcing newlines between class members. So I'm wondering if it would be better to keep just line-between-class-methods for now, and evaluate proposals for other class members separately if people want more customizability.

[mysticatea]

My opinion is not strongly. So I'm OK if the rule checks only methods and just ignores blank lines between a method and a semicolon, and between consecutive semicolons. But, I'm guessing lines-between-class-methods is going to be deprecated in future by adding fields.

[not-an-aardvark]

What if we call it lines-between-class-members instead? That way, we can add options to it for fields in the future.

[mysticatea]

I guess this should be continue.

[eslintbot]

LGTM

[eslintbot]

LGTM

[eslintbot]

LGTM

[eslintbot]

LGTM

[kaicataldo]

One small comment about the docs, but otherwise this LGTM. Thanks for all your hard work on this!

[kaicataldo]

Can we make this example more clear? It's not obvious how it's different from the default option.

Maybe something like:

class Foo{
  bar(){} // single line class member
  baz(){
    // multi line class member
  }

  qux(){}
}

[eslintbot]

LGTM

[eslintbot]

LGTM

[eslintbot]

LGTM

[btmills]

Nice work on this, @aladdin-add! I have just one request for an additional test, then once that's done LGTM!

[btmills]

Can you add a test that covers this check?

[aladdin-add]

@btmills I found this check is unused -- it is always true. removed it and added some tests.

[eslintbot]

LGTM

[btmills]

LGTM, thanks @aladdin-add!

[eslintbot]

LGTM