Skip to content

Prevent merged class/namespace from overlapping with Record<string, unknown> #47088

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

Merged
merged 3 commits into from
Jan 7, 2022
Merged
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
11 changes: 8 additions & 3 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21000,9 +21000,14 @@ namespace ts {
* with no call or construct signatures.
*/
function isObjectTypeWithInferableIndex(type: Type): boolean {
return type.flags & TypeFlags.Intersection ? every((type as IntersectionType).types, isObjectTypeWithInferableIndex) :
!!(type.symbol && (type.symbol.flags & (SymbolFlags.ObjectLiteral | SymbolFlags.TypeLiteral | SymbolFlags.Enum | SymbolFlags.ValueModule)) !== 0 &&
!typeHasCallOrConstructSignatures(type)) || !!(getObjectFlags(type) & ObjectFlags.ReverseMapped && isObjectTypeWithInferableIndex((type as ReverseMappedType).source));
return type.flags & TypeFlags.Intersection
? every((type as IntersectionType).types, isObjectTypeWithInferableIndex)
: !!(
type.symbol
&& (type.symbol.flags & (SymbolFlags.ObjectLiteral | SymbolFlags.TypeLiteral | SymbolFlags.Enum | SymbolFlags.ValueModule)) !== 0
&& !(type.symbol.flags & SymbolFlags.Class)
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason not to just use !(type.symbol.flags & SymbolFlags.Class) (dropping the “and is also a value module”)? It looks to me like we believe namespaces generally have an inferable index, and classes don’t, and we want the classiness property to trump the namespacey property in this calculation. It doesn’t seem to need the specificity of “classes merged with namespaces.”

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably not; things still work as expected with that simplification. I just took the solution suggested in the issue and thumbs-up'd by Ryan.

Copy link
Member Author

Choose a reason for hiding this comment

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

Switched it over.

&& !typeHasCallOrConstructSignatures(type)
) || !!(getObjectFlags(type) & ObjectFlags.ReverseMapped && isObjectTypeWithInferableIndex((type as ReverseMappedType).source));
}

function createSymbolWithType(source: Symbol, type: Type | undefined) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
tests/cases/compiler/mergedClassNamespaceRecordCast.ts(3,1): error TS2352: Conversion of type 'C1' to type 'Record<string, unknown>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
Index signature for type 'string' is missing in type 'C1'.
tests/cases/compiler/mergedClassNamespaceRecordCast.ts(9,1): error TS2352: Conversion of type 'C2' to type 'Record<string, unknown>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
Index signature for type 'string' is missing in type 'C2'.
tests/cases/compiler/mergedClassNamespaceRecordCast.ts(12,10): error TS2339: Property 'unrelated' does not exist on type 'C2'.


==== tests/cases/compiler/mergedClassNamespaceRecordCast.ts (3 errors) ====
class C1 { foo() {} }

new C1() as Record<string, unknown>;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2352: Conversion of type 'C1' to type 'Record<string, unknown>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
!!! error TS2352: Index signature for type 'string' is missing in type 'C1'.


class C2 { foo() {} }
namespace C2 { export const unrelated = 3; }

new C2() as Record<string, unknown>;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2352: Conversion of type 'C2' to type 'Record<string, unknown>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
!!! error TS2352: Index signature for type 'string' is missing in type 'C2'.

C2.unrelated
new C2().unrelated
~~~~~~~~~
!!! error TS2339: Property 'unrelated' does not exist on type 'C2'.


namespace C3 { export const unrelated = 3; }

C3 as Record<string, unknown>;

45 changes: 45 additions & 0 deletions tests/baselines/reference/mergedClassNamespaceRecordCast.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
//// [mergedClassNamespaceRecordCast.ts]
class C1 { foo() {} }

new C1() as Record<string, unknown>;


class C2 { foo() {} }
namespace C2 { export const unrelated = 3; }

new C2() as Record<string, unknown>;

C2.unrelated
new C2().unrelated


namespace C3 { export const unrelated = 3; }

C3 as Record<string, unknown>;


//// [mergedClassNamespaceRecordCast.js]
var C1 = /** @class */ (function () {
function C1() {
}
C1.prototype.foo = function () { };
return C1;
}());
new C1();
var C2 = /** @class */ (function () {
function C2() {
}
C2.prototype.foo = function () { };
return C2;
}());
(function (C2) {
C2.unrelated = 3;
})(C2 || (C2 = {}));
new C2();
C2.unrelated;
new C2().unrelated;
var C3;
(function (C3) {
C3.unrelated = 3;
})(C3 || (C3 = {}));
C3;
39 changes: 39 additions & 0 deletions tests/baselines/reference/mergedClassNamespaceRecordCast.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
=== tests/cases/compiler/mergedClassNamespaceRecordCast.ts ===
class C1 { foo() {} }
>C1 : Symbol(C1, Decl(mergedClassNamespaceRecordCast.ts, 0, 0))
>foo : Symbol(C1.foo, Decl(mergedClassNamespaceRecordCast.ts, 0, 10))

new C1() as Record<string, unknown>;
>C1 : Symbol(C1, Decl(mergedClassNamespaceRecordCast.ts, 0, 0))
>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --))


