Skip to content

Prevent getTypeAtLocation crash on type-only import clause - #64468

Merged
Andrew Branch (andrewbranch) merged 3 commits into
microsoft:mainfrom
lsh4711:fix-type-only-import-clause-crash
Sep 29, 2026
Merged

Andrew Branch (andrewbranch) merged 3 commits into
microsoft:mainfrom
lsh4711:fix-type-only-import-clause-crash

Conversation

@lsh4711

@lsh4711 lsh4711 commented Sep 26, 2026 •

Copy link
Copy Markdown

Fixes #64467

Context

I'm building syscript, a compiler that compiles TypeScript syntax to C for systems
programming. It uses the TypeScript 7 API to type-check the program and then walks
every node, requesting its type in a batch with getTypeAtLocation. That walk crashed
the API server on the first file that used import type.

Problem

checker.GetTypeAtLocation panics with a nil pointer dereference when called on the
ImportClause of a type-only import without a default binding:

// types.ts
export type U = number;
// main.ts
import type { U } from "./types";
import type * as types from "./types";
panic: runtime error: invalid memory address or nil pointer dereference
checker.(*Checker).tryGetDeclaredTypeOfSymbol
checker.(*Checker).getDeclaredTypeOfSymbol
checker.(*Checker).getTypeOfNode
checker.(*Checker).GetTypeAtLocation

It reproduces with the latest nightly (typescript@7.1.0-dev.20260926.1).

Cause

ast.IsTypeDeclaration returns true for a type-only ImportClause, so getTypeOfNode
takes the type-declaration branch and passes the result of getSymbolOfDeclaration
straight to getDeclaredTypeOfSymbol. An import clause only has a symbol when it has
a default binding (import type X from "..."), so for import type { U } and
import type * as ns the symbol is nil.

The missing check looks historical rather than intentional. When this branch was
written, isTypeDeclaration only covered type parameters, classes, interfaces, type
aliases and enums, which always have a symbol. #35200 (type-only imports and exports)
added ImportClause, ImportSpecifier and ExportSpecifier; the specifiers always
have a symbol, but a clause without a default binding does not. The same unguarded
branch is still in Strada's getTypeOfNode, and this code was ported from it.

The nil symbol is expected: the binder only declares a symbol for an import clause with a
default binding. The bug is that IsTypeDeclaration classifies a type-only clause as a type
declaration even when it has no default binding and therefore declares nothing.

Fix

Make ast.IsTypeDeclaration return true for a type-only ImportClause only when it has a
default binding. A clause without one now takes the same path in getTypeOfNode as a regular
import clause, which already handles a missing symbol.

The other callers of IsTypeDeclaration (addUndefinedToGlobalsOrErrorOnRedeclaration, the
deprecation check, reportUnusedLocal and IsTypeDeclarationName) only look at symbol
declarations or named nodes, so their behavior doesn't change.

Tests

Added TestGetTypeAtLocationOfTypeOnlyImportClause in internal/checker. It requests
the type of a named and a namespace type-only import clause and checks that both match
the type of an equivalent regular import clause, and that import type D (a default
binding) still resolves to the same type as a reference to D. It panics without the fix
and passes with it.

Ran the pre-submission checklist from CONTRIBUTING.md. Everything passes except
internal/vfs/osvfs TestOS/Realpath, which also fails on main in my environment
because my home directory is a symlink (/home → /var/home, Fedora Atomic).


I used Claude Code for this change; I've reviewed it and will handle the review.

Copilot AI balanced review requested due to automatic review settings September 26, 2026 14:04
@github-project-automation github-project-automation Bot moved this to Not started in PR Backlog Sep 26, 2026
@typescript-automation typescript-automation Bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Sep 26, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@lsh4711

lsh4711 commented Sep 26, 2026

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@lsh4711
lsh4711 requested a balanced review from Copilot September 26, 2026 19:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread tsc/internal/checker/checker.go Outdated
// In this case, we call getSymbolOfDeclaration instead of getSymbolAtLocation because it is a declaration
symbol := c.getSymbolOfDeclaration(node)
return c.getDeclaredTypeOfSymbol(symbol)
if symbol != nil {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems like a band-aid. Why is this nil in the first place?

@lsh4711 lsh4711 Sep 28, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks, you're right. The nil symbol itself is expected: the binder only declares a symbol for
an ImportClause that has a default binding (bindImportClause checks Name() != nil). For
import type { U } and import type * as ns, the declarations are the ImportSpecifier and
NamespaceImport, and the clause itself declares nothing.

The actual problem is that ast.IsTypeDeclaration still treats such a clause (import type { U }) as a type
declaration. I've changed it to require a default binding and removed the nil check, so these
clauses now take the same path as a regular import clause. The other callers of
IsTypeDeclaration only see symbol declarations or named nodes, so they aren't affected.
I also extended the test to cover import type D to make sure a default binding still
resolves as before.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The fix addresses the nil-symbol path with focused regression coverage; only a minor comment grammar issue remains.

Review effort: Balanced
Findings: 1 Low severity

Open (1)

Comment thread tsc/internal/checker/checker_test.go Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@typescript-automation typescript-automation Bot added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Sep 29, 2026
@andrewbranch
Andrew Branch (andrewbranch) added this pull request to the merge queue Sep 29, 2026
Merged via the queue into microsoft:main with commit f34fb21 Sep 29, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Milestone Bug PRs that fix a bug with a specific milestone

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

getTypeAtLocation crashes on the ImportClause of a type-only import

4 participants