Rule Suggestion: class-requires-members

[ChrisMBarr]

At my work we name our classes with certain suffixes that correspond to AngularJS functionality like MyPageController MyFeatureService etc.

Due to typo-related bugs in the past and some conventions we like to use I've written a rule that enforces certain properties and/or methods to exist on when a certain class name pattern is matched.

For example:

• All classes ending in "Controller" we require to have a public method named activate so that we can consistently call the same method on all controllers.
• All classes ending in a more broad pattern that matches all AngularJS features are requires to have public static $inject property.

I can we-write this internal rule to be more generic for use publicly here, but I want to make sure it's something that is worth the time & effort before I spend any time on it.

Here's an example of what the below rule configuration could look like.

"class-requires-members": [true, [
  {
    //No class name pattern specified, so all classes are required to have this method
    "member-name": "activate"
  },
  {
    //Classes ending with "Controller" are required to have a static property named "$inject"
    "class-name-pattern": ".+Controller$",
    "member-name": "$inject",
    "property": true,
    "static": true
  }
]]

Some Notes

• when no "class-name-pattern" is specified, it would apply to all classes.
• "class-name-pattern" would match a regex
• unless "property": true is specified, it would be assumed to be a method
• unless "private": true is specified, it would be assumed to be public

Here's an example of what the above configuration would produce for some test data

class SomeFeature1Controller {
      ~~~~~~~~~~~~~~~~~~~~~~ [The class 'SomeFeature1Controller' is required to have a public static property named '$inject']
      ~~~~~~~~~~~~~~~~~~~~~~ [The class 'SomeFeature1Controller' is required to have a public method named 'activate']
}

class SomeFeature2Controller {
      ~~~~~~~~~~~~~~~~~~~~~~ [The class 'SomeFeature2Controller' is required to have a public method named 'activate']
      static $inject = [];
}

class SomeFeature3Controller {
      ~~~~~~~~~~~~~~~~~~~~~~ [The class 'SomeFeature3Controller' s required to have a public static property named '$inject']
      activate = () => {
            //do stuff
      }
}

class SomeFeature4Controller {
      ~~~~~~~~~~~~~~~~~~~~~~ [The class 'SomeFeature4Controller' is required to have a public static property named '$inject']
      ~~~~~~~~~~~~~~~~~~~~~~ [The class 'SomeFeature4Controller' is required to have a public method named 'activate']

      public inject = []; //wrong name and not static!

      private _activate = () => {
            //wrong name and private!
      }
}

class SomeFeatureWithoutSuffix {
      ~~~~~~~~~~~~~~~~~~~~~~~~ [The class 'SomeFeatureWithoutSuffix' is required to have a public method named 'activate']
}

class SomeOtherFeatureWithoutSuffix {
      activate = () => {
            //Yay!
      }
}

---

So, would a rule like this be appropriate to include in this project?

[andy-hanson]

Is this related to microsoft/TypeScript#13462? Seems like you could be getting better static checking than just that a method with the name exists.

[ChrisMBarr]

No, not related. That's the first I've seen that actually.

[andy-hanson]

It makes more sense to me to have class Foo implements Controller, and get errors from the type checker, than to have class FooController and have a lint rule with a regexp walk the AST and look for methods with certain names.

[ChrisMBarr]

Yes I agree that would be ideal, but that would require you to remember to add implements Controller on every controller class. The rule would be a catch all for those that forgot.

Plus, it's not possible to use an interface to require a static property on a class, where it could be enforced with a lint rule.

At my organization we've run into issues where someone added public $inject (not static) or public static inject (typo, missing dollar sign) instead of public static $inject. Stuff like that is really difficult to track down why it's broken, which is why I made a lint rule that finds this specific to how we use use AngularJS.

[JoshuaKGoldberg]

@ChrisMBarr this seems like a rule tailored to very few types of applications (really just Angular, right?), so it'd be better off in tslint-microsoft-contrib or in a standalone repository. If you're still interested, let's continue discussion in either of those.