Skip to content

Report errors for usage of private types when generating declaration file #161

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 6 commits into from
Aug 6, 2014
Merged

Report errors for usage of private types when generating declaration file #161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bbb36dc
Make the symbol writing api on the text writer
sheetalkamat Jul 18, 2014
999b7fe
Checker and emitter changes to report errors on inaccessibility of sy…
sheetalkamat Jul 19, 2014
09ec1bb
Report error on Class/Interface heritage clause if it cant be accessed
sheetalkamat Jul 19, 2014
9fd95fc
Report errors if the type parameter uses constraint that is using pri…
sheetalkamat Jul 19, 2014
0e76a82
Fix the incorrect error reporting and declaration file emit for the t…
sheetalkamat Aug 4, 2014
e31aa9e
Added more comments and todos as per code review feedback
sheetalkamat Aug 6, 2014
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
73 changes: 63 additions & 10 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ module ts {
}

// If symbol is directly available by its name in the symbol table
if (isAccessible(symbols[symbol.name])) {
if (hasProperty(symbols, symbol.name) && isAccessible(symbols[symbol.name])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I have added a lookup method for Map, it would be more consistent.

Copy link
Member Author

Choose a reason for hiding this comment

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

i will update that when I resolve merge conflicts.

return symbol;
}

Expand All @@ -668,7 +668,7 @@ module ts {
var qualify = false;
forEachSymbolTableInScope(enclosingDeclaration, symbolTable => {
// If symbol of this name is not available in the symbol table we are ok
if (!symbolTable[symbol.name]) {
if (!hasProperty(symbolTable, symbol.name)) {
// Continue to the next symbol table
return false;
}
Expand All @@ -693,6 +693,52 @@ module ts {
return qualify
}

function isSymbolAccessible(symbol: Symbol, enclosingDeclaration: Node, meaning: SymbolFlags): SymbolAccessiblityResult {
if (symbol && enclosingDeclaration && !(symbol.flags & SymbolFlags.TypeParameter)) {
var initialSymbol = symbol;
var meaningToLook = meaning;
while (symbol) {
// Symbol is accessible if it by itself is accessible
var accessibleSymbol = getAccessibleSymbol(symbol, enclosingDeclaration, meaningToLook);
if (accessibleSymbol) {
Copy link
Contributor

Choose a reason for hiding this comment

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

question: If we could not find a symbol at this location, should we just break? what is the point of examining the parent if this link in the chain is not accessible?

Copy link
Member Author

Choose a reason for hiding this comment

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

module m {
export class c {
}
}

var x: typeof m.c

In this example when we look if the symbol constructor function m.c is accessible, we will start with the symbol for c. Since c is not directly in the scope - the answer here that is accessible symbol is going to be undefined. But that doesn't mean m.c is not accessible. It is accessible if the parent m is accessible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Note I added the detailed comment

if (forEach(accessibleSymbol.declarations, declaration => !isDeclarationVisible(declaration))) {
return {
accessibility: SymbolAccessibility.NotAccessible,
errorSymbolName: symbolToString(initialSymbol, enclosingDeclaration, meaning),
errorModuleName: symbol !== initialSymbol ? symbolToString(symbol, enclosingDeclaration, SymbolFlags.Namespace) : undefined
};
}
return { accessibility: SymbolAccessibility.Accessible };
}

// TODO(shkamat): Handle static method of class

// If we havent got the accessible symbol doesnt mean the symbol is actually inaccessible.
// It could be qualified symbol and hence verify the path
// eg:
// module m {
// export class c {
// }
// }
// var x: typeof m.c
// In the above example when we start with checking if typeof m.c symbol is accessible,
// we are going to see if c can be accessed in scope directly.
// But it cant, hence the accessible is going to be undefined, but that doesnt mean m.c is accessible
// It is accessible if the parent m is accessible because then m.c can be accessed through qualification
meaningToLook = SymbolFlags.Namespace;
Copy link
Contributor

Choose a reason for hiding this comment

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

question: what if we were looking for a static method on a class, the parent is not a module in this case.. how would that wok?

Copy link
Member Author

Choose a reason for hiding this comment

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

i still have todo on that, i will add a todo comment here specifically.

symbol = symbol.parent;
}

// This is a local symbol that cannot be named
return {
accessibility: SymbolAccessibility.CannotBeNamed,
errorSymbolName: symbolToString(initialSymbol, enclosingDeclaration, meaning),
};
}

return { accessibility: SymbolAccessibility.Accessible };
}

// Enclosing declaration is optional when we dont want to get qualified name in the enclosing declaration scope
// Meaning needs to be specified if the enclosing declaration is given
function symbolToString(symbol: Symbol, enclosingDeclaration?: Node, meaning?: SymbolFlags) {
Expand Down Expand Up @@ -728,10 +774,15 @@ module ts {
return getSymbolName(symbol);
}

function writeSymbolToTextWriter(symbol: Symbol, enclosingDeclaration: Node, meaning: SymbolFlags, writer: TextWriter) {
writer.write(symbolToString(symbol, enclosingDeclaration, meaning));
}

function createSingleLineTextWriter() {
var result = "";
return {
write(s: string) { result += s; },
writeSymbol(symbol: Symbol, enclosingDeclaration?: Node, meaning?: SymbolFlags) { writeSymbolToTextWriter(symbol, enclosingDeclaration, meaning, this); },
writeLine() { result += " "; },
increaseIndent() { },
decreaseIndent() { },
Expand All @@ -758,7 +809,7 @@ module ts {
writeTypeReference(<TypeReference>type);
}
else if (type.flags & (TypeFlags.Class | TypeFlags.Interface | TypeFlags.Enum | TypeFlags.TypeParameter)) {
writer.write(symbolToString(type.symbol, enclosingDeclaration, SymbolFlags.Type));
writer.writeSymbol(type.symbol, enclosingDeclaration, SymbolFlags.Type);
}
else if (type.flags & TypeFlags.Anonymous) {
writeAnonymousType(<ObjectType>type, allowFunctionOrConstructorTypeLiteral);
Expand All @@ -780,7 +831,7 @@ module ts {
writer.write("[]");
}
else {
writer.write(symbolToString(type.target.symbol, enclosingDeclaration, SymbolFlags.Type));
writer.writeSymbol(type.target.symbol, enclosingDeclaration, SymbolFlags.Type);
writer.write("<");
for (var i = 0; i < type.typeArguments.length; i++) {
if (i > 0) {
Expand Down Expand Up @@ -814,7 +865,7 @@ module ts {

function writeTypeofSymbol(type: ObjectType) {
writer.write("typeof ");
writer.write(symbolToString(type.symbol, enclosingDeclaration, SymbolFlags.Value));
writer.writeSymbol(type.symbol, enclosingDeclaration, SymbolFlags.Value);
}

function writeLiteralType(type: ObjectType, allowFunctionOrConstructorTypeLiteral: boolean) {
Expand Down Expand Up @@ -870,7 +921,7 @@ module ts {
if (p.flags & (SymbolFlags.Function | SymbolFlags.Method) && !getPropertiesOfType(t).length) {
var signatures = getSignaturesOfType(t, SignatureKind.Call);
for (var j = 0; j < signatures.length; j++) {
writer.write(symbolToString(p));
writer.writeSymbol(p);
if (isOptionalProperty(p)) {
writer.write("?");
}
Expand All @@ -880,7 +931,7 @@ module ts {
}
}
else {
writer.write(symbolToString(p));
writer.writeSymbol(p);
if (isOptionalProperty(p)) {
writer.write("?");
}
Expand All @@ -902,7 +953,7 @@ module ts {
writer.write(", ");
}
var tp = signature.typeParameters[i];
writer.write(symbolToString(tp.symbol));
writer.writeSymbol(tp.symbol);
var constraint = getConstraintOfTypeParameter(tp);
if (constraint) {
writer.write(" extends ");
Expand All @@ -920,7 +971,7 @@ module ts {
if (getDeclarationFlagsFromSymbol(p) & NodeFlags.Rest) {
writer.write("...");
}
writer.write(symbolToString(p));
writer.writeSymbol(p);
if (p.valueDeclaration.flags & NodeFlags.QuestionMark || (<VariableDeclaration>p.valueDeclaration).initializer) {
writer.write("?");
}
Expand Down Expand Up @@ -6105,7 +6156,9 @@ module ts {
isDeclarationVisible: isDeclarationVisible,
isImplementationOfOverload: isImplementationOfOverload,
writeTypeAtLocation: writeTypeAtLocation,
writeReturnTypeOfSignatureDeclaration: writeReturnTypeOfSignatureDeclaration
writeReturnTypeOfSignatureDeclaration: writeReturnTypeOfSignatureDeclaration,
writeSymbol: writeSymbolToTextWriter,
isSymbolAccessible: isSymbolAccessible
};
checkProgram();
return emitFiles(resolver);
Expand Down
22 changes: 22 additions & 0 deletions src/compiler/diagnosticInformationMap.generated.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,28 @@ module ts {
A_constructor_implementation_cannot_be_declared_in_an_ambient_context: { code: 1111, category: DiagnosticCategory.Error, key: "A constructor implementation cannot be declared in an ambient context." },
A_class_member_cannot_be_declared_optional: { code: 1112, category: DiagnosticCategory.Error, key: "A class member cannot be declared optional." },
Duplicate_identifier_0: { code: 2000, category: DiagnosticCategory.Error, key: "Duplicate identifier '{0}'." },
Extends_clause_of_exported_class_0_has_or_is_using_private_name_1: { code: 2018, category: DiagnosticCategory.Error, key: "Extends clause of exported class '{0}' has or is using private name '{1}'." },
Implements_clause_of_exported_class_0_has_or_is_using_private_name_1: { code: 2019, category: DiagnosticCategory.Error, key: "Implements clause of exported class '{0}' has or is using private name '{1}'." },
Extends_clause_of_exported_interface_0_has_or_is_using_private_name_1: { code: 2020, category: DiagnosticCategory.Error, key: "Extends clause of exported interface '{0}' has or is using private name '{1}'." },
Extends_clause_of_exported_class_0_has_or_is_using_name_1_from_private_module_2: { code: 2021, category: DiagnosticCategory.Error, key: "Extends clause of exported class '{0}' has or is using name '{1}' from private module '{2}'." },
Implements_clause_of_exported_class_0_has_or_is_using_name_1_from_private_module_2: { code: 2022, category: DiagnosticCategory.Error, key: "Implements clause of exported class '{0}' has or is using name '{1}' from private module '{2}'." },
Extends_clause_of_exported_interface_0_has_or_is_using_name_1_from_private_module_2: { code: 2023, category: DiagnosticCategory.Error, key: "Extends clause of exported interface '{0}' has or is using name '{1}' from private module '{2}'." },
Type_parameter_0_of_constructor_signature_from_exported_interface_has_or_is_using_private_name_1: { code: 2208, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of constructor signature from exported interface has or is using private name '{1}'." },
Type_parameter_0_of_call_signature_from_exported_interface_has_or_is_using_private_name_1: { code: 2209, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of call signature from exported interface has or is using private name '{1}'." },
Type_parameter_0_of_public_static_method_from_exported_class_has_or_is_using_private_name_1: { code: 2210, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of public static method from exported class has or is using private name '{1}'." },
Type_parameter_0_of_public_method_from_exported_class_has_or_is_using_private_name_1: { code: 2211, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of public method from exported class has or is using private name '{1}'." },
Type_parameter_0_of_method_from_exported_interface_has_or_is_using_private_name_1: { code: 2212, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of method from exported interface has or is using private name '{1}'." },
Type_parameter_0_of_exported_function_has_or_is_using_private_name_1: { code: 2213, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported function has or is using private name '{1}'." },
Type_parameter_0_of_constructor_signature_from_exported_interface_has_or_is_using_name_1_from_private_module_2: { code: 2214, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of constructor signature from exported interface has or is using name '{1}' from private module '{2}'." },
Type_parameter_0_of_call_signature_from_exported_interface_has_or_is_using_name_1_from_private_module_2: { code: 2215, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of call signature from exported interface has or is using name '{1}' from private module '{2}'." },
Type_parameter_0_of_public_static_method_from_exported_class_has_or_is_using_name_1_from_private_module_2: { code: 2216, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of public static method from exported class has or is using name '{1}' from private module '{2}'." },
Type_parameter_0_of_public_method_from_exported_class_has_or_is_using_name_1_from_private_module_2: { code: 2217, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of public method from exported class has or is using name '{1}' from private module '{2}'." },
Type_parameter_0_of_method_from_exported_interface_has_or_is_using_name_1_from_private_module_2: { code: 2218, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of method from exported interface has or is using name '{1}' from private module '{2}'." },
Type_parameter_0_of_exported_function_has_or_is_using_name_1_from_private_module_2: { code: 2219, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported function has or is using name '{1}' from private module '{2}'." },
Type_parameter_0_of_exported_class_has_or_is_using_private_name_1: { code: 2220, 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: 2221, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported interface has or is using private name '{1}'." },
Type_parameter_0_of_exported_class_has_or_is_using_name_1_from_private_module_2: { code: 2222, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported class has or is using name '{1}' from private module '{2}'." },
Type_parameter_0_of_exported_interface_has_or_is_using_name_1_from_private_module_2: { code: 2223, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported interface has or is using name '{1}' from private module '{2}'." },
new_T_cannot_be_used_to_create_an_array_Use_new_Array_T_instead: { code: 2068, category: DiagnosticCategory.Error, key: "'new T[]' cannot be used to create an array. Use 'new Array<T>()' instead." },
Multiple_constructor_implementations_are_not_allowed: { code: 2070, category: DiagnosticCategory.Error, key: "Multiple constructor implementations are not allowed." },
A_class_may_only_implement_another_class_or_interface: { code: 2074, category: DiagnosticCategory.Error, key: "A class may only implement another class or interface." },
Expand Down
88 changes: 88 additions & 0 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,94 @@
"category": "Error",
"code": 2000
},
"Extends clause of exported class '{0}' has or is using private name '{1}'.": {
"category": "Error",
"code": 2018
},
"Implements clause of exported class '{0}' has or is using private name '{1}'.": {
"category": "Error",
"code": 2019
},
"Extends clause of exported interface '{0}' has or is using private name '{1}'.": {
"category": "Error",
"code": 2020
},
"Extends clause of exported class '{0}' has or is using name '{1}' from private module '{2}'.": {
"category": "Error",
"code": 2021
},
"Implements clause of exported class '{0}' has or is using name '{1}' from private module '{2}'.": {
"category": "Error",
"code": 2022
},
"Extends clause of exported interface '{0}' has or is using name '{1}' from private module '{2}'.": {
"category": "Error",
"code": 2023
},
"Type parameter '{0}' of constructor signature from exported interface has or is using private name '{1}'.": {
"category": "Error",
"code": 2208
},
"Type parameter '{0}' of call signature from exported interface has or is using private name '{1}'.": {
"category": "Error",
"code": 2209
},
"Type parameter '{0}' of public static method from exported class has or is using private name '{1}'.": {
"category": "Error",
"code": 2210
},
"Type parameter '{0}' of public method from exported class has or is using private name '{1}'.": {
"category": "Error",
"code": 2211
},
"Type parameter '{0}' of method from exported interface has or is using private name '{1}'.": {
"category": "Error",
"code": 2212
},
"Type parameter '{0}' of exported function has or is using private name '{1}'.": {
"category": "Error",
"code": 2213
},
"Type parameter '{0}' of constructor signature from exported interface has or is using name '{1}' from private module '{2}'.": {
"category": "Error",
"code": 2214
},
"Type parameter '{0}' of call signature from exported interface has or is using name '{1}' from private module '{2}'.": {
"category": "Error",
"code": 2215
},
"Type parameter '{0}' of public static method from exported class has or is using name '{1}' from private module '{2}'.": {
"category": "Error",
"code": 2216
},
"Type parameter '{0}' of public method from exported class has or is using name '{1}' from private module '{2}'.": {
"category": "Error",
"code": 2217
},
"Type parameter '{0}' of method from exported interface has or is using name '{1}' from private module '{2}'.": {
"category": "Error",
"code": 2218
},
"Type parameter '{0}' of exported function has or is using name '{1}' from private module '{2}'.": {
"category": "Error",
"code": 2219
},
"Type parameter '{0}' of exported class has or is using private name '{1}'.": {
"category": "Error",
"code": 2220
},
"Type parameter '{0}' of exported interface has or is using private name '{1}'.": {
"category": "Error",
"code": 2221
},
"Type parameter '{0}' of exported class has or is using name '{1}' from private module '{2}'.": {
"category": "Error",
"code": 2222
},
"Type parameter '{0}' of exported interface has or is using name '{1}' from private module '{2}'.": {
"category": "Error",
"code": 2223
},
"'new T[]' cannot be used to create an array. Use 'new Array<T>()' instead.": {
"category": "Error",
"code": 2068
Expand Down
Loading