fix(eslint-plugin): [explicit-function-return-type] Add handling for …

…class properties (#502)

packages/eslint-plugin/docs/rules/explicit-function-return-type.md

@@ -61,12 +61,19 @@ class Test {
 
 The rule accepts an options object with the following properties:
 
-- `allowExpressions` if true, only functions which are part of a declaration will be checked
-- `allowTypedFunctionExpressions` if true, type annotations are also allowed on the variable
-  of a function expression rather than on the function directly.
+```ts
+type Options = {
+  // if true, only functions which are part of a declaration will be checked
+  allowExpressions?: boolean;
+  // if true, type annotations are also allowed on the variable of a function expression rather than on the function directly.
+  allowTypedFunctionExpressions?: boolean;
+};
 
-By default, `allowExpressions: false` and `allowTypedFunctionExpressions: false` are used,
-meaning all declarations and expressions _must_ have a return type.
+const defaults = {
+  allowExpressions: false,
+  allowTypedFunctionExpressions: false,
+};
+```
 
 ### allowExpressions
 
@@ -88,6 +95,20 @@ const foo = arr.map(i => i * i);
 
 ### allowTypedFunctionExpressions
 
+Examples of **incorrect** code for this rule with `{ allowTypedFunctionExpressions: true }`:
+
+```ts
+let arrowFn = () => 'test';
+
+let funcExpr = function() {
+  return 'test';
+};
+
+let objectProp = {
+  foo: () => 1,
+};
+```
+
 Examples of additional **correct** code for this rule with `{ allowTypedFunctionExpressions: true }`:
 
 ```ts
@@ -100,21 +121,20 @@ let funcExpr: FuncType = function() {
 };
 
 let asTyped = (() => '') as () => string;
+let caasTyped = <() => string>(() => '');
 
 interface ObjectType {
   foo(): number;
 }
 let objectProp: ObjectType = {
   foo: () => 1,
 };
-
-interface ObjectType {
-  foo(): number;
-}
-
-let asObjectProp = {
+let objectPropAs = {
   foo: () => 1,
 } as ObjectType;
+let objectPropCast = <ObjectType>{
+  foo: () => 1,
+};
 ```
 
 ## When Not To Use It

packages/eslint-plugin/src/rules/explicit-function-return-type.ts

@@ -5,6 +5,7 @@ type Options = [
   {
     allowExpressions?: boolean;
     allowTypedFunctionExpressions?: boolean;
+    allowUntypedSetters?: boolean;
   }
 ];
 type MessageIds = 'missingReturnType';
@@ -41,15 +42,17 @@ export default util.createRule<Options, MessageIds>({
     {
       allowExpressions: false,
       allowTypedFunctionExpressions: false,
+      allowUntypedSetters: true,
     },
   ],
   create(context, [options]) {
     /**
      * Checks if a node is a constructor.
      * @param node The node to check
      */
-    function isConstructor(node: TSESTree.Node): boolean {
+    function isConstructor(node: TSESTree.Node | undefined): boolean {
       return (
+        !!node &&
         node.type === AST_NODE_TYPES.MethodDefinition &&
         node.kind === 'constructor'
       );
@@ -58,14 +61,17 @@ export default util.createRule<Options, MessageIds>({
     /**
      * Checks if a node is a setter.
      */
-    function isSetter(node: TSESTree.Node): boolean {
+    function isSetter(node: TSESTree.Node | undefined): boolean {
       return (
-        node.type === AST_NODE_TYPES.MethodDefinition && node.kind === 'set'
+        !!node &&
+        node.type === AST_NODE_TYPES.MethodDefinition &&
+        node.kind === 'set'
       );
     }
 
     /**
      * Checks if a node is a variable declarator with a type annotation.
+     * `const x: Foo = ...`
      */
     function isVariableDeclaratorWithTypeAnnotation(
       node: TSESTree.Node,
@@ -76,41 +82,62 @@ export default util.createRule<Options, MessageIds>({
       );
     }
 
+    /**
+     * Checks if a node is a class property with a type annotation.
+     * `public x: Foo = ...`
+     */
+    function isClassPropertyWithTypeAnnotation(node: TSESTree.Node): boolean {
+      return (
+        node.type === AST_NODE_TYPES.ClassProperty && !!node.typeAnnotation
+      );
+    }
+
+    /**
+     * Checks if a node is a type cast
+     * `(() => {}) as Foo`
+     * `<Foo>(() => {})`
+     */
+    function isTypeCast(node: TSESTree.Node): boolean {
+      return (
+        node.type === AST_NODE_TYPES.TSAsExpression ||
+        node.type === AST_NODE_TYPES.TSTypeAssertion
+      );
+    }
+
     /**
      * Checks if a node belongs to:
-     * const x: Foo = { prop: () => {} }
+     * `const x: Foo = { prop: () => {} }`
+     * `const x = { prop: () => {} } as Foo`
+     * `const x = <Foo>{ prop: () => {} }`
      */
-    function isPropertyOfObjectVariableDeclaratorWithTypeAnnotation(
-      node: TSESTree.Node,
+    function isPropertyOfObjectWithType(
+      parent: TSESTree.Node | undefined,
     ): boolean {
-      let parent = node.parent;
       if (!parent || parent.type !== AST_NODE_TYPES.Property) {
         return false;
       }
-      parent = parent.parent;
-      if (!parent || parent.type !== AST_NODE_TYPES.ObjectExpression) {
+      parent = parent.parent; // this shouldn't happen, checking just in case
+      /* istanbul ignore if */ if (
+        !parent ||
+        parent.type !== AST_NODE_TYPES.ObjectExpression
+      ) {
         return false;
       }
-      parent = parent.parent;
-      return !!parent && isVariableDeclaratorWithTypeAnnotation(parent);
-    }
 
-    function isPropertyOfObjectInAsExpression(node: TSESTree.Node): boolean {
-      let parent = node.parent;
-      if (!parent || parent.type !== AST_NODE_TYPES.Property) {
+      parent = parent.parent; // this shouldn't happen, checking just in case
+      /* istanbul ignore if */ if (!parent) {
         return false;
       }
-      parent = parent.parent;
-      if (!parent || parent.type !== AST_NODE_TYPES.ObjectExpression) {
-        return false;
-      }
-      parent = parent.parent;
-      return !!parent && parent.type === AST_NODE_TYPES.TSAsExpression;
+
+      return (
+        isTypeCast(parent) ||
+        isClassPropertyWithTypeAnnotation(parent) ||
+        isVariableDeclaratorWithTypeAnnotation(parent)
+      );
     }
 
     /**
      * Checks if a function declaration/expression has a return type.
-     * @param node The node representing a function.
      */
     function checkFunctionReturnType(
       node:
@@ -119,22 +146,14 @@ export default util.createRule<Options, MessageIds>({
         | TSESTree.FunctionExpression,
     ): void {
       if (
-        options.allowExpressions &&
-        node.type !== AST_NODE_TYPES.FunctionDeclaration &&
-        node.parent &&
-        node.parent.type !== AST_NODE_TYPES.VariableDeclarator &&
-        node.parent.type !== AST_NODE_TYPES.MethodDefinition
+        node.returnType ||
+        isConstructor(node.parent) ||
+        isSetter(node.parent)
       ) {
         return;
       }
 
-      if (
-        !node.returnType &&
-        node.parent &&
-        !isConstructor(node.parent) &&
-        !isSetter(node.parent) &&
-        util.isTypeScriptFile(context.getFilename())
-      ) {
+      if (util.isTypeScriptFile(context.getFilename())) {
         context.report({
           node,
           messageId: 'missingReturnType',
@@ -144,20 +163,29 @@ export default util.createRule<Options, MessageIds>({
 
     /**
      * Checks if a function declaration/expression has a return type.
-     * @param {ASTNode} node The node representing a function.
      */
     function checkFunctionExpressionReturnType(
       node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression,
     ): void {
-      if (
-        options.allowTypedFunctionExpressions &&
-        node.parent &&
-        (isVariableDeclaratorWithTypeAnnotation(node.parent) ||
-          isPropertyOfObjectVariableDeclaratorWithTypeAnnotation(node) ||
-          node.parent.type === AST_NODE_TYPES.TSAsExpression ||
-          isPropertyOfObjectInAsExpression(node))
-      ) {
-        return;
+      if (node.parent) {
+        if (options.allowTypedFunctionExpressions) {
+          if (
+            isTypeCast(node.parent) ||
+            isVariableDeclaratorWithTypeAnnotation(node.parent) ||
+            isClassPropertyWithTypeAnnotation(node.parent) ||
+            isPropertyOfObjectWithType(node.parent)
+          ) {
+            return;
+          }
+        }
+
+        if (
+          options.allowExpressions &&
+          node.parent.type !== AST_NODE_TYPES.VariableDeclarator &&
+          node.parent.type !== AST_NODE_TYPES.MethodDefinition
+        ) {
+          return;
+        }
       }
 
       checkFunctionReturnType(node);

packages/eslint-plugin/tests/rules/explicit-function-return-type.test.ts

@@ -125,6 +125,11 @@ var funcExpr: Foo = function() { return 'test'; };
       code: `const x = (() => {}) as Foo`,
       options: [{ allowTypedFunctionExpressions: true }],
     },
+    {
+      filename: 'test.ts',
+      code: `const x = <Foo>(() => {})`,
+      options: [{ allowTypedFunctionExpressions: true }],
+    },
     {
       filename: 'test.ts',
       code: `
@@ -137,8 +142,29 @@ const x = {
     {
       filename: 'test.ts',
       code: `
+const x = <Foo>{
+  foo: () => {},
+}
+      `,
+      options: [{ allowTypedFunctionExpressions: true }],
+    },
+    {
+      filename: 'test.ts',
+      code: `
 const x: Foo = {
   foo: () => {},
+}
+      `,
+      options: [{ allowTypedFunctionExpressions: true }],
+    },
+    // https://github.com/typescript-eslint/typescript-eslint/issues/484
+    {
+      filename: 'test.ts',
+      code: `
+type MethodType = () => void;
+
+class App {
+  private method: MethodType = () => {}
 }
       `,
       options: [{ allowTypedFunctionExpressions: true }],