Skip to content

Error when variable is circularly referenced in type annotation #2991

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

Merged
merged 9 commits into from
May 5, 2015
Merged

Error when variable is circularly referenced in type annotation #2991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
05f51dc
Error when variable is circularly referenced in type annotation
ahejlsberg May 1, 2015
a0f4478
Merge branch 'master' into circularVar
ahejlsberg May 1, 2015
a133684
Modifying test
ahejlsberg May 1, 2015
8600fef
Accepting new baselines
ahejlsberg May 1, 2015
2792614
Improved circularity detection for types
ahejlsberg May 2, 2015
d0e5269
Accepting new baselines
ahejlsberg May 2, 2015
7efd93a
Adding test
ahejlsberg May 2, 2015
4186167
Removing 'resolvingType' special type
ahejlsberg May 3, 2015
eeb23ad
Addressing CR feedback
ahejlsberg May 5, 2015
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
159 changes: 95 additions & 64 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,11 @@ module ts {
let undefinedType = createIntrinsicType(TypeFlags.Undefined | TypeFlags.ContainsUndefinedOrNull, "undefined");
let nullType = createIntrinsicType(TypeFlags.Null | TypeFlags.ContainsUndefinedOrNull, "null");
let unknownType = createIntrinsicType(TypeFlags.Any, "unknown");
let resolvingType = createIntrinsicType(TypeFlags.Any, "__resolving__");

let emptyObjectType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined);
let anyFunctionType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined);
let noConstraintType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined);

let anySignature = createSignature(undefined, undefined, emptyArray, anyType, 0, false, false);
let unknownSignature = createSignature(undefined, undefined, emptyArray, unknownType, 0, false, false);

Expand All @@ -118,14 +117,17 @@ module ts {
let getGlobalParameterDecoratorType: () => ObjectType;
let getGlobalPropertyDecoratorType: () => ObjectType;
let getGlobalMethodDecoratorType: () => ObjectType;

let tupleTypes: Map<TupleType> = {};
let unionTypes: Map<UnionType> = {};
let stringLiteralTypes: Map<StringLiteralType> = {};
let emitExtends = false;
let emitDecorate = false;
let emitParam = false;

let resolutionTargets: Object[] = [];
let resolutionResults: boolean[] = [];

let mergedSymbols: Symbol[] = [];
let symbolLinks: SymbolLinks[] = [];
let nodeLinks: NodeLinks[] = [];
Expand Down Expand Up @@ -1980,7 +1982,7 @@ module ts {
}
}

