Skip to content

Commit d6e3417

Browse files
Copilotjakebailey
andcommitted
Port remaining collision checks from checkCollisionsForDeclarationName
- Add checkCollisionWithGlobalPromiseInGeneratedCode for Promise collision detection - Add recordPotentialCollisionWithReflectInGeneratedCode and checkReflectCollision for Reflect collision detection - Add checkClassNameCollisionWithObject for Object class name collision detection - Update checkCollisionsForDeclarationName to match TypeScript checker structure - Add potentialReflectCollisions array to Checker struct - Process Reflect collisions in checkSourceFile - Accept updated baselines for new collision checks Co-authored-by: jakebailey <[email protected]>
1 parent 6d0c26e commit d6e3417

File tree

31 files changed

+249
-290
lines changed

31 files changed

+249
-290
lines changed

internal/checker/checker.go

Lines changed: 78 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -775,6 +775,7 @@ type Checker struct {
775775
flowNodeReachable map[*ast.FlowNode]bool
776776
flowNodePostSuper map[*ast.FlowNode]bool
777777
potentialWeakMapSetCollisions []*ast.Node
778+
potentialReflectCollisions []*ast.Node
778779
renamedBindingElementsInTypes []*ast.Node
779780
contextualInfos []ContextualInfo
780781
inferenceContextInfos []InferenceContextInfo
@@ -2128,6 +2129,7 @@ func (c *Checker) checkSourceFile(ctx context.Context, sourceFile *ast.SourceFil
21282129
c.checkGrammarSourceFile(sourceFile)
21292130
c.renamedBindingElementsInTypes = nil
21302131
c.potentialWeakMapSetCollisions = nil
2132+
c.potentialReflectCollisions = nil
21312133
c.checkSourceElements(sourceFile.Statements.Nodes)
21322134
c.checkDeferredNodes(sourceFile)
21332135
if ast.IsExternalOrCommonJSModule(sourceFile) {
@@ -2151,6 +2153,12 @@ func (c *Checker) checkSourceFile(ctx context.Context, sourceFile *ast.SourceFil
21512153
}
21522154
c.potentialWeakMapSetCollisions = nil
21532155
}
2156+
if len(c.potentialReflectCollisions) > 0 {
2157+
for _, node := range c.potentialReflectCollisions {
2158+
c.checkReflectCollision(node)
2159+
}
2160+
c.potentialReflectCollisions = nil
2161+
}
21542162
c.ctx = nil
21552163
links.typeChecked = true
21562164
}
@@ -10158,14 +10166,19 @@ func (c *Checker) assignBindingElementTypes(pattern *ast.Node, parentType *Type)
1015810166
}
1015910167

1016010168
func (c *Checker) checkCollisionsForDeclarationName(node *ast.Node, name *ast.Node) {
10169+
if name == nil {
10170+
return
10171+
}
1016110172
c.checkCollisionWithRequireExportsInGeneratedCode(node, name)
10173+
c.checkCollisionWithGlobalPromiseInGeneratedCode(node, name)
1016210174
c.recordPotentialCollisionWithWeakMapSetInGeneratedCode(node, name)
10163-
switch {
10164-
case name == nil:
10165-
return
10166-
case ast.IsClassLike(node):
10175+
c.recordPotentialCollisionWithReflectInGeneratedCode(node, name)
10176+
if ast.IsClassLike(node) {
1016710177
c.checkTypeNameIsReserved(name, diagnostics.Class_name_cannot_be_0)
10168-
case ast.IsEnumDeclaration(node):
10178+
if node.Flags&ast.NodeFlagsAmbient == 0 {
10179+
c.checkClassNameCollisionWithObject(name)
10180+
}
10181+
} else if ast.IsEnumDeclaration(node) {
1016910182
c.checkTypeNameIsReserved(name, diagnostics.Enum_name_cannot_be_0)
1017010183
}
1017110184
}
@@ -10248,6 +10261,66 @@ func (c *Checker) checkWeakMapSetCollision(node *ast.Node) {
1024810261
}
1024910262
}
1025010263

