Skip to content

Commit e49ff08

Browse files
mhegazymhegazy
committed
Merge pull request #763 from Microsoft/protectedCompletion
Protected completion
2 parents 438aa89 + 4227e58 commit e49ff08

14 files changed

+577
-74
lines changed

‎src/compiler/checker.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ module ts {
9292
getContextualType: getContextualType,
9393
getFullyQualifiedName: getFullyQualifiedName,
9494
getResolvedSignature: getResolvedSignature,
95-
getEnumMemberValue: getEnumMemberValue
95+
getEnumMemberValue: getEnumMemberValue,
96+
isValidPropertyAccess: isValidPropertyAccess
9697
};
9798

9899
var undefinedSymbol = createSymbol(SymbolFlags.Property | SymbolFlags.Transient, "undefined");
@@ -4239,6 +4240,25 @@ module ts {
42394240
return anyType;
42404241
}
42414242

4243+
function isValidPropertyAccess(node: PropertyAccess, propertyName: string): boolean {
4244+
var type = checkExpression(node.left);
4245+
if (type !== unknownType && type !== anyType) {
4246+
var apparentType = getApparentType(getWidenedType(type));
4247+
var prop = getPropertyOfApparentType(apparentType, propertyName);
4248+
if (prop && prop.parent && prop.parent.flags & SymbolFlags.Class) {
4249+
if (node.left.kind === SyntaxKind.SuperKeyword && getDeclarationKindFromSymbol(prop) !== SyntaxKind.Method) {
4250+
return false;
4251+
}
4252+
else {
4253+
var diagnosticsCount = diagnostics.length;
4254+
checkClassPropertyAccess(node, type, prop);
4255+
return diagnostics.length === diagnosticsCount
4256+
}
4257+
}
4258+
}
4259+
return true;
4260+
}
4261+
42424262
function checkIndexedAccess(node: IndexedAccess): Type {
42434263
var objectType = checkExpression(node.object);
42444264
var indexType = checkExpression(node.index);

‎src/compiler/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,8 @@ module ts {
655655
// Returns the constant value of this enum member, or 'undefined' if the enum member has a
656656
// computed value.
657657
getEnumMemberValue(node: EnumMember): number;
658+
659+
isValidPropertyAccess(node: PropertyAccess, propertyName: string): boolean;
658660
}
659661

660662
export interface TextWriter {

‎src/harness/fourslash.ts

Lines changed: 65 additions & 60 deletions
Large diffs are not rendered by default.

‎src/services/services.ts

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2038,15 +2038,6 @@ module ts {
20382038
return (SyntaxKind.FirstPunctuation <= kind && kind <= SyntaxKind.LastPunctuation);
20392039
}
20402040

2041-
function isVisibleWithinClassDeclaration(symbol: Symbol, containingClass: Declaration): boolean {
2042-
var declaration = symbol.declarations && symbol.declarations[0];
2043-
if (declaration && (declaration.flags & NodeFlags.Private)) {
2044-
var declarationClass = getAncestor(declaration, SyntaxKind.ClassDeclaration);
2045-
return containingClass === declarationClass;
2046-
}
2047-
return true;
2048-
}
2049-
20502041
function filterContextualMembersList(contextualMemberSymbols: Symbol[], existingMembers: Declaration[]): Symbol[] {
20512042
if (!existingMembers || existingMembers.length === 0) {
20522043
return contextualMemberSymbols;
@@ -2150,15 +2141,14 @@ module ts {
21502141
// Right of dot member completion list
21512142
if (isRightOfDot) {
21522143
var symbols: Symbol[] = [];
2153-
var containingClass = getAncestor(mappedNode, SyntaxKind.ClassDeclaration);
21542144
isMemberCompletion = true;
21552145

21562146
if (mappedNode.kind === SyntaxKind.Identifier || mappedNode.kind === SyntaxKind.QualifiedName || mappedNode.kind === SyntaxKind.PropertyAccess) {
21572147
var symbol = typeInfoResolver.getSymbolInfo(mappedNode);
21582148
if (symbol && symbol.flags & SymbolFlags.HasExports) {
21592149
// Extract module or enum members
21602150
forEachValue(symbol.exports, symbol => {
2161-
if (isVisibleWithinClassDeclaration(symbol, containingClass)) {
2151+
if (typeInfoResolver.isValidPropertyAccess(<PropertyAccess>(mappedNode.parent), symbol.name)) {
21622152
symbols.push(symbol);
21632153
}
21642154
});
@@ -2170,7 +2160,7 @@ module ts {
21702160
if (apparentType) {
21712161
// Filter private properties
21722162
forEach(apparentType.getApparentProperties(), symbol => {
2173-
if (isVisibleWithinClassDeclaration(symbol, containingClass)) {
2163+
if (typeInfoResolver.isValidPropertyAccess(<PropertyAccess>(mappedNode.parent), symbol.name)) {
21742164
symbols.push(symbol);
21752165
}
21762166
});

‎tests/cases/fourslash/completionListInstanceProtectedMembers.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////class Base {
4+
//// private privateMethod() { }
5+
//// private privateProperty;
6+
////
7+
//// protected protectedMethod() { }
8+
//// protected protectedProperty;
9+
////
10+
//// public publicMethod() { }
11+
//// public publicProperty;
12+
////
13+
//// protected protectedOverriddenMethod() { }
14+
//// protected protectedOverriddenProperty;
15+
////
16+
//// test() {
17+
//// this./*1*/;
18+
////
19+
//// var b: Base;
20+
//// var c: C1;
21+
////
22+
//// b./*2*/;
23+
//// c./*3*/;
24+
//// }
25+
////}
26+
////
27+
////class C1 extends Base {
28+
//// protected protectedOverriddenMethod() { }
29+
//// protected protectedOverriddenProperty;
30+
////}
31+
32+
33+
// Same class, everything is visible
34+
goTo.marker("1");
35+
verify.memberListContains('privateMethod');
36+
verify.memberListContains('privateProperty');
37+
verify.memberListContains('protectedMethod');
38+
verify.memberListContains('protectedProperty');
39+
verify.memberListContains('publicMethod');
40+
verify.memberListContains('publicProperty');
41+
verify.memberListContains('protectedOverriddenMethod');
42+
verify.memberListContains('protectedOverriddenProperty');
43+
44+
goTo.marker("2");
45+
verify.memberListContains('privateMethod');
46+
verify.memberListContains('privateProperty');
47+
verify.memberListContains('protectedMethod');
48+
verify.memberListContains('protectedProperty');
49+
verify.memberListContains('publicMethod');
50+
verify.memberListContains('publicProperty');
51+
verify.memberListContains('protectedOverriddenMethod');
52+
verify.memberListContains('protectedOverriddenProperty');
53+
54+
// Can not access protected properties overridden in subclass
55+
goTo.marker("3");
56+
verify.memberListContains('privateMethod');
57+
verify.memberListContains('privateProperty');
58+
verify.memberListContains('protectedMethod');
59+
verify.memberListContains('protectedProperty');
60+
verify.memberListContains('publicMethod');
61+
verify.memberListContains('publicProperty');
62+
verify.not.memberListContains('protectedOverriddenMethod');
63+
verify.not.memberListContains('protectedOverriddenProperty');

‎tests/cases/fourslash/completionListInstanceProtectedMembers2.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////class Base {
4+
//// private privateMethod() { }
5+
//// private privateProperty;
6+
////
7+
//// protected protectedMethod() { }
8+
//// protected protectedProperty;
9+
////
10+
//// public publicMethod() { }
11+
//// public publicProperty;
12+
////
13+
//// protected protectedOverriddenMethod() { }
14+
//// protected protectedOverriddenProperty;
15+
////}
16+
////
17+
////class C1 extends Base {
18+
//// protected protectedOverriddenMethod() { }
19+
//// protected protectedOverriddenProperty;
20+
////
21+
//// test() {
22+
//// this./*1*/;
23+
//// super./*2*/;
24+
////
25+
//// var b: Base;
26+
//// var c: C1;
27+
////
28+
//// b./*3*/;
29+
//// c./*4*/;
30+
//// }
31+
////}
32+
33+
34+
// Same class, everything is visible
35+
goTo.marker("1");
36+
verify.not.memberListContains('privateMethod');
37+
verify.not.memberListContains('privateProperty');
38+
verify.memberListContains('protectedMethod');
39+
verify.memberListContains('protectedProperty');
40+
verify.memberListContains('publicMethod');
41+
verify.memberListContains('publicProperty');
42+
verify.memberListContains('protectedOverriddenMethod');
43+
verify.memberListContains('protectedOverriddenProperty');
44+
45+
// Can not access properties on super
46+
goTo.marker("2");
47+
verify.not.memberListContains('privateMethod');
48+
verify.not.memberListContains('privateProperty');
49+
verify.memberListContains('protectedMethod');
50+
verify.not.memberListContains('protectedProperty');
51+
verify.memberListContains('publicMethod');
52+
verify.not.memberListContains('publicProperty');
53+
verify.memberListContains('protectedOverriddenMethod');
54+
verify.not.memberListContains('protectedOverriddenProperty');
55+
56+
// Can not access protected properties through base class
57+
goTo.marker("3");
58+
verify.not.memberListContains('privateMethod');
59+
verify.not.memberListContains('privateProperty');
60+
verify.not.memberListContains('protectedMethod');
61+
verify.not.memberListContains('protectedProperty');
62+
verify.memberListContains('publicMethod');
63+
verify.memberListContains('publicProperty');
64+
verify.not.memberListContains('protectedOverriddenMethod');
65+
verify.not.memberListContains('protectedOverriddenProperty');
66+
67+
// Same class, everything is visible
68+
goTo.marker("4");
69+
verify.not.memberListContains('privateMethod');
70+
verify.not.memberListContains('privateProperty');
71+
verify.memberListContains('protectedMethod');
72+
verify.memberListContains('protectedProperty');
73+
verify.memberListContains('publicMethod');
74+
verify.memberListContains('publicProperty');
75+
verify.memberListContains('protectedOverriddenMethod');
76+
verify.memberListContains('protectedOverriddenProperty');

‎tests/cases/fourslash/completionListInstanceProtectedMembers3.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////class Base {
4+
//// private privateMethod() { }
5+
//// private privateProperty;
6+
////
7+
//// protected protectedMethod() { }
8+
//// protected protectedProperty;
9+
////
10+
//// public publicMethod() { }
11+
//// public publicProperty;
12+
////
13+
//// protected protectedOverriddenMethod() { }
14+
//// protected protectedOverriddenProperty;
15+
////}
16+
////
17+
////class C1 extends Base {
18+
//// protected protectedOverriddenMethod() { }
19+
//// protected protectedOverriddenProperty;
20+
////}
21+
////
22+
//// var b: Base;
23+
//// var c: C1;
24+
//// b./*1*/;
25+
//// c./*2*/;
26+
27+
// Only public properties are visible outside the class
28+
goTo.marker("1");
29+
verify.not.memberListContains('privateMethod');
30+
verify.not.memberListContains('privateProperty');
31+
verify.not.memberListContains('protectedMethod');
32+
verify.not.memberListContains('protectedProperty');
33+
verify.memberListContains('publicMethod');
34+
verify.memberListContains('publicProperty');
35+
verify.not.memberListContains('protectedOverriddenMethod');
36+
verify.not.memberListContains('protectedOverriddenProperty');
37+
38+
goTo.marker("2");
39+
verify.not.memberListContains('privateMethod');
40+
verify.not.memberListContains('privateProperty');
41+
verify.not.memberListContains('protectedMethod');
42+
verify.not.memberListContains('protectedProperty');
43+
verify.memberListContains('publicMethod');
44+
verify.memberListContains('publicProperty');
45+
verify.not.memberListContains('protectedOverriddenMethod');
46+
verify.not.memberListContains('protectedOverriddenProperty');

‎tests/cases/fourslash/completionListInstanceProtectedMembers4.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////class Base {
4+
//// private privateMethod() { }
5+
//// private privateProperty;
6+
////
7+
//// protected protectedMethod() { }
8+
//// protected protectedProperty;
9+
////
10+
//// public publicMethod() { }
11+
//// public publicProperty;
12+
////
13+
//// protected protectedOverriddenMethod() { }
14+
//// protected protectedOverriddenProperty;
15+
////}
16+
////
17+
////class C1 extends Base {
18+
//// public protectedOverriddenMethod() { }
19+
//// public protectedOverriddenProperty;
20+
////}
21+
////
22+
//// var c: C1;
23+
//// c./*1*/
24+
25+
26+
goTo.marker("1");
27+
verify.not.memberListContains('privateMethod');
28+
verify.not.memberListContains('privateProperty');
29+
verify.not.memberListContains('protectedMethod');
30+
verify.not.memberListContains('protectedProperty');
31+
verify.memberListContains('publicMethod');
32+
verify.memberListContains('publicProperty');
33+
verify.memberListContains('protectedOverriddenMethod');
34+
verify.memberListContains('protectedOverriddenProperty');

‎tests/cases/fourslash/completionListProtectedMembers.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/// <reference path="fourslash.ts"/>
2+
3+
////class Base {
4+
//// protected y;
5+
//// constructor(protected x) {}
6+
//// method() { this./*1*/; }
7+
////}
8+
////class D1 extends Base {
9+
//// protected z;
10+
//// method1() { this./*2*/; }
11+
////}
12+
////class D2 extends Base {
13+
//// method2() { this./*3*/; }
14+
////}
15+
////class D3 extends D1 {
16+
//// method2() { this./*4*/; }
17+
////}
18+
////var b: Base;
19+
////f./*5*/
20+
21+
goTo.marker("1");
22+
verify.memberListContains("y");
23+
verify.memberListContains("x");
24+
verify.not.memberListContains("z");
25+
26+
goTo.marker("2");
27+
verify.memberListContains("y");
28+
verify.memberListContains("x");
29+
verify.memberListContains("z");
30+
31+
goTo.marker("3");
32+
verify.memberListContains("y");
33+
verify.memberListContains("x");
34+
verify.not.memberListContains("z");
35+
36+
goTo.marker("4");
37+
verify.memberListContains("y");
38+
verify.memberListContains("x");
39+
verify.memberListContains("z");
40+
41+
goTo.marker("5");
42+
verify.not.memberListContains("x");
43+
verify.not.memberListContains("y");
44+
verify.not.memberListContains("z");

0 commit comments

Comments
 (0)