Skip to content

Derive disposability checks from known symbols rather than global interfaces #62122

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.

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
54 changes: 43 additions & 11 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17542,14 +17542,51 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return (deferredGlobalIteratorReturnResultType ||= getGlobalType("IteratorReturnResult" as __String, /*arity*/ 1, reportErrors)) || emptyGenericType;
}

function getGlobalDisposableType(reportErrors: boolean) {
function _getGlobalDisposableType(reportErrors: boolean) {
return (deferredGlobalDisposableType ||= getGlobalType("Disposable" as __String, /*arity*/ 0, reportErrors)) || emptyObjectType;
}

function getGlobalAsyncDisposableType(reportErrors: boolean) {
function _getGlobalAsyncDisposableType(reportErrors: boolean) {
return (deferredGlobalAsyncDisposableType ||= getGlobalType("AsyncDisposable" as __String, /*arity*/ 0, reportErrors)) || emptyObjectType;
}

function checkTypeIsDisposable(type: Type): boolean {
if (type.flags & (TypeFlags.Null | TypeFlags.Undefined)) {
return true; // null and undefined are allowed
}

// Handle union types
if (type.flags & TypeFlags.Union) {
return every((type as UnionType).types, checkTypeIsDisposable);
}

const disposePropertyName = getPropertyNameForKnownSymbolName("dispose");
const disposeProperty = getPropertyOfType(type, disposePropertyName);

return !!disposeProperty && !(disposeProperty.flags & SymbolFlags.Optional);
}

function checkTypeIsAsyncDisposable(type: Type): boolean {
if (type.flags & (TypeFlags.Null | TypeFlags.Undefined)) {
return true; // null and undefined are allowed
}

// Handle union types
if (type.flags & TypeFlags.Union) {
return every((type as UnionType).types, checkTypeIsAsyncDisposable);
}

const asyncDisposePropertyName = getPropertyNameForKnownSymbolName("asyncDispose");
const asyncDisposeProperty = getPropertyOfType(type, asyncDisposePropertyName);

if (asyncDisposeProperty && !(asyncDisposeProperty.flags & SymbolFlags.Optional)) {
return true;
}

// For await using, also check for Symbol.dispose as a fallback
return checkTypeIsDisposable(type);
}

function getGlobalTypeOrUndefined(name: __String, arity = 0): ObjectType | undefined {
const symbol = getGlobalSymbol(name, SymbolFlags.Type, /*diagnostic*/ undefined);
return symbol && getTypeOfGlobalSymbol(symbol, arity) as GenericType;
Expand Down Expand Up @@ -44999,18 +45036,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
checkTypeAssignableToAndOptionallyElaborate(initializerType, type, node, initializer, /*headMessage*/ undefined);
const blockScopeKind = getCombinedNodeFlagsCached(node) & NodeFlags.BlockScoped;
if (blockScopeKind === NodeFlags.AwaitUsing) {
const globalAsyncDisposableType = getGlobalAsyncDisposableType(/*reportErrors*/ true);
const globalDisposableType = getGlobalDisposableType(/*reportErrors*/ true);
if (globalAsyncDisposableType !== emptyObjectType && globalDisposableType !== emptyObjectType) {
const optionalDisposableType = getUnionType([globalAsyncDisposableType, globalDisposableType, nullType, undefinedType]);
checkTypeAssignableTo(widenTypeForVariableLikeDeclaration(initializerType, node), optionalDisposableType, initializer, Diagnostics.The_initializer_of_an_await_using_declaration_must_be_either_an_object_with_a_Symbol_asyncDispose_or_Symbol_dispose_method_or_be_null_or_undefined);
if (!checkTypeIsAsyncDisposable(initializerType)) {
error(initializer, Diagnostics.The_initializer_of_an_await_using_declaration_must_be_either_an_object_with_a_Symbol_asyncDispose_or_Symbol_dispose_method_or_be_null_or_undefined);
}
}
else if (blockScopeKind === NodeFlags.Using) {
const globalDisposableType = getGlobalDisposableType(/*reportErrors*/ true);
if (globalDisposableType !== emptyObjectType) {
const optionalDisposableType = getUnionType([globalDisposableType, nullType, undefinedType]);
checkTypeAssignableTo(widenTypeForVariableLikeDeclaration(initializerType, node), optionalDisposableType, initializer, Diagnostics.The_initializer_of_a_using_declaration_must_be_either_an_object_with_a_Symbol_dispose_method_or_be_null_or_undefined);
if (!checkTypeIsDisposable(initializerType)) {
error(initializer, Diagnostics.The_initializer_of_a_using_declaration_must_be_either_an_object_with_a_Symbol_dispose_method_or_be_null_or_undefined);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
usingDeclarationWithGlobalInterfaceModification.ts(26,17): error TS2850: The initializer of a 'using' declaration must be either an object with a '[Symbol.dispose]()' method, or be 'null' or 'undefined'.


==== usingDeclarationWithGlobalInterfaceModification.ts (1 errors) ====
// Test case that demonstrates the issue from https://github.com/microsoft/TypeScript/issues/62121
// When an empty global Disposable interface is declared, it should NOT affect
// the checking for Symbol.dispose properties

declare global {
interface Disposable {}
}

// This should pass - has Symbol.dispose method
const validDisposable = {
[Symbol.dispose]() {
// disposed
}
};

// This should fail - no Symbol.dispose method
const invalidDisposable = {
cleanup() {
// cleanup
}
};

// With the fix, the checker should directly check for Symbol.dispose properties
// rather than relying on assignability to the global Disposable interface
using valid = validDisposable; // should pass
using invalid = invalidDisposable; // should error
~~~~~~~~~~~~~~~~~
!!! error TS2850: The initializer of a 'using' declaration must be either an object with a '[Symbol.dispose]()' method, or be 'null' or 'undefined'.

export {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
//// [tests/cases/compiler/usingDeclarationWithGlobalInterfaceModification.ts] ////

//// [usingDeclarationWithGlobalInterfaceModification.ts]
// Test case that demonstrates the issue from https://github.com/microsoft/TypeScript/issues/62121
// When an empty global Disposable interface is declared, it should NOT affect
// the checking for Symbol.dispose properties

declare global {
interface Disposable {}
}

// This should pass - has Symbol.dispose method
const validDisposable = {
[Symbol.dispose]() {
// disposed
}
};

// This should fail - no Symbol.dispose method
const invalidDisposable = {
cleanup() {
// cleanup
}
};

// With the fix, the checker should directly check for Symbol.dispose properties
// rather than relying on assignability to the global Disposable interface
using valid = validDisposable; // should pass
using invalid = invalidDisposable; // should error

export {};

//// [usingDeclarationWithGlobalInterfaceModification.js]
// Test case that demonstrates the issue from https://github.com/microsoft/TypeScript/issues/62121
// When an empty global Disposable interface is declared, it should NOT affect
// the checking for Symbol.dispose properties
// This should pass - has Symbol.dispose method
const validDisposable = {
[Symbol.dispose]() {
// disposed
}
};
// This should fail - no Symbol.dispose method
const invalidDisposable = {
cleanup() {
// cleanup
}
};
// With the fix, the checker should directly check for Symbol.dispose properties
// rather than relying on assignability to the global Disposable interface
using valid = validDisposable; // should pass
using invalid = invalidDisposable; // should error
export {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//// [tests/cases/compiler/usingDeclarationWithGlobalInterfaceModification.ts] ////

=== usingDeclarationWithGlobalInterfaceModification.ts ===
// Test case that demonstrates the issue from https://github.com/microsoft/TypeScript/issues/62121
// When an empty global Disposable interface is declared, it should NOT affect
// the checking for Symbol.dispose properties

declare global {
>global : Symbol(global, Decl(usingDeclarationWithGlobalInterfaceModification.ts, 0, 0))

interface Disposable {}
>Disposable : Symbol(Disposable, Decl(lib.esnext.disposable.d.ts, --, --), Decl(usingDeclarationWithGlobalInterfaceModification.ts, 4, 16))
}

// This should pass - has Symbol.dispose method
const validDisposable = {
>validDisposable : Symbol(validDisposable, Decl(usingDeclarationWithGlobalInterfaceModification.ts, 9, 5))

[Symbol.dispose]() {
>[Symbol.dispose] : Symbol([Symbol.dispose], Decl(usingDeclarationWithGlobalInterfaceModification.ts, 9, 25))
>Symbol.dispose : Symbol(SymbolConstructor.dispose, Decl(lib.esnext.disposable.d.ts, --, --))
>Symbol : Symbol(Symbol, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.symbol.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2019.symbol.d.ts, --, --))
>dispose : Symbol(SymbolConstructor.dispose, Decl(lib.esnext.disposable.d.ts, --, --))

// disposed
}
};

// This should fail - no Symbol.dispose method
const invalidDisposable = {
>invalidDisposable : Symbol(invalidDisposable, Decl(usingDeclarationWithGlobalInterfaceModification.ts, 16, 5))

cleanup() {
>cleanup : Symbol(cleanup, Decl(usingDeclarationWithGlobalInterfaceModification.ts, 16, 27))

// cleanup
}
};

// With the fix, the checker should directly check for Symbol.dispose properties
// rather than relying on assignability to the global Disposable interface
using valid = validDisposable; // should pass
>valid : Symbol(valid, Decl(usingDeclarationWithGlobalInterfaceModification.ts, 24, 5))
>validDisposable : Symbol(validDisposable, Decl(usingDeclarationWithGlobalInterfaceModification.ts, 9, 5))

using invalid = invalidDisposable; // should error
>invalid : Symbol(invalid, Decl(usingDeclarationWithGlobalInterfaceModification.ts, 25, 5))
>invalidDisposable : Symbol(invalidDisposable, Decl(usingDeclarationWithGlobalInterfaceModification.ts, 16, 5))

export {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
//// [tests/cases/compiler/usingDeclarationWithGlobalInterfaceModification.ts] ////

=== usingDeclarationWithGlobalInterfaceModification.ts ===
// Test case that demonstrates the issue from https://github.com/microsoft/TypeScript/issues/62121
// When an empty global Disposable interface is declared, it should NOT affect
// the checking for Symbol.dispose properties

declare global {
>global : any
> : ^^^

interface Disposable {}
}

// This should pass - has Symbol.dispose method
const validDisposable = {
>validDisposable : { [Symbol.dispose](): void; }
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>{ [Symbol.dispose]() { // disposed }} : { [Symbol.dispose](): void; }
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Symbol.dispose]() {
>[Symbol.dispose] : () => void
> : ^^^^^^^^^^
>Symbol.dispose : unique symbol
> : ^^^^^^^^^^^^^
>Symbol : SymbolConstructor
> : ^^^^^^^^^^^^^^^^^
>dispose : unique symbol
> : ^^^^^^^^^^^^^

// disposed
}
};

// This should fail - no Symbol.dispose method
const invalidDisposable = {
>invalidDisposable : { cleanup(): void; }
> : ^^^^^^^^^^^^^^^^^^^^
>{ cleanup() { // cleanup }} : { cleanup(): void; }
> : ^^^^^^^^^^^^^^^^^^^^

cleanup() {
>cleanup : () => void
> : ^^^^^^^^^^

// cleanup
}
};

// With the fix, the checker should directly check for Symbol.dispose properties
// rather than relying on assignability to the global Disposable interface
using valid = validDisposable; // should pass
>valid : { [Symbol.dispose](): void; }
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>validDisposable : { [Symbol.dispose](): void; }
> : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

using invalid = invalidDisposable; // should error
>invalid : { cleanup(): void; }
> : ^^^^^^^^^^^^^^^^^^^^
>invalidDisposable : { cleanup(): void; }
> : ^^^^^^^^^^^^^^^^^^^^

export {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// @target: esnext
// @lib: esnext

// Test case that demonstrates the issue from https://github.com/microsoft/TypeScript/issues/62121
// When an empty global Disposable interface is declared, it should NOT affect
// the checking for Symbol.dispose properties

declare global {
interface Disposable {}
}

// This should pass - has Symbol.dispose method
const validDisposable = {
[Symbol.dispose]() {
// disposed
}
};

// This should fail - no Symbol.dispose method
const invalidDisposable = {
cleanup() {
// cleanup
}
};

// With the fix, the checker should directly check for Symbol.dispose properties
// rather than relying on assignability to the global Disposable interface
using valid = validDisposable; // should pass
using invalid = invalidDisposable; // should error

export {};