class C2 { foo() {} }
>C2 : Symbol(C2, Decl(mergedClassNamespaceRecordCast.ts, 2, 36), Decl(mergedClassNamespaceRecordCast.ts, 5, 21))
>foo : Symbol(C2.foo, Decl(mergedClassNamespaceRecordCast.ts, 5, 10))

namespace C2 { export const unrelated = 3; }
>C2 : Symbol(C2, Decl(mergedClassNamespaceRecordCast.ts, 2, 36), Decl(mergedClassNamespaceRecordCast.ts, 5, 21))
>unrelated : Symbol(unrelated, Decl(mergedClassNamespaceRecordCast.ts, 6, 27))

new C2() as Record<string, unknown>;
>C2 : Symbol(C2, Decl(mergedClassNamespaceRecordCast.ts, 2, 36), Decl(mergedClassNamespaceRecordCast.ts, 5, 21))
>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --))

C2.unrelated
>C2.unrelated : Symbol(C2.unrelated, Decl(mergedClassNamespaceRecordCast.ts, 6, 27))
>C2 : Symbol(C2, Decl(mergedClassNamespaceRecordCast.ts, 2, 36), Decl(mergedClassNamespaceRecordCast.ts, 5, 21))
>unrelated : Symbol(C2.unrelated, Decl(mergedClassNamespaceRecordCast.ts, 6, 27))

new C2().unrelated
>C2 : Symbol(C2, Decl(mergedClassNamespaceRecordCast.ts, 2, 36), Decl(mergedClassNamespaceRecordCast.ts, 5, 21))


namespace C3 { export const unrelated = 3; }
>C3 : Symbol(C3, Decl(mergedClassNamespaceRecordCast.ts, 11, 18))
>unrelated : Symbol(unrelated, Decl(mergedClassNamespaceRecordCast.ts, 14, 27))

C3 as Record<string, unknown>;
>C3 : Symbol(C3, Decl(mergedClassNamespaceRecordCast.ts, 11, 18))
>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --))

46 changes: 46 additions & 0 deletions tests/baselines/reference/mergedClassNamespaceRecordCast.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
=== tests/cases/compiler/mergedClassNamespaceRecordCast.ts ===
class C1 { foo() {} }
>C1 : C1
>foo : () => void

new C1() as Record<string, unknown>;
>new C1() as Record<string, unknown> : Record<string, unknown>
>new C1() : C1
>C1 : typeof C1


class C2 { foo() {} }
>C2 : C2
>foo : () => void

namespace C2 { export const unrelated = 3; }
>C2 : typeof C2
>unrelated : 3
>3 : 3

new C2() as Record<string, unknown>;
>new C2() as Record<string, unknown> : Record<string, unknown>
>new C2() : C2
>C2 : typeof C2

C2.unrelated
>C2.unrelated : 3
>C2 : typeof C2
>unrelated : 3

new C2().unrelated
>new C2().unrelated : any
>new C2() : C2
>C2 : typeof C2
>unrelated : any


namespace C3 { export const unrelated = 3; }
>C3 : typeof C3
>unrelated : 3
>3 : 3

C3 as Record<string, unknown>;
>C3 as Record<string, unknown> : Record<string, unknown>
>C3 : typeof C3

17 changes: 17 additions & 0 deletions tests/cases/compiler/mergedClassNamespaceRecordCast.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
class C1 { foo() {} }

new C1() as Record<string, unknown>;


class C2 { foo() {} }
namespace C2 { export const unrelated = 3; }

new C2() as Record<string, unknown>;

C2.unrelated
new C2().unrelated


namespace C3 { export const unrelated = 3; }

C3 as Record<string, unknown>;