8000 fix(eslint-plugin): [explicit-function-return-type] Add handling for … · vieron/typescript-eslint@2c36325 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2c36325

Browse files
bradzacherbradzacher
authored
fix(eslint-plugin): [explicit-function-return-type] Add handling for class properties (typescript-eslint#502)
1 parent 3219aa7 commit 2c36325

File tree

3 files changed

+129
-55
lines changed

3 files changed

+129
-55
lines changed

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

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,19 @@ class Test {
6161

6262
The rule accepts an options object with the following properties:
6363

64-
- `allowExpressions` if true, only functions which are part of a declaration will be checked
65-
- `allowTypedFunctionExpressions` if true, type annotations are also allowed on the variable
66-
of a function expression rather than on the function directly.
64+
```ts
65+
type Options = {
66+
// if true, only functions which are part of a declaration will be checked
67+
allowExpressions?: boolean;
68+
// if true, type annotations are also allowed on the variable of a function expression rather than on the function directly.
69+
allowTypedFunctionExpressions?: boolean;
70+
};
6771

68-
By default, `allowExpressions: false` and `allowTypedFunctionExpressions: false` are used,
69-
meaning all declarations and expressions _must_ have a return type.
72+
const defaults = {
73+
allowExpressions: false,
74+
allowTypedFunctionExpressions: false,
75+
};
76+
```
7077

7178
### allowExpressions
7279

@@ -88,6 +95,20 @@ const foo = arr.map(i => i * i);
8895

8996
### allowTypedFunctionExpressions
9097

98+
Examples of **incorrect** code for this rule with `{ allowTypedFunctionExpressions: true }`:
99+
100+
```ts
101+
let arrowFn = () => 'test';
102+
103+
let funcExpr = function() {
104+
return 'test';
105+
};
106+
107+
let objectProp = {
108+
foo: () => 1,
109+
};
110+
```
111+
91112
Examples of additional **correct** code for this rule with `{ allowTypedFunctionExpressions: true }`:
92113

93114
```ts
@@ -100,21 +121,20 @@ let funcExpr: FuncType = function() {
100121
};
101122

102123
let asTyped = (() => '') as () => string;
124+
let caasTyped = <() => string>(() => '');
103125

104126
interface ObjectType {
105127
foo(): number;
106128
}
107129
let objectProp: ObjectType = {
108130
foo: () => 1,
109131
};
110-
111-
interface ObjectType {
112-
foo(): number;
113-
}
114-
115-
let asObjectProp = {
132+
let objectPropAs = {
116133
foo: () => 1,
117134
} as ObjectType;
135+
let objectPropCast = <ObjectType>{
136+
foo: () => 1,
137+
};
118138
```
119139

120140
## When Not To Use It

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

Lines changed: 72 additi 23D3 ons & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ type Options = [
55
{
66
allowExpressions?: boolean;
77
allowTypedFunctionExpressions?: boolean;
8+
allowUntypedSetters?: boolean;
89
}
910
];
1011
type MessageIds = 'missingReturnType';
@@ -41,15 +42,17 @@ export default util.createRule<Options, MessageIds>({
4142
{
4243
allowExpressions: false,
4344
allowTypedFunctionExpressions: false,
45+
allowUntypedSetters: true,
4446
},
4547
],
4648
create(context, [options]) {
4749
/**
4850
* Checks if a node is a constructor.
4951
* @param node The node to check
5052
*/
51-
function isConstructor(node: TSESTree.Node): boolean {
53+
function isConstructor(node: TSESTree.Node | undefined): boolean {
5254
return (
55+
!!node &&
5356
node.type === AST_NODE_TYPES.MethodDefinition &&
5457
node.kind === 'constructor'
5558
);
@@ -58,14 +61,17 @@ export default util.createRule<Options, MessageIds>({
5861
/**
5962
* Checks if a node is a setter.
6063
*/
61-
function isSetter(node: TSESTree.Node): boolean {
64+
function isSetter(node: TSESTree.Node | undefined): boolean {
6265
return (
63-
node.type === AST_NODE_TYPES.MethodDefinition && node.kind === 'set'
66+
!!node &&
67+
node.type === AST_NODE_TYPES.MethodDefinition &&
68+
node.kind === 'set'
6469
);
6570
}
6671

6772
/**
6873
* Checks if a node is a variable declarator with a type annotation.
74+
* `const x: Foo = ...`
6975
*/
7076
function isVariableDeclaratorWithTypeAnnotation(
7177
node: TSESTree.Node,
@@ -76,41 +82,62 @@ export default util.createRule<Options, MessageIds>({
7682
);
7783
}
7884

85+
/**
86+
* Checks if a node is a class property with a type annotation.
87+
* `public x: Foo = ...`
88+
*/
89+
function isClassPropertyWithTypeAnnotation(node: TSESTree.Node): boolean {
90+
return (
91+
node.type === AST_NODE_TYPES.ClassProperty && !!node.typeAnnotation
92+
);
93+
}
94+
95+
/**
96+
* Checks if a node is a type cast
97+
* `(() => {}) as Foo`
98+
* `<Foo>(() => {})`
99+
*/
100+
function isTypeCast(node: TSESTree.Node): boolean {
101+
return (
102+
node.type === AST_NODE_TYPES.TSAsExpression ||
103+
node.type === AST_NODE_TYPES.TSTypeAssertion
104+
);
105+
}
106+
79107
/**
80108
* Checks if a node belongs to:
81-
* const x: Foo = { prop: () => {} }
109+
* `const x: Foo = { prop: () => {} }`
110+
* `const x = { prop: () => {} } as Foo`
111+
* `const x = <Foo>{ prop: () => {} }`
82112
*/
83-
function isPropertyOfObjectVariableDeclaratorWithTypeAnnotation(
84-
node: TSESTree.Node,
113+
function isPropertyOfObjectWithType(
114+
parent: TSESTree.Node | undefined,
85115
): boolean {
86-
let parent = node.parent;
87116
if (!parent || parent.type !== AST_NODE_TYPES.Property) {
88117
return false;
89118
}
90-
parent = parent.parent;
91-
if (!parent || parent.type !== AST_NODE_TYPES.ObjectExpression) {
119+
parent = parent.parent; // this shouldn't happen, checking just in case
120+
/* istanbul ignore if */ if (
121+
!parent ||
122+
parent.type !== AST_NODE_TYPES.ObjectExpression
123+
) {
92124
return false;
93125
}
94-
parent = parent.parent;
95-
return !!parent && isVariableDeclaratorWithTypeAnnotation(parent);
96-
}
97126

98-
function isPropertyOfObjectInAsExpression(node: TSESTree.Node): boolean {
99-
let parent = node.parent;
100-
if (!parent || parent.type !== AST_NODE_TYPES.Property) {
127+
parent = parent.parent; // this shouldn't happen, checking just in case
128+
/* istanbul ignore if */ if (!parent) {
101129
return false;
102130
}
103-
parent = parent.parent;
104-
if (!parent || parent.type !== AST_NODE_TYPES.ObjectExpression) {
105-
return false;
106-
}
107-
parent = parent.parent;
108-
return !!parent && parent.type === AST_NODE_TYPES.TSAsExpression;
131+
132+
return (
133+
isTypeCast(parent) ||
134+
isClassPropertyWithTypeAnnotation(parent) ||
135+
isVariableDeclaratorWithTypeAnnotation(parent)
136+
);
109137
}
110138

111139
/**
112140
* Checks if a function declaration/expression has a return type.
113-
* @param node The node representing a function.
114141
*/
115142
function checkFunctionReturnType(
116143
node:
@@ -119,22 +146,14 @@ export default util.createRule<Options, MessageIds>({
119146
| TSESTree.FunctionExpression,
120147
): void {
121148
if (
122-
options.allowExpressions &&
123-
node.type !== AST_NODE_TYPES.FunctionDeclaration &&
124-
node.parent &&
125-
node.parent.type !== AST_NODE_TYPES.VariableDeclarator &&
126-
node.parent.type !== AST_NODE_TYPES.MethodDefinition
149+
node.returnType ||
150+
isConstructor(node.parent) ||
151+
isSetter(node.parent)
127152
) {
128153
return;
129154
}
130155

131-
if (
132-
!node.returnType &&
133-
node.parent &&
134-
!isConstructor(node.parent) &&
135-
!isSetter(node.parent) &&
136-
util.isTypeScriptFile(context.getFilename())
137-
) {
156+
if (util.isTypeScriptFile(context.getFilename())) {
138157
context.report({
139158
node,
140159
messageId: 'missingReturnType',
144163

145164
/**
146165
* Checks if a function declaration/expression has a return type.
147-
* @param {ASTNode} node The node representing a function.
148166
*/
149167
function checkFunctionExpressionReturnType(
150168
node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression,
151169
): void {
152-
if (
153-
options.allowTypedFunctionExpressions &&
154-
node.parent &&
155-
(isVariableDeclaratorWithTypeAnnotation(node.parent) ||
156-
isPropertyOfObjectVariableDeclaratorWithTypeAnnotation(node) ||
157-
node.parent.type === AST_NODE_TYPES.TSAsExpression ||
158-
isPropertyOfObjectInAsExpression(node))
159-
) {
160-
return;
170+
if (node.parent) {
171+
if (options.allowTypedFunctionExpressions) {
172+
if (
173+
isTypeCast(node.parent) ||
174+
isVariableDeclaratorWithTypeAnnotation(node.parent) ||
175+
isClassPropertyWithTypeAnnotation(node.parent) ||
176+
isPropertyOfObjectWithType(node.parent)
177+
) {
178+
return;
179+
}
180+
}
181+
182+
if (
183+
options.allowExpressions &&
184+
node.parent.type !== AST_NODE_TYPES.VariableDeclarator &&
185+
node.parent.type !== AST_NODE_TYPES.MethodDefinition
186+
) {
187+
return;
188+
}
161189
}
162190

163191
checkFunctionReturnType(node);

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,11 @@ var funcExpr: Foo = function() { return 'test'; };
125125
code: `const x = (() => {}) as Foo`,
126126
options: [{ allowTypedFunctionExpressions: true }],
127127
},
128+
{
129+
filename: 'test.ts',
130+
code: `const x = <Foo>(() => {})`,
131+
options: [{ allowTypedFunctionExpressions: true }],
132+
},
128133
{
129134
filename: 'test.ts',
130135
code: `
@@ -137,8 +142,29 @@ const x = {
137142
{
138143
filename: 'test.ts',
139144
code: `
145+
const x = <Foo>{
146+
foo: () => {},
147+
}
148+
`,
149+
options: [{ allowTypedFunctionExpressions: true }],
150+
},
151+
{
152+
filename: 'test.ts',
153+
code: `
140154
const x: Foo = {
141155
foo: () => {},
156+
}
157+
`,
158+
options: [{ allowTypedFunctionExpressions: true }],
159+
},
160+
// https://github.com/typescript-eslint/typescript-eslint/issues/484
161+
{
162+
filename: 'test.ts',
163+
code: `
164+
type MethodType = () => void;
165+
166+
class App {
167+
private method: MethodType = () => {}
142168
}
143169
`,
144170
options: [{ allowTypedFunctionExpressions: true }],

0 commit comments

Comments
 (0)
0