Assume parameters are `const` for the purposes of control flow analysis

[RyanCavanaugh]

Reference issue: #7719.

We currently consider this code to be invalid:

function fn(x: number|string) {
  if (typeof x === 'number') {
    doSomething(() => x.toFixed());
  }
}

because the callback given to doSomething might be executed "later" and there "might be" (please ignore for the purposes of this issue the fact that there isn't) an assignment to x which invalidates the type guard.

The recommended workaround is to write

function fn(x: number|string) {
  const y = x;
  if (typeof y === 'number') {
    doSomething(() => y.toFixed());
  }
}

which is annoying/silly/undiscoverable. A proposed change is to allow e.g. function fn(const x: number|string but 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 as const (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.

[RyanCavanaugh]

Discussed alternative was to treat function expression as if they are always immediately invoked, which is obviously possibly-wrong for other reasons

[tinganho]

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 async keyword as a type annotation might conflict with the meaning of async declarations in async/await which makes a function returning a promise. An alternative is to use nonimmediate, though I think that keyword is too long.

[yortus]

@tinganho what about all the callback signatures already declared in .d.ts files 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 an async annotation, that might lead to a lot of incorrect type inference in existing code.

[tinganho]

@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 --strictCallbackChecking to opt in for the new functionality described above?

[yortus]

add a flag

They will love that :wink: :flags: :crossed_flags: :us:

[zpdDG4gta8XKpMCd]

(clears the throat) https://github.com/Microsoft/TypeScript/issues/7770 might be related somehow, despite it is widely believed that

pure is a bit impractical

[gcnew]

An 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

[zpdDG4gta8XKpMCd]

Simply add a flag :)

The design team might need to seriously consider switching to a flag-driven-development paradigm.

[RyanCavanaugh]

Proposal: 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.

[zpdDG4gta8XKpMCd]

@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 --strictNullChecks and --noImplicitAny which 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

[tinganho]

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.

[yortus]

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.

[use-strict]

...or do a flag cleanup at the cost of breaking some backwards compatibility.

\ sorry for the off-topic

[gcnew]

@aleksey-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 const instead of var. 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 @tinganho proposes - maybe it could be a part of --strictNullChecks? Though it's definitely useful as a standalone feature.

[ahejlsberg]

Returning to the original topic... I think a reasonable compromise here would be to consider parameters const when 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 a null or undefined into a default value) but then kept constant. I don't think that's too big of a deal though.