no pure functions inside of es6 classes

[pwmckenna]

I'd like to write a rule that complains if a class method does not use this. Sort of the opposite of no-invalid-this.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[eslintbot]

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

[michaelficarra]

👍 That would be great.

[ilyavolodin]

Hmmm.. It seems to be a fairly limiting rule. I'm also not sure what would be a good reason to discourage using pure functions. @pwmckenna could you provide more information about why you are looking for this rule and follow our new rule proposal template?

[michaelficarra]

@ilyavolodin If it doesn't mention this, it can be safely made static. There's no reason to put a function like this on a prototype.

[lo1tuma]

I would definitely use such a rule 👍.

[ilyavolodin]

Ok, sounds good to me, in that case. Thanks @michaelficarra

[pwmckenna]

@ilyavolodin I'll definitely use that template next time, thanks for linking. I'd be happy to make a first pass at implementing this, though I suspect I'll need to ask for help along the way. I'll have something up in the next couple of days.

[nzakas]

@pwmckenna I'd still like to see the info from the rule proposal posted to this issue. It really helps both with the evaluation and communicating to others the intention of this rule.

[pwmckenna]

@nzakas definitely!

When the rules will warn. Include a description as well as sample code.

This rule will warn whenever this is not used from within a class method.

Good:

class A {
    constructor() {}
    static foo() {}
    increment() {this.count++;}
}

Bad:

class A {
    add(a, b) {return a + b;}
}

Whether the rule prevents an error or is stylistic.

This rule is stylistic, but has performance implications. If a function is not using this, it is unnecessarily copied whenever a new instance of the class is created.

Why the rule should be in the core instead of creating a custom rule.

Totally up to the maintainers. I think its a pretty fundamental thing, making a distinction between class members and static functions, but again, its mostly a style issue, so I can make it a custom rule as well.

Are you willing to create the rule yourself?

#5152

I went ahead and wrote it because its interesting, so I'm happy even if it doesn't make it in. Great project. Thanks for all the hard work!

[oriSomething]

@michaelficarra if there is a reason not to use this inside class instance functions is because classes like React.Components where functions are being called outside the class as an events hook.

[kaicataldo]

@pwmckenna Would you like to open a PR? Looks like you did a lot of work on this already!

[DrewML]

Just my 2 cents, but this rule may make more sense as a part of eslint-plugin-react.

The use-case for React makes sense, but I'm not sure how often this would be desirable with typical ES6 classes.

As an example, a developer might create a class with a method that doesn't use this, but is called from another method within the class. In that scenario, the developer may intend to make it possible for a consumer of the class to override that method for a single instance. Example:

class Foo {
   doSomething() {
      // some code here
      this.onSomething();
   }

   onSomething() {
      console.log('foo');
   }
}

var foo = new Foo();
// Consumer can override
foo.onSomething = () => console.log('bar');

With this rule enabled, it would encourage the developer of the Foo class to move the onSomething function into closure scope, rather than putting it on the prototype. This can obviously be addressed by disabling the rule for that section of the code, but figured I'd throw out the use-case.

[gyandeeps]

Sounds like it should be named prefer-static. Webstorm has built-in support for this. very helpful 😄

[nzakas]

There's been no progress on this issue for months. Unless someone steps forward to implement, it's time to close this issue.

[gyandeeps]

working on this