Improve valueDeclaration for js module merges (#24707)

Nearly everything in a merge of JS special assignments looks like a
valueDeclaration. This commit ensures that intermediate "module
declarations" are not used when a better valueDeclaration is available:

```js
// File1.js
var X = {}
X.Y.Z = class { }

// File2.js
X.Y = {}
```

In the above example, the `Y` in `X.Y.Z = class { }` was used as the
valueDeclaration for `Y` because it appeared before `X.Y = {}` in the
compilation.

This change exposed a bug in binding, #24703, that required a change in
typeFromPropertyAssignmentOutOfOrder. The test still fails for the
original reason it was created, and the new bug #24703 contains a repro.

Author: sandersn

src/compiler/binder.ts

@@ -236,7 +236,7 @@ namespace ts {
             if (symbolFlags & SymbolFlags.Value) {
                 const { valueDeclaration } = symbol;
                 if (!valueDeclaration ||
-                    (valueDeclaration.kind !== node.kind && valueDeclaration.kind === SyntaxKind.ModuleDeclaration)) {
+                    (valueDeclaration.kind !== node.kind && isEffectiveModuleDeclaration(valueDeclaration))) {
                     // other kinds of value declarations take precedence over modules
                     symbol.valueDeclaration = node;
                 }

src/compiler/checker.ts

@@ -921,7 +921,7 @@ namespace ts {
                 target.flags |= source.flags;
                 if (source.valueDeclaration &&
                     (!target.valueDeclaration ||
-                        (target.valueDeclaration.kind === SyntaxKind.ModuleDeclaration && source.valueDeclaration.kind !== SyntaxKind.ModuleDeclaration))) {
+                     isEffectiveModuleDeclaration(target.valueDeclaration) && !isEffectiveModuleDeclaration(source.valueDeclaration))) {
                     // other kinds of value declarations take precedence over modules
                     target.valueDeclaration = source.valueDeclaration;
                 }

src/compiler/utilities.ts

@@ -451,6 +451,16 @@ namespace ts {
         return isModuleDeclaration(node) && isStringLiteral(node.name);
     }
 
+    /**
+     * An effective module (namespace) declaration is either
+     * 1. An actual declaration: namespace X { ... }
+     * 2. A Javascript declaration, which is:
+     *    An identifier in a nested property access expression: Y in `X.Y.Z = { ... }`
+     */
+    export function isEffectiveModuleDeclaration(node: Node) {
+        return isModuleDeclaration(node) || isIdentifier(node);
+    }
+
     /** Given a symbol for a module, checks that it is a shorthand ambient module. */
     export function isShorthandAmbientModuleSymbol(moduleSymbol: Symbol): boolean {
         return isShorthandAmbientModule(moduleSymbol.valueDeclaration);

tests/baselines/reference/typeFromPropertyAssignment24.symbols

@@ -0,0 +1,47 @@
+=== tests/cases/conformance/salsa/usage.js ===
+// note that usage is first in the compilation
+Outer.Inner.Message = function() {
+>Outer.Inner.Message : Symbol(Outer.Inner.Message, Decl(usage.js, 0, 0))
+>Outer.Inner : Symbol(Outer.Inner, Decl(usage.js, 1, 6), Decl(def.js, 0, 14))
+>Outer : Symbol(Outer, Decl(usage.js, 0, 0), Decl(def.js, 0, 3), Decl(def.js, 0, 14))
+>Inner : Symbol(Outer.Inner, Decl(usage.js, 1, 6), Decl(def.js, 0, 14))
+>Message : Symbol(Outer.Inner.Message, Decl(usage.js, 0, 0))
+
+};
+
+var y = new Outer.Inner()
+>y : Symbol(y, Decl(usage.js, 4, 3))
+>Outer.Inner : Symbol(Outer.Inner, Decl(usage.js, 1, 6), Decl(def.js, 0, 14))
+>Outer : Symbol(Outer, Decl(usage.js, 0, 0), Decl(def.js, 0, 3), Decl(def.js, 0, 14))
+>Inner : Symbol(Outer.Inner, Decl(usage.js, 1, 6), Decl(def.js, 0, 14))
+
+y.name
+>y.name : Symbol(Inner.name, Decl(def.js, 1, 21))
+>y : Symbol(y, Decl(usage.js, 4, 3))
+>name : Symbol(Inner.name, Decl(def.js, 1, 21))
+
+/** @type {Outer.Inner} should be instance type, not static type */
+var x;
+>x : Symbol(x, Decl(usage.js, 7, 3))
+
+x.name
+>x.name : Symbol(Inner.name, Decl(def.js, 1, 21))
+>x : Symbol(x, Decl(usage.js, 7, 3))
+>name : Symbol(Inner.name, Decl(def.js, 1, 21))
+
+=== tests/cases/conformance/salsa/def.js ===
+var Outer = {}
+>Outer : Symbol(Outer, Decl(usage.js, 0, 0), Decl(def.js, 0, 3), Decl(def.js, 0, 14))
+
+Outer.Inner = class {
+>Outer.Inner : Symbol(Outer.Inner, Decl(usage.js, 1, 6), Decl(def.js, 0, 14))
+>Outer : Symbol(Outer, Decl(usage.js, 0, 0), Decl(def.js, 0, 3), Decl(def.js, 0, 14))
+>Inner : Symbol(Outer.Inner, Decl(usage.js, 1, 6), Decl(def.js, 0, 14))
+
+  name() {
+>name : Symbol(Inner.name, Decl(def.js, 1, 21))
+
+    return 'hi'
+  }
+}
+

tests/baselines/reference/typeFromPropertyAssignment24.types

@@ -0,0 +1,54 @@
+=== tests/cases/conformance/salsa/usage.js ===
+// note that usage is first in the compilation
+Outer.Inner.Message = function() {
+>Outer.Inner.Message = function() {} : () => void
+>Outer.Inner.Message : () => void
+>Outer.Inner : typeof Inner
+>Outer : { [x: string]: any; Inner: typeof Inner; }
+>Inner : typeof Inner
+>Message : () => void
+>function() {} : () => void
+
+};
+
+var y = new Outer.Inner()
+>y : Inner
+>new Outer.Inner() : Inner
+>Outer.Inner : typeof Inner
+>Outer : { [x: string]: any; Inner: typeof Inner; }
+>Inner : typeof Inner
+
+y.name
+>y.name : () => string
+>y : Inner
+>name : () => string
+
+/** @type {Outer.Inner} should be instance type, not static type */
+var x;
+>x : Inner
+
+x.name
+>x.name : () => string
+>x : Inner
+>name : () => string
+
+=== tests/cases/conformance/salsa/def.js ===
+var Outer = {}
+>Outer : { [x: string]: any; Inner: typeof Inner; }
+>{} : { [x: string]: any; Inner: typeof Inner; }
+
+Outer.Inner = class {
+>Outer.Inner = class {  name() {    return 'hi'  }} : typeof Inner
+>Outer.Inner : typeof Inner
+>Outer : { [x: string]: any; Inner: typeof Inner; }
+>Inner : typeof Inner
+>class {  name() {    return 'hi'  }} : typeof Inner
+
+  name() {
+>name : () => string
+
+    return 'hi'
+>'hi' : "hi"
+  }
+}
+

tests/baselines/reference/typeFromPropertyAssignmentOutOfOrder.symbols

@@ -1,34 +1,37 @@
 === tests/cases/conformance/salsa/index.js ===
-Common.Item = class I {}
->Common.Item : Symbol(Common.Item, Decl(index.js, 0, 0))
->Common : Symbol(Common, Decl(index.js, 0, 0), Decl(roots.js, 0, 3))
->Item : Symbol(Common.Item, Decl(index.js, 0, 0))
->I : Symbol(I, Decl(index.js, 0, 13))
+First.Item = class I {}
+>First.Item : Symbol(First.Item, Decl(index.js, 0, 0))
+>First : Symbol(First, Decl(index.js, 0, 0), Decl(roots.js, 0, 3))
+>Item : Symbol(First.Item, Decl(index.js, 0, 0))
+>I : Symbol(I, Decl(index.js, 0, 12))
 
-Common.Object = class extends Common.Item {}
->Common.Object : Symbol(Common.Object, Decl(index.js, 0, 24))
->Common : Symbol(Common, Decl(index.js, 0, 0), Decl(roots.js, 0, 3))
->Object : Symbol(Common.Object, Decl(index.js, 0, 24))
->Common.Item : Symbol(Common.Item, Decl(index.js, 0, 0))
->Common : Symbol(Common, Decl(index.js, 0, 0), Decl(roots.js, 0, 3))
->Item : Symbol(Common.Item, Decl(index.js, 0, 0))
+Common.Object = class extends First.Item {}
+>Common.Object : Symbol(Common.Object, Decl(index.js, 0, 23))
+>Common : Symbol(Common, Decl(index.js, 0, 23), Decl(roots.js, 1, 3))
+>Object : Symbol(Common.Object, Decl(index.js, 0, 23))
+>First.Item : Symbol(First.Item, Decl(index.js, 0, 0))
+>First : Symbol(First, Decl(index.js, 0, 0), Decl(roots.js, 0, 3))
+>Item : Symbol(First.Item, Decl(index.js, 0, 0))
 
 Workspace.Object = class extends Common.Object {}
->Workspace.Object : Symbol(Workspace.Object, Decl(index.js, 1, 44))
->Workspace : Symbol(Workspace, Decl(index.js, 1, 44), Decl(roots.js, 1, 3))
->Object : Symbol(Workspace.Object, Decl(index.js, 1, 44))
->Common.Object : Symbol(Common.Object, Decl(index.js, 0, 24))
->Common : Symbol(Common, Decl(index.js, 0, 0), Decl(roots.js, 0, 3))
->Object : Symbol(Common.Object, Decl(index.js, 0, 24))
+>Workspace.Object : Symbol(Workspace.Object, Decl(index.js, 1, 43))
+>Workspace : Symbol(Workspace, Decl(index.js, 1, 43), Decl(roots.js, 2, 3))
+>Object : Symbol(Workspace.Object, Decl(index.js, 1, 43))
+>Common.Object : Symbol(Common.Object, Decl(index.js, 0, 23))
+>Common : Symbol(Common, Decl(index.js, 0, 23), Decl(roots.js, 1, 3))
+>Object : Symbol(Common.Object, Decl(index.js, 0, 23))
 
 /** @type {Workspace.Object} */
 var am;
 >am : Symbol(am, Decl(index.js, 6, 3))
 
 === tests/cases/conformance/salsa/roots.js ===
+var First = {};
+>First : Symbol(First, Decl(index.js, 0, 0), Decl(roots.js, 0, 3))
+
 var Common = {};
->Common : Symbol(Common, Decl(index.js, 0, 0), Decl(roots.js, 0, 3))
+>Common : Symbol(Common, Decl(index.js, 0, 23), Decl(roots.js, 1, 3))
 
 var Workspace = {};
->Workspace : Symbol(Workspace, Decl(index.js, 1, 44), Decl(roots.js, 1, 3))
+>Workspace : Symbol(Workspace, Decl(index.js, 1, 43), Decl(roots.js, 2, 3))
 

tests/baselines/reference/typeFromPropertyAssignmentOutOfOrder.types

@@ -1,42 +1,46 @@
 === tests/cases/conformance/salsa/index.js ===
-Common.Item = class I {}
->Common.Item = class I {} : typeof I
->Common.Item : typeof I
->Common : typeof Common
+First.Item = class I {}
+>First.Item = class I {} : typeof I
+>First.Item : typeof I
+>First : { [x: string]: any; Item: typeof I; }
 >Item : typeof I
 >class I {} : typeof I
 >I : typeof I
 
-Common.Object = class extends Common.Item {}
->Common.Object = class extends Common.Item {} : typeof Object
+Common.Object = class extends First.Item {}
+>Common.Object = class extends First.Item {} : typeof Object
 >Common.Object : typeof Object
->Common : typeof Common
+>Common : { [x: string]: any; Object: typeof Object; }
 >Object : typeof Object
->class extends Common.Item {} : typeof Object
->Common.Item : I
->Common : typeof Common
+>class extends First.Item {} : typeof Object
+>First.Item : I
+>First : { [x: string]: any; Item: typeof I; }
 >Item : typeof I
 
 Workspace.Object = class extends Common.Object {}
 >Workspace.Object = class extends Common.Object {} : typeof Object
 >Workspace.Object : typeof Object
->Workspace : typeof Workspace
+>Workspace : { [x: string]: any; Object: typeof Object; }
 >Object : typeof Object
 >class extends Common.Object {} : typeof Object
 >Common.Object : Object
->Common : typeof Common
+>Common : { [x: string]: any; Object: typeof Object; }
 >Object : typeof Object
 
 /** @type {Workspace.Object} */
 var am;
 >am : Object
 
 === tests/cases/conformance/salsa/roots.js ===
+var First = {};
+>First : { [x: string]: any; Item: typeof I; }
+>{} : { [x: string]: any; Item: typeof I; }
+
 var Common = {};
->Common : typeof Common
->{} : { [x: string]: any; Item: typeof I; Object: typeof Object; }
+>Common : { [x: string]: any; Object: typeof Object; }
+>{} : { [x: string]: any; Object: typeof Object; }
 
 var Workspace = {};
->Workspace : typeof Workspace
+>Workspace : { [x: string]: any; Object: typeof Object; }
 >{} : { [x: string]: any; Object: typeof Object; }
 

tests/cases/conformance/salsa/typeFromPropertyAssignment24.ts

@@ -0,0 +1,21 @@
+// @noEmit: true
+// @checkJs: true
+// @allowJs: true
+// @Filename: usage.js
+// note that usage is first in the compilation
+Outer.Inner.Message = function() {
+};
+
+var y = new Outer.Inner()
+y.name
+/** @type {Outer.Inner} should be instance type, not static type */
+var x;
+x.name
+
+// @Filename: def.js
+var Outer = {}
+Outer.Inner = class {
+  name() {
+    return 'hi'
+  }
+}

tests/cases/conformance/salsa/typeFromPropertyAssignmentOutOfOrder.ts

@@ -3,14 +3,15 @@
 // @checkJs: true
 // @target: es3
 // @filename: index.js
-Common.Item = class I {}
-Common.Object = class extends Common.Item {}
+First.Item = class I {}
+Common.Object = class extends First.Item {}
 
 Workspace.Object = class extends Common.Object {}
 
 /** @type {Workspace.Object} */
 var am;
 
 // @filename: roots.js
+var First = {};
 var Common = {};
 var Workspace = {};