Skip to content

Commit

Permalink
Allow duplicate identifiers as long as their declarations span multip…
Browse files Browse the repository at this point in the history
…le blocks

Fixes microsoft#8675
  • Loading branch information
RyanCavanaugh committed May 24, 2016
1 parent 27292e4 commit 675d176
Show file tree
Hide file tree
Showing 64 changed files with 731 additions and 487 deletions.
114 changes: 108 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12954,6 +12954,82 @@ namespace ts {
}
}

function checkClassForDuplicateDeclarations(node: ClassLikeDeclaration) {
const getter = 1, setter = 2, property = getter | setter;

const instanceNames: Map<number> = {};
const staticNames: Map<number> = {};
for (const member of node.members) {
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 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.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, getTextOfNode(location));
}
else {
names[name] = prev | meaning;
}
}
else {
names[name] = meaning;
}
}
}

function checkObjectTypeForDuplicateDeclarations(node: TypeLiteralNode | InterfaceDeclaration) {
const names: Map<boolean> = {};
for (const member of node.members) {
if (member.kind == SyntaxKind.PropertySignature) {
let memberName: string;
switch (member.name.kind) {
case SyntaxKind.StringLiteral:
case SyntaxKind.NumericLiteral:
case SyntaxKind.Identifier:
memberName = (member.name as LiteralExpression | Identifier).text;
break;
default:
continue;
}

if (hasProperty(names, memberName)) {
error(member.symbol.valueDeclaration.name, Diagnostics.Duplicate_identifier_0, memberName);
error(member.name, Diagnostics.Duplicate_identifier_0, memberName);
}
else {
names[memberName] = true;
}
}
}
}

