Fix unused type parameter checking to skip overload signatures and only report on implementations

[Copilot]

TypeScript-Go was incorrectly reporting unused type parameters on overload signatures in addition to implementations. TypeScript only reports these errors on implementations (functions with bodies), not on overload signatures (functions without bodies).

Consider this overload scenario:

// These should NOT error (overload signatures - no bodies)
export function tryParseJson<T>(text: string): unknown;
export function tryParseJson<T>(text: string, predicate: (parsed: unknown) => parsed is T): T | undefined;
// This SHOULD error (implementation - has body, T is unused)
export function tryParseJson<T>() {
    throw new Error("noop")
}

Before: TypeScript-Go reported errors on all 3 declarations
After: TypeScript-Go reports errors only on the implementation, matching TypeScript's behavior

Root Cause

In checkUnusedIdentifiers(), the code correctly handled unused parameters by only checking them on implementations (if node.Body() != nil), but it was checking unused type parameters on ALL function signatures, regardless of whether they were overloads or implementations.

Fix

Moved the checkUnusedTypeParameters(node) call inside the if node.Body() != nil block, making it consistent with parameter checking and matching TypeScript's behavior.

This maintains the intentional diagnostic difference - type parameters continue to use TS6196 ('T' is declared but never used.) rather than TS6133 ('T' is declared but its value is never read.), as type variables are not "values" that can be "read".

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[jakebailey]

@copilot What you just fixed is an intentional diff; type variables are not "values" that can be "read", so we changed the message used in the new code. Undo everything you've done in this PR.

However, there is an inconsistency. Reread #1617 (comment) and use the test case from the comment:

// @strict: true
// @noUnusedLocals: true
// @noUnusedParameters: true
// @target: esnext

export function tryParseJson<T>(text: string): unknown;
export function tryParseJson<T>(text: string, predicate: (parsed: unknown) => parsed is T): T | undefined;
export function tryParseJson<T>() {
    throw new Error("noop")
}

export function tryParseJson2<T>(text: string): unknown;
export function tryParseJson2<T>(text: string, predicate: (parsed: unknown) => parsed is T): T | undefined;
export function tryParseJson2() {
    throw new Error("noop")
}

export function tryParseJson3<T>(_text: string): unknown {
    throw new Error("noop")
}

The old codebase only issued error on the first and third declarations, but the new code errors on tryParseJson2 too, which it should not.

[Copilot]

@copilot What you just fixed is an intentional diff; type variables are not "values" that can be "read", so we changed the message used in the new code. Undo everything you've done in this PR.

Howe...

You're absolutely right. I've reverted all the diagnostic changes in c72979a. The intentional diff should remain - type variables are not "values" that can be "read".

I'm now investigating the real issue with overload handling inconsistency where TypeScript only errors on the first and third declarations but TypeScript-Go errors on all of them.

[jakebailey]

Wow, we actually didn't have a test for this.

[jakebailey]

The original code for checkUnusedIdentifiers in checker.ts is:

case SyntaxKind.Constructor:
case SyntaxKind.FunctionExpression:
case SyntaxKind.FunctionDeclaration:
case SyntaxKind.ArrowFunction:
case SyntaxKind.MethodDeclaration:
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
    // Only report unused parameters on the implementation, not overloads.
    if (node.body) {
        checkUnusedLocalsAndParameters(node, addDiagnostic);
    }
    checkUnusedTypeParameters(node, addDiagnostic);
    break;

Which very much implies that we were checking unused type parameters on these.

I think the difference is actually in checkUnusedTypeParameters, which did this:

        // Only report errors on the last declaration for the type parameter container;
        // this ensures that all uses have been accounted for.
        const declarations = getSymbolOfDeclaration(node).declarations;
        if (!declarations || last(declarations) !== node) return;

[jakebailey]

So, what I'm saying is, I think that this PR is actually correct as it skips the unused check exactly like the other code.

[Copilot]

Pull Request Overview

This PR fixes an issue where TypeScript-Go was incorrectly reporting unused type parameter errors on function overload signatures in addition to implementations, while TypeScript only reports these errors on implementations (functions with bodies).

Key changes:

• Moved unused type parameter checking to only occur on function implementations (those with bodies)
• Added comprehensive test cases to verify the fix across different overload scenarios
• Updated baseline test files to reflect the corrected behavior

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
testdata/tests/cases/compiler/overloadUnusedTypeParameters.ts	New test file with overload scenarios to verify unused type parameter checking behavior
testdata/baselines/reference/compiler/overloadUnusedTypeParameters.types	Type information baseline for the new test
testdata/baselines/reference/compiler/overloadUnusedTypeParameters.symbols	Symbol information baseline for the new test
testdata/baselines/reference/compiler/overloadUnusedTypeParameters.js	JavaScript output baseline for the new test
testdata/baselines/reference/compiler/overloadUnusedTypeParameters.errors.txt	Error output baseline showing only implementations report unused type parameter errors
internal/checker/checker.go	Core fix moving checkUnusedTypeParameters call inside the body check condition