Skip to content

Commit ed26563

Browse files
committed
Merge pull request #317 from Microsoft/order_of_overloads
Disallow statements between overloads fixed #75 and #92
2 parents 853288b + a62b47e commit ed26563

File tree

65 files changed

+275
-198
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+275
-198
lines changed

src/compiler/checker.ts

Lines changed: 75 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4872,47 +4872,101 @@ module ts {
48724872
var hasOverloads = false;
48734873
var bodyDeclaration: FunctionDeclaration;
48744874
var lastSeenNonAmbientDeclaration: FunctionDeclaration;
4875+
var previousDeclaration: FunctionDeclaration;
4876+
48754877
var declarations = symbol.declarations;
48764878
var isConstructor = (symbol.flags & SymbolFlags.Constructor) !== 0;
4879+
4880+
function reportImplementationExpectedError(node: FunctionDeclaration): void {
4881+
var seen = false;
4882+
var subsequentNode = forEachChild(node.parent, c => {
4883+
if (seen) {
4884+
return c;
4885+
}
4886+
else {
4887+
seen = c === node;
4888+
}
4889+
});
4890+
if (subsequentNode) {
4891+
if (subsequentNode.kind === node.kind) {
4892+
var errorNode: Node = (<FunctionDeclaration>subsequentNode).name || subsequentNode;
4893+
if (node.name && (<FunctionDeclaration>subsequentNode).name && node.name.text === (<FunctionDeclaration>subsequentNode).name.text) {
4894+
// the only situation when this is possible (same kind\same name but different symbol) - mixed static and instance class members
4895+
Debug.assert(node.kind === SyntaxKind.Method);
4896+
Debug.assert((node.flags & NodeFlags.Static) !== (subsequentNode.flags & NodeFlags.Static));
4897+
var diagnostic = node.flags & NodeFlags.Static ? Diagnostics.Function_overload_must_be_static : Diagnostics.Function_overload_must_not_be_static;
4898+
error(errorNode, diagnostic);
4899+
return;
4900+
}
4901+
else if ((<FunctionDeclaration>subsequentNode).body) {
4902+
error(errorNode, Diagnostics.Function_implementation_name_must_be_0, identifierToString(node.name));
4903+
return;
4904+
}
4905+
}
4906+
}
4907+
var errorNode: Node = node.name || node;
4908+
if (isConstructor) {
4909+
error(errorNode, Diagnostics.Constructor_implementation_is_missing);
4910+
}
4911+
else {
4912+
error(errorNode, Diagnostics.Function_implementation_is_missing_or_not_immediately_following_the_declaration);
4913+
}
4914+
}
4915+
4916+
// when checking exported function declarations across modules check only duplicate implementations
4917+
// names and consistensy of modifiers are verified when we check local symbol
4918+
var isExportSymbolInsideModule = symbol.parent && symbol.parent.flags & SymbolFlags.Module;
48774919
for (var i = 0; i < declarations.length; i++) {
48784920
var node = <FunctionDeclaration>declarations[i];
4921+
var inAmbientContext = isInAmbientContext(node);
4922+
var inAmbientContextOrInterface = node.parent.kind === SyntaxKind.InterfaceDeclaration || node.parent.kind === SyntaxKind.TypeLiteral || inAmbientContext;
4923+
if (inAmbientContextOrInterface) {
4924+
// check if declarations are consecutive only if they are non-ambient
4925+
// 1. ambient declarations can be interleaved
4926+
// i.e. this is legal
4927+
// declare function foo();
4928+
// declare function bar();
4929+
// declare function foo();
4930+
// 2. mixing ambient and non-ambient declarations is a separate error that will be reported - do not want to report an extra one
4931+
previousDeclaration = undefined;
4932+
}
4933+
48794934
if (node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.Method || node.kind === SyntaxKind.Constructor) {
48804935
var currentNodeFlags = getEffectiveDeclarationFlags(node, flagsToCheck);
48814936
someNodeFlags |= currentNodeFlags;
48824937
allNodeFlags &= currentNodeFlags;
48834938

4884-
var inAmbientContext = isInAmbientContext(node);
4885-
var inAmbientContextOrInterface = node.parent.kind === SyntaxKind.InterfaceDeclaration || node.parent.kind === SyntaxKind.TypeLiteral || inAmbientContext;
4886-
if (!inAmbientContextOrInterface) {
4887-
lastSeenNonAmbientDeclaration = node;
4939+
if (node.body && bodyDeclaration) {
4940+
if (isConstructor) {
4941+
error(node, Diagnostics.Multiple_constructor_implementations_are_not_allowed);
4942+
}
4943+
else {
4944+
error(node, Diagnostics.Duplicate_function_implementation);
4945+
}
4946+
}
4947+
else if (!isExportSymbolInsideModule && previousDeclaration && previousDeclaration.parent === node.parent && previousDeclaration.end !== node.pos) {
4948+
reportImplementationExpectedError(previousDeclaration);
48884949
}
48894950

48904951
if (node.body) {
4891-
if (bodyDeclaration) {
4892-
if (isConstructor) {
4893-
error(node, Diagnostics.Multiple_constructor_implementations_are_not_allowed);
4894-
}
4895-
else {
4896-
error(node, Diagnostics.Duplicate_function_implementation);
4897-
}
4898-
}
4899-
else {
4952+
if (!bodyDeclaration) {
49004953
bodyDeclaration = node;
49014954
}
49024955
}
49034956
else {
49044957
hasOverloads = true;
49054958
}
4959+
4960+
previousDeclaration = node;
4961+
4962+
if (!inAmbientContextOrInterface) {
4963+
lastSeenNonAmbientDeclaration = node;
4964+
}
49064965
}
49074966
}
49084967

4909-
if (lastSeenNonAmbientDeclaration && !lastSeenNonAmbientDeclaration.body) {
4910-
if (isConstructor) {
4911-
error(lastSeenNonAmbientDeclaration, Diagnostics.Constructor_implementation_expected);
4912-
}
4913-
else {
4914-
error(lastSeenNonAmbientDeclaration, Diagnostics.Function_implementation_expected);
4915-
}
4968+
if (!isExportSymbolInsideModule && lastSeenNonAmbientDeclaration && !lastSeenNonAmbientDeclaration.body) {
4969+
reportImplementationExpectedError(lastSeenNonAmbientDeclaration);
49164970
}
49174971

49184972
if (hasOverloads) {
@@ -4985,7 +5039,7 @@ module ts {
49855039
}
49865040
});
49875041

4988-
var commonDeclarationSpace = exportedDeclarationSpaces & nonExportedDeclarationSpaces
5042+
var commonDeclarationSpace = exportedDeclarationSpaces & nonExportedDeclarationSpaces;
49895043

49905044
if (commonDeclarationSpace) {
49915045
// declaration spaces for exported and non-exported declarations intersect

src/compiler/diagnosticInformationMap.generated.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,12 @@ module ts {
158158
Duplicate_number_index_signature: { code: 2233, category: DiagnosticCategory.Error, key: "Duplicate number index signature." },
159159
All_declarations_of_an_interface_must_have_identical_type_parameters: { code: 2234, category: DiagnosticCategory.Error, key: "All declarations of an interface must have identical type parameters." },
160160
Expression_resolves_to_variable_declaration_i_that_compiler_uses_to_initialize_rest_parameter: { code: 2235, category: DiagnosticCategory.Error, key: "Expression resolves to variable declaration '_i' that compiler uses to initialize rest parameter." },
161-
Constructor_implementation_expected: { code: 2240, category: DiagnosticCategory.Error, key: "Constructor implementation expected." },
161+
Function_implementation_name_must_be_0: { code: 2239, category: DiagnosticCategory.Error, key: "Function implementation name must be '{0}'." },
162+
Constructor_implementation_is_missing: { code: 2240, category: DiagnosticCategory.Error, key: "Constructor implementation is missing." },
162163
An_export_assignment_cannot_be_used_in_a_module_with_other_exported_elements: { code: 2245, category: DiagnosticCategory.Error, key: "An export assignment cannot be used in a module with other exported elements." },
163164
A_parameter_property_is_only_allowed_in_a_constructor_implementation: { code: 2246, category: DiagnosticCategory.Error, key: "A parameter property is only allowed in a constructor implementation." },
165+
Function_overload_must_be_static: { code: 2247, category: DiagnosticCategory.Error, key: "Function overload must be static." },
166+
Function_overload_must_not_be_static: { code: 2248, category: DiagnosticCategory.Error, key: "Function overload must not be static." },
164167
Circular_definition_of_import_alias_0: { code: 3000, category: DiagnosticCategory.Error, key: "Circular definition of import alias '{0}'." },
165168
Cannot_find_name_0: { code: 3001, category: DiagnosticCategory.Error, key: "Cannot find name '{0}'." },
166169
Module_0_has_no_exported_member_1: { code: 3002, category: DiagnosticCategory.Error, key: "Module '{0}' has no exported member '{1}'." },
@@ -277,7 +280,7 @@ module ts {
277280
Types_of_parameters_0_and_1_are_incompatible_Colon: { code: -9999999, category: DiagnosticCategory.Error, key: "Types of parameters '{0}' and '{1}' are incompatible:" },
278281
Unknown_identifier_0: { code: -9999999, category: DiagnosticCategory.Error, key: "Unknown identifier '{0}'." },
279282
Property_0_is_inaccessible: { code: -9999999, category: DiagnosticCategory.Error, key: "Property '{0}' is inaccessible." },
280-
Function_implementation_expected: { code: -9999999, category: DiagnosticCategory.Error, key: "Function implementation expected." },
283+
Function_implementation_is_missing_or_not_immediately_following_the_declaration: { code: -9999999, category: DiagnosticCategory.Error, key: "Function implementation is missing or not immediately following the declaration." },
281284
Property_0_of_type_1_is_not_assignable_to_string_index_type_2: { code: -9999999, category: DiagnosticCategory.Error, key: "Property '{0}' of type '{1}' is not assignable to string index type '{2}'." },
282285
Property_0_of_type_1_is_not_assignable_to_numeric_index_type_2: { code: -9999999, category: DiagnosticCategory.Error, key: "Property '{0}' of type '{1}' is not assignable to numeric index type '{2}'." },
283286
Numeric_index_type_0_is_not_assignable_to_string_index_type_1: { code: -9999999, category: DiagnosticCategory.Error, key: "Numeric index type '{0}' is not assignable to string index type '{1}'." },

src/compiler/diagnosticMessages.json

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -623,8 +623,12 @@
623623
"Expression resolves to variable declaration '_i' that compiler uses to initialize rest parameter.": {
624624
"category": "Error",
625625
"code": 2235
626-
},
627-
"Constructor implementation expected.": {
626+
},
627+
"Function implementation name must be '{0}'.": {
628+
"category": "Error",
629+
"code": 2239
630+
},
631+
"Constructor implementation is missing.": {
628632
"category": "Error",
629633
"code": 2240
630634
},
@@ -636,6 +640,14 @@
636640
"category": "Error",
637641
"code": 2246
638642
},
643+
"Function overload must be static.": {
644+
"category": "Error",
645+
"code": 2247
646+
},
647+
"Function overload must not be static.": {
648+
"category": "Error",
649+
"code": 2248
650+
},
639651
"Circular definition of import alias '{0}'.": {
640652
"category": "Error",
641653
"code": 3000
@@ -905,7 +917,6 @@
905917
"category": "Error",
906918
"code": 7020
907919
},
908-
909920
"Variable declaration list cannot be empty.": {
910921
"category": "Error",
911922
"code": -9999999
@@ -1126,7 +1137,7 @@
11261137
"category": "Error",
11271138
"code": -9999999
11281139
},
1129-
"Function implementation expected.": {
1140+
"Function implementation is missing or not immediately following the declaration.": {
11301141
"category": "Error",
11311142
"code": -9999999
11321143
},

tests/baselines/reference/ClassDeclaration10.errors.txt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
class C {
33
constructor();
44
~~~~~~~~~~~~~~
5-
!!! Constructor implementation expected.
5+
!!! Constructor implementation is missing.
66
foo();
7-
~~~~~~
8-
!!! Function implementation expected.
7+
~~~
8+
!!! Function implementation is missing or not immediately following the declaration.
99
}

tests/baselines/reference/ClassDeclaration11.errors.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@
22
class C {
33
constructor();
44
~~~~~~~~~~~~~~
5-
!!! Constructor implementation expected.
5+
!!! Constructor implementation is missing.
66
foo() { }
77
}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
==== tests/cases/compiler/ClassDeclaration13.ts (1 errors) ====
22
class C {
33
foo();
4-
~~~~~~
5-
!!! Function implementation expected.
64
bar() { }
5+
~~~
6+
!!! Function implementation name must be 'foo'.
77
}
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
==== tests/cases/compiler/ClassDeclaration14.ts (2 errors) ====
22
class C {
33
foo();
4-
~~~~~~
5-
!!! Function implementation expected.
4+
~~~
5+
!!! Function implementation is missing or not immediately following the declaration.
66
constructor();
77
~~~~~~~~~~~~~~
8-
!!! Constructor implementation expected.
8+
!!! Constructor implementation is missing.
99
}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
==== tests/cases/compiler/ClassDeclaration15.ts (1 errors) ====
22
class C {
33
foo();
4-
~~~~~~
5-
!!! Function implementation expected.
4+
~~~
5+
!!! Function implementation is missing or not immediately following the declaration.
66
constructor() { }
77
}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
==== tests/cases/compiler/ClassDeclaration21.ts (1 errors) ====
22
class C {
33
0();
4-
~~~~
5-
!!! Function implementation expected.
64
1() { }
5+
~
6+
!!! Function implementation name must be '0'.
77
}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
==== tests/cases/compiler/ClassDeclaration22.ts (1 errors) ====
22
class C {
33
"foo"();
4-
~~~~~~~~
5-
!!! Function implementation expected.
64
"bar"() { }
5+
~~~~~
6+
!!! Function implementation name must be '"foo"'.
77
}

tests/baselines/reference/ClassDeclaration25.errors.txt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@
55
}
66
class List<U> implements IList<U> {
77
data(): U;
8-
~~~~~~~~~~
9-
!!! Function implementation expected.
8+
~~~~
9+
!!! Function implementation is missing or not immediately following the declaration.
1010
next(): string;
11-
~~~~~~~~~~~~~~~
12-
!!! Function implementation expected.
11+
~~~~
12+
!!! Function implementation is missing or not immediately following the declaration.
1313
}
1414

tests/baselines/reference/ClassDeclaration8.errors.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22
class C {
33
constructor();
44
~~~~~~~~~~~~~~
5-
!!! Constructor implementation expected.
5+
!!! Constructor implementation is missing.
66
}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
==== tests/cases/compiler/ClassDeclaration9.ts (1 errors) ====
22
class C {
33
foo();
4-
~~~~~~
5-
!!! Function implementation expected.
4+
~~~
5+
!!! Function implementation is missing or not immediately following the declaration.
66
}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
==== tests/cases/compiler/FunctionDeclaration3.ts (1 errors) ====
22
function foo();
3-
~~~~~~~~~~~~~~~
4-
!!! Function implementation expected.
3+
~~~
4+
!!! Function implementation is missing or not immediately following the declaration.
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
==== tests/cases/compiler/FunctionDeclaration4.ts (1 errors) ====
22
function foo();
3-
~~~~~~~~~~~~~~~
4-
!!! Function implementation expected.
5-
function bar() { }
3+
function bar() { }
4+
~~~
5+
!!! Function implementation name must be 'foo'.
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
==== tests/cases/compiler/FunctionDeclaration6.ts (1 errors) ====
22
{
33
function foo();
4-
~~~~~~~~~~~~~~~
5-
!!! Function implementation expected.
64
function bar() { }
5+
~~~
6+
!!! Function implementation name must be 'foo'.
77
}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
==== tests/cases/compiler/FunctionDeclaration7.ts (1 errors) ====
22
module M {
33
function foo();
4-
~~~~~~~~~~~~~~~
5-
!!! Function implementation expected.
4+
~~~
5+
!!! Function implementation is missing or not immediately following the declaration.
66
}

tests/baselines/reference/assignmentCompatFunctionsWithOptionalArgs.errors.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
==== tests/cases/compiler/assignmentCompatFunctionsWithOptionalArgs.ts (3 errors) ====
22
function foo(x: { id: number; name?: string; }): void;
3-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4-
!!! Function implementation expected.
3+
~~~
4+
!!! Function implementation is missing or not immediately following the declaration.
55
foo({ id: 1234 }); // Ok
66
foo({ id: 1234, name: "hello" }); // Ok
77
foo({ id: 1234, name: false }); // Error, name of wrong type

tests/baselines/reference/callOverloads1.errors.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
}
99

1010
function Foo(); // error
11-
~~~~~~~~~~~~~~~
12-
!!! Function implementation expected.
11+
~~~
12+
!!! Function implementation is missing or not immediately following the declaration.
1313
~~~
1414
!!! Duplicate identifier 'Foo'.
1515
function F1(s:string);

0 commit comments

Comments
 (0)