function checkTypeForDuplicateIndexSignatures(node: Node) {
if (node.kind === SyntaxKind.InterfaceDeclaration) {
const nodeSymbol = getSymbolOfNode(node);
Expand Down Expand Up @@ -13238,6 +13314,7 @@ namespace ts {
const type = getTypeFromTypeLiteralOrFunctionOrConstructorTypeNode(node);
checkIndexConstraints(type);
checkTypeForDuplicateIndexSignatures(node);
checkObjectTypeForDuplicateDeclarations(node);
}
}

Expand Down Expand Up @@ -14430,6 +14507,10 @@ namespace ts {
if (node.initializer) {
checkTypeAssignableTo(checkExpressionCached(node.initializer), declarationType, node, /*headMessage*/ undefined);
}
if (!areDeclarationFlagsIdentical(node, symbol.valueDeclaration)) {
error(symbol.valueDeclaration.name, Diagnostics.All_declarations_of_0_must_have_identical_modifiers, declarationNameToString(node.name));
error(node.name, Diagnostics.All_declarations_of_0_must_have_identical_modifiers, declarationNameToString(node.name));
}
}
if (node.kind !== SyntaxKind.PropertyDeclaration && node.kind !== SyntaxKind.PropertySignature) {
// We know we don't have a binding pattern or computed name here
Expand All @@ -14444,6 +14525,21 @@ namespace ts {
}
}

function areDeclarationFlagsIdentical(left: Declaration, right: Declaration) {
if (hasQuestionToken(left) !== hasQuestionToken(right)) {
return false;
}

const interestingFlags = NodeFlags.Private |
NodeFlags.Protected |
NodeFlags.Async |
NodeFlags.Abstract |
NodeFlags.Readonly |
NodeFlags.Static;

return (left.flags & interestingFlags) === (right.flags & interestingFlags);
}

function checkVariableDeclaration(node: VariableDeclaration) {
checkGrammarVariableDeclaration(node);
return checkVariableLikeDeclaration(node);
Expand Down Expand Up @@ -15237,6 +15333,7 @@ namespace ts {
const typeWithThis = getTypeWithThisArgument(type);
const staticType = <ObjectType>getTypeOfSymbol(symbol);
checkTypeParameterListsIdentical(node, symbol);
checkClassForDuplicateDeclarations(node);

const baseTypeNode = getClassExtendsHeritageClauseElement(node);
if (baseTypeNode) {
Expand Down Expand Up @@ -15514,6 +15611,7 @@ namespace ts {
checkIndexConstraints(type);
}
}
checkObjectTypeForDuplicateDeclarations(node);
}
forEach(getInterfaceBaseTypeNodes(node), heritageElement => {
if (!isSupportedExpressionWithTypeArguments(heritageElement)) {
Expand Down Expand Up @@ -18152,7 +18250,6 @@ namespace ts {
name.kind === SyntaxKind.ComputedPropertyName) {
// If the name is not a ComputedPropertyName, the grammar checking will skip it
checkGrammarComputedPropertyName(<ComputedPropertyName>name);
continue;
}

if (prop.kind === SyntaxKind.ShorthandPropertyAssignment && !inDestructuring && (<ShorthandPropertyAssignment>prop).objectAssignmentInitializer) {
Expand Down Expand Up @@ -18198,17 +18295,22 @@ namespace ts {
Debug.fail("Unexpected syntax kind:" + prop.kind);
}

if (!hasProperty(seen, (<Identifier>name).text)) {
seen[(<Identifier>name).text] = currentKind;
const effectiveName = getPropertyNameForPropertyNameNode(name);
if (effectiveName === undefined) {
continue;
}

if (!hasProperty(seen, effectiveName)) {
seen[effectiveName] = currentKind;
}
else {
const existingKind = seen[(<Identifier>name).text];
const existingKind = seen[effectiveName];
if (currentKind === Property && existingKind === Property) {
continue;
grammarErrorOnNode(name, Diagnostics.Duplicate_identifier_0, getTextOfNode(name));
}
else if ((currentKind & GetOrSetAccessor) && (existingKind & GetOrSetAccessor)) {
if (existingKind !== GetOrSetAccessor && currentKind !== existingKind) {
seen[(<Identifier>name).text] = currentKind | existingKind;
seen[effectiveName] = currentKind | existingKind;
}
else {
return grammarErrorOnNode(name, Diagnostics.An_object_literal_cannot_have_multiple_get_Slashset_accessors_with_the_same_name);
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1927,6 +1927,10 @@
"category": "Error",
"code": 2686
},
"All declarations of '{0}' must have identical modifiers.": {
"category": "Error",
"code": 2687
},
"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
"code": 4000
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2025,7 +2025,7 @@ namespace ts {
BlockScopedVariableExcludes = Value,

ParameterExcludes = Value,
PropertyExcludes = Value,
PropertyExcludes = None,
EnumMemberExcludes = Value,
FunctionExcludes = Value & ~(Function | ValueModule),
ClassExcludes = (Value | Type) & ~(ValueModule | Interface), // class-interface mergability done in checker.ts
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1774,7 +1774,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) {
Expand Down
11 changes: 1 addition & 10 deletions tests/baselines/reference/binaryIntegerLiteralError.errors.txt
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
tests/cases/conformance/es6/binaryAndOctalIntegerLiteral/binaryIntegerLiteralError.ts(2,17): error TS1005: ',' expected.
tests/cases/conformance/es6/binaryAndOctalIntegerLiteral/binaryIntegerLiteralError.ts(3,17): error TS1005: ',' expected.
tests/cases/conformance/es6/binaryAndOctalIntegerLiteral/binaryIntegerLiteralError.ts(6,5): error TS2300: Duplicate identifier '0b11010'.
tests/cases/conformance/es6/binaryAndOctalIntegerLiteral/binaryIntegerLiteralError.ts(7,5): error TS2300: Duplicate identifier '26'.
tests/cases/conformance/es6/binaryAndOctalIntegerLiteral/binaryIntegerLiteralError.ts(8,5): error TS2300: Duplicate identifier '"26"'.


==== tests/cases/conformance/es6/binaryAndOctalIntegerLiteral/binaryIntegerLiteralError.ts (5 errors) ====
==== tests/cases/conformance/es6/binaryAndOctalIntegerLiteral/binaryIntegerLiteralError.ts (2 errors) ====
// error
var bin1 = 0B1102110;
~~~~
Expand All @@ -16,13 +13,7 @@ tests/cases/conformance/es6/binaryAndOctalIntegerLiteral/binaryIntegerLiteralErr

var obj1 = {
0b11010: "hi",
~~~~~~~
!!! error TS2300: Duplicate identifier '0b11010'.
26: "Hello",
~~
!!! error TS2300: Duplicate identifier '26'.
"26": "world",
~~~~
!!! error TS2300: Duplicate identifier '"26"'.
};

Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
tests/cases/conformance/types/objectTypeLiteral/callSignatures/callSignaturesWithParameterInitializers2.ts(4,14): error TS2371: A parameter initializer is only allowed in a function or constructor implementation.
tests/cases/conformance/types/objectTypeLiteral/callSignatures/callSignaturesWithParameterInitializers2.ts(11,9): error TS2371: A parameter initializer is only allowed in a function or constructor implementation.
tests/cases/conformance/types/objectTypeLiteral/callSignatures/callSignaturesWithParameterInitializers2.ts(20,5): error TS2300: Duplicate identifier 'foo'.
tests/cases/conformance/types/objectTypeLiteral/callSignatures/callSignaturesWithParameterInitializers2.ts(20,9): error TS2371: A parameter initializer is only allowed in a function or constructor implementation.
tests/cases/conformance/types/objectTypeLiteral/callSignatures/callSignaturesWithParameterInitializers2.ts(20,15): error TS1005: '{' expected.
tests/cases/conformance/types/objectTypeLiteral/callSignatures/callSignaturesWithParameterInitializers2.ts(21,5): error TS2300: Duplicate identifier 'foo'.


==== tests/cases/conformance/types/objectTypeLiteral/callSignatures/callSignaturesWithParameterInitializers2.ts (6 errors) ====
==== tests/cases/conformance/types/objectTypeLiteral/callSignatures/callSignaturesWithParameterInitializers2.ts (4 errors) ====
// Optional parameters allow initializers only in implementation signatures
// All the below declarations are errors

Expand All @@ -31,15 +29,11 @@ tests/cases/conformance/types/objectTypeLiteral/callSignatures/callSignaturesWit

var b = {
foo(x = 1), // error
~~~
!!! error TS2300: Duplicate identifier 'foo'.
~~~~~
!!! error TS2371: A parameter initializer is only allowed in a function or constructor implementation.
~
!!! error TS1005: '{' expected.
foo(x = 1) { }, // error
~~~
!!! error TS2300: Duplicate identifier 'foo'.
}

b.foo();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,44 +1,38 @@
tests/cases/conformance/classes/classDeclarations/classAndInterfaceMergeConflictingMembers.ts(2,12): error TS2300: Duplicate identifier 'x'.
tests/cases/conformance/classes/classDeclarations/classAndInterfaceMergeConflictingMembers.ts(6,5): error TS2300: Duplicate identifier 'x'.
tests/cases/conformance/classes/classDeclarations/classAndInterfaceMergeConflictingMembers.ts(10,15): error TS2300: Duplicate identifier 'x'.
tests/cases/conformance/classes/classDeclarations/classAndInterfaceMergeConflictingMembers.ts(14,5): error TS2300: Duplicate identifier 'x'.
tests/cases/conformance/classes/classDeclarations/classAndInterfaceMergeConflictingMembers.ts(18,13): error TS2300: Duplicate identifier 'x'.
tests/cases/conformance/classes/classDeclarations/classAndInterfaceMergeConflictingMembers.ts(22,5): error TS2300: Duplicate identifier 'x'.
tests/cases/conformance/classes/classDeclarations/classAndInterfaceMergeConflictingMembers.ts(10,15): error TS2686: All declarations of 'x' must have identical modifiers.
tests/cases/conformance/classes/classDeclarations/classAndInterfaceMergeConflictingMembers.ts(14,5): error TS2686: All declarations of 'x' must have identical modifiers.
tests/cases/conformance/classes/classDeclarations/classAndInterfaceMergeConflictingMembers.ts(18,13): error TS2686: All declarations of 'x' must have identical modifiers.
tests/cases/conformance/classes/classDeclarations/classAndInterfaceMergeConflictingMembers.ts(22,5): error TS2686: All declarations of 'x' must have identical modifiers.


==== tests/cases/conformance/classes/classDeclarations/classAndInterfaceMergeConflictingMembers.ts (6 errors) ====
==== tests/cases/conformance/classes/classDeclarations/classAndInterfaceMergeConflictingMembers.ts (4 errors) ====
declare class C1 {
public x : number;
~
!!! error TS2300: Duplicate identifier 'x'.
}

interface C1 {
x : number;
~
!!! error TS2300: Duplicate identifier 'x'.
}

declare class C2 {
protected x : number;
~
!!! error TS2300: Duplicate identifier 'x'.
!!! error TS2686: All declarations of 'x' must have identical modifiers.
}

interface C2 {
x : number;
~
!!! error TS2300: Duplicate identifier 'x'.
!!! error TS2686: All declarations of 'x' must have identical modifiers.
}

declare class C3 {
private x : number;
~
!!! error TS2300: Duplicate identifier 'x'.
!!! error TS2686: All declarations of 'x' must have identical modifiers.
}

interface C3 {
x : number;
~
!!! error TS2300: Duplicate identifier 'x'.
!!! error TS2686: All declarations of 'x' must have identical modifiers.
}
27 changes: 0 additions & 27 deletions tests/baselines/reference/classAndInterfaceWithSameName.errors.txt

This file was deleted.

26 changes: 26 additions & 0 deletions tests/baselines/reference/classAndInterfaceWithSameName.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
=== tests/cases/conformance/classes/classDeclarations/classAndInterfaceWithSameName.ts ===
class C { foo: string; }
>C : Symbol(C, Decl(classAndInterfaceWithSameName.ts, 0, 0), Decl(classAndInterfaceWithSameName.ts, 0, 24))
>foo : Symbol(C.foo, Decl(classAndInterfaceWithSameName.ts, 0, 9), Decl(classAndInterfaceWithSameName.ts, 1, 13))

interface C { foo: string; }
>C : Symbol(C, Decl(classAndInterfaceWithSameName.ts, 0, 0), Decl(classAndInterfaceWithSameName.ts, 0, 24))
>foo : Symbol(C.foo, Decl(classAndInterfaceWithSameName.ts, 0, 9), Decl(classAndInterfaceWithSameName.ts, 1, 13))

module M {
>M : Symbol(M, Decl(classAndInterfaceWithSameName.ts, 1, 28))

class D {
>D : Symbol(D, Decl(classAndInterfaceWithSameName.ts, 3, 10), Decl(classAndInterfaceWithSameName.ts, 6, 5))

bar: string;
>bar : Symbol(D.bar, Decl(classAndInterfaceWithSameName.ts, 4, 13), Decl(classAndInterfaceWithSameName.ts, 8, 17))
}

interface D {
>D : Symbol(D, Decl(classAndInterfaceWithSameName.ts, 3, 10), Decl(classAndInterfaceWithSameName.ts, 6, 5))

bar: string;
>bar : Symbol(D.bar, Decl(classAndInterfaceWithSameName.ts, 4, 13), Decl(classAndInterfaceWithSameName.ts, 8, 17))
}
}
Loading

0 comments on commit 675d176

Please sign in to comment.