Better errors for types with same name #1575
Conversation
| @@ -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 'Path.Utils' is not assignable to type 'Shape.Utils'. | |||
There was a problem hiding this comment.
This is actually a regression - I think it should be 'typeof Path.Utils' and 'typeof Shape.Utils'.
|
I think you'll need to modify |
|
Makes sense. I'll try something along those lines and see what I come up with. |
| @@ -1096,9 +1096,11 @@ module ts { | |||
| } | |||
|
|
|||
| // Get qualified name | |||
| if (enclosingDeclaration && | |||
| if ((enclosingDeclaration && | |||
There was a problem hiding this comment.
please find a way to simplify this 'if' conditional. One option is to introduce locals above to make it clearer what you are testing for.
|
|
||
| // Get qualified name if there is an enclosing declaration and it is not a | ||
| // type parameter or we have a flag telling us to | ||
| var needFullyQualifiedName = (enclosingDeclaration && !(symbol.flags & SymbolFlags.TypeParameter)); |
There was a problem hiding this comment.
Type parameters still shouldn't be qualified even when TypeFormatFlags.UseFullyQualifiedType
…eparemeters even if typeformatflag is present
| } | ||
| 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); | ||
| buildSymbolDisplay(typeAlias, writer, enclosingDeclaration, SymbolFlags.Type, /*flags*/ undefined, flags); |
There was a problem hiding this comment.
Use None, not undefined.
|
Need to bring this back up on the radar - sorry for the lack of follow-up. I'll try to CR it when I get in. |
|
|
||
| // Get qualified name if the symbol is not a type parameter | ||
| // and there is an enclosing declaration or we specifically | ||
| // asked for it |
|
Looking good so far! Just need to address the comments I've left. |
|
Sweet, I'll get it fixed up when I get a chance. |
|
LGTM 👍 |
|
@DanielRosenwasser Any other work that needs to be done here? |
|
I don't think so, but I'd like to just get sign off from another member before pulling in the change. |
…ameName Better errors for types with same name
|
Thanks! This is going to save users a lot of mental anguish. 😄 |
Fix for issue #1419. Added a pretty unobtrusive fix but I'm wondering if I need to have more complicated logic (like typeToString() does) to handle certain cases that simply calling getFullyQualifiedName doesn't take care of.