Merge pull request microsoft#3048 from Microsoft/duplicateIdentifierBindingElement

Duplicate identifier binding element

Author: yuit

src/compiler/binder.ts

@@ -440,6 +440,18 @@ module ts {
                     else if (isBlockOrCatchScoped(<Declaration>node)) {
                         bindBlockScopedVariableDeclaration(<Declaration>node);
                     }
+                    else if (isParameterDeclaration(<VariableLikeDeclaration>node)) {
+                        // It is safe to walk up parent chain to find whether the node is a destructing parameter declaration
+                        // because its parent chain has already been set up, since parents are set before descending into children.
+                        //
+                        // If node is a binding element in parameter declaration, we need to use ParameterExcludes.
+                        // Using ParameterExcludes flag allows the compiler to report an error on duplicate identifiers in Parameter Declaration
+                        // For example:
+                        //      function foo([a,a]) {} // Duplicate Identifier error
+                        //      function bar(a,a) {}   // Duplicate Identifier error, parameter declaration in this case is handled in bindParameter
+                        //                             // which correctly set excluded symbols
+                        bindDeclaration(<Declaration>node, SymbolFlags.FunctionScopedVariable, SymbolFlags.ParameterExcludes, /*isBlockScopeContainer*/ false);
+                    }
                     else {
                         bindDeclaration(<Declaration>node, SymbolFlags.FunctionScopedVariable, SymbolFlags.FunctionScopedVariableExcludes, /*isBlockScopeContainer*/ false);
                     }

src/compiler/checker.ts

@@ -2051,13 +2051,6 @@ module ts {
             return resolutionResults.pop();
         }
 
-        function getRootDeclaration(node: Node): Node {
-            while (node.kind === SyntaxKind.BindingElement) {
-                node = node.parent.parent;
-            }
-            return node;
-        }
-
         function getDeclarationContainer(node: Node): Node {
             node = getRootDeclaration(node);
 
@@ -9206,13 +9199,6 @@ module ts {
             }
         }
 
-        function isParameterDeclaration(node: VariableLikeDeclaration) {
-            while (node.kind === SyntaxKind.BindingElement) {
-                node = <VariableLikeDeclaration>node.parent.parent;
-            }
-            return node.kind === SyntaxKind.Parameter;
-        }
-
         // Check that a parameter initializer contains no references to parameters declared to the right of itself
         function checkParameterInitializer(node: VariableLikeDeclaration): void {
             if (getRootDeclaration(node).kind !== SyntaxKind.Parameter) {

src/compiler/utilities.ts

@@ -1146,6 +1146,18 @@ module ts {
         }
         return false;
     }
+
+    export function isParameterDeclaration(node: VariableLikeDeclaration) {
+        let root = getRootDeclaration(node);
+        return root.kind === SyntaxKind.Parameter;
+    }
+
+    export function getRootDeclaration(node: Node): Node {
+        while (node.kind === SyntaxKind.BindingElement) {
+            node = node.parent.parent;
+        }
+        return node;
+    }
 
     export function nodeStartsNewLexicalEnvironment(n: Node): boolean {
         return isFunctionLike(n) || n.kind === SyntaxKind.ModuleDeclaration || n.kind === SyntaxKind.SourceFile;

tests/baselines/reference/declarationEmitDestructuring2.errors.txt

@@ -0,0 +1,25 @@
+tests/cases/compiler/declarationEmitDestructuring2.ts(3,13): error TS2300: Duplicate identifier 'a'.
+tests/cases/compiler/declarationEmitDestructuring2.ts(3,17): error TS2300: Duplicate identifier 'b'.
+tests/cases/compiler/declarationEmitDestructuring2.ts(3,23): error TS2300: Duplicate identifier 'c'.
+tests/cases/compiler/declarationEmitDestructuring2.ts(3,41): error TS2300: Duplicate identifier 'a'.
+tests/cases/compiler/declarationEmitDestructuring2.ts(3,44): error TS2300: Duplicate identifier 'b'.
+tests/cases/compiler/declarationEmitDestructuring2.ts(3,47): error TS2300: Duplicate identifier 'c'.
+
+
+==== tests/cases/compiler/declarationEmitDestructuring2.ts (6 errors) ====
+    function f({x = 10, y: [a, b, c, d] = [1, 2, 3, 4]} = { x: 10, y: [2, 4, 6, 8] }) { }
+    function g([a, b, c, d] = [1, 2, 3, 4]) { }
+    function h([a, [b], [[c]], {x = 10, y: [a, b, c], z: {a1, b1}}]){ }
+                ~
+!!! error TS2300: Duplicate identifier 'a'.
+                    ~
+!!! error TS2300: Duplicate identifier 'b'.
+                          ~
+!!! error TS2300: Duplicate identifier 'c'.
+                                            ~
+!!! error TS2300: Duplicate identifier 'a'.
+                                               ~
+!!! error TS2300: Duplicate identifier 'b'.
+                                                  ~
+!!! error TS2300: Duplicate identifier 'c'.
+    function h1([a, [b], [[c]], {x = 10, y = [1, 2, 3], z: {a1, b1}}]){ }

tests/baselines/reference/declarationEmitDestructuring2.symbols

@@ -0,0 +1,111 @@
+tests/cases/conformance/es6/destructuring/destructuringParameterDeclaration1ES6.ts(96,18): error TS2300: Duplicate identifier 'number'.
+tests/cases/conformance/es6/destructuring/destructuringParameterDeclaration1ES6.ts(96,26): error TS2300: Duplicate identifier 'number'.
+tests/cases/conformance/es6/destructuring/destructuringParameterDeclaration1ES6.ts(96,34): error TS2300: Duplicate identifier 'number'.
+
+
+==== tests/cases/conformance/es6/destructuring/destructuringParameterDeclaration1ES6.ts (3 errors) ====
+    // Conformance for emitting ES6
+    
+    // A parameter declaration may specify either an identifier or a binding pattern.
+    // The identifiers specified in parameter declarations and binding patterns
+    // in a parameter list must be unique within that parameter list.
+    
+    // If the declaration includes a type annotation, the parameter is of that type
+    function a1([a, b, [[c]]]: [number, number, string[][]]) { }
+    function a2(o: { x: number, a: number }) { }
+    function a3({j, k, l: {m, n}, q: [a, b, c]}: { j: number, k: string, l: { m: boolean, n: number }, q: (number|string)[] }) { };
+    function a4({x, a}: { x: number, a: number }) { }
+    
+    a1([1, 2, [["world"]]]);
+    a1([1, 2, [["world"]], 3]);
+    
+    
+    // If the declaration includes an initializer expression (which is permitted only
+    // when the parameter list occurs in conjunction with a function body),
+    // the parameter type is the widened form (section 3.11) of the type of the initializer expression.
+    
+    function b1(z = [undefined, null]) { };
+    function b2(z = null, o = { x: 0, y: undefined }) { }
+    function b3({z: {x, y: {j}}} = { z: { x: "hi", y: { j: 1 } } }) { }
+    
+    interface F1 {
+        b5(z, y, [, a, b], {p, m: { q, r}});
+    }
+    
+    function b6([a, z, y] = [undefined, null, undefined]) { }
+    function b7([[a], b, [[c, d]]] = [[undefined], undefined, [[undefined, undefined]]]) { }
+    
+    b1([1, 2, 3]);  // z is widen to the type any[]
+    b2("string", { x: 200, y: "string" });
+    b2("string", { x: 200, y: true });
+    
+    
+    // If the declaration specifies a binding pattern, the parameter type is the implied type of that binding pattern (section 5.1.3)
+    enum Foo { a }
+    function c0({z: {x, y: {j}}}) { }
+    function c1({z} = { z: 10 }) { }
+    function c2({z = 10}) { }
+    function c3({b}: { b: number|string} = { b: "hello" }) { }
+    function c5([a, b, [[c]]]) { }
+    function c6([a, b, [[c=1]]]) { }
+    
+    c0({z : { x: 1, y: { j: "world" } }});      // Implied type is { z: {x: any, y: {j: any}} }
+    c0({z : { x: "string", y: { j: true } }});  // Implied type is { z: {x: any, y: {j: any}} }
+    
+    c1();             // Implied type is {z:number}?
+    c1({ z: 1 })      // Implied type is {z:number}? 
+    
+    c2({});         // Implied type is {z?: number}
+    c2({z:1});      // Implied type is {z?: number}
+    
+    c3({ b: 1 });     // Implied type is { b: number|string }.
+    
+    c5([1, 2, [["string"]]]);               // Implied type is is [any, any, [[any]]]
+    c5([1, 2, [["string"]], false, true]);  // Implied type is is [any, any, [[any]]]
+    
+    
+    // A parameter can be marked optional by following its name or binding pattern with a question mark (?)
+    // or by including an initializer.
+    
+    interface F2 {
+        d3([a, b, c]?);
+        d4({x, y, z}?);
+        e0([a, b, c]);
+    }
+    
+    class C2 implements F2 {
+        constructor() { }
+        d3() { }
+        d4() { }
+        e0([a, b, c]) { }
+    }
+    
+    class C3 implements F2 {
+        d3([a, b, c]) { }
+        d4({x, y, z}) { }
+        e0([a, b, c]) { }
+    }
+    
+    function d5({x, y} = { x: 1, y: 2 }) { }
+    d5();  // Parameter is optional as its declaration included an initializer
+    
+    // Destructuring parameter declarations do not permit type annotations on the individual binding patterns,
+    // as such annotations would conflict with the already established meaning of colons in object literals.
+    // Type annotations must instead be written on the top- level parameter declaration
+    
+    function e1({x: number}) { }  // x has type any NOT number
+    function e2({x}: { x: number }) { }  // x is type number
+    function e3({x}: { x?: number }) { }  // x is an optional with type number
+    function e4({x: [number,string,any] }) { }  // x has type [any, any, any]
+    function e5({x: [a, b, c]}: { x: [number, number, number] }) { }  // x has type [any, any, any]
+    
+    function e6({x: [number, number, number]}) { }  // error, duplicate identifier;
+                     ~~~~~~
+!!! error TS2300: Duplicate identifier 'number'.
+                             ~~~~~~
+!!! error TS2300: Duplicate identifier 'number'.
+                                     ~~~~~~
+!!! error TS2300: Duplicate identifier 'number'.
+    
+    
+    

tests/baselines/reference/declarationEmitDestructuring2.types

@@ -94,7 +94,7 @@ function e3({x}: { x?: number }) { }  // x is an optional with type number
 function e4({x: [number,string,any] }) { }  // x has type [any, any, any]
 function e5({x: [a, b, c]}: { x: [number, number, number] }) { }  // x has type [any, any, any]
 
-function e6({x: [number, number, number]}) { }  // should be an error, duplicate identifier;
+function e6({x: [number, number, number]}) { }  // error, duplicate identifier;
 
 
 
@@ -166,4 +166,4 @@ function e2({ x }) { } // x is type number
 function e3({ x }) { } // x is an optional with type number
 function e4({ x: [number, string, any] }) { } // x has type [any, any, any]
 function e5({ x: [a, b, c] }) { } // x has type [any, any, any]
-function e6({ x: [number, number, number] }) { } // should be an error, duplicate identifier;
+function e6({ x: [number, number, number] }) { } // error, duplicate identifier;