fix(eslint-plugin): handle DoBootstrap correctly in lifecycle rules (#…

…243)
angular-eslint · Jan 11, 2021 · 5010b3f · 5010b3f
1 parent 27ca1bf
commit 5010b3f
Show file tree

Hide file tree

Showing 11 changed files with 343 additions and 147 deletions.
diff --git a/packages/eslint-plugin/src/rules/contextual-decorator.ts b/packages/eslint-plugin/src/rules/contextual-decorator.ts
@@ -1,12 +1,11 @@
 import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
 
 import {
-  getDecoratorName,
-  isAngularClassDecorator,
   isAngularInnerClassDecorator,
-  isNotNullOrUndefined,
   AngularClassDecoratorKeys,
   ANGULAR_CLASS_DECORATOR_MAPPER,
+  getAngularClassDecorator,
+  getDecoratorName,
 } from '../utils/utils';
 import { createESLintRule } from '../utils/create-eslint-rule';
 
@@ -62,7 +61,7 @@ function validateNode(
     return;
   }
 
-  const classDecoratorName = getClassDecoratorName(classDeclaration);
+  const classDecoratorName = getAngularClassDecorator(classDeclaration);
 
   if (!classDecoratorName) {
     return;
@@ -87,15 +86,6 @@ function lookupTheClassDeclaration(
   return null;
 }
 
-function getClassDecoratorName(
-  classDeclaration: TSESTree.ClassDeclaration,
-): AngularClassDecoratorKeys | undefined {
-  return (classDeclaration.decorators || [])
-    .map(getDecoratorName)
-    .filter(isNotNullOrUndefined)
-    .find(isAngularClassDecorator);
-}
-
 function validateDecorator(
   context: TSESLint.RuleContext<MessageIds, []>,
   decorator: TSESTree.Decorator,

diff --git a/packages/eslint-plugin/src/rules/contextual-lifecycle.ts b/packages/eslint-plugin/src/rules/contextual-lifecycle.ts
@@ -1,16 +1,15 @@
 import { TSESTree } from '@typescript-eslint/experimental-utils';
 import { createESLintRule } from '../utils/create-eslint-rule';
 import {
-  PIPE_CLASS_DECORATOR,
+  COMPONENT_CLASS_DECORATOR,
   DIRECTIVE_CLASS_DECORATOR,
   INJECTABLE_CLASS_DECORATOR,
   MODULE_CLASS_DECORATOR,
+  PIPE_CLASS_DECORATOR,
 } from '../utils/selectors';
-
 import {
   ANGULAR_CLASS_DECORATOR_LIFECYCLE_METHOD_MAPPER,
   AngularClassDecorators,
-  getClassName,
   getDeclaredMethods,
   getMethodName,
   isAngularLifecycleMethod,
@@ -26,61 +25,60 @@ export default createESLintRule<Options, MessageIds>({
     type: 'problem',
     docs: {
       description:
-        'Ensures that classes use allowed lifecycle method in its body',
+        'Ensures that lifecycle methods are used in a correct context',
       category: 'Possible Errors',
       recommended: 'error',
     },
     schema: [],
     messages: {
-      contextuaLifecycle: `The method {{methodName}} is not allowed for class {{className}} because it is decorated with {{decorator}}`,
+      contextuaLifecycle: `This lifecycle method is not called for {{decorator}}`,
     },
   },
   defaultOptions: [],
   create(context) {
     function checkContext(
-      node: TSESTree.Decorator,
+      { parent }: TSESTree.Decorator,
       decorator: AngularClassDecorators,
     ) {
-      const className = getClassName(node);
-      const classParent = node.parent as TSESTree.ClassDeclaration;
+      const classDeclaration = parent as TSESTree.ClassDeclaration;
       const allowedMethods = ANGULAR_CLASS_DECORATOR_LIFECYCLE_METHOD_MAPPER.get(
         decorator,
       );
-
-      const declaredMethods = getDeclaredMethods(classParent);
+      const declaredMethods = getDeclaredMethods(classDeclaration);
 
       for (const method of declaredMethods) {
         const methodName = getMethodName(method);
+
         if (
           !isAngularLifecycleMethod(methodName) ||
-          (allowedMethods && allowedMethods.has(methodName))
-        )
+          allowedMethods?.has(methodName)
+        ) {
           continue;
+        }
 
         context.report({
           node: method.key,
           messageId: 'contextuaLifecycle',
-          data: {
-            methodName,
-            className,
-            decorator,
-          },
+          data: { decorator },
         });
       }
     }
 
     return {
-      [PIPE_CLASS_DECORATOR](node: TSESTree.Decorator) {
-        checkContext(node, AngularClassDecorators.Pipe);
+      [COMPONENT_CLASS_DECORATOR](node: TSESTree.Decorator) {
+        checkContext(node, AngularClassDecorators.Component);
+      },
+      [DIRECTIVE_CLASS_DECORATOR](node: TSESTree.Decorator) {
+        checkContext(node, AngularClassDecorators.Directive);
       },
       [INJECTABLE_CLASS_DECORATOR](node: TSESTree.Decorator) {
         checkContext(node, AngularClassDecorators.Injectable);
       },
       [MODULE_CLASS_DECORATOR](node: TSESTree.Decorator) {
         checkContext(node, AngularClassDecorators.NgModule);
       },
-      [DIRECTIVE_CLASS_DECORATOR](node: TSESTree.Decorator) {
-        checkContext(node, AngularClassDecorators.Directive);
+      [PIPE_CLASS_DECORATOR](node: TSESTree.Decorator) {
+        checkContext(node, AngularClassDecorators.Pipe);
       },
     };
   },

diff --git a/packages/eslint-plugin/src/rules/no-empty-lifecycle-method.ts b/packages/eslint-plugin/src/rules/no-empty-lifecycle-method.ts
@@ -1,11 +1,9 @@
 import { TSESTree } from '@typescript-eslint/experimental-utils';
 import { createESLintRule } from '../utils/create-eslint-rule';
-import { COMPONENT_OR_DIRECTIVE_CLASS_DECORATOR } from '../utils/selectors';
-
 import {
-  getDeclaredMethods,
-  getMethodName,
-  isAngularLifecycleMethod,
+  ANGULAR_LIFECYCLE_METHODS,
+  getAngularClassDecorator,
+  toPattern,
 } from '../utils/utils';
 
 type Options = [];
@@ -17,39 +15,34 @@ export default createESLintRule<Options, MessageIds>({
   meta: {
     type: 'suggestion',
     docs: {
-      description: 'Disallows declaring empty lifecycle hook methods',
+      description: 'Disallows declaring empty lifecycle methods',
       category: 'Best Practices',
       recommended: 'error',
     },
     schema: [],
     messages: {
-      noEmptyLifecycleMethod: `Lifecycle method {{methodName}} should not be empty`,
+      noEmptyLifecycleMethod: 'Lifecycle methods should not be empty',
     },
   },
   defaultOptions: [],
   create(context) {
-    const isMethodEmpty = (node: TSESTree.MethodDefinition): boolean => {
-      return !node.value.body || node.value.body.body.length === 0;
-    };
+    const angularLifecycleMethodsPattern = toPattern([
+      ...ANGULAR_LIFECYCLE_METHODS,
+    ]);
 
     return {
-      [COMPONENT_OR_DIRECTIVE_CLASS_DECORATOR](node: TSESTree.Decorator) {
-        const classParent = node.parent as TSESTree.ClassDeclaration;
-        const declaredMethods = getDeclaredMethods(classParent);
+      [`MethodDefinition[key.name=${angularLifecycleMethodsPattern}][value.body.body.length=0]`](
+        node: TSESTree.MethodDefinition,
+      ) {
+        const classDeclaration = node.parent!
+          .parent as TSESTree.ClassDeclaration;
 
-        for (const method of declaredMethods) {
-          const methodName = getMethodName(method);
-          if (!isAngularLifecycleMethod(methodName) || !isMethodEmpty(method))
-            continue;
+        if (!getAngularClassDecorator(classDeclaration)) return;
 
-          context.report({
-            node: method.key,
-            messageId: 'noEmptyLifecycleMethod',
-            data: {
-              methodName,
-            },
-          });
-        }
+        context.report({
+          node: node.key,
+          messageId: 'noEmptyLifecycleMethod',
+        });
       },
     };
   },

diff --git a/packages/eslint-plugin/src/rules/no-lifecycle-call.ts b/packages/eslint-plugin/src/rules/no-lifecycle-call.ts
@@ -22,12 +22,11 @@ export default createESLintRule<Options, MessageIds>({
   defaultOptions: [],
   create(context) {
     return {
-      MemberExpression: (node) => {
-        const {
-          object: { type: nodeObjectType },
-          parent,
-          property,
-        } = node;
+      MemberExpression: ({
+        object: { type: nodeObjectType },
+        parent,
+        property,
+      }) => {
         const isSuperCall = nodeObjectType === 'Super';
 
         if (

diff --git a/packages/eslint-plugin/src/rules/use-lifecycle-interface.ts b/packages/eslint-plugin/src/rules/use-lifecycle-interface.ts
@@ -2,17 +2,17 @@ import { TSESTree } from '@typescript-eslint/experimental-utils';
 import { createESLintRule } from '../utils/create-eslint-rule';
 import {
   AngularLifecycleInterfaces,
+  AngularLifecycleMethodKeys,
+  ANGULAR_LIFECYCLE_METHODS,
+  getAngularClassDecorator,
   getDeclaredAngularLifecycleInterfaces,
-  getDeclaredMethods,
   getLifecycleInterfaceByMethodName,
-  getMethodName,
-  isAngularLifecycleMethod,
+  toPattern,
 } from '../utils/utils';
 
 type Options = [];
 export type MessageIds = 'useLifecycleInterface';
 export const RULE_NAME = 'use-lifecycle-interface';
-
 const STYLE_GUIDE_LINK = 'https://angular.io/styleguide#style-09-01';
 
 export default createESLintRule<Options, MessageIds>({
@@ -21,7 +21,7 @@ export default createESLintRule<Options, MessageIds>({
     type: 'suggestion',
     docs: {
       description:
-        'Ensures classes implement lifecycle interfaces corresponding to the declared lifecycle methods',
+        'Ensures that classes implement lifecycle interfaces corresponding to the declared lifecycle methods',
       category: 'Best Practices',
       recommended: 'warn',
     },
@@ -32,34 +32,36 @@ export default createESLintRule<Options, MessageIds>({
   },
   defaultOptions: [],
   create(context) {
-    return {
-      ClassDeclaration(node: TSESTree.ClassDeclaration) {
-        const declaredLifecycleInterfaces = getDeclaredAngularLifecycleInterfaces(
-          node,
-        );
-        const declaredMethods = getDeclaredMethods(node);
+    const angularLifecycleMethodsPattern = toPattern([
+      ...ANGULAR_LIFECYCLE_METHODS,
+    ]);
 
-        for (const method of declaredMethods) {
-          const methodName = getMethodName(method);
+    return {
+      [`MethodDefinition[key.name=${angularLifecycleMethodsPattern}]`]({
+        key,
+        parent,
+      }: TSESTree.MethodDefinition) {
+        const classDeclaration = parent!.parent as TSESTree.ClassDeclaration;
 
-          if (!isAngularLifecycleMethod(methodName)) continue;
+        if (!getAngularClassDecorator(classDeclaration)) return;
 
-          const interfaceName = getLifecycleInterfaceByMethodName(methodName);
-          const isMethodImplemented = declaredLifecycleInterfaces.includes(
-            AngularLifecycleInterfaces[interfaceName],
-          );
+        const declaredLifecycleInterfaces = getDeclaredAngularLifecycleInterfaces(
+          classDeclaration,
+        );
+        const methodName = (key as TSESTree.Identifier)
+          .name as AngularLifecycleMethodKeys;
+        const interfaceName = getLifecycleInterfaceByMethodName(methodName);
+        const isMethodImplemented = declaredLifecycleInterfaces.includes(
+          AngularLifecycleInterfaces[interfaceName],
+        );
 
-          if (isMethodImplemented) continue;
+        if (isMethodImplemented) return;
 
-          context.report({
-            node: method.key,
-            messageId: 'useLifecycleInterface',
-            data: {
-              interfaceName,
-              methodName,
-            },
-          });
-        }
+        context.report({
+          node: key,
+          messageId: 'useLifecycleInterface',
+          data: { interfaceName, methodName },
+        });
       },
     };
   },