Skip to content
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

Add "No empty method" to no-empty for ES6 methods #4605

Closed
allenmanning opened this Issue Dec 4, 2015 · 9 comments

Comments

Projects
None yet
6 participants
@allenmanning
Copy link

allenmanning commented Dec 4, 2015

It would check for methods like this

class Person

...

  getName () {
  }
@eslintbot

This comment has been minimized.

Copy link

eslintbot commented Dec 4, 2015

@allenmanning Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage label Dec 4, 2015

@allenmanning allenmanning changed the title Consider "No empty method" rule for ES6 Classes New Rule Proposal: "No empty method" rule for ES6 Classes Dec 4, 2015

@platinumazure

This comment has been minimized.

Copy link
Member

platinumazure commented Dec 4, 2015

What exactly defines "empty"? Should a comment make it non-empty, like in our empty block rule?

@mysticatea

This comment has been minimized.

Copy link
Member

mysticatea commented Dec 4, 2015

I think that to add an option which covers functions into no-empty is fine.

@nzakas nzakas closed this Dec 4, 2015

@nzakas nzakas reopened this Dec 4, 2015

@nzakas nzakas added enhancement rule accepted and removed triage labels Dec 4, 2015

@nzakas nzakas changed the title New Rule Proposal: "No empty method" rule for ES6 Classes Add "No empty method" to no-empty for ES6 methods Dec 4, 2015

@nzakas nzakas added the help wanted label Dec 4, 2015

@kaicataldo

This comment has been minimized.

Copy link
Member

kaicataldo commented Dec 5, 2015

I'll work on this over the next few days.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Dec 5, 2015

Agree, adding an option to no-empty that covers this case is fine. We probably want the option to be "methods" so that all functions aren't checked.

@mysticatea

This comment has been minimized.

Copy link
Member

mysticatea commented Dec 5, 2015

@nzakas To be clear, does that "methods" contain ES5 style methods and getter/setter?

var obj = {
    foo: function() {},

    get foo2() {}
    set foo2(value) {}
};

obj.bar = function() {};

function Foo() {
    this.hello = function() {};
}

Foo.prototype.hello2 = function() {};
@mysticatea

This comment has been minimized.

Copy link
Member

mysticatea commented Dec 5, 2015

Ah, sorry, the title says "ES6 methods".
But is it good?

kaicataldo added a commit to kaicataldo/eslint that referenced this issue Dec 6, 2015

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Dec 6, 2015

We should keep "methods" to mean ES6 methods that are defined by shorthand notation in objects and classes.

@allenmanning

This comment has been minimized.

Copy link
Author

allenmanning commented Dec 7, 2015

Thank you.

One thing to consider as well is empty ES6 Constructors, or constructors that only call super().

@nzakas nzakas closed this in 1ab0438 Dec 8, 2015

nzakas added a commit that referenced this issue Dec 8, 2015

Merge pull request #4617 from kaicataldo/fixes4605
Update: Add 'method' option to no-empty (fixes #4605)

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.