10264+
func (c *Checker) checkCollisionWithGlobalPromiseInGeneratedCode(node *ast.Node, name *ast.Node) {
10265+
if name == nil || c.languageVersion >= core.ScriptTargetES2017 || !c.needCollisionCheckForIdentifier(node, name, "Promise") {
10266+
return
10267+
}
10268+
// Uninstantiated modules shouldn't do this check
10269+
if ast.IsModuleDeclaration(node) && ast.GetModuleInstanceState(node) != ast.ModuleInstanceStateInstantiated {
10270+
return
10271+
}
10272+
// In case of variable declaration, node.parent is variable statement so look at the variable statement's parent
10273+
parent := ast.GetDeclarationContainer(node)
10274+
// Note: TypeScript checks for HasAsyncFunctions flag, but since that flag is not yet ported to Go,
10275+
// we conservatively check all external/CommonJS modules. This may produce false positives
10276+
// but is safer than missing real issues.
10277+
if ast.IsSourceFile(parent) && ast.IsExternalOrCommonJSModule(parent.AsSourceFile()) {
10278+
// If the declaration happens to be in external module, report error that Promise is a reserved identifier.
10279+
c.errorSkippedOnNoEmit(name, diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_a_module_containing_async_functions, scanner.DeclarationNameToString(name), scanner.DeclarationNameToString(name))
10280+
}
10281+
}
10282+
10283+
func (c *Checker) recordPotentialCollisionWithReflectInGeneratedCode(node *ast.Node, name *ast.Node) {
10284+
if name != nil && c.languageVersion <= core.ScriptTargetES2021 && c.needCollisionCheckForIdentifier(node, name, "Reflect") {
10285+
c.potentialReflectCollisions = append(c.potentialReflectCollisions, node)
10286+
}
10287+
}
10288+
10289+
func (c *Checker) checkReflectCollision(node *ast.Node) {
10290+
hasCollision := false
10291+
if ast.IsClassExpression(node) {
10292+
// ClassExpression names don't contribute to their containers, but do matter for any of their block-scoped members.
10293+
for _, member := range node.Members() {
10294+
if c.nodeLinks.Get(member).flags&NodeCheckFlagsContainsSuperPropertyInStaticInitializer != 0 {
10295+
hasCollision = true
10296+
break
10297+
}
10298+
}
10299+
} else if ast.IsFunctionExpression(node) {
10300+
// FunctionExpression names don't contribute to their containers, but do matter for their contents
10301+
if c.nodeLinks.Get(node).flags&NodeCheckFlagsContainsSuperPropertyInStaticInitializer != 0 {
10302+
hasCollision = true
10303+
}
10304+
} else {
10305+
container := ast.GetEnclosingBlockScopeContainer(node)
10306+
if container != nil && c.nodeLinks.Get(container).flags&NodeCheckFlagsContainsSuperPropertyInStaticInitializer != 0 {
10307+
hasCollision = true
10308+
}
10309+
}
10310+
if hasCollision {
10311+
name := node.Name()
10312+
if name != nil && ast.IsIdentifier(name) {
10313+
c.errorSkippedOnNoEmit(node, diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_when_emitting_super_references_in_static_initializers, scanner.DeclarationNameToString(name), "Reflect")
10314+
}
10315+
}
10316+
}
10317+
10318+
func (c *Checker) checkClassNameCollisionWithObject(name *ast.Node) {
10319+
if name.Text() == "Object" && c.program.GetEmitModuleFormatOfFile(ast.GetSourceFileOfNode(name)) < core.ModuleKindES2015 {
10320+
c.error(name, diagnostics.Class_name_cannot_be_Object_when_targeting_ES5_and_above_with_module_0, core.ModuleKind(c.moduleKind).String())
10321+
}
10322+
}
10323+
1025110324
func (c *Checker) checkTypeOfExpression(node *ast.Node) *Type {
1025210325
c.checkExpression(node.Expression())
1025310326
return c.typeofType
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
checkForObjectTooStrict.ts(3,18): error TS2725: Class name cannot be 'Object' when targeting ES5 and above with module CommonJS.
2+
3+
4+
==== checkForObjectTooStrict.ts (1 errors) ====
5+
module Foo {
6+
7+
export class Object {
8+
~~~~~~
9+
!!! error TS2725: Class name cannot be 'Object' when targeting ES5 and above with module CommonJS.
10+
11+
}
12+
13+
}
14+
15+
16+
17+
class Bar extends Foo.Object { // should work
18+
19+
constructor () {
20+
21+
super();
22+
23+
}
24+
25+
}
26+
27+
28+
class Baz extends Object {
29+
30+
constructor () { // ERROR, as expected
31+
32+
super();
33+
34+
}
35+
36+
}
37+

testdata/baselines/reference/submodule/compiler/checkForObjectTooStrict.errors.txt.diff

Lines changed: 0 additions & 41 deletions
This file was deleted.

testdata/baselines/reference/submodule/compiler/instanceofOperator.errors.txt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
1+
instanceofOperator.ts(7,11): error TS2725: Class name cannot be 'Object' when targeting ES5 and above with module CommonJS.
12
instanceofOperator.ts(12,5): error TS2358: The left-hand side of an 'instanceof' expression must be of type 'any', an object type or a type parameter.
23
instanceofOperator.ts(15,20): error TS2359: The right-hand side of an 'instanceof' expression must be either of type 'any', a class, function, or other type assignable to the 'Function' interface type, or an object type with a 'Symbol.hasInstance' method.
34
instanceofOperator.ts(16,23): error TS2359: The right-hand side of an 'instanceof' expression must be either of type 'any', a class, function, or other type assignable to the 'Function' interface type, or an object type with a 'Symbol.hasInstance' method.
45
instanceofOperator.ts(19,5): error TS2358: The left-hand side of an 'instanceof' expression must be of type 'any', an object type or a type parameter.
56
instanceofOperator.ts(21,5): error TS2358: The left-hand side of an 'instanceof' expression must be of type 'any', an object type or a type parameter.
67

78

8-
==== instanceofOperator.ts (5 errors) ====
9+
==== instanceofOperator.ts (6 errors) ====
910
// Spec:
1011
// The instanceof operator requires the left operand to be of type Any or an object type, and the right
1112
// operand to be of type Any or a subtype of the ‘Function’ interface type. The result is always of the
1213
// Boolean primitive type.
1314

1415
module test {
1516
class Object { }
17+
~~~~~~
18+
!!! error TS2725: Class name cannot be 'Object' when targeting ES5 and above with module CommonJS.
1619
var obj: Object;
1720

1821

testdata/baselines/reference/submodule/compiler/instanceofOperator.errors.txt.diff

Lines changed: 0 additions & 25 deletions
This file was deleted.

testdata/baselines/reference/submodule/compiler/moduleInTypePosition1.errors.txt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
moduleInTypePosition1_0.ts(1,14): error TS2529: Duplicate identifier 'Promise'. Compiler reserves name 'Promise' in top level scope of a module containing async functions.
12
moduleInTypePosition1_1.ts(3,14): error TS2709: Cannot use namespace 'WinJS' as a type.
23

34

@@ -8,8 +9,10 @@ moduleInTypePosition1_1.ts(3,14): error TS2709: Cannot use namespace 'WinJS' as
89
~~~~~
910
!!! error TS2709: Cannot use namespace 'WinJS' as a type.
1011

11-
==== moduleInTypePosition1_0.ts (0 errors) ====
12+
==== moduleInTypePosition1_0.ts (1 errors) ====
1213
export class Promise {
14+
~~~~~~~
15+
!!! error TS2529: Duplicate identifier 'Promise'. Compiler reserves name 'Promise' in top level scope of a module containing async functions.
1316
foo: string;
1417
}
1518

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
--- old.moduleInTypePosition1.errors.txt
2+
+++ new.moduleInTypePosition1.errors.txt
3+
@@= skipped -0, +0 lines =@@
4+
+moduleInTypePosition1_0.ts(1,14): error TS2529: Duplicate identifier 'Promise'. Compiler reserves name 'Promise' in top level scope of a module containing async functions.
5+
moduleInTypePosition1_1.ts(3,14): error TS2709: Cannot use namespace 'WinJS' as a type.
6+
7+
8+
@@= skipped -7, +8 lines =@@
9+
~~~~~
10+
!!! error TS2709: Cannot use namespace 'WinJS' as a type.
11+
12+
-==== moduleInTypePosition1_0.ts (0 errors) ====
13+
+==== moduleInTypePosition1_0.ts (1 errors) ====
14+
export class Promise {
15+
+ ~~~~~~~
16+
+!!! error TS2529: Duplicate identifier 'Promise'. Compiler reserves name 'Promise' in top level scope of a module containing async functions.
17+
foo: string;
18+
}
19+
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
staticInstanceResolution3_0.ts(1,14): error TS2529: Duplicate identifier 'Promise'. Compiler reserves name 'Promise' in top level scope of a module containing async functions.
2+
3+
4+
==== staticInstanceResolution3_1.ts (0 errors) ====
5+
///<reference path='staticInstanceResolution3_0.ts'/>
6+
import WinJS = require('./staticInstanceResolution3_0');
7+
WinJS.Promise.timeout(10);
8+
==== staticInstanceResolution3_0.ts (1 errors) ====
9+
export class Promise {
10+
~~~~~~~
11+
!!! error TS2529: Duplicate identifier 'Promise'. Compiler reserves name 'Promise' in top level scope of a module containing async functions.
12+
static timeout(delay: number): Promise {
13+
return null;
14+
}
15+
}
16+
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--- old.staticInstanceResolution3.errors.txt
2+
+++ new.staticInstanceResolution3.errors.txt
3+
@@= skipped -0, +0 lines =@@
4+
-<no content>
5+
+staticInstanceResolution3_0.ts(1,14): error TS2529: Duplicate identifier 'Promise'. Compiler reserves name 'Promise' in top level scope of a module containing async functions.
6+
+
7+
+
8+
+==== staticInstanceResolution3_1.ts (0 errors) ====
9+
+ ///<reference path='staticInstanceResolution3_0.ts'/>
10+
+ import WinJS = require('./staticInstanceResolution3_0');
11+
+ WinJS.Promise.timeout(10);
12+
+==== staticInstanceResolution3_0.ts (1 errors) ====
13+
+ export class Promise {
14+
+ ~~~~~~~
15+
+!!! error TS2529: Duplicate identifier 'Promise'. Compiler reserves name 'Promise' in top level scope of a module containing async functions.
16+
+ static timeout(delay: number): Promise {
17+
+ return null;
18+
+ }
19+
+ }
20+
+

testdata/baselines/reference/submodule/compiler/staticInstanceResolution5.errors.txt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
staticInstanceResolution5_0.ts(1,14): error TS2529: Duplicate identifier 'Promise'. Compiler reserves name 'Promise' in top level scope of a module containing async functions.
12
staticInstanceResolution5_1.ts(4,14): error TS2709: Cannot use namespace 'WinJS' as a type.
23
staticInstanceResolution5_1.ts(5,23): error TS2709: Cannot use namespace 'WinJS' as a type.
34
staticInstanceResolution5_1.ts(6,16): error TS2709: Cannot use namespace 'WinJS' as a type.
@@ -17,8 +18,10 @@ staticInstanceResolution5_1.ts(6,16): error TS2709: Cannot use namespace 'WinJS'
1718
~~~~~
1819
!!! error TS2709: Cannot use namespace 'WinJS' as a type.
1920

20-
==== staticInstanceResolution5_0.ts (0 errors) ====
21+
==== staticInstanceResolution5_0.ts (1 errors) ====
2122
export class Promise {
23+
~~~~~~~
24+
!!! error TS2529: Duplicate identifier 'Promise'. Compiler reserves name 'Promise' in top level scope of a module containing async functions.
2225
static timeout(delay: number): Promise {
2326
return null;
2427
}

0 commit comments

Comments
 (0)