Skip to content

Commit

Permalink
Addressing CR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
ahejlsberg committed Sep 17, 2014
1 parent ce6b623 commit 4aa04a9
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 10 deletions.
17 changes: 13 additions & 4 deletions src/compiler/checker.ts
Expand Up @@ -2713,12 +2713,12 @@ module ts {
if (sourceProp === targetProp) {
return true;
}
var sourcePropVisibility = getDeclarationFlagsFromSymbol(sourceProp) & (NodeFlags.Private || NodeFlags.Protected);
var targetPropVisibility = getDeclarationFlagsFromSymbol(targetProp) & (NodeFlags.Private || NodeFlags.Protected);
if (sourcePropVisibility !== targetPropVisibility) {
var sourcePropAccessibility = getDeclarationFlagsFromSymbol(sourceProp) & (NodeFlags.Private | NodeFlags.Protected);
var targetPropAccessibility = getDeclarationFlagsFromSymbol(targetProp) & (NodeFlags.Private | NodeFlags.Protected);
if (sourcePropAccessibility !== targetPropAccessibility) {
return false;
}
if (sourcePropVisibility) {
if (sourcePropAccessibility) {
return getTargetSymbol(sourceProp) === getTargetSymbol(targetProp) && relate(getTypeOfSymbol(sourceProp), getTypeOfSymbol(targetProp), reportErrors);
}
else {
Expand Down Expand Up @@ -4011,26 +4011,35 @@ module ts {

function isClassPropertyAccessible(node: PropertyAccess, type: Type, prop: Symbol): boolean {
var flags = getDeclarationFlagsFromSymbol(prop);
// Public properties are always accessible
if (!(flags & (NodeFlags.Private | NodeFlags.Protected))) {
return true;
}
// 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 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;
}
// 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;
}
// 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;
}
}
// A protected property is accessible in the declaring class and classes derived from it

This comment has been minimized.

Copy link
@CyrusNajmabadi

CyrusNajmabadi Sep 18, 2014

Contributor

great comments thanks. it really makes it easy to read through and understand what's going on.

return hasBaseType(enclosingClass, declaringClass);

This comment has been minimized.

Copy link
@CyrusNajmabadi

CyrusNajmabadi Sep 18, 2014

Contributor

this line doesn't match the comment. You don't check for the case that you're in the declaring calss, only that you're in a class derived form it.

This comment has been minimized.

Copy link
@ahejlsberg

ahejlsberg Sep 18, 2014

Author Member

Actually, hasBaseType returns true for the type itself so the check is correct. But maybe we need a better name for hasBaseType. Ideas?

}

Expand Down
12 changes: 6 additions & 6 deletions src/compiler/diagnosticMessages.json
Expand Up @@ -1012,18 +1012,18 @@
"category": "Error",
"code": 2441
},
"Types have separate declarations of a private property '{0}'.": {
"Types have separate declarations of a private property '{0}'.": {
"category": "Error",
"code": 2442
},
"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 type '{2}'.": {
"category": "Error",
"code": 2443
},
"Property '{0}' is protected in type '{1}' but public in type '{2}'.": {
},
"Property '{0}' is protected in type '{1}' but public in type '{2}'.": {
"category": "Error",
"code": 2444
},
},

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

0 comments on commit 4aa04a9

Please sign in to comment.