google / google/closure-compiler

Type narrowing failure?

Open
#3,018 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
7.7k
Forks
1.2k
Avg merge
2d 12h
Merged PRs (30d)
6

Description

(Filed per request from @concavelenz after internal discussion:)

Compiling
```JS
/**
* Add all reachable objects to a list, recursively.
* @param {*} node JavaScript value to search.
* @param {!Array} objects Array to add objects to.
*/
function add(node, objects) {
if (!node || (typeof node !== 'object' && typeof node !== 'function') ||
objects.includes(node)) {
return;
}
objects.push(node);
var names = Object.getOwnPropertyNames(node);
for (var i = 0; i < names.length; i++) {
add(node[names[i]], objects);
}
}
```
with -O=ADVANCED_OPTIMIZATIONS [gives some unexpected warnings](https://closure-compiler-debugger.appspot.com/#input0%3D%252F**%250A%2520*%2520Add%2520all%2520reachable%2520objects%2520to%2520a%2520list%252C%2520recursively.%250A%2520*%2520%2540param%2520%257B*%257D%2520node%2520JavaScript%2520value%2520to%2520search.%250A%2520*%2520%2540param%2520%257B!Array%253C!Object%253E%257D%2520objects%2520Array%2520to%2520add%2520objects%2520to.%250A%2520*%252F%250Afunction%2520add(node%252C%2520objects)%2520%257B%250A%2520%2520if%2520(!node%2520%257C%257C%2520(typeof%2520node%2520!%253D%253D%2520'object'%2520%2526%2526%2520typeof%2520node%2520!%253D%253D%2520'function')%2520%257C%257C%250A%2520%2520%2520%2520%2520%2520objects.includes(node))%2520%257B%250A%2520%2520%2520%2520return%253B%250A%2520%2520%257D%250A%2520%2520objects.push(node)%253B%250A%2520%2520var%2520names%2520%253D%2520Object.getOwnPropertyNames(node)%253B%250A%2520%2520for%2520(var%2520i%2520%253D%25200%253B%2520i%2520%253C%2520names.length%253B%2520i%252B%252B)%2520%257B%250A%2520%2520%2520%2520add(node%255Bnames%255Bi%255D%255D%252C%2520objects)%253B%250A%2520%2520%257D%250A%257D%250A%26input1%26conformanceConfig%26externs%26refasterjs-template%26includeDefaultExterns%3Dtrue%26ENABLE_ALL_DIAGNOSTIC_GROUPS%3Dtrue%26CHECK_TYPES%3Dtrue%26STRICT_CHECK_TYPES%3Dtrue%26TRANSPILE%3Dtrue%26PRESERVE_TYPE_ANNOTATIONS%3Dtrue%26PRETTY_PRINT%3Dtrue):
```
input0:8: WARNING - actual parameter 1 of Array.prototype.includes does not match formal parameter
found : *
required: Object
objects.includes(node)) {
^^^^

input0:11: WARNING - actual parameter 1 of Array.prototype.push does not match formal parameter
found : *
required: (Object|undefined)
objects.push(node);
^^^^

input0:12: WARNING - actual parameter 1 of Object.getOwnPropertyNames does not match formal parameter
found : *
required: Object
var names = Object.getOwnPropertyNames(node);
^^^^
0 error(s), 3 warning(s), 100.0% typed
```
I say "unexpected" because usually the compiler is very good at type narrowing based on conditionals, so I was expecting that that after evaluating `!node || (typeof node !== 'object' && typeof node !== 'function') ||` it would infer that node must be an object in the call `objects.includes(node)`, and similarly in the call to `.push` if it is reached.

Of course, it is possible that the compiler has correctly inferred that node could be an object but not an `Object`—but this seems to be contrary to its usual tendency to conflate `x` being non-primitive with `x instanceof Object`, so I'm not sure why it isn't doing that here.

(Interestingly, if I change the declaration at the top to `@param {!Object|string|number|boolean|undefined|null|Symbol} node`, then the error changes to:
```
input0:8: WARNING - actual parameter 1 of Array.prototype.includes does not match formal parameter
found : (Object|boolean|number|string)
required: Object
objects.includes(node)) {
^^^^
```
which suggests that the problem is that it has not understood that `typeof node !== 'object' && typeof node !== 'function'` will evaluate to true for booleans, numbers and strings.)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.