Skip to content

Conversation

Xvezda
Copy link
Owner

@Xvezda Xvezda commented Jun 22, 2025

  • Fix tests
  • Type narrowing for non-callable node

@Copilot Copilot AI review requested due to automatic review settings June 22, 2025 04:23
@Xvezda Xvezda changed the title feature/tests Support type narrowing for non-callable node Jun 22, 2025
@Xvezda Xvezda merged commit de4f4d0 into master Jun 22, 2025
1 check passed
@Xvezda Xvezda deleted the feature/tests branch June 22, 2025 04:24
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 getCalleeDeclaration in src/utils.js to handle declarations arrays and multiple symbol declarations.
  • Modified the no-undocumented-throws rule 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() {
Copy link

Copilot AI Jun 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

Comment on lines +358 to +359
// If there are multiple declarations, we take the first one.
declaration = type.symbol.declarations[0];
Copy link

Copilot AI Jun 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Suggested change
// 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];

Copilot uses AI. Check for mistakes.

Comment on lines +149 to +155
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();
Copy link

Copilot AI Jun 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Suggested change
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();

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant