Skip to content

Improve inference by not considering thisless functions to be context-sensitive #62243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+3,339 −55
Open

Improve inference by not considering thisless functions to be context-sensitive #62243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4e92f82
Improve inference by not considering thisless functions to be context…
Andarist Aug 6, 2025
c7f90c9
cleanup `ContainerFlags.IsControlFlowContainer`
Andarist Aug 6, 2025
c29601c
add tests
Andarist Aug 6, 2025
cf4544d
fix propagation condition
Andarist Aug 7, 2025
5fed81c
add test cases
Andarist Aug 9, 2025
c069fbb
fmt
Andarist Aug 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,7 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
const saveExceptionTarget = currentExceptionTarget;
const saveActiveLabelList = activeLabelList;
const saveHasExplicitReturn = hasExplicitReturn;
const saveSeenThisKeyword = seenThisKeyword;
const isImmediatelyInvoked = (
containerFlags & ContainerFlags.IsFunctionExpression &&
!hasSyntacticModifier(node, ModifierFlags.Async) &&
Expand All @@ -1036,19 +1037,22 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
currentContinueTarget = undefined;
activeLabelList = undefined;
hasExplicitReturn = false;
seenThisKeyword = false;
bindChildren(node);
// Reset all reachability check related flags on node (for incremental scenarios)
node.flags &= ~NodeFlags.ReachabilityAndEmitFlags;
// Reset flags (for incremental scenarios)
node.flags &= ~(NodeFlags.ReachabilityAndEmitFlags | NodeFlags.ContainsThis);
Copy link
Preview

Copilot AI Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The comment above this line mentions 'Reset all reachability check related flags' but now it also resets ContainsThis. Consider updating the comment to reflect that other flags are also being reset.

Suggested change
node.flags &= ~(NodeFlags.ReachabilityAndEmitFlags | NodeFlags.ContainsThis);
// Reset reachability, emit, and ContainsThis flags (for incremental scenarios)
node.flags &= ~(NodeFlags.ReachabilityAndEmitFlags | NodeFlags.ContainsThis);

Copilot uses AI. Check for mistakes.

if (!(currentFlow.flags & FlowFlags.Unreachable) && containerFlags & ContainerFlags.IsFunctionLike && nodeIsPresent((node as FunctionLikeDeclaration | ClassStaticBlockDeclaration).body)) {
node.flags |= NodeFlags.HasImplicitReturn;
if (hasExplicitReturn) node.flags |= NodeFlags.HasExplicitReturn;
(node as FunctionLikeDeclaration | ClassStaticBlockDeclaration).endFlowNode = currentFlow;
}
if (seenThisKeyword) {
node.flags |= NodeFlags.ContainsThis;
}
if (node.kind === SyntaxKind.SourceFile) {
node.flags |= emitFlags;
(node as SourceFile).endFlowNode = currentFlow;
}

if (currentReturnTarget) {
addAntecedent(currentReturnTarget, currentFlow);
currentFlow = finishFlowLabel(currentReturnTarget);
Expand All @@ -1065,12 +1069,15 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
currentExceptionTarget = saveExceptionTarget;
activeLabelList = saveActiveLabelList;
hasExplicitReturn = saveHasExplicitReturn;
seenThisKeyword = node.kind === SyntaxKind.ArrowFunction ? saveSeenThisKeyword || seenThisKeyword : saveSeenThisKeyword;
}
else if (containerFlags & ContainerFlags.IsInterface) {
const saveSeenThisKeyword = seenThisKeyword;
seenThisKeyword = false;
bindChildren(node);
Debug.assertNotNode(node, isIdentifier); // ContainsThis cannot overlap with HasExtendedUnicodeEscape on Identifier
node.flags = seenThisKeyword ? node.flags | NodeFlags.ContainsThis : node.flags & ~NodeFlags.ContainsThis;
seenThisKeyword = saveSeenThisKeyword;
}
else {
bindChildren(node);
Expand Down Expand Up @@ -2873,6 +2880,9 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
}
// falls through
case SyntaxKind.ThisKeyword:
if (node.kind === SyntaxKind.ThisKeyword) {
seenThisKeyword = true;
}
// TODO: Why use `isExpression` here? both Identifier and ThisKeyword are expressions.
if (currentFlow && (isExpression(node) || parent.kind === SyntaxKind.ShorthandPropertyAssignment)) {
(node as Identifier | ThisExpression).flowNode = currentFlow;
Expand Down Expand Up @@ -3941,28 +3951,27 @@ export function getContainerFlags(node: Node): ContainerFlags {
// falls through
case SyntaxKind.Constructor:
case SyntaxKind.FunctionDeclaration:
case SyntaxKind.ClassStaticBlockDeclaration:
return ContainerFlags.IsContainer | ContainerFlags.IsControlFlowContainer | ContainerFlags.HasLocals | ContainerFlags.IsFunctionLike;
case SyntaxKind.MethodSignature:
case SyntaxKind.CallSignature:
case SyntaxKind.JSDocSignature:
case SyntaxKind.JSDocFunctionType:
case SyntaxKind.FunctionType:
case SyntaxKind.ConstructSignature:
case SyntaxKind.ConstructorType:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type-level AST nodes had ContainerFlags.IsControlFlowContainer here. This was messing up some of the changes I made since it was interfering with the implemented seenThisKeyword tracking. I don't see why those would be considered control flow containers and there are no tests proving it was needed.

Other changes in this function are basically of the same kind - I just removed ContainerFlags.IsControlFlowContainer from the type-level nodes.

case SyntaxKind.ClassStaticBlockDeclaration:
return ContainerFlags.IsContainer | ContainerFlags.IsControlFlowContainer | ContainerFlags.HasLocals | ContainerFlags.IsFunctionLike;
return ContainerFlags.IsContainer | ContainerFlags.HasLocals | ContainerFlags.IsFunctionLike;

case SyntaxKind.JSDocImportTag:
// treat as a container to prevent using an enclosing effective host, ensuring import bindings are scoped correctly
return ContainerFlags.IsContainer | ContainerFlags.IsControlFlowContainer | ContainerFlags.HasLocals;
return ContainerFlags.IsContainer | ContainerFlags.HasLocals;

case SyntaxKind.FunctionExpression:
case SyntaxKind.ArrowFunction:
return ContainerFlags.IsContainer | ContainerFlags.IsControlFlowContainer | ContainerFlags.HasLocals | ContainerFlags.IsFunctionLike | ContainerFlags.IsFunctionExpression;

case SyntaxKind.ModuleBlock:
return ContainerFlags.IsControlFlowContainer;
case SyntaxKind.PropertyDeclaration:
return (node as PropertyDeclaration).initializer ? ContainerFlags.IsControlFlowContainer : 0;

case SyntaxKind.CatchClause:
case SyntaxKind.ForStatement:
Expand Down
7 changes: 4 additions & 3 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21135,9 +21135,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const { initializer } = node as JsxAttribute;
return !!initializer && isContextSensitive(initializer);
}
case SyntaxKind.JsxExpression: {
case SyntaxKind.JsxExpression:
case SyntaxKind.YieldExpression: {
// It is possible to that node.expression is undefined (e.g <div x={} />)
const { expression } = node as JsxExpression;
const { expression } = node as JsxExpression | YieldExpression;
return !!expression && isContextSensitive(expression);
}
}
Expand All @@ -21146,7 +21147,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function isContextSensitiveFunctionLikeDeclaration(node: FunctionLikeDeclaration): boolean {
return hasContextSensitiveParameters(node) || hasContextSensitiveReturnExpression(node);
return hasContextSensitiveParameters(node) || hasContextSensitiveReturnExpression(node) || !!(getFunctionFlags(node) & FunctionFlags.Generator && node.body && forEachYieldExpression(node.body as Block, isContextSensitive));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously generators were always context-sensitive as they can't even be arrow functions. At times, they are truly context-sensitive in cases like:

declare function test(
  gen: () => Generator<(arg: number) => string, void, void>,
): void;

test(function* () {
  yield (arg) => String(arg);
});

So I had to add this extra forEachYieldExpression to cover for this

}

function hasContextSensitiveReturnExpression(node: FunctionLikeDeclaration) {
Expand Down
24 changes: 14 additions & 10 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2864,19 +2864,24 @@ export function forEachReturnStatement<T>(body: Block | Statement, visitor: (stm
}
}

// Warning: This has the same semantics as the forEach family of functions,
// in that traversal terminates in the event that 'visitor' supplies a truthy value.
/** @internal */
export function forEachYieldExpression(body: Block, visitor: (expr: YieldExpression) => void): void {
export function forEachYieldExpression<T>(body: Block, visitor: (expr: YieldExpression) => T): T | undefined {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this basically changes this function to work in the same way as forEachReturnStatement defined above

return traverse(body);

function traverse(node: Node): void {
function traverse(node: Node): T | undefined {
switch (node.kind) {
case SyntaxKind.YieldExpression:
visitor(node as YieldExpression);
const value = visitor(node as YieldExpression);
if (value) {
return value;
}
const operand = (node as YieldExpression).expression;
if (operand) {
traverse(operand);
if (!operand) {
return;
}
return;
return traverse(operand);
case SyntaxKind.EnumDeclaration:
case SyntaxKind.InterfaceDeclaration:
case SyntaxKind.ModuleDeclaration:
Expand All @@ -2889,14 +2894,13 @@ export function forEachYieldExpression(body: Block, visitor: (expr: YieldExpress
if (node.name && node.name.kind === SyntaxKind.ComputedPropertyName) {
// Note that we will not include methods/accessors of a class because they would require
// first descending into the class. This is by design.
traverse(node.name.expression);
return;
return traverse(node.name.expression);
}
Copy link
Preview

Copilot AI Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a double semicolon at the end of this line. Remove the extra semicolon.

Copilot uses AI. Check for mistakes.

}
else if (!isPartOfTypeNode(node)) {
// This is the general case, which should include mostly expressions and statements.
// Also includes NodeArrays.
forEachChild(node, traverse);
return forEachChild(node, traverse);
}
}
}
Expand Down Expand Up @@ -10825,7 +10829,7 @@ export function hasContextSensitiveParameters(node: FunctionLikeDeclaration): bo
// an implicit 'this' parameter which is subject to contextual typing.
const parameter = firstOrUndefined(node.parameters);
if (!(parameter && parameterIsThisKeyword(parameter))) {
return true;
return !!(node.flags & NodeFlags.ContainsThis);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/circularlySimplifyingConditionalTypesNoCrash.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ declare var connect: Connect;
const myStoreConnect: Connect = function(
>myStoreConnect : Connect
> : ^^^^^^^
>function( mapStateToProps?: any, mapDispatchToProps?: any, mergeProps?: any, options: unknown = {},) { return connect( mapStateToProps, mapDispatchToProps, mergeProps, options, );} : <TStateProps, TOwnProps>(mapStateToProps?: any, mapDispatchToProps?: any, mergeProps?: any, options?: unknown) => InferableComponentEnhancerWithProps<TStateProps, Omit<P, Extract<keyof TStateProps, keyof P>> & TOwnProps>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this just changes the inferred type to match the type inferred from the equivalent arrow function with type parameters, it's purely a result of making a thisless function context-insensitive

> : ^ ^^ ^^ ^^^ ^^ ^^^ ^^ ^^^ ^^ ^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>function( mapStateToProps?: any, mapDispatchToProps?: any, mergeProps?: any, options: unknown = {},) { return connect( mapStateToProps, mapDispatchToProps, mergeProps, options, );} : (mapStateToProps?: any, mapDispatchToProps?: any, mergeProps?: any, options?: unknown) => InferableComponentEnhancerWithProps<TStateProps, Omit<P, Extract<keyof TStateProps, keyof P>> & TOwnProps>
> : ^ ^^^ ^^ ^^^ ^^ ^^^ ^^ ^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

mapStateToProps?: any,
>mapStateToProps : any
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/classCanExtendConstructorFunction.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
first.js(23,9): error TS2554: Expected 1 arguments, but got 0.
first.js(31,5): error TS2416: Property 'load' in type 'Sql' is not assignable to the same property in base type 'Wagon'.
Type '(files: string[], format: "csv" | "json" | "xmlolololol") => void' is not assignable to type '(supplies?: any[]) => void'.
Type '(files: string[], format: "csv" | "json" | "xmlolololol") => void' is not assignable to type '(supplies?: any[] | undefined) => void'.
Target signature provides too few arguments. Expected 2 or more, but got 1.
first.js(47,24): error TS2507: Type '(numberEaten: number) => void' is not a constructor function type.
generic.js(19,19): error TS2554: Expected 1 arguments, but got 0.
generic.js(20,32): error TS2345: Argument of type 'number' is not assignable to parameter of type '{ claim: "ignorant" | "malicious"; }'.
second.ts(8,25): error TS2507: Type '(numberEaten: number) => void' is not a constructor function type.
second.ts(14,7): error TS2417: Class static side 'typeof Conestoga' incorrectly extends base class static side 'typeof Wagon'.
Types of property 'circle' are incompatible.
Type '(others: (typeof Wagon)[]) => number' is not assignable to type '(wagons?: Wagon[]) => number'.
Type '(others: (typeof Wagon)[]) => number' is not assignable to type '(wagons?: Wagon[] | undefined) => number'.
Types of parameters 'others' and 'wagons' are incompatible.
Type 'Wagon[]' is not assignable to type '(typeof Wagon)[]'.
Property 'circle' is missing in type 'Wagon' but required in type 'typeof Wagon'.
Expand Down Expand Up @@ -52,7 +52,7 @@ second.ts(17,15): error TS2345: Argument of type 'string' is not assignable to p
load(files, format) {
~~~~
!!! error TS2416: Property 'load' in type 'Sql' is not assignable to the same property in base type 'Wagon'.
!!! error TS2416: Type '(files: string[], format: "csv" | "json" | "xmlolololol") => void' is not assignable to type '(supplies?: any[]) => void'.
!!! error TS2416: Type '(files: string[], format: "csv" | "json" | "xmlolololol") => void' is not assignable to type '(supplies?: any[] | undefined) => void'.
!!! error TS2416: Target signature provides too few arguments. Expected 2 or more, but got 1.
if (format === "xmlolololol") {
throw new Error("please do not use XML. It was a joke.");
Expand Down Expand Up @@ -95,7 +95,7 @@ second.ts(17,15): error TS2345: Argument of type 'string' is not assignable to p
~~~~~~~~~
!!! error TS2417: Class static side 'typeof Conestoga' incorrectly extends base class static side 'typeof Wagon'.
!!! error TS2417: Types of property 'circle' are incompatible.
!!! error TS2417: Type '(others: (typeof Wagon)[]) => number' is not assignable to type '(wagons?: Wagon[]) => number'.
!!! error TS2417: Type '(others: (typeof Wagon)[]) => number' is not assignable to type '(wagons?: Wagon[] | undefined) => number'.
!!! error TS2417: Types of parameters 'others' and 'wagons' are incompatible.
!!! error TS2417: Type 'Wagon[]' is not assignable to type '(typeof Wagon)[]'.
!!! error TS2417: Property 'circle' is missing in type 'Wagon' but required in type 'typeof Wagon'.
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/generatorTypeCheck62.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ export const Nothing2: Strategy<State> = strategy("Nothing", function*(state: St
export const Nothing3: Strategy<State> = strategy("Nothing", function* (state: State) {
>Nothing3 : Strategy<State>
> : ^^^^^^^^^^^^^^^
>strategy("Nothing", function* (state: State) { yield ; return state; // `return`/`TReturn` isn't supported by `strategy`, so this should error.}) : (a: State) => IterableIterator<State, void>
> : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>strategy("Nothing", function* (state: State) { yield ; return state; // `return`/`TReturn` isn't supported by `strategy`, so this should error.}) : (a: any) => IterableIterator<any, void>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly here, this is a result of inferSignatureInstantiationForOverloadFailure no longer skipping the generator function on the basis it's context-sensitive (inferSignatureInstantiationForOverloadFailure uses CheckMode.SkipContextSensitive)

> : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>strategy : <T extends StrategicState>(stratName: string, gen: (a: T) => IterableIterator<T | undefined, void>) => (a: T) => IterableIterator<T | undefined, void>
> : ^ ^^^^^^^^^ ^^ ^^ ^^ ^^ ^^^^^
>"Nothing" : "Nothing"
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/jsdocTemplateTag.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
forgot.js(23,1): error TS2322: Type '(keyframes: any[]) => void' is not assignable to type '(keyframes: Keyframe[] | PropertyIndexedKeyframes, options?: number | KeyframeAnimationOptions) => Animation'.
forgot.js(23,1): error TS2322: Type '(keyframes: Array<any>) => void' is not assignable to type '(keyframes: Keyframe[] | PropertyIndexedKeyframes, options?: number | KeyframeAnimationOptions) => Animation'.
Type 'void' is not assignable to type 'Animation'.


Expand Down Expand Up @@ -27,6 +27,6 @@ forgot.js(23,1): error TS2322: Type '(keyframes: any[]) => void' is not assignab
*/
Element.prototype.animate = function(keyframes) {};
~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2322: Type '(keyframes: any[]) => void' is not assignable to type '(keyframes: Keyframe[] | PropertyIndexedKeyframes, options?: number | KeyframeAnimationOptions) => Animation'.
!!! error TS2322: Type '(keyframes: Array<any>) => void' is not assignable to type '(keyframes: Keyframe[] | PropertyIndexedKeyframes, options?: number | KeyframeAnimationOptions) => Animation'.
!!! error TS2322: Type 'void' is not assignable to type 'Animation'.

4 changes: 2 additions & 2 deletions tests/baselines/reference/redefineArray.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
redefineArray.ts(1,1): error TS2741: Property 'isArray' is missing in type '<T>(n: number, s: string) => number' but required in type 'ArrayConstructor'.
redefineArray.ts(1,1): error TS2741: Property 'isArray' is missing in type '(n: number, s: string) => number' but required in type 'ArrayConstructor'.


==== redefineArray.ts (1 errors) ====
Array = function (n:number, s:string) {return n;};
~~~~~
!!! error TS2741: Property 'isArray' is missing in type '<T>(n: number, s: string) => number' but required in type 'ArrayConstructor'.
!!! error TS2741: Property 'isArray' is missing in type '(n: number, s: string) => number' but required in type 'ArrayConstructor'.
!!! related TS2728 lib.es5.d.ts:--:--: 'isArray' is declared here.
8 changes: 4 additions & 4 deletions tests/baselines/reference/redefineArray.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@

=== redefineArray.ts ===
Array = function (n:number, s:string) {return n;};
>Array = function (n:number, s:string) {return n;} : <T>(n: number, s: string) => number
> : ^ ^^ ^^ ^^ ^^ ^^^^^^^^^^^
>Array = function (n:number, s:string) {return n;} : (n: number, s: string) => number
> : ^ ^^ ^^ ^^ ^^^^^^^^^^^
>Array : ArrayConstructor
> : ^^^^^^^^^^^^^^^^
>function (n:number, s:string) {return n;} : <T>(n: number, s: string) => number
> : ^ ^^ ^^ ^^ ^^ ^^^^^^^^^^^
>function (n:number, s:string) {return n;} : (n: number, s: string) => number
> : ^ ^^ ^^ ^^ ^^^^^^^^^^^
>n : number
> : ^^^^^^
>s : string
Expand Down
Loading
Loading