Repository navigation
Assume parameters are const for the purposes of control flow analysis #10180
Description
Activity
- added this to the milestone
on Aug 5, 2016 Discussed alternative was to treat function expression as if they are always immediately invoked, which is obviously possibly-wrong for other reasons
Discussed alternative was to treat function expression as if they are always immediately invoked, which is obviously possibly-wrong for other reasons
I wonder if you have considered the idea of annotating callback parameters as async(non-immediate) and default on sync(immediate)?
Immediate case:
function doSomething(callback: () => any) { callback(); } function fn(x: number|string) { if (typeof x === 'number') { doSomething(() => x.toFixed()); // No error. } }
Non-immediate case:
function doSomething(callback: async () => any) { setTimeout(callback, 0); } function fn(x: number|string) { if (typeof x === 'number') { doSomething(() => x.toFixed()); // Error x is 'number|string' } }
Sync callbacks is only assignable to sync callbacks and vice versa for Async callbacks:
function doSomething(callback: () => any) { setTimeout(callback, 0); // Error a sync callback is not assignable to an async one. }
function doSomething1(callback: () => any) { callback(); } function doSomething2(callback: () => any) { doSomething1(callback); // No error }
Though re-using
asynckeyword as a type annotation might conflict with the meaning of async declarations inasync/awaitwhich makes a function returning a promise. An alternative is to usenonimmediate, though I think that keyword is too long.Reacted by Yahiko UzumakiTingan Ho (@tinganho) what about all the callback signatures already declared in
.d.tsfiles and in existing code? Some are called immediately and some are not. If the compiler started assuming they are immediately called because they don't have anasyncannotation, that might lead to a lot of incorrect type inference in existing code.Troy Gerwien (@yortus) what about a classic solution, add a flag:
Defaulting on async with no assignment checks as it is today with. And add a flag
--strictCallbackCheckingto opt in for the new functionality described above?add a flag
They will love that 😉 🎏 🎌 🇺🇸
Reacted by Tingan Ho(clears the throat) #7770 might be related somehow, despite it is widely believed that
pureis a bit impracticalAn easy solution would be to add a flag that all parameters are const (which is a good programming practice anyway), e.g.
--constParams. Then, if you feel very kind, let/var/const? modifiers might be allowed as an escape hatch.I think there are multiple merits of this solution:
- it doesn't rely on complex, fragile machinery
- it is reliable and the results are predictable (no additional load for the programmer)
- it improves coding style
- it's easy to implement
Simply add a flag :)
The design team might need to seriously consider switching to a flag-driven-development paradigm.
Reacted by Daniel RosenwasserProposal: Allow for the first 64 compiler flags to be specified as an integer literal. Then people can just be like "Oh I'm using TypeScript variant 41956651792133412" and then only merge codebases if they have compatible flags.
Reacted by Brian Terlson, Yegor Roganov, Anders Hejlsberg, Daniel Rosenwasser, Marius Schulz and Maxime QuandalleReacted by Marin MarinovReacted by Alexandru CiucăMarin Marinov (@gcnew) the problem with flags is that in order to handle them mathematically correctly you need to consider all 2^N cases where N - is the total number of flags.
Example, say there were 2 flags already
--strictNullChecksand--noImplicitAnywhich needed to be taken into account for every critical part of code that one way or another might depend on either or both. Which gives us 4 cases:function doSomethingVeryCritical() { if (strictNullChecks) { if (noImplicitAny) { // strictNullChecks and noImplicitAny } else { // strictNullChecks and no noImplicitAny } } else { if (noImplicitAny) { // no strictNullChecks and noImplicitAny } else { // no strictNullChecks and no noImplicitAny } } }Now here comes you and say, hey, let's add another one, because we need it:
--constParams
And then the design team in order to stay correct need to double all possible cases:function doSomethingVeryCritical() { if (strictNullChecks) { if (noImplicitAny) { // strictNullChecks and noImplicitAny if (constParams) { // more fun } else { // more fun } } else { // strictNullChecks and no noImplicitAny if (constParams) { // more fun } else { // more fun } } } else { if (noImplicitAny) { // no strictNullChecks and noImplicitAny if (constParams) { // more fun } else { // more fun } } else { // no strictNullChecks and no noImplicitAny if (constParams) { // more fun } else { // more fun } } } }the alternative is not to be 100% correct by dropping some cases from considerations, which may or may not lead to serious bugs and your own frustration
think about it
Though in most situations it is either add a flag or break someone else code. I dislike both cases, but faced with that situation I would rather add a flag than to break someone's code.
That being said, whatever solution you cook up here it could also be baked into
--strictNullChecks. So we don't need add another flag, though it would break everyone trying out the old--strictNullChecks.The combinatorial explosion of flags seems inevitable in a monolithic tool that has wide and ever-growing variety of use-cases and is called on to support them all, even when some demands are contradictory.
The alternative is to build in extensibility (#6508) and let people choose the compile-time plugins they need instead of demanding flags in core. And they can write their own plugins too. Babel has gone that way. Clearly extensibility has it's own problems too, as anyone familiar with babel can attest.
...or do a flag cleanup at the cost of breaking some backwards compatibility.
** sorry for the off-topic
Reacted by Tingan HoAleksey-Bykov Yes, I do agree that introducing flags adds a disproportionate amount of complexity. Especially true for logic-heavy flags. Luckily it's not the case with const params as it should be handled with ease on parser level. There is no explosion of logic - parameters are just tagged as
constinstead ofvar. The rest of the compiler should not be affected at all, but pick it up automatically according to the existing rules.I was thinking the same as what Tingan Ho (@tinganho) proposes - maybe it could be a part of
--strictNullChecks? Though it's definitely useful as a standalone feature.Returning to the original topic... I think a reasonable compromise here would be to consider parameters
constwhen there are no assignments to them anywhere. That is reasonably easy for the compiler to check. With this rule the original example would work, as would most other examples I've seen. The one case we wouldn't cover is when a parameter is modified before it is captured (e.g. to turn anullorundefinedinto a default value) but then kept constant. I don't think that's too big of a deal though.Reacted by Yahiko Uzumaki, Kitson Kelly, Maxime Quandalle, Alexandru Ciucă and Aluan Haddad- locked and limited conversation to collaborators
on Jun 19, 2018
Reference issue: #7719.
We currently consider this code to be invalid:
because the callback given to
doSomethingmight be executed "later" and there "might be" (please ignore for the purposes of this issue the fact that there isn't) an assignment toxwhich invalidates the type guard.The recommended workaround is to write
which is annoying/silly/undiscoverable. A proposed change is to allow e.g.
function fn(const x: number|stringbut this is cumbersome given that the vast majority of parameters are not reassigned.For the purposes of control flow analysis, a simpler and more understandable solution is to assume that parameters are
const. Assignments would still be allowed, but treating them asconst(again, only for the purposes of control flow analysis) means we would optimistically assume that e.g.doSomething's callback is immediately invoked, which is going to be effectively correct the great majority of the time in cases where it matters.