Skip to content

Commit

Permalink
Improved error messages for private and protected member access
Browse files Browse the repository at this point in the history
  • Loading branch information
@ahejlsberg
ahejlsberg committed Sep 19, 2014
1 parent 08188b0 commit c990c42
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 23 deletions.
42 changes: 23 additions & 19 deletions src/compiler/checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2972,7 +2972,7 @@ module ts {
var targetClass = <InterfaceType>getDeclaredTypeOfSymbol(targetProp.parent);
if (!sourceClass || !hasBaseType(sourceClass, targetClass)) {
if (reportErrors) {
reportError(Diagnostics.Property_0_is_protected_but_type_1_is_not_derived_from_type_2,
reportError(Diagnostics.Property_0_is_protected_but_type_1_is_not_a_class_derived_from_2,
symbolToString(targetProp), typeToString(sourceClass || source), typeToString(targetClass));
}
return false;
Expand Down Expand Up @@ -4012,38 +4012,42 @@ module ts {
return s.valueDeclaration ? s.valueDeclaration.flags : s.flags & SymbolFlags.Prototype ? NodeFlags.Public | NodeFlags.Static : 0;
}

function isClassPropertyAccessible(node: PropertyAccess, type: Type, prop: Symbol): boolean {
function checkClassPropertyAccess(node: PropertyAccess, type: Type, prop: Symbol) {
var flags = getDeclarationFlagsFromSymbol(prop);
// Public properties are always accessible
if (!(flags & (NodeFlags.Private | NodeFlags.Protected))) {
return true;
return;
}
// Property is known to be private or protected at this point
// Private and protected properties are never accessible outside a class declaration
var enclosingClassDeclaration = getAncestor(node, SyntaxKind.ClassDeclaration);
if (!enclosingClassDeclaration) {
return false;
}
// Get the declaring and enclosing class instance types
var enclosingClassDeclaration = getAncestor(node, SyntaxKind.ClassDeclaration);
var enclosingClass = enclosingClassDeclaration ? <InterfaceType>getDeclaredTypeOfSymbol(getSymbolOfNode(enclosingClassDeclaration)) : undefined;
var declaringClass = <InterfaceType>getDeclaredTypeOfSymbol(prop.parent);
var enclosingClass = <InterfaceType>getDeclaredTypeOfSymbol(getSymbolOfNode(enclosingClassDeclaration));
// Private property is accessible if declaring and enclosing class are the same
if (flags & NodeFlags.Private) {
return declaringClass === enclosingClass;
if (declaringClass !== enclosingClass) {
error(node, Diagnostics.Property_0_is_private_and_only_accessible_within_class_1, symbolToString(prop), typeToString(declaringClass));
}
return;
}
// Property is known to be protected at this point
// All protected properties of a supertype are accessible in a super access
if (node.left.kind === SyntaxKind.SuperKeyword) {
return true;
return;
}
// A protected property is accessible in the declaring class and classes derived from it
if (!enclosingClass || !hasBaseType(enclosingClass, declaringClass)) {
error(node, Diagnostics.Property_0_is_protected_and_only_accessible_within_class_1_and_its_subclasses, symbolToString(prop), typeToString(declaringClass));
return;
}
// No further restrictions for static properties
if (flags & NodeFlags.Static) {
return;
}
// An instance property must be accessed through an instance of the enclosing class
if (!(flags & NodeFlags.Static)) {
if (!(getTargetType(type).flags & (TypeFlags.Class | TypeFlags.Interface) && hasBaseType(<InterfaceType>type, enclosingClass))) {
return false;
}
if (!(getTargetType(type).flags & (TypeFlags.Class | TypeFlags.Interface) && hasBaseType(<InterfaceType>type, enclosingClass))) {
error(node, Diagnostics.Property_0_is_protected_and_only_accessible_through_an_instance_of_class_1, symbolToString(prop), typeToString(enclosingClass));
}
// A protected property is accessible in the declaring class and classes derived from it
return hasBaseType(enclosingClass, declaringClass);
}

function checkPropertyAccess(node: PropertyAccess) {
Expand Down Expand Up @@ -4074,8 +4078,8 @@ module ts {
if (node.left.kind === SyntaxKind.SuperKeyword && getDeclarationKindFromSymbol(prop) !== SyntaxKind.Method) {
error(node.right, Diagnostics.Only_public_and_protected_methods_of_the_base_class_are_accessible_via_the_super_keyword);
}
else if (!isClassPropertyAccessible(node, type, prop)) {
error(node, Diagnostics.Property_0_is_inaccessible, getFullyQualifiedName(prop));
else {
checkClassPropertyAccess(node, type, prop);
}
}
return getTypeOfSymbol(prop);
Expand Down
6 changes: 4 additions & 2 deletions src/compiler/diagnosticInformationMap.generated.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ module ts {
super_property_access_is_permitted_only_in_a_constructor_member_function_or_member_accessor_of_a_derived_class: { code: 2338, category: DiagnosticCategory.Error, key: "'super' property access is permitted only in a constructor, member function, or member accessor of a derived class" },
Property_0_does_not_exist_on_type_1: { code: 2339, category: DiagnosticCategory.Error, key: "Property '{0}' does not exist on type '{1}'." },
Only_public_and_protected_methods_of_the_base_class_are_accessible_via_the_super_keyword: { code: 2340, category: DiagnosticCategory.Error, key: "Only public and protected methods of the base class are accessible via the 'super' keyword" },
Property_0_is_inaccessible: { code: 2341, category: DiagnosticCategory.Error, key: "Property '{0}' is inaccessible." },
Property_0_is_private_and_only_accessible_within_class_1: { code: 2341, category: DiagnosticCategory.Error, key: "Property '{0}' is private and only accessible within class '{1}'." },
An_index_expression_argument_must_be_of_type_string_number_or_any: { code: 2342, category: DiagnosticCategory.Error, key: "An index expression argument must be of type 'string', 'number', or 'any'." },
Type_0_does_not_satisfy_the_constraint_1_Colon: { code: 2343, category: DiagnosticCategory.Error, key: "Type '{0}' does not satisfy the constraint '{1}':" },
Type_0_does_not_satisfy_the_constraint_1: { code: 2344, category: DiagnosticCategory.Error, key: "Type '{0}' does not satisfy the constraint '{1}'." },
Expand Down Expand Up @@ -256,8 +256,10 @@ module ts {
Import_declaration_conflicts_with_local_declaration_of_0: { code: 2440, category: DiagnosticCategory.Error, key: "Import declaration conflicts with local declaration of '{0}'" },
Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_an_external_module: { code: 2441, category: DiagnosticCategory.Error, key: "Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of an external module." },
Types_have_separate_declarations_of_a_private_property_0: { code: 2442, category: DiagnosticCategory.Error, key: "Types have separate declarations of a private property '{0}'." },
Property_0_is_protected_but_type_1_is_not_derived_from_type_2: { code: 2443, category: DiagnosticCategory.Error, key: "Property '{0}' is protected but type '{1}' is not derived from type '{2}'." },
Property_0_is_protected_but_type_1_is_not_a_class_derived_from_2: { code: 2443, category: DiagnosticCategory.Error, key: "Property '{0}' is protected but type '{1}' is not a class derived from '{2}'." },
Property_0_is_protected_in_type_1_but_public_in_type_2: { code: 2444, category: DiagnosticCategory.Error, key: "Property '{0}' is protected in type '{1}' but public in type '{2}'." },
Property_0_is_protected_and_only_accessible_within_class_1_and_its_subclasses: { code: 2445, category: DiagnosticCategory.Error, key: "Property '{0}' is protected and only accessible within class '{1}' and its subclasses." },
Property_0_is_protected_and_only_accessible_through_an_instance_of_class_1: { code: 2446, category: DiagnosticCategory.Error, key: "Property '{0}' is protected and only accessible through an instance of class '{1}'." },
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_name_1_from_private_module_2: { code: 4001, 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_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}'." },
Expand Down
12 changes: 10 additions & 2 deletions src/compiler/diagnosticMessages.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@
"category": "Error",
"code": 2340
},
"Property '{0}' is inaccessible.": {
"Property '{0}' is private and only accessible within class '{1}'.": {
"category": "Error",
"code": 2341
},
Expand Down Expand Up @@ -1016,14 +1016,22 @@
"category": "Error",
"code": 2442
},
"Property '{0}' is protected but type '{1}' is not a class derived from type '{2}'.": {
"Property '{0}' is protected but type '{1}' is not a class derived from '{2}'.": {
"category": "Error",
"code": 2443
},
"Property '{0}' is protected in type '{1}' but public in type '{2}'.": {
"category": "Error",
"code": 2444
},
"Property '{0}' is protected and only accessible within class '{1}' and its subclasses.": {
"category": "Error",
"code": 2445
},
"Property '{0}' is protected and only accessible through an instance of class '{1}'.": {
"category": "Error",
"code": 2446
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down

1 comment on commit c990c42

@mhegazy
Copy link
Contributor

Choose a reason for hiding this comment

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

Error messages are much better. thanks!

Please sign in to comment.