CR feedback

microsoft · May 24, 2016 · 2ec60fa · 2ec60fa
1 parent 4138873
commit 2ec60fa
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 47 deletions.
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -12899,49 +12899,39 @@ namespace ts {
             const instanceNames: Map<number> = {};
             const staticNames: Map<number> = {};
             for (const member of node.members) {
-                let memberName: string;
-                if (member.name) {
-                    switch (member.name.kind) {
-                        case SyntaxKind.StringLiteral:
-                        case SyntaxKind.NumericLiteral:
-                        case SyntaxKind.Identifier:
-                            memberName = (member.name as LiteralExpression | Identifier).text;
-                            break;
-                        default:
-                            continue;
+                if (member.kind === SyntaxKind.Constructor) {
+                    for (const param of (member as ConstructorDeclaration).parameters) {
+                        if (isParameterPropertyDeclaration(param)) {
+                            addName(instanceNames, param.name, (param.name as Identifier).text, property);
+                        }
                     }
                 }
+                else {
+                    const static = forEach(member.modifiers, m => m.kind === SyntaxKind.StaticKeyword);
+                    const names = static ? staticNames : instanceNames;
 
-                const static = forEach(member.modifiers, m => m.kind === SyntaxKind.StaticKeyword);
-                const names = static ? staticNames : instanceNames;
-                switch (member.kind) {
-                    case SyntaxKind.Constructor:
-                        for (const param of (member as ConstructorDeclaration).parameters) {
-                            if (isParameterPropertyDeclaration(param)) {
-                                addName(names, param.name, (param.name as Identifier).text, property);
-                            }
-                        }
-                        break;
-
-                    case SyntaxKind.GetAccessor:
-                        addName(names, member.name, memberName, getter);
-                        break;
+                    const memberName = member.name && getPropertyNameForPropertyNameNode(member.name);
+                    switch (member.kind) {
+                        case SyntaxKind.GetAccessor:
+                            addName(names, member.name, memberName, getter);
+                            break;
 
-                    case SyntaxKind.SetAccessor:
-                        addName(names, member.name, memberName, setter);
-                        break;
+                        case SyntaxKind.SetAccessor:
+                            addName(names, member.name, memberName, setter);
+                            break;
 
-                    case SyntaxKind.PropertyDeclaration:
-                        addName(names, member.name, memberName, property);
-                        break;
+                        case SyntaxKind.PropertyDeclaration:
+                            addName(names, member.name, memberName, property);
+                            break;
+                    }
                 }
             }
 
             function addName(names: Map<number>, location: Node, name: string, meaning: number) {
                 if (hasProperty(names, name)) {
                     const prev = names[name];
                     if (prev & meaning) {
-                        error(location, Diagnostics.Duplicate_identifier_0, name);
+                        error(location, Diagnostics.Duplicate_identifier_0, getTextOfNode(location));
                     }
                     else {
                         names[name] = prev | meaning;

diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -1767,7 +1767,7 @@ namespace ts {
     }
 
     export function getPropertyNameForPropertyNameNode(name: DeclarationName): string {
-        if (name.kind === SyntaxKind.Identifier || name.kind === SyntaxKind.StringLiteral || name.kind === SyntaxKind.NumericLiteral) {
+        if (name.kind === SyntaxKind.Identifier || name.kind === SyntaxKind.StringLiteral || name.kind === SyntaxKind.NumericLiteral || name.kind === SyntaxKind.Parameter) {
             return (<Identifier | LiteralExpression>name).text;
         }
         if (name.kind === SyntaxKind.ComputedPropertyName) {

diff --git a/.../baselines/reference/constructSignatureWithAccessibilityModifiersOnParameters2.errors.txt b/.../baselines/reference/constructSignatureWithAccessibilityModifiersOnParameters2.errors.txt
@@ -1,10 +1,7 @@
 tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatureWithAccessibilityModifiersOnParameters2.ts(4,17): error TS2369: A parameter property is only allowed in a constructor implementation.
 tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatureWithAccessibilityModifiersOnParameters2.ts(4,27): error TS2369: A parameter property is only allowed in a constructor implementation.
-tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatureWithAccessibilityModifiersOnParameters2.ts(5,24): error TS2300: Duplicate identifier 'x'.
-tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatureWithAccessibilityModifiersOnParameters2.ts(5,35): error TS2300: Duplicate identifier 'y'.
 tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatureWithAccessibilityModifiersOnParameters2.ts(9,17): error TS2369: A parameter property is only allowed in a constructor implementation.
 tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatureWithAccessibilityModifiersOnParameters2.ts(9,25): error TS2686: All declarations of 'x' must have identical modifiers.
-tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatureWithAccessibilityModifiersOnParameters2.ts(10,24): error TS2300: Duplicate identifier 'x'.
 tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatureWithAccessibilityModifiersOnParameters2.ts(10,24): error TS2686: All declarations of 'x' must have identical modifiers.
 tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatureWithAccessibilityModifiersOnParameters2.ts(14,17): error TS2369: A parameter property is only allowed in a constructor implementation.
 tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatureWithAccessibilityModifiersOnParameters2.ts(19,10): error TS2369: A parameter property is only allowed in a constructor implementation.
@@ -17,7 +14,7 @@ tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatur
 tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatureWithAccessibilityModifiersOnParameters2.ts(35,10): error TS2369: A parameter property is only allowed in a constructor implementation.
 
 
-==== tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatureWithAccessibilityModifiersOnParameters2.ts (17 errors) ====
+==== tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatureWithAccessibilityModifiersOnParameters2.ts (14 errors) ====
     // Parameter properties are not valid in overloads of constructors
 
     class C {
@@ -27,10 +24,6 @@ tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatur
                               ~~~~~~~~~
 !!! error TS2369: A parameter property is only allowed in a constructor implementation.
         constructor(public x, private y) { }
-                           ~
-!!! error TS2300: Duplicate identifier 'x'.
-                                      ~
-!!! error TS2300: Duplicate identifier 'y'.
     }
 
     class C2 {
@@ -41,8 +34,6 @@ tests/cases/conformance/types/objectTypeLiteral/callSignatures/constructSignatur
 !!! error TS2686: All declarations of 'x' must have identical modifiers.
         constructor(public x) { }
                            ~
-!!! error TS2300: Duplicate identifier 'x'.
-                           ~
 !!! error TS2686: All declarations of 'x' must have identical modifiers.
     }
 

diff --git a/tests/baselines/reference/duplicateIdentifierComputedName.errors.txt b/tests/baselines/reference/duplicateIdentifierComputedName.errors.txt
@@ -0,0 +1,11 @@
+tests/cases/compiler/duplicateIdentifierComputedName.ts(3,5): error TS2300: Duplicate identifier 'a'.
+
+
+==== tests/cases/compiler/duplicateIdentifierComputedName.ts (1 errors) ====
+    class C {
+        ["a"]: string;
+        ["a"]: string;
+        ~~~~~
+!!! error TS2300: Duplicate identifier 'a'.
+    }
+
diff --git a/tests/baselines/reference/duplicateIdentifierComputedName.js b/tests/baselines/reference/duplicateIdentifierComputedName.js
@@ -0,0 +1,13 @@
+//// [duplicateIdentifierComputedName.ts]
+class C {
+    ["a"]: string;
+    ["a"]: string;
+}
+
+
+//// [duplicateIdentifierComputedName.js]
+var C = (function () {
+    function C() {
+    }
+    return C;
+}());
diff --git a/tests/baselines/reference/parameterPropertyInConstructor2.errors.txt b/tests/baselines/reference/parameterPropertyInConstructor2.errors.txt
@@ -1,9 +1,8 @@
 tests/cases/compiler/parameterPropertyInConstructor2.ts(3,5): error TS2394: Overload signature is not compatible with function implementation.
 tests/cases/compiler/parameterPropertyInConstructor2.ts(3,17): error TS2369: A parameter property is only allowed in a constructor implementation.
-tests/cases/compiler/parameterPropertyInConstructor2.ts(4,24): error TS2300: Duplicate identifier 'names'.
 
 
-==== tests/cases/compiler/parameterPropertyInConstructor2.ts (3 errors) ====
+==== tests/cases/compiler/parameterPropertyInConstructor2.ts (2 errors) ====
     module mod {
       class Customers {
         constructor(public names: string);
@@ -12,8 +11,6 @@ tests/cases/compiler/parameterPropertyInConstructor2.ts(4,24): error TS2300: Dup
                     ~~~~~~~~~~~~~~~~~~~~
 !!! error TS2369: A parameter property is only allowed in a constructor implementation.
         constructor(public names: string, public ages: number) {
-                           ~~~~~
-!!! error TS2300: Duplicate identifier 'names'.
         }
       }
     }

diff --git a/tests/baselines/reference/symbolProperty44.errors.txt b/tests/baselines/reference/symbolProperty44.errors.txt
@@ -1,8 +1,9 @@
 tests/cases/conformance/es6/Symbols/symbolProperty44.ts(2,9): error TS2300: Duplicate identifier '[Symbol.hasInstance]'.
 tests/cases/conformance/es6/Symbols/symbolProperty44.ts(5,9): error TS2300: Duplicate identifier '[Symbol.hasInstance]'.
+tests/cases/conformance/es6/Symbols/symbolProperty44.ts(5,9): error TS2300: Duplicate identifier '__@hasInstance'.
 
 
-==== tests/cases/conformance/es6/Symbols/symbolProperty44.ts (2 errors) ====
+==== tests/cases/conformance/es6/Symbols/symbolProperty44.ts (3 errors) ====
     class C {
         get [Symbol.hasInstance]() {
             ~~~~~~~~~~~~~~~~~~~~
@@ -12,6 +13,8 @@ tests/cases/conformance/es6/Symbols/symbolProperty44.ts(5,9): error TS2300: Dupl
         get [Symbol.hasInstance]() {
             ~~~~~~~~~~~~~~~~~~~~
 !!! error TS2300: Duplicate identifier '[Symbol.hasInstance]'.
+            ~~~~~~~~~~~~~~~~~~~~
+!!! error TS2300: Duplicate identifier '__@hasInstance'.
             return "";
         }
     }
diff --git a/tests/cases/compiler/duplicateIdentifierComputedName.ts b/tests/cases/compiler/duplicateIdentifierComputedName.ts
@@ -0,0 +1,4 @@
+class C {
+    ["a"]: string;
+    ["a"]: string;
+}