Addressing CR feedback

ahejlsberg · ahejlsberg · commit 4aa04a93b3b0 · 2014-09-17T16:36:38.000-07:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -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 {
@@ -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
             return hasBaseType(enclosingClass, declaringClass);
         }
 
diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json
@@ -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",

Original file line number	Diff line number	Diff line change
`@@ -2713,12 +2713,12 @@ module ts {`
`2713`	`2713`	`if (sourceProp === targetProp) {`
`2714`	`2714`	`return true;`
`2715`	`2715`	`}`
`2716`		`- var sourcePropVisibility = getDeclarationFlagsFromSymbol(sourceProp) & (NodeFlags.Private \|\| NodeFlags.Protected);`
`2717`		`- var targetPropVisibility = getDeclarationFlagsFromSymbol(targetProp) & (NodeFlags.Private \|\| NodeFlags.Protected);`
`2718`		`- if (sourcePropVisibility !== targetPropVisibility) {`
	`2716`	`+ var sourcePropAccessibility = getDeclarationFlagsFromSymbol(sourceProp) & (NodeFlags.Private \| NodeFlags.Protected);`
	`2717`	`+ var targetPropAccessibility = getDeclarationFlagsFromSymbol(targetProp) & (NodeFlags.Private \| NodeFlags.Protected);`
	`2718`	`+ if (sourcePropAccessibility !== targetPropAccessibility) {`
`2719`	`2719`	`return false;`
`2720`	`2720`	`}`
`2721`		`- if (sourcePropVisibility) {`
	`2721`	`+ if (sourcePropAccessibility) {`
`2722`	`2722`	`return getTargetSymbol(sourceProp) === getTargetSymbol(targetProp) && relate(getTypeOfSymbol(sourceProp), getTypeOfSymbol(targetProp), reportErrors);`
`2723`	`2723`	`}`
`2724`	`2724`	`else {`
`@@ -4011,26 +4011,35 @@ module ts {`
`4011`	`4011`
`4012`	`4012`	`function isClassPropertyAccessible(node: PropertyAccess, type: Type, prop: Symbol): boolean {`
`4013`	`4013`	`var flags = getDeclarationFlagsFromSymbol(prop);`
	`4014`	`+ // Public properties are always accessible`
`4014`	`4015`	`if (!(flags & (NodeFlags.Private \| NodeFlags.Protected))) {`
`4015`	`4016`	`return true;`
`4016`	`4017`	`}`
	`4018`	`+ // Property is known to be private or protected at this point`
	`4019`	`+ // Private and protected properties are never accessible outside a class declaration`
`4017`	`4020`	`var enclosingClassDeclaration = getAncestor(node, SyntaxKind.ClassDeclaration);`
`4018`	`4021`	`if (!enclosingClassDeclaration) {`
`4019`	`4022`	`return false;`
`4020`	`4023`	`}`
	`4024`	`+ // Get the declaring and enclosing class instance types`
`4021`	`4025`	`var declaringClass = <InterfaceType>getDeclaredTypeOfSymbol(prop.parent);`
`4022`	`4026`	`var enclosingClass = <InterfaceType>getDeclaredTypeOfSymbol(getSymbolOfNode(enclosingClassDeclaration));`
	`4027`	`+ // Private property is accessible if declaring and enclosing class are the same`
`4023`	`4028`	`if (flags & NodeFlags.Private) {`
`4024`	`4029`	`return declaringClass === enclosingClass;`
`4025`	`4030`	`}`
	`4031`	`+ // Property is known to be protected at this point`
	`4032`	`+ // All protected properties of a supertype are accessible in a super access`
`4026`	`4033`	`if (node.left.kind === SyntaxKind.SuperKeyword) {`
`4027`	`4034`	`return true;`
`4028`	`4035`	`}`
	`4036`	`+ // An instance property must be accessed through an instance of the enclosing class`
`4029`	`4037`	`if (!(flags & NodeFlags.Static)) {`
`4030`	`4038`	`if (!(getTargetType(type).flags & (TypeFlags.Class \| TypeFlags.Interface) && hasBaseType(<InterfaceType>type, enclosingClass))) {`
`4031`	`4039`	`return false;`
`4032`	`4040`	`}`
`4033`	`4041`	`}`
	`4042`	`+ // A protected property is accessible in the declaring class and classes derived from it`
`4034`	`4043`	`return hasBaseType(enclosingClass, declaringClass);`
`4035`	`4044`	`}`
`4036`	`4045`