Repository navigation
checker.ts :: checkModuleDeclaration(): Redundant type guard erroneously narrows node type to 'never' #45572
Description
Bug Report
I have a ready, very easy fix for this bug, so if you indeed recognize this as a bug, please assign it to me, and I'll submit a PR.
🔎 Search Terms
checkModuleDeclaration node type never
checker.ts
isExternalModuleAugmentation
Property 'parent' does not exist on type 'never' (ts2339)
🕗 Version & Regression Information
This is a logical bug in checker.ts that should arise in pretty much any TypeScript version that supports type guards/predicaments. However, since isAmbientModule()'s result is assigned to a variable, and control flow of aliased conditional expressions is introduced only in v4.4.0, this issue will only appear in v4.4.0+ (or any other version with pull request #44730)
Judging by related commits that I've found by using git blame -L37720,37722 src/compiler/checker.ts and
git log -L37720,37722:src/compiler/checker.ts, the bug was introduced somewhere around v2.8-rc:
- 4a963a2 – Dec 22, 2015, seems to be v1.7.5 or v1.8.0-beta – Added the
isExternalModuleAugmentation()check (which will become redundant in the next commit) - 5e593ac – Mar 6, 2018, seems to be v2.8-rc – Added a type predicate to
isExternalModuleAugmentation(), which created a redundancy with parentisAmbientModulecheck.
⏯ Playground Link
💻 Code
checker.ts : line 37664
function checkModuleDeclaration(node: ModuleDeclaration) {
// ...
// Control flow analysis of aliased conditional expressions
// (e.g. type guard result saved in a variable) are introduced
// only in v4.4.0 https://github.com/microsoft/TypeScript/pull/44730
const isAmbientExternalModule = isAmbientModule(node);
// ...
if (isAmbientExternalModule) {
// Before v4.4.0, node here is of type 'TNode'
// After v4.4.0, node here is of type 'AmbientModuleDeclaration'
if (isExternalModuleAugmentation(node)) {
// isExternalModuleAugmentation() contains a redundant isAmbientModule() check
}
else if (isGlobalSourceFile(node.parent)) {
// ^^^^^^^^^^^^
// Here, tsc v4.5.0-dev.20210824 (and all others) give an error:
// Property 'parent' does not exist on type 'never'.ts(2339)
// Because of the redundant check in if (isExternalModuleAugmentation(node)) ...,
// control flows deduces node's type to be 'never'.
// ...
} else {
// ...
}
}
// ...
}🙁 Actual behavior
Covered in the code examples/playground.
🙂 Expected behavior
Covered in the code examples/playground.
Activity
[-]Redundant type guard erroneously narrows node to 'never' in checkModuleDeclaration() in checker.ts[/-][+]checker.ts :: checkModuleDeclaration(): Redundant type guard erroneously narrows node type to 'never' in [/+][-]checker.ts :: checkModuleDeclaration(): Redundant type guard erroneously narrows node type to 'never' in [/-][+]checker.ts :: checkModuleDeclaration(): Redundant type guard erroneously narrows node type to 'never'[/+]andrewbranch commentedon Aug 25, 2021
I think your sources are out of date; this was fixed as soon as it became a problem. It now reads
The compiler compiles itself in CI, so we automatically catch things like this as we go.
rafasofizada commentedon Aug 25, 2021
EDIT: Okay, it seems like just adding a type annotation solves this issue. But how?
isAmbientExternalModuleshould be inferred as a boolean anyway, because it is a result of a type guard. Adding abooleantype annotation shouldn't change the fact thatisAmbientModule()andisExternalModuleAugmentation()intersect andnodeis inferred asneverin subsequent conditional statements. I still think the issue should be reopened, because the real solution to this is to replaceisExternalModuleAugmentation()withisModuleAugmentationExternal().Andrew Branch (@andrewbranch) that doesn't really change anything about the issue, does it? The issue is not in the type definition of isAmbientExternalModule, it is in the intersection of
isAmbientModule()andisExternalModuleAugmentation(), defined inutilities.ts.The line
if (isExternalModuleAugmentation(node))should beif (isModuleAugmentationExternal(node))instead, asisExternalModuleAugmentation()already includesisAmbientModule()check inside of it.I think the issue should be reopened. The bug is still there.
