From 54d222bbb3a434fc2cbd248485d0ac123f45df6d Mon Sep 17 00:00:00 2001 From: Jason Weinzierl Date: Fri, 18 Apr 2025 09:31:17 -0500 Subject: [PATCH] fix: check implicit return types for finnish, no-finnish, and no-misused-observables --- src/etc/get-type-services.ts | 11 ++++++++++- tests/rules/finnish.test.ts | 10 ++++++++++ tests/rules/no-finnish.test.ts | 4 ++++ tests/rules/no-misused-observables.test.ts | 15 +++++++++------ 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/etc/get-type-services.ts b/src/etc/get-type-services.ts index 5f38ba6c..55bc15f8 100644 --- a/src/etc/get-type-services.ts +++ b/src/etc/get-type-services.ts @@ -38,7 +38,16 @@ export function getTypeServices< || ts.isMethodDeclaration(tsNode) || ts.isFunctionExpression(tsNode) ) { - tsTypeNode = tsNode.type ?? tsNode.body; // TODO(#57): this doesn't work for Block bodies. + if (tsNode.type) { + tsTypeNode = tsNode.type; + } else if (tsNode.body && ts.isBlock(tsNode.body)) { + const returnStatement = tsNode.body.statements.find(ts.isReturnStatement); + if (returnStatement?.expression) { + tsTypeNode = returnStatement.expression; + } + } else { + tsTypeNode = tsNode.body; + } } else if ( ts.isCallSignatureDeclaration(tsNode) || ts.isMethodSignature(tsNode) diff --git a/tests/rules/finnish.test.ts b/tests/rules/finnish.test.ts index 562237f2..a78f1e90 100644 --- a/tests/rules/finnish.test.ts +++ b/tests/rules/finnish.test.ts @@ -23,6 +23,7 @@ ruleTester({ types: true }).run('finnish', finnishRule, { someArray.forEach((element$: Observable) => {}); function someFunction$(someParam$: Observable): Observable { return someParam; } + function someImplicitReturnFunction$(someParam$: Observable) { return someParam; } class SomeClass { someProperty$: Observable; @@ -30,6 +31,7 @@ ruleTester({ types: true }).run('finnish', finnishRule, { get someGetter$(): Observable { throw new Error("Some error."); } set someSetter$(someParam$: Observable) {} someMethod$(someParam$: Observable): Observable { return someParam; } + someImplicitReturnMethod$(someParam$: Observable) { return someParam; } } interface SomeInterface { @@ -131,6 +133,7 @@ ruleTester({ types: true }).run('finnish', finnishRule, { const someObservable$ = of(0); const someArray = [someObservable$]; function someFunction(someParam$: Observable): Observable { return someParam$; } + function someImplicitReturnFunction(someParam$: Observable) { return someParam$; } `, options: [{ functions: false }], }, @@ -141,6 +144,7 @@ ruleTester({ types: true }).run('finnish', finnishRule, { class SomeClass { someMethod(someParam$: Observable): Observable { return someParam$; } + someImplicitReturnMethod(someParam$: Observable) { return someParam$; } } interface SomeInterface { @@ -269,6 +273,8 @@ ruleTester({ types: true }).run('finnish', finnishRule, { const someArray = [someObservable$]; function someFunction(someParam$: Observable): Observable { return someParam$; } ~~~~~~~~~~~~ [shouldBeFinnish] + function someImplicitReturnFunction(someParam$: Observable) { return someParam$; } + ~~~~~~~~~~~~~~~~~~~~~~~~~~ [shouldBeFinnish] `, ), fromFixture( @@ -279,6 +285,8 @@ ruleTester({ types: true }).run('finnish', finnishRule, { class SomeClass { someMethod(someParam$: Observable): Observable { return someParam$; } ~~~~~~~~~~ [shouldBeFinnish] + someImplicitReturnMethod(someParam$: Observable) { return someParam$; } + ~~~~~~~~~~~~~~~~~~~~~~~~ [shouldBeFinnish] } interface SomeInterface { @@ -413,6 +421,8 @@ ruleTester({ types: true }).run('finnish', finnishRule, { class SomeClass { someMethod(someValue: any): Observable { return of(someValue); } ~~~~~~~~~~ [shouldBeFinnish] + someImplicitReturnMethod(someValue: any) { return of(someValue); } + ~~~~~~~~~~~~~~~~~~~~~~~~ [shouldBeFinnish] } interface SomeInterface { diff --git a/tests/rules/no-finnish.test.ts b/tests/rules/no-finnish.test.ts index bd7e983e..a77ca07b 100644 --- a/tests/rules/no-finnish.test.ts +++ b/tests/rules/no-finnish.test.ts @@ -119,6 +119,10 @@ ruleTester({ types: true }).run('no-finnish', noFinnishRule, { someMethod$(someParam$: Observable): Observable { return someParam; } ~~~~~~~~~~~ [forbidden] ~~~~~~~~~~ [forbidden] + someImplicitReturnMethod$(someParam: Observable) { + ~~~~~~~~~~~~~~~~~~~~~~~~~ [forbidden] + return someParam; + } } `, ), diff --git a/tests/rules/no-misused-observables.test.ts b/tests/rules/no-misused-observables.test.ts index 32ac0221..e1e75689 100644 --- a/tests/rules/no-misused-observables.test.ts +++ b/tests/rules/no-misused-observables.test.ts @@ -44,12 +44,6 @@ ruleTester({ types: true }).run('no-misused-observables', noMisusedObservablesRu new Foo(() => 42, 0); new Foo; `, - stripIndent` - // couldReturnType is bugged for block body implicit return types (#57) - import { of } from "rxjs"; - - [1, 2, 3].forEach(i => { return of(i); }); - `, // #endregion valid; void return argument // #region valid; void return attribute { @@ -300,6 +294,15 @@ ruleTester({ types: true }).run('no-misused-observables', noMisusedObservablesRu ~~~~~~~~~~ [forbiddenVoidReturnArgument] `, ), + fromFixture( + stripIndent` + // void return argument; block body; implicit return type + import { of } from "rxjs"; + + [1, 2, 3].forEach(i => { return of(i); }); + ~~~~~~~~~~~~~~~~~~~~~~ [forbiddenVoidReturnArgument] + `, + ), fromFixture( stripIndent` // void return argument; block body; union return