Support type narrowing for non-callable node#136
Conversation
Xvezda
commented
Jun 22, 2025
- Fix tests
- Type narrowing for non-callable node
There was a problem hiding this comment.
Pull Request Overview
This PR fixes existing tests for the no-undocumented-throws rule and adds type‐based narrowing so that calls returning generator‐like types are skipped.
- Updated and added several test scenarios to exercise promise chains, getters/setters, and union‐typed methods.
- Enhanced
getCalleeDeclarationinsrc/utils.jsto handle declarations arrays and multiple symbol declarations. - Modified the
no-undocumented-throwsrule to early‐return on call/new expressions whose return type is generator-like.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/rules/no-undocumented-throws.test.js | Fixed failing tests, removed outdated JSDoc, and added new test cases. |
| src/utils.js | Improved resolution of declaration from TypeScript symbols. |
| src/rules/no-undocumented-throws.js | Added a switch to skip calls/new expressions returning generator types. |
| /** | ||
| * @throws {Promise<TypeError>} | ||
| */ | ||
| function foo() { |
There was a problem hiding this comment.
This test removed the @throws JSDoc but does not specify an errors expectation, so it will incorrectly pass. Add errors: [{ messageId: 'missingThrowsTag' }] to this test case to assert the missing tag is reported.
| // If there are multiple declarations, we take the first one. | ||
| declaration = type.symbol.declarations[0]; |
There was a problem hiding this comment.
[nitpick] Although you guard for declarations?.length above, consider capturing const decls = type.symbol.declarations to avoid reaccessing the optional chain—this also makes the assignment clearer and prevents any future undefined access.
| // If there are multiple declarations, we take the first one. | |
| declaration = type.symbol.declarations[0]; | |
| const decls = type.symbol.declarations; | |
| // If there are multiple declarations, we take the first one. | |
| declaration = decls[0]; |
| if (services.tsNodeToESTreeNodeMap.has(calleeDeclaration)) { | ||
| const signature = getCallSignature( | ||
| services, | ||
| services.tsNodeToESTreeNodeMap.get(calleeDeclaration) | ||
| ); | ||
|
|
||
| const returnType = signature?.getReturnType(); | ||
| if (returnType && isGeneratorLike(returnType)) return; | ||
| const returnType = signature?.getReturnType(); |
There was a problem hiding this comment.
[nitpick] Generator-like checks only run when the declaration is in tsNodeToESTreeNodeMap. If the signature isn’t mapped there, generator return types won’t be skipped. Consider obtaining the signature unconditionally or adjusting the guard.
| if (services.tsNodeToESTreeNodeMap.has(calleeDeclaration)) { | |
| const signature = getCallSignature( | |
| services, | |
| services.tsNodeToESTreeNodeMap.get(calleeDeclaration) | |
| ); | |
| const returnType = signature?.getReturnType(); | |
| if (returnType && isGeneratorLike(returnType)) return; | |
| const returnType = signature?.getReturnType(); | |
| const signature = checker.getResolvedSignature(calleeDeclaration); | |
| if (signature) { | |
| const returnType = signature.getReturnType(); |