Skip to content

Better errors for types with same name #1575

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.

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

Merged
35 changes: 22 additions & 13 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,7 @@ module ts {
* Enclosing declaration is optional when we don't want to get qualified name in the enclosing declaration scope
* Meaning needs to be specified if the enclosing declaration is given
*/
function buildSymbolDisplay(symbol: Symbol, writer: SymbolWriter, enclosingDeclaration?: Node, meaning?: SymbolFlags, flags?: SymbolFormatFlags): void {
function buildSymbolDisplay(symbol: Symbol, writer: SymbolWriter, enclosingDeclaration?: Node, meaning?: SymbolFlags, flags?: SymbolFormatFlags, typeFlags?: TypeFormatFlags): void {
var parentSymbol: Symbol;
function appendParentTypeArgumentsAndSymbolName(symbol: Symbol): void {
if (parentSymbol) {
Expand Down Expand Up @@ -1095,11 +1095,12 @@ module ts {
}
}

// Get qualified name
if (enclosingDeclaration &&
// TypeParameters do not need qualification
!(symbol.flags & SymbolFlags.TypeParameter)) {

// Get qualified name if the symbol is not a type parameter
// and there is an enclosing declaration or we specifically
// asked for it
var isTypeParameter = symbol.flags & SymbolFlags.TypeParameter;
var typeFormatFlag = TypeFormatFlags.UseFullyQualifiedType & typeFlags;
if (!isTypeParameter && (enclosingDeclaration || typeFormatFlag)) {
walkSymbol(symbol, meaning);
return;
}
Expand All @@ -1122,7 +1123,8 @@ module ts {
writeTypeReference(<TypeReference>type, flags);
}
else if (type.flags & (TypeFlags.Class | TypeFlags.Interface | TypeFlags.Enum | TypeFlags.TypeParameter)) {
buildSymbolDisplay(type.symbol, writer, enclosingDeclaration, SymbolFlags.Type);
// The specified symbol flags need to be reinterpreted as type flags
buildSymbolDisplay(type.symbol, writer, enclosingDeclaration, SymbolFlags.Type, SymbolFormatFlags.None, flags);
Copy link
Member

Choose a reason for hiding this comment

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

This is subtle - leave a comment saying that in these cases, the specified symbol flags need to be reinterpreted as type flags.

}
else if (type.flags & TypeFlags.Tuple) {
writeTupleType(<TupleType>type);
Expand Down Expand Up @@ -1193,17 +1195,18 @@ module ts {
function writeAnonymousType(type: ObjectType, flags: TypeFormatFlags) {
// Always use 'typeof T' for type of class, enum, and module objects
if (type.symbol && type.symbol.flags & (SymbolFlags.Class | SymbolFlags.Enum | SymbolFlags.ValueModule)) {
writeTypeofSymbol(type);
writeTypeofSymbol(type, flags);
}
// Use 'typeof T' for types of functions and methods that circularly reference themselves
else if (shouldWriteTypeOfFunctionSymbol()) {
writeTypeofSymbol(type);
writeTypeofSymbol(type, flags);
}
else if (typeStack && contains(typeStack, type)) {
// If type is an anonymous type literal in a type alias declaration, use type alias name
var typeAlias = getTypeAliasForTypeLiteral(type);
if (typeAlias) {
buildSymbolDisplay(typeAlias, writer, enclosingDeclaration, SymbolFlags.Type);
// The specified symbol flags need to be reinterpreted as type flags
buildSymbolDisplay(typeAlias, writer, enclosingDeclaration, SymbolFlags.Type, SymbolFormatFlags.None, flags);
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as for TypeFlags.Class | TypeFlags.Interface | TypeFlags.Enum | TypeFlags.TypeParameter

}
else {
// Recursive usage, use any
Expand Down Expand Up @@ -1237,10 +1240,10 @@ module ts {
}
}

function writeTypeofSymbol(type: ObjectType) {
function writeTypeofSymbol(type: ObjectType, typeFormatFlags?: TypeFormatFlags) {
writeKeyword(writer, SyntaxKind.TypeOfKeyword);
writeSpace(writer);
buildSymbolDisplay(type.symbol, writer, enclosingDeclaration, SymbolFlags.Value);
buildSymbolDisplay(type.symbol, writer, enclosingDeclaration, SymbolFlags.Value, SymbolFormatFlags.None, typeFormatFlags);
}

function getIndexerParameterName(type: ObjectType, indexKind: IndexKind, fallbackName: string): string {
Expand Down Expand Up @@ -3508,7 +3511,13 @@ module ts {
}
if (reportErrors) {
headMessage = headMessage || Diagnostics.Type_0_is_not_assignable_to_type_1;
reportError(headMessage, typeToString(source), typeToString(target));
var sourceType = typeToString(source);
var targetType = typeToString(target);
Copy link
Contributor

Choose a reason for hiding this comment

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

i would write this as:

if (sourceType === targetType) {
    sourceType = typeToString(...);
    targetType = typeToString(...);
}

reportError(headMessage, sourceType, targetType)

if (sourceType === targetType) {
sourceType = typeToString(source, /*enclosingDeclaration*/ undefined, TypeFormatFlags.UseFullyQualifiedType);
targetType = typeToString(target, /*enclosingDeclaration*/ undefined, TypeFormatFlags.UseFullyQualifiedType);
}
reportError(headMessage, sourceType, targetType);
}
return Ternary.False;
}
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1074,6 +1074,7 @@ module ts {
WriteOwnNameForAnyLike = 0x00000010, // Write symbol's own name instead of 'any' for any like types (eg. unknown, __resolving__ etc)
WriteTypeArgumentsOfSignature = 0x00000020, // Write the type arguments instead of type parameters of the signature
InElementType = 0x00000040, // Writing an array or union element type
UseFullyQualifiedType = 0x00000080, // Write out the fully qualified type name (eg. Module.Type, instead of Type)
}

export const enum SymbolFormatFlags {
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/clodulesDerivedClasses.errors.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
tests/cases/compiler/clodulesDerivedClasses.ts(9,7): error TS2417: Class static side 'typeof Path' incorrectly extends base class static side 'typeof Shape'.
Types of property 'Utils' are incompatible.
Type 'typeof Utils' is not assignable to type 'typeof Utils'.
Type 'typeof Path.Utils' is not assignable to type 'typeof Shape.Utils'.
Property 'convert' is missing in type 'typeof Utils'.


Expand All @@ -17,7 +17,7 @@ tests/cases/compiler/clodulesDerivedClasses.ts(9,7): error TS2417: Class static
~~~~
!!! error TS2417: Class static side 'typeof Path' incorrectly extends base class static side 'typeof Shape'.
!!! error TS2417: Types of property 'Utils' are incompatible.
!!! error TS2417: Type 'typeof Utils' is not assignable to type 'typeof Utils'.
!!! error TS2417: Type 'typeof Path.Utils' is not assignable to type 'typeof Shape.Utils'.
!!! error TS2417: Property 'convert' is missing in type 'typeof Utils'.
name: string;

Expand Down
22 changes: 22 additions & 0 deletions tests/baselines/reference/differentTypesWithSameName.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
tests/cases/compiler/differentTypesWithSameName.ts(16,15): error TS2345: Argument of type 'variable' is not assignable to parameter of type 'm.variable'.


==== tests/cases/compiler/differentTypesWithSameName.ts (1 errors) ====
module m {
export class variable{
s: string;
}
export function doSomething(v: m.variable) {

}
}

class variable {
t: number;
}


var v: variable = new variable();
m.doSomething(v);
~
!!! error TS2345: Argument of type 'variable' is not assignable to parameter of type 'm.variable'.
38 changes: 38 additions & 0 deletions tests/baselines/reference/differentTypesWithSameName.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
//// [differentTypesWithSameName.ts]
module m {
export class variable{
s: string;
}
export function doSomething(v: m.variable) {

}
}

class variable {
t: number;
}


var v: variable = new variable();
m.doSomething(v);

//// [differentTypesWithSameName.js]
var m;
(function (m) {
var variable = (function () {
function variable() {
}
return variable;
})();
m.variable = variable;
function doSomething(v) {
}
m.doSomething = doSomething;
})(m || (m = {}));
var variable = (function () {
function variable() {
}
return variable;
})();
var v = new variable();
m.doSomething(v);
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ tests/cases/conformance/statements/VariableStatements/everyTypeWithAnnotationAnd
Type 'string' is not assignable to type 'number'.
tests/cases/conformance/statements/VariableStatements/everyTypeWithAnnotationAndInvalidInitializer.ts(50,5): error TS2322: Type 'typeof N' is not assignable to type 'typeof M'.
Types of property 'A' are incompatible.
Type 'typeof A' is not assignable to type 'typeof A'.
Type 'A' is not assignable to type 'A'.
Type 'typeof N.A' is not assignable to type 'typeof M.A'.
Type 'N.A' is not assignable to type 'M.A'.
Property 'name' is missing in type 'A'.
tests/cases/conformance/statements/VariableStatements/everyTypeWithAnnotationAndInvalidInitializer.ts(51,5): error TS2322: Type 'A' is not assignable to type 'A'.
tests/cases/conformance/statements/VariableStatements/everyTypeWithAnnotationAndInvalidInitializer.ts(51,5): error TS2322: Type 'N.A' is not assignable to type 'M.A'.
tests/cases/conformance/statements/VariableStatements/everyTypeWithAnnotationAndInvalidInitializer.ts(52,5): error TS2322: Type '(x: number) => boolean' is not assignable to type '(x: number) => string'.
Type 'boolean' is not assignable to type 'string'.

Expand Down Expand Up @@ -124,12 +124,12 @@ tests/cases/conformance/statements/VariableStatements/everyTypeWithAnnotationAnd
~~~~~~~
!!! error TS2322: Type 'typeof N' is not assignable to type 'typeof M'.
!!! error TS2322: Types of property 'A' are incompatible.
!!! error TS2322: Type 'typeof A' is not assignable to type 'typeof A'.
!!! error TS2322: Type 'A' is not assignable to type 'A'.
!!! error TS2322: Type 'typeof N.A' is not assignable to type 'typeof M.A'.
!!! error TS2322: Type 'N.A' is not assignable to type 'M.A'.
!!! error TS2322: Property 'name' is missing in type 'A'.
var aClassInModule: M.A = new N.A();
~~~~~~~~~~~~~~
!!! error TS2322: Type 'A' is not assignable to type 'A'.
!!! error TS2322: Type 'N.A' is not assignable to type 'M.A'.
var aFunctionInModule: typeof M.F2 = F2;
~~~~~~~~~~~~~~~~~
!!! error TS2322: Type '(x: number) => boolean' is not assignable to type '(x: number) => string'.
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/qualify.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ tests/cases/compiler/qualify.ts(47,13): error TS2322: Type 'I4' is not assignabl
tests/cases/compiler/qualify.ts(48,13): error TS2322: Type 'I4' is not assignable to type '(k: I3) => void'.
tests/cases/compiler/qualify.ts(49,13): error TS2322: Type 'I4' is not assignable to type '{ k: I3; }'.
Property 'k' is missing in type 'I4'.
tests/cases/compiler/qualify.ts(58,5): error TS2322: Type 'I' is not assignable to type 'I'.
tests/cases/compiler/qualify.ts(58,5): error TS2322: Type 'I' is not assignable to type 'T.I'.
Property 'p' is missing in type 'I'.


Expand Down Expand Up @@ -93,7 +93,7 @@ tests/cases/compiler/qualify.ts(58,5): error TS2322: Type 'I' is not assignable
var y:I;
var x:T.I=y;
~
!!! error TS2322: Type 'I' is not assignable to type 'I'.
!!! error TS2322: Type 'I' is not assignable to type 'T.I'.
!!! error TS2322: Property 'p' is missing in type 'I'.


16 changes: 16 additions & 0 deletions tests/cases/compiler/differentTypesWithSameName.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
module m {
export class variable{
s: string;
}
export function doSomething(v: m.variable) {

}
}

class variable {
t: number;
}


var v: variable = new variable();
m.doSomething(v);