google / google/closure-compiler

Flow-sensitive type specialization of qualified names should back off when owners escape.

Open
#2,824 2 comments 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

The [following][1] should probably warn but does not:

```js
function assertNonNull(/** !Object */ arg) {}
class Foo {
constructor() { /** @type {?Object} */ this.bar; }
clear() { this.bar = null; }
baz() {
this.bar = {};
this.clear();
assertNonNull(this.bar);
}
}
```

Note that if `this.bar = {};` is commented out then it does warn correctly. So it's using the flow information about the qualified name `this.bar`, but `this` is escaped by the call to `this.clear()`.

Neither OTI nor NTI handle this correctly. It should be simple enough during type inference to look for any qualified names passed either as arguments or as receiver in any function calls and add some sort of "invalidate" marker for them to the LinkedFlowScope. Then when LinkedFlowScope looks up a variable, if it comes across any prefix that's invalidated, it stops immediately and falls back on the declared type.

One trick is that we'd like to be able to do this *after* pure function analysis, but I don't know if that's feasible to run before type checking (it currently runs after property disambiguation and various code removal passes, which seems important).

This will likely break a lot of existing usages, so we'd probably need to roll it out behind a flag.

[1]: https://closure-compiler-debugger.appspot.com/#input0%3Dclass%2520Foo%2520%257B%250A%2520%2520constructor()%2520%257B%2520%252F**%2520%2540type%2520%257B%253FObject%257D%2520*%252F%2520this.bar%253B%2520%257D%250A%2520%2520clear()%2520%257B%2520this.bar%2520%253D%2520null%253B%2520%257D%250A%2520%2520baz()%2520%257B%250A%2520%2520%2520%2520this.bar%2520%253D%2520%257B%257D%253B%250A%2520%2520%2520%2520this.clear()%253B%250A%2520%2520%2520%2520assertNonNull(this.bar)%253B%250A%2520%2520%257D%250A%257D%250A%26input1%26conformanceConfig%26externs%3Dfunction%2520assertNonNull(%252F**%2520!Object%2520*%252F%2520arg)%2520%257B%257D%26refasterjs-template%26includeDefaultExterns%3Dtrue%26CHECK_SYMBOLS%3Dtrue%26MISSING_PROPERTIES%3Dtrue%26TRANSPILE%3Dtrue%26CHECK_TYPES%3Dtrue%26CLOSURE_PASS%3Dtrue%26PRESERVE_TYPE_ANNOTATIONS%3Dtrue%26PRETTY_PRINT%3Dtrue

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.