function collectLinkedAliases(node: Identifier): Node[]{
function collectLinkedAliases(node: Identifier): Node[] {
var exportSymbol: Symbol;
if (node.parent && node.parent.kind === SyntaxKind.ExportAssignment) {
exportSymbol = resolveName(node.parent, node.text, SymbolFlags.Value | SymbolFlags.Type | SymbolFlags.Namespace, Diagnostics.Cannot_find_name_0, node);
Expand Down Expand Up @@ -2014,6 +2016,38 @@ module ts {
}
}

// Push an entry on the type resolution stack. If an entry with the given target is not already on the stack,
// a new entry with that target and an associated result value of true is pushed on the stack, and the value
// true is returned. Otherwise, a circularity has occurred and the result values of the existing entry and
// all entries pushed after it are changed to false, and the value false is returned. The target object provides
// a unique identity for a particular type resolution result: Symbol instances are used to track resolution of
// SymbolLinks.type, SymbolLinks instances are used to track resolution of SymbolLinks.declaredType, and
// Signature instances are used to track resolution of Signature.resolvedReturnType.
function pushTypeResolution(target: Object): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

I realize the target object only matters for identity of the result. But I'd rather have one type that gets pushed onto the stack. I noticed below, the things that get pushed can be of type Symbol, SymbolLinks or Signature. I would really prefer to converge on one type if possible, just to keep things organized.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, it seems very easy to pass in the same object from two different callers by mistake, even when tracking different properties. I wish we could pass a stronger proxy for the property, rather than passing a different owner object in a way that is unrelated to which property we want to track.

let i = 0;
let count = resolutionTargets.length;
while (i < count && resolutionTargets[i] !== target) {
i++;
}
if (i < count) {
do {
resolutionResults[i++] = false;
}
while (i < count);
return false;
}
resolutionTargets.push(target);
resolutionResults.push(true);
return true;
}

// Pop an entry from the type resolution stack and return its associated result value. The result value will
// be true if no circularities were detected, or false if a circularity was found.
function popTypeResolution(): boolean {
resolutionTargets.pop();
return resolutionResults.pop();
}

function getRootDeclaration(node: Node): Node {
while (node.kind === SyntaxKind.BindingElement) {
node = node.parent.parent;
Expand Down Expand Up @@ -2271,20 +2305,27 @@ module ts {
return links.type = checkExpression((<ExportAssignment>declaration).expression);
}
// Handle variable, parameter or property
links.type = resolvingType;
let type = getWidenedTypeForVariableLikeDeclaration(<VariableLikeDeclaration>declaration, /*reportErrors*/ true);
if (links.type === resolvingType) {
links.type = type;
if (!pushTypeResolution(symbol)) {
return unknownType;
}
}
else if (links.type === resolvingType) {
links.type = anyType;
if (compilerOptions.noImplicitAny) {
let diagnostic = (<VariableLikeDeclaration>symbol.valueDeclaration).type ?
Diagnostics._0_implicitly_has_type_any_because_it_is_referenced_directly_or_indirectly_in_its_own_type_annotation :
Diagnostics._0_implicitly_has_type_any_because_it_is_does_not_have_a_type_annotation_and_is_referenced_directly_or_indirectly_in_its_own_initializer;
error(symbol.valueDeclaration, diagnostic, symbolToString(symbol));
let type = getWidenedTypeForVariableLikeDeclaration(<VariableLikeDeclaration>declaration, /*reportErrors*/ true);
if (!popTypeResolution()) {
if ((<VariableLikeDeclaration>symbol.valueDeclaration).type) {
// Variable has type annotation that circularly references the variable itself
type = unknownType;
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for cleaning that up

error(symbol.valueDeclaration, Diagnostics._0_is_referenced_directly_or_indirectly_in_its_own_type_annotation,
symbolToString(symbol));
}
else {
// Variable has initializer that circularly references the variable itself
type = anyType;
if (compilerOptions.noImplicitAny) {
error(symbol.valueDeclaration, Diagnostics._0_implicitly_has_type_any_because_it_is_does_not_have_a_type_annotation_and_is_referenced_directly_or_indirectly_in_its_own_initializer,
symbolToString(symbol));
}
}
}
links.type = type;
}
return links.type;
}
Expand All @@ -2308,19 +2349,13 @@ module ts {

function getTypeOfAccessors(symbol: Symbol): Type {
let links = getSymbolLinks(symbol);
checkAndStoreTypeOfAccessors(symbol, links);
return links.type;
}

function checkAndStoreTypeOfAccessors(symbol: Symbol, links?: SymbolLinks) {
links = links || getSymbolLinks(symbol);
if (!links.type) {
links.type = resolvingType;
if (!pushTypeResolution(symbol)) {
return unknownType;
}
let getter = <AccessorDeclaration>getDeclarationOfKind(symbol, SyntaxKind.GetAccessor);
let setter = <AccessorDeclaration>getDeclarationOfKind(symbol, SyntaxKind.SetAccessor);

let type: Type;

// First try to see if the user specified a return type on the get-accessor.
let getterReturnType = getAnnotatedAccessorType(getter);
if (getterReturnType) {
Expand All @@ -2342,23 +2377,20 @@ module ts {
if (compilerOptions.noImplicitAny) {
error(setter, Diagnostics.Property_0_implicitly_has_type_any_because_its_set_accessor_lacks_a_type_annotation, symbolToString(symbol));
}

type = anyType;
}
}
}

if (links.type === resolvingType) {
links.type = type;
}
}
else if (links.type === resolvingType) {
links.type = anyType;
if (compilerOptions.noImplicitAny) {
let getter = <AccessorDeclaration>getDeclarationOfKind(symbol, SyntaxKind.GetAccessor);
error(getter, Diagnostics._0_implicitly_has_return_type_any_because_it_does_not_have_a_return_type_annotation_and_is_referenced_directly_or_indirectly_in_one_of_its_return_expressions, symbolToString(symbol));
if (!popTypeResolution()) {
type = anyType;
if (compilerOptions.noImplicitAny) {
let getter = <AccessorDeclaration>getDeclarationOfKind(symbol, SyntaxKind.GetAccessor);
error(getter, Diagnostics._0_implicitly_has_return_type_any_because_it_does_not_have_a_return_type_annotation_and_is_referenced_directly_or_indirectly_in_one_of_its_return_expressions, symbolToString(symbol));
}
}
links.type = type;
}
return links.type;
}

function getTypeOfFuncClassEnumModule(symbol: Symbol): Type {
Expand Down Expand Up @@ -2451,7 +2483,7 @@ module ts {
return result;
}

function getBaseTypes(type: InterfaceType): ObjectType[]{
function getBaseTypes(type: InterfaceType): ObjectType[] {
let typeWithBaseTypes = <InterfaceTypeWithBaseTypes>type;
if (!typeWithBaseTypes.baseTypes) {
if (type.symbol.flags & SymbolFlags.Class) {
Expand Down Expand Up @@ -2536,17 +2568,18 @@ module ts {
function getDeclaredTypeOfTypeAlias(symbol: Symbol): Type {
let links = getSymbolLinks(symbol);
if (!links.declaredType) {
links.declaredType = resolvingType;
// Note that we use the links object as the target here because the symbol object is used as the unique
// identity for resolution of the 'type' property in SymbolLinks.
if (!pushTypeResolution(links)) {
return unknownType;
}
let declaration = <TypeAliasDeclaration>getDeclarationOfKind(symbol, SyntaxKind.TypeAliasDeclaration);
let type = getTypeFromTypeNode(declaration.type);
if (links.declaredType === resolvingType) {
links.declaredType = type;
if (!popTypeResolution()) {
type = unknownType;
error(declaration.name, Diagnostics.Type_alias_0_circularly_references_itself, symbolToString(symbol));
}
}
else if (links.declaredType === resolvingType) {
links.declaredType = unknownType;
let declaration = <TypeAliasDeclaration>getDeclarationOfKind(symbol, SyntaxKind.TypeAliasDeclaration);
error(declaration.name, Diagnostics.Type_alias_0_circularly_references_itself, symbolToString(symbol));
links.declaredType = type;
}
return links.declaredType;
}
Expand Down Expand Up @@ -3150,7 +3183,9 @@ module ts {

function getReturnTypeOfSignature(signature: Signature): Type {
if (!signature.resolvedReturnType) {
signature.resolvedReturnType = resolvingType;
if (!pushTypeResolution(signature)) {
return unknownType;
}
let type: Type;
if (signature.target) {
type = instantiateType(getReturnTypeOfSignature(signature.target), signature.mapper);
Expand All @@ -3161,21 +3196,19 @@ module ts {
else {
type = getReturnTypeFromBody(<FunctionLikeDeclaration>signature.declaration);
}
if (signature.resolvedReturnType === resolvingType) {
signature.resolvedReturnType = type;
}
}
else if (signature.resolvedReturnType === resolvingType) {
signature.resolvedReturnType = anyType;
if (compilerOptions.noImplicitAny) {
let declaration = <Declaration>signature.declaration;
if (declaration.name) {
error(declaration.name, Diagnostics._0_implicitly_has_return_type_any_because_it_does_not_have_a_return_type_annotation_and_is_referenced_directly_or_indirectly_in_one_of_its_return_expressions, declarationNameToString(declaration.name));
}
else {
error(declaration, Diagnostics.Function_implicitly_has_return_type_any_because_it_does_not_have_a_return_type_annotation_and_is_referenced_directly_or_indirectly_in_one_of_its_return_expressions);
if (!popTypeResolution()) {
type = anyType;
if (compilerOptions.noImplicitAny) {
let declaration = <Declaration>signature.declaration;
if (declaration.name) {
error(declaration.name, Diagnostics._0_implicitly_has_return_type_any_because_it_does_not_have_a_return_type_annotation_and_is_referenced_directly_or_indirectly_in_one_of_its_return_expressions, declarationNameToString(declaration.name));
}
else {
error(declaration, Diagnostics.Function_implicitly_has_return_type_any_because_it_does_not_have_a_return_type_annotation_and_is_referenced_directly_or_indirectly_in_one_of_its_return_expressions);
}
}
}
signature.resolvedReturnType = type;
}
return signature.resolvedReturnType;
}
Expand Down Expand Up @@ -7342,10 +7375,9 @@ module ts {
if (isContextSensitive(node)) {
assignContextualParameterTypes(signature, contextualSignature, contextualMapper || identityMapper);
}
if (!node.type) {
signature.resolvedReturnType = resolvingType;
if (!node.type && !signature.resolvedReturnType) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why doesn't this one have to change to the new model?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is already guarded by the ContextChecked flag and will never be re-entered.

let returnType = getReturnTypeFromBody(node, contextualMapper);
if (signature.resolvedReturnType === resolvingType) {
if (!signature.resolvedReturnType) {
signature.resolvedReturnType = returnType;
}
}
Expand Down Expand Up @@ -8359,8 +8391,7 @@ module ts {
}
}
}

checkAndStoreTypeOfAccessors(getSymbolOfNode(node));
getTypeOfAccessors(getSymbolOfNode(node));
}

checkFunctionLikeDeclaration(node);
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/diagnosticInformationMap.generated.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ module ts {
An_interface_can_only_extend_an_identifier_Slashqualified_name_with_optional_type_arguments: { code: 2499, category: DiagnosticCategory.Error, key: "An interface can only extend an identifier/qualified-name with optional type arguments." },
A_class_can_only_implement_an_identifier_Slashqualified_name_with_optional_type_arguments: { code: 2500, category: DiagnosticCategory.Error, key: "A class can only implement an identifier/qualified-name with optional type arguments." },
A_rest_element_cannot_contain_a_binding_pattern: { code: 2501, category: DiagnosticCategory.Error, key: "A rest element cannot contain a binding pattern." },
_0_is_referenced_directly_or_indirectly_in_its_own_type_annotation: { code: 2502, category: DiagnosticCategory.Error, key: "'{0}' is referenced directly or indirectly in its own type annotation." },
Import_declaration_0_is_using_private_name_1: { code: 4000, category: DiagnosticCategory.Error, key: "Import declaration '{0}' is using private name '{1}'." },
Type_parameter_0_of_exported_class_has_or_is_using_private_name_1: { code: 4002, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported class has or is using private name '{1}'." },
Type_parameter_0_of_exported_interface_has_or_is_using_private_name_1: { code: 4004, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported interface has or is using private name '{1}'." },
Expand Down Expand Up @@ -514,7 +515,6 @@ module ts {
Object_literal_s_property_0_implicitly_has_an_1_type: { code: 7018, category: DiagnosticCategory.Error, key: "Object literal's property '{0}' implicitly has an '{1}' type." },
Rest_parameter_0_implicitly_has_an_any_type: { code: 7019, category: DiagnosticCategory.Error, key: "Rest parameter '{0}' implicitly has an 'any[]' type." },
Call_signature_which_lacks_return_type_annotation_implicitly_has_an_any_return_type: { code: 7020, category: DiagnosticCategory.Error, key: "Call signature, which lacks return-type annotation, implicitly has an 'any' return type." },
_0_implicitly_has_type_any_because_it_is_referenced_directly_or_indirectly_in_its_own_type_annotation: { code: 7021, category: DiagnosticCategory.Error, key: "'{0}' implicitly has type 'any' because it is referenced directly or indirectly in its own type annotation." },
_0_implicitly_has_type_any_because_it_is_does_not_have_a_type_annotation_and_is_referenced_directly_or_indirectly_in_its_own_initializer: { code: 7022, category: DiagnosticCategory.Error, key: "'{0}' implicitly has type 'any' because it is does not have a type annotation and is referenced directly or indirectly in its own initializer." },
_0_implicitly_has_return_type_any_because_it_does_not_have_a_return_type_annotation_and_is_referenced_directly_or_indirectly_in_one_of_its_return_expressions: { code: 7023, category: DiagnosticCategory.Error, key: "'{0}' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions." },
Function_implicitly_has_return_type_any_because_it_does_not_have_a_return_type_annotation_and_is_referenced_directly_or_indirectly_in_one_of_its_return_expressions: { code: 7024, category: DiagnosticCategory.Error, key: "Function implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions." },
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1440,6 +1440,10 @@
"category": "Error",
"code": 2501
},
"'{0}' is referenced directly or indirectly in its own type annotation.": {
"category": "Error",
"code": 2502
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down Expand Up @@ -2048,10 +2052,6 @@
"category": "Error",
"code": 7020
},
"'{0}' implicitly has type 'any' because it is referenced directly or indirectly in its own type annotation.": {
"category": "Error",
"code": 7021
},
"'{0}' implicitly has type 'any' because it is does not have a type annotation and is referenced directly or indirectly in its own initializer.": {
"category": "Error",
"code": 7022
Expand Down
Loading