Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve non-ambient class and function merge error #33441

Closed
wants to merge 1 commit into from
Closed

Improve non-ambient class and function merge error #33441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26436,12 +26436,21 @@ namespace ts {
}

if (hasNonAmbientClass && !isConstructor && symbol.flags & SymbolFlags.Function) {
// A non-ambient class cannot be an implementation for a non-constructor function/class merge
// TODO: The below just replicates our older error from when classes and functions were
// entirely unable to merge - a more helpful message like "Class declaration cannot implement overload list"
// might be warranted. :shrug:
const relatedDiagnostics = filter(declarations, d => d.kind === SyntaxKind.ClassDeclaration)
.map(d => createDiagnosticForNode(d, Diagnostics.Consider_adding_a_declare_modifier_to_this_class));

forEach(declarations, declaration => {
addDuplicateDeclarationError(getNameOfDeclaration(declaration) || declaration, Diagnostics.Duplicate_identifier_0, symbolName(symbol), filter(declarations, d => d !== declaration));
const diagnostic = declaration.kind === SyntaxKind.ClassDeclaration
? Diagnostics.Class_declaration_cannot_implement_overload_list_for_0
: declaration.kind === SyntaxKind.FunctionDeclaration
? Diagnostics.Function_with_bodies_can_only_merge_with_classes_that_are_ambient
: undefined;
if (diagnostic) {
addRelatedInfo(
error(getNameOfDeclaration(declaration) || declaration, diagnostic, symbolName(symbol)),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it safe to provide symbolName(symbol) even if the diagnostic doesn't specify a parameter?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, from reading the code, it looks for the params in the string and matches the values to that - basically it'll just not do anything 👍

   export function formatStringFromArgs(text: string, args: ArrayLike<string | number>, baseIndex = 0): string {
        return text.replace(/{(\d+)}/g, (_match, index: string) => "" + Debug.assertDefined(args[+index + baseIndex]));
    }

...relatedDiagnostics
);
}
});
}

Expand Down
13 changes: 13 additions & 0 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2693,6 +2693,14 @@
"category": "Error",
"code": 2773
},
"Class declaration cannot implement overload list for '{0}'.": {
"category": "Error",
"code": 2774
},
"Function with bodies can only merge with classes that are ambient.": {
Copy link
Contributor Author

@austincummings austincummings Sep 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "bodies" right? Or can there only be one function body?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, there's only one body, I think we should probably make these two error messages:

  • Class declaration cannot implement overload list for '{0}'.
  • Functions with bodies can only merge with classes that are ambient.

"category": "Error",
"code": 2775
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down Expand Up @@ -4194,6 +4202,11 @@
"code": 6502
},

"Consider adding a 'declare' modifier to this class.": {
"category": "Message",
"code": 6503
},

"Variable '{0}' implicitly has an '{1}' type.": {
"category": "Error",
"code": 7005
Expand Down
12 changes: 6 additions & 6 deletions tests/baselines/reference/augmentedTypesClass2a.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
tests/cases/compiler/augmentedTypesClass2a.ts(2,7): error TS2300: Duplicate identifier 'c2'.
tests/cases/compiler/augmentedTypesClass2a.ts(2,7): error TS2300: Duplicate identifier 'c2'.
tests/cases/compiler/augmentedTypesClass2a.ts(3,10): error TS2300: Duplicate identifier 'c2'.
tests/cases/compiler/augmentedTypesClass2a.ts(2,7): error TS2774: Class declaration cannot implement overload list for 'c2'.
tests/cases/compiler/augmentedTypesClass2a.ts(3,10): error TS2300: Duplicate identifier 'c2'.
tests/cases/compiler/augmentedTypesClass2a.ts(3,10): error TS2775: Function with bodies can only merge with classes that are ambient.
tests/cases/compiler/augmentedTypesClass2a.ts(4,5): error TS2300: Duplicate identifier 'c2'.


Expand All @@ -10,15 +10,15 @@ tests/cases/compiler/augmentedTypesClass2a.ts(4,5): error TS2300: Duplicate iden
class c2 { public foo() { } } // error
~~
!!! error TS2300: Duplicate identifier 'c2'.
!!! related TS6203 tests/cases/compiler/augmentedTypesClass2a.ts:3:10: 'c2' was also declared here.
~~
!!! error TS2300: Duplicate identifier 'c2'.
!!! error TS2774: Class declaration cannot implement overload list for 'c2'.
!!! related TS6503 tests/cases/compiler/augmentedTypesClass2a.ts:2:7: Consider adding a 'declare' modifier to this class.
function c2() { } // error
~~
!!! error TS2300: Duplicate identifier 'c2'.
!!! related TS6203 tests/cases/compiler/augmentedTypesClass2a.ts:2:7: 'c2' was also declared here.
~~
!!! error TS2300: Duplicate identifier 'c2'.
!!! error TS2775: Function with bodies can only merge with classes that are ambient.
!!! related TS6503 tests/cases/compiler/augmentedTypesClass2a.ts:2:7: Consider adding a 'declare' modifier to this class.
var c2 = () => { }
~~
!!! error TS2300: Duplicate identifier 'c2'.
24 changes: 12 additions & 12 deletions tests/baselines/reference/augmentedTypesFunction.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ tests/cases/compiler/augmentedTypesFunction.ts(6,10): error TS2393: Duplicate fu
tests/cases/compiler/augmentedTypesFunction.ts(7,10): error TS2393: Duplicate function implementation.
tests/cases/compiler/augmentedTypesFunction.ts(9,10): error TS2300: Duplicate identifier 'y2a'.
tests/cases/compiler/augmentedTypesFunction.ts(10,5): error TS2300: Duplicate identifier 'y2a'.
tests/cases/compiler/augmentedTypesFunction.ts(13,10): error TS2300: Duplicate identifier 'y3'.
tests/cases/compiler/augmentedTypesFunction.ts(14,7): error TS2300: Duplicate identifier 'y3'.
tests/cases/compiler/augmentedTypesFunction.ts(16,10): error TS2300: Duplicate identifier 'y3a'.
tests/cases/compiler/augmentedTypesFunction.ts(17,7): error TS2300: Duplicate identifier 'y3a'.
tests/cases/compiler/augmentedTypesFunction.ts(13,10): error TS2775: Function with bodies can only merge with classes that are ambient.
tests/cases/compiler/augmentedTypesFunction.ts(14,7): error TS2774: Class declaration cannot implement overload list for 'y3'.
tests/cases/compiler/augmentedTypesFunction.ts(16,10): error TS2775: Function with bodies can only merge with classes that are ambient.
tests/cases/compiler/augmentedTypesFunction.ts(17,7): error TS2774: Class declaration cannot implement overload list for 'y3a'.
tests/cases/compiler/augmentedTypesFunction.ts(20,10): error TS2567: Enum declarations can only merge with namespace or other enum declarations.
tests/cases/compiler/augmentedTypesFunction.ts(21,6): error TS2567: Enum declarations can only merge with namespace or other enum declarations.

Expand Down Expand Up @@ -39,21 +39,21 @@ tests/cases/compiler/augmentedTypesFunction.ts(21,6): error TS2567: Enum declara
// function then class
function y3() { } // error
~~
!!! error TS2300: Duplicate identifier 'y3'.
!!! related TS6203 tests/cases/compiler/augmentedTypesFunction.ts:14:7: 'y3' was also declared here.
!!! error TS2775: Function with bodies can only merge with classes that are ambient.
!!! related TS6503 tests/cases/compiler/augmentedTypesFunction.ts:14:7: Consider adding a 'declare' modifier to this class.
class y3 { } // error
~~
!!! error TS2300: Duplicate identifier 'y3'.
!!! related TS6203 tests/cases/compiler/augmentedTypesFunction.ts:13:10: 'y3' was also declared here.
!!! error TS2774: Class declaration cannot implement overload list for 'y3'.
!!! related TS6503 tests/cases/compiler/augmentedTypesFunction.ts:14:7: Consider adding a 'declare' modifier to this class.

function y3a() { } // error
~~~
!!! error TS2300: Duplicate identifier 'y3a'.
!!! related TS6203 tests/cases/compiler/augmentedTypesFunction.ts:17:7: 'y3a' was also declared here.
!!! error TS2775: Function with bodies can only merge with classes that are ambient.
!!! related TS6503 tests/cases/compiler/augmentedTypesFunction.ts:17:7: Consider adding a 'declare' modifier to this class.
class y3a { public foo() { } } // error
~~~
!!! error TS2300: Duplicate identifier 'y3a'.
!!! related TS6203 tests/cases/compiler/augmentedTypesFunction.ts:16:10: 'y3a' was also declared here.
!!! error TS2774: Class declaration cannot implement overload list for 'y3a'.
!!! related TS6503 tests/cases/compiler/augmentedTypesFunction.ts:17:7: Consider adding a 'declare' modifier to this class.

// function then enum
function y4() { } // error
Expand Down
14 changes: 7 additions & 7 deletions tests/baselines/reference/callOverloads1.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
tests/cases/compiler/callOverloads1.ts(1,7): error TS2300: Duplicate identifier 'Foo'.
tests/cases/compiler/callOverloads1.ts(9,10): error TS2300: Duplicate identifier 'Foo'.
tests/cases/compiler/callOverloads1.ts(1,7): error TS2774: Class declaration cannot implement overload list for 'Foo'.
tests/cases/compiler/callOverloads1.ts(9,10): error TS2391: Function implementation is missing or not immediately following the declaration.
tests/cases/compiler/callOverloads1.ts(9,10): error TS2775: Function with bodies can only merge with classes that are ambient.


==== tests/cases/compiler/callOverloads1.ts (3 errors) ====
class Foo { // error
~~~
!!! error TS2300: Duplicate identifier 'Foo'.
!!! related TS6203 tests/cases/compiler/callOverloads1.ts:9:10: 'Foo' was also declared here.
!!! error TS2774: Class declaration cannot implement overload list for 'Foo'.
!!! related TS6503 tests/cases/compiler/callOverloads1.ts:1:7: Consider adding a 'declare' modifier to this class.
bar1() { /*WScript.Echo("bar1");*/ }

constructor(x: any) {
Expand All @@ -17,10 +17,10 @@ tests/cases/compiler/callOverloads1.ts(9,10): error TS2391: Function implementat

function Foo(); // error
~~~
!!! error TS2300: Duplicate identifier 'Foo'.
!!! related TS6203 tests/cases/compiler/callOverloads1.ts:1:7: 'Foo' was also declared here.
~~~
!!! error TS2391: Function implementation is missing or not immediately following the declaration.
~~~
!!! error TS2775: Function with bodies can only merge with classes that are ambient.
!!! related TS6503 tests/cases/compiler/callOverloads1.ts:1:7: Consider adding a 'declare' modifier to this class.
function F1(s:string);
function F1(a:any) { return a;}

Expand Down
12 changes: 6 additions & 6 deletions tests/baselines/reference/callOverloads2.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
tests/cases/compiler/callOverloads2.ts(1,7): error TS2300: Duplicate identifier 'Foo'.
tests/cases/compiler/callOverloads2.ts(9,10): error TS2300: Duplicate identifier 'Foo'.
tests/cases/compiler/callOverloads2.ts(1,7): error TS2774: Class declaration cannot implement overload list for 'Foo'.
tests/cases/compiler/callOverloads2.ts(9,10): error TS2775: Function with bodies can only merge with classes that are ambient.
tests/cases/compiler/callOverloads2.ts(11,10): error TS2389: Function implementation name must be 'Foo'.
tests/cases/compiler/callOverloads2.ts(11,10): error TS2393: Duplicate function implementation.
tests/cases/compiler/callOverloads2.ts(12,10): error TS2393: Duplicate function implementation.
Expand All @@ -9,8 +9,8 @@ tests/cases/compiler/callOverloads2.ts(14,10): error TS2391: Function implementa
==== tests/cases/compiler/callOverloads2.ts (6 errors) ====
class Foo { // error
~~~
!!! error TS2300: Duplicate identifier 'Foo'.
!!! related TS6203 tests/cases/compiler/callOverloads2.ts:9:10: 'Foo' was also declared here.
!!! error TS2774: Class declaration cannot implement overload list for 'Foo'.
!!! related TS6503 tests/cases/compiler/callOverloads2.ts:1:7: Consider adding a 'declare' modifier to this class.
bar1() { /*WScript.Echo("bar1");*/ }

constructor(x: any) {
Expand All @@ -20,8 +20,8 @@ tests/cases/compiler/callOverloads2.ts(14,10): error TS2391: Function implementa

function Foo(); // error
~~~
!!! error TS2300: Duplicate identifier 'Foo'.
!!! related TS6203 tests/cases/compiler/callOverloads2.ts:1:7: 'Foo' was also declared here.
!!! error TS2775: Function with bodies can only merge with classes that are ambient.
!!! related TS6503 tests/cases/compiler/callOverloads2.ts:1:7: Consider adding a 'declare' modifier to this class.

function F1(s:string) {return s;} // error
~~
Expand Down
23 changes: 10 additions & 13 deletions tests/baselines/reference/callOverloads3.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,24 @@
tests/cases/compiler/callOverloads3.ts(1,10): error TS2300: Duplicate identifier 'Foo'.
tests/cases/compiler/callOverloads3.ts(2,10): error TS2300: Duplicate identifier 'Foo'.
tests/cases/compiler/callOverloads3.ts(1,10): error TS2775: Function with bodies can only merge with classes that are ambient.
tests/cases/compiler/callOverloads3.ts(2,10): error TS2391: Function implementation is missing or not immediately following the declaration.
tests/cases/compiler/callOverloads3.ts(3,7): error TS2300: Duplicate identifier 'Foo'.
tests/cases/compiler/callOverloads3.ts(2,10): error TS2775: Function with bodies can only merge with classes that are ambient.
tests/cases/compiler/callOverloads3.ts(3,7): error TS2774: Class declaration cannot implement overload list for 'Foo'.


==== tests/cases/compiler/callOverloads3.ts (4 errors) ====
function Foo():Foo; // error
~~~
!!! error TS2300: Duplicate identifier 'Foo'.
!!! related TS6203 tests/cases/compiler/callOverloads3.ts:2:10: 'Foo' was also declared here.
!!! related TS6204 tests/cases/compiler/callOverloads3.ts:3:7: and here.
!!! error TS2775: Function with bodies can only merge with classes that are ambient.
!!! related TS6503 tests/cases/compiler/callOverloads3.ts:3:7: Consider adding a 'declare' modifier to this class.
function Foo(s:string):Foo; // error
~~~
!!! error TS2300: Duplicate identifier 'Foo'.
!!! related TS6203 tests/cases/compiler/callOverloads3.ts:1:10: 'Foo' was also declared here.
!!! related TS6204 tests/cases/compiler/callOverloads3.ts:3:7: and here.
~~~
!!! error TS2391: Function implementation is missing or not immediately following the declaration.
~~~
!!! error TS2775: Function with bodies can only merge with classes that are ambient.
!!! related TS6503 tests/cases/compiler/callOverloads3.ts:3:7: Consider adding a 'declare' modifier to this class.
class Foo { // error
~~~
!!! error TS2300: Duplicate identifier 'Foo'.
!!! related TS6203 tests/cases/compiler/callOverloads3.ts:1:10: 'Foo' was also declared here.
!!! related TS6204 tests/cases/compiler/callOverloads3.ts:2:10: and here.
!!! error TS2774: Class declaration cannot implement overload list for 'Foo'.
!!! related TS6503 tests/cases/compiler/callOverloads3.ts:3:7: Consider adding a 'declare' modifier to this class.
bar1() { /*WScript.Echo("bar1");*/ }
constructor(x: any) {
// WScript.Echo("Constructor function has executed");
Expand Down
23 changes: 10 additions & 13 deletions tests/baselines/reference/callOverloads4.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,24 @@
tests/cases/compiler/callOverloads4.ts(1,10): error TS2300: Duplicate identifier 'Foo'.
tests/cases/compiler/callOverloads4.ts(2,10): error TS2300: Duplicate identifier 'Foo'.
tests/cases/compiler/callOverloads4.ts(1,10): error TS2775: Function with bodies can only merge with classes that are ambient.
tests/cases/compiler/callOverloads4.ts(2,10): error TS2391: Function implementation is missing or not immediately following the declaration.
tests/cases/compiler/callOverloads4.ts(3,7): error TS2300: Duplicate identifier 'Foo'.
tests/cases/compiler/callOverloads4.ts(2,10): error TS2775: Function with bodies can only merge with classes that are ambient.
tests/cases/compiler/callOverloads4.ts(3,7): error TS2774: Class declaration cannot implement overload list for 'Foo'.


==== tests/cases/compiler/callOverloads4.ts (4 errors) ====
function Foo():Foo; // error
~~~
!!! error TS2300: Duplicate identifier 'Foo'.
!!! related TS6203 tests/cases/compiler/callOverloads4.ts:2:10: 'Foo' was also declared here.
!!! related TS6204 tests/cases/compiler/callOverloads4.ts:3:7: and here.
!!! error TS2775: Function with bodies can only merge with classes that are ambient.
!!! related TS6503 tests/cases/compiler/callOverloads4.ts:3:7: Consider adding a 'declare' modifier to this class.
function Foo(s:string):Foo; // error
~~~
!!! error TS2300: Duplicate identifier 'Foo'.
!!! related TS6203 tests/cases/compiler/callOverloads4.ts:1:10: 'Foo' was also declared here.
!!! related TS6204 tests/cases/compiler/callOverloads4.ts:3:7: and here.
~~~
!!! error TS2391: Function implementation is missing or not immediately following the declaration.
~~~
!!! error TS2775: Function with bodies can only merge with classes that are ambient.
!!! related TS6503 tests/cases/compiler/callOverloads4.ts:3:7: Consider adding a 'declare' modifier to this class.
class Foo { // error
~~~
!!! error TS2300: Duplicate identifier 'Foo'.
!!! related TS6203 tests/cases/compiler/callOverloads4.ts:1:10: 'Foo' was also declared here.
!!! related TS6204 tests/cases/compiler/callOverloads4.ts:2:10: and here.
!!! error TS2774: Class declaration cannot implement overload list for 'Foo'.
!!! related TS6503 tests/cases/compiler/callOverloads4.ts:3:7: Consider adding a 'declare' modifier to this class.
bar1() { /*WScript.Echo("bar1");*/ }
constructor(s: string);
constructor(x: any) {
Expand Down
23 changes: 10 additions & 13 deletions tests/baselines/reference/callOverloads5.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,24 @@
tests/cases/compiler/callOverloads5.ts(1,10): error TS2300: Duplicate identifier 'Foo'.
tests/cases/compiler/callOverloads5.ts(2,10): error TS2300: Duplicate identifier 'Foo'.
tests/cases/compiler/callOverloads5.ts(1,10): error TS2775: Function with bodies can only merge with classes that are ambient.
tests/cases/compiler/callOverloads5.ts(2,10): error TS2391: Function implementation is missing or not immediately following the declaration.
tests/cases/compiler/callOverloads5.ts(3,7): error TS2300: Duplicate identifier 'Foo'.
tests/cases/compiler/callOverloads5.ts(2,10): error TS2775: Function with bodies can only merge with classes that are ambient.
tests/cases/compiler/callOverloads5.ts(3,7): error TS2774: Class declaration cannot implement overload list for 'Foo'.


==== tests/cases/compiler/callOverloads5.ts (4 errors) ====
function Foo():Foo; // error
~~~
!!! error TS2300: Duplicate identifier 'Foo'.
!!! related TS6203 tests/cases/compiler/callOverloads5.ts:2:10: 'Foo' was also declared here.
!!! related TS6204 tests/cases/compiler/callOverloads5.ts:3:7: and here.
!!! error TS2775: Function with bodies can only merge with classes that are ambient.
!!! related TS6503 tests/cases/compiler/callOverloads5.ts:3:7: Consider adding a 'declare' modifier to this class.
function Foo(s:string):Foo; // error
~~~
!!! error TS2300: Duplicate identifier 'Foo'.
!!! related TS6203 tests/cases/compiler/callOverloads5.ts:1:10: 'Foo' was also declared here.
!!! related TS6204 tests/cases/compiler/callOverloads5.ts:3:7: and here.
~~~
!!! error TS2391: Function implementation is missing or not immediately following the declaration.
~~~
!!! error TS2775: Function with bodies can only merge with classes that are ambient.
!!! related TS6503 tests/cases/compiler/callOverloads5.ts:3:7: Consider adding a 'declare' modifier to this class.
class Foo { // error
~~~
!!! error TS2300: Duplicate identifier 'Foo'.
!!! related TS6203 tests/cases/compiler/callOverloads5.ts:1:10: 'Foo' was also declared here.
!!! related TS6204 tests/cases/compiler/callOverloads5.ts:2:10: and here.
!!! error TS2774: Class declaration cannot implement overload list for 'Foo'.
!!! related TS6503 tests/cases/compiler/callOverloads5.ts:3:7: Consider adding a 'declare' modifier to this class.
bar1(s:string);
bar1(n:number);
bar1(a:any) { /*WScript.Echo(a);*/ }
Expand Down
Loading