Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Add prefer-function-over-method rule #2037

Merged
merged 4 commits into from Jan 21, 2017

Conversation

andy-hanson
Copy link
Contributor

@andy-hanson andy-hanson commented Jan 14, 2017

PR checklist

  • Addresses an existing issue: #0000
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update
  • Enable CircleCI for your fork (https://circleci.com/add-projects)

What changes did you make?

Added the no-unused-this rule, which warns for methods that don't use this (they could be functions).

Also, once microsoft/TypeScript#13217 is in, I'll add an exception for override, since that's a legitimate case where you would need to write a method that might not use this.

@ajafff
Copy link
Contributor

ajafff commented Jan 14, 2017

Just a suggestion:
You could detect methods that only calls itself recursive.

class Foo {
    bar() { this.bar(); }
    ~~~ [lorem ipsum]
}

If you want to go the easy way, you could "borrow" my implementation for this rule: https://github.com/ajafff/tslint-consistent-codestyle/blob/master/rules/preferStaticMethodRule.ts

@andy-hanson
Copy link
Contributor Author

andy-hanson commented Jan 14, 2017

@ajafff Thanks! Have you considered turning some of your rules into pull requests here?
no-collapsible-if and no-unnecessary-else were some I was considering working on, but won't have to, it seems.

@ajafff
Copy link
Contributor

ajafff commented Jan 18, 2017

On second thought the name of this rule is a bit misleading.
For me no-unused-this would be something like the following:

// function is not using this, but declares it in parameter list
function doStuff(this: Foo, bar: Bar) {
                 ~~~~~~~~~ [unused this]
    return bar;
}

export class Rule extends Lint.Rules.AbstractRule {
/* tslint:disable:object-literal-sort-keys */
public static metadata: Lint.IRuleMetadata = {
ruleName: "no-unused-this",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name is misleading since it does allow this. I don't have a good name, but what about something like prefer-functions-over-methods

@andy-hanson
Copy link
Contributor Author

andy-hanson commented Jan 19, 2017

How about method-must-use-this?

@nchen63
Copy link
Contributor

nchen63 commented Jan 19, 2017

That sort of makes it sound like the rule is suggesting that this should be added to the method.

@andy-hanson andy-hanson changed the title Add no-unused-this rule Add prefer-function-over-method rule Jan 21, 2017
@nchen63 nchen63 merged commit eff3bc8 into palantir:master Jan 21, 2017
@nchen63
Copy link
Contributor

nchen63 commented Jan 21, 2017

@andy-hanson thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants