Skip to content

Commit

Permalink
Completion list for a class extending another class should contain me…
Browse files Browse the repository at this point in the history
…mbers from base class

Handles #7158
  • Loading branch information
sheetalkamat committed May 2, 2017
1 parent 1db4f96 commit 945f675
Show file tree
Hide file tree
Showing 9 changed files with 313 additions and 57 deletions.
23 changes: 0 additions & 23 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -679,10 +679,6 @@ namespace ts {
return type.flags & TypeFlags.Object ? (<ObjectType>type).objectFlags : 0;
}

function getCheckFlags(symbol: Symbol): CheckFlags {
return symbol.flags & SymbolFlags.Transient ? (<TransientSymbol>symbol).checkFlags : 0;
}

function isGlobalSourceFile(node: Node) {
return node.kind === SyntaxKind.SourceFile && !isExternalOrCommonJsModule(<SourceFile>node);
}
Expand Down Expand Up @@ -13914,25 +13910,6 @@ namespace ts {
return s.valueDeclaration ? s.valueDeclaration.kind : SyntaxKind.PropertyDeclaration;
}

function getDeclarationModifierFlagsFromSymbol(s: Symbol): ModifierFlags {
if (s.valueDeclaration) {
const flags = getCombinedModifierFlags(s.valueDeclaration);
return s.parent && s.parent.flags & SymbolFlags.Class ? flags : flags & ~ModifierFlags.AccessibilityModifier;
}
if (getCheckFlags(s) & CheckFlags.Synthetic) {
const checkFlags = (<TransientSymbol>s).checkFlags;
const accessModifier = checkFlags & CheckFlags.ContainsPrivate ? ModifierFlags.Private :
checkFlags & CheckFlags.ContainsPublic ? ModifierFlags.Public :
ModifierFlags.Protected;
const staticModifier = checkFlags & CheckFlags.ContainsStatic ? ModifierFlags.Static : 0;
return accessModifier | staticModifier;
}
if (s.flags & SymbolFlags.Prototype) {
return ModifierFlags.Public | ModifierFlags.Static;
}
return 0;
}

function getDeclarationNodeFlagsFromSymbol(s: Symbol): NodeFlags {
return s.valueDeclaration ? getCombinedNodeFlags(s.valueDeclaration) : 0;
}
Expand Down
23 changes: 23 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4198,6 +4198,29 @@ namespace ts {
// Firefox has Object.prototype.watch
return options.watch && options.hasOwnProperty("watch");
}

export function getCheckFlags(symbol: Symbol): CheckFlags {
return symbol.flags & SymbolFlags.Transient ? (<TransientSymbol>symbol).checkFlags : 0;
}

export function getDeclarationModifierFlagsFromSymbol(s: Symbol): ModifierFlags {
if (s.valueDeclaration) {
const flags = getCombinedModifierFlags(s.valueDeclaration);
return s.parent && s.parent.flags & SymbolFlags.Class ? flags : flags & ~ModifierFlags.AccessibilityModifier;
}
if (getCheckFlags(s) & CheckFlags.Synthetic) {
const checkFlags = (<TransientSymbol>s).checkFlags;
const accessModifier = checkFlags & CheckFlags.ContainsPrivate ? ModifierFlags.Private :
checkFlags & CheckFlags.ContainsPublic ? ModifierFlags.Public :
ModifierFlags.Protected;
const staticModifier = checkFlags & CheckFlags.ContainsStatic ? ModifierFlags.Static : 0;
return accessModifier | staticModifier;
}
if (s.flags & SymbolFlags.Prototype) {
return ModifierFlags.Public | ModifierFlags.Static;
}
return 0;
}
}

namespace ts {
Expand Down
92 changes: 87 additions & 5 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace ts.Completions {
return undefined;
}

const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, requestJsDocTagName, requestJsDocTag } = completionData;
const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, requestJsDocTagName, requestJsDocTag, hasFilteredClassMemberKeywords } = completionData;

if (requestJsDocTagName) {
// If the current position is a jsDoc tag name, only tag names should be provided for completion
Expand Down Expand Up @@ -52,16 +52,19 @@ namespace ts.Completions {
sortText: "0",
});
}
else {
else if (!hasFilteredClassMemberKeywords) {
return undefined;
}
}

getCompletionEntriesFromSymbols(symbols, entries, location, /*performCharacterChecks*/ true, typeChecker, compilerOptions.target, log);
}

if (hasFilteredClassMemberKeywords) {
addRange(entries, classMemberKeywordCompletions);
}
// Add keywords if this is not a member completion list
if (!isMemberCompletion && !requestJsDocTag && !requestJsDocTagName) {
else if (!isMemberCompletion && !requestJsDocTag && !requestJsDocTagName) {
addRange(entries, keywordCompletions);
}

Expand Down Expand Up @@ -406,7 +409,7 @@ namespace ts.Completions {
}

if (requestJsDocTagName || requestJsDocTag) {
return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, requestJsDocTagName, requestJsDocTag };
return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, requestJsDocTagName, requestJsDocTag, hasFilteredClassMemberKeywords: false };
}

if (!insideJsDocTagExpression) {
Expand Down Expand Up @@ -505,6 +508,7 @@ namespace ts.Completions {
let isGlobalCompletion = false;
let isMemberCompletion: boolean;
let isNewIdentifierLocation: boolean;
let hasFilteredClassMemberKeywords = false;
let symbols: Symbol[] = [];

if (isRightOfDot) {
Expand Down Expand Up @@ -542,7 +546,7 @@ namespace ts.Completions {

log("getCompletionData: Semantic work: " + (timestamp() - semanticStart));

return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), requestJsDocTagName, requestJsDocTag };
return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), requestJsDocTagName, requestJsDocTag, hasFilteredClassMemberKeywords };

function getTypeScriptMemberSymbols(): void {
// Right of dot member completion list
Expand Down Expand Up @@ -599,6 +603,7 @@ namespace ts.Completions {
function tryGetGlobalSymbols(): boolean {
let objectLikeContainer: ObjectLiteralExpression | BindingPattern;
let namedImportsOrExports: NamedImportsOrExports;
let classLikeContainer: ClassLikeDeclaration;
let jsxContainer: JsxOpeningLikeElement;

if (objectLikeContainer = tryGetObjectLikeCompletionContainer(contextToken)) {
Expand All @@ -611,6 +616,11 @@ namespace ts.Completions {
return tryGetImportOrExportClauseCompletionSymbols(namedImportsOrExports);
}

if (classLikeContainer = tryGetClassLikeCompletionContainer(contextToken)) {
// cursor inside class declaration
return tryGetClassLikeCompletionSymbols(classLikeContainer);
}

if (jsxContainer = tryGetContainingJsxElement(contextToken)) {
let attrsType: Type;
if ((jsxContainer.kind === SyntaxKind.JsxSelfClosingElement) || (jsxContainer.kind === SyntaxKind.JsxOpeningElement)) {
Expand Down Expand Up @@ -913,6 +923,31 @@ namespace ts.Completions {
return true;
}

/**
* Aggregates relevant symbols for completion in class declaration
* Relevant symbols are stored in the captured 'symbols' variable.
*
* @returns true if 'symbols' was successfully populated; false otherwise.
*/
function tryGetClassLikeCompletionSymbols(classLikeDeclaration: ClassLikeDeclaration): boolean {
// We're looking up possible property names from parent type.
isMemberCompletion = true;
// Declaring new property/method/accessor
isNewIdentifierLocation = true;
// Has keywords for class elements
hasFilteredClassMemberKeywords = true;

const baseTypeNode = getClassExtendsHeritageClauseElement(classLikeDeclaration);
if (baseTypeNode) {
const baseType = typeChecker.getTypeAtLocation(baseTypeNode);
// List of property symbols of base type that are not private
symbols = filter(typeChecker.getPropertiesOfType(baseType),
baseProperty => !(getDeclarationModifierFlagsFromSymbol(baseProperty) & ModifierFlags.Private));
}

return true;
}

/**
* Returns the immediate owning object literal or binding pattern of a context token,
* on the condition that one exists and that the context implies completion should be given.
Expand Down Expand Up @@ -953,6 +988,38 @@ namespace ts.Completions {
return undefined;
}

/**
* Returns the immediate owning class declaration of a context token,
* on the condition that one exists and that the context implies completion should be given.
*/
function tryGetClassLikeCompletionContainer(contextToken: Node): ClassLikeDeclaration {
if (contextToken) {
switch (contextToken.kind) {
case SyntaxKind.OpenBraceToken: // class c { |
if (isClassLike(contextToken.parent)) {
return contextToken.parent;
}
break;

// class c {getValue(): number; | }
case SyntaxKind.CommaToken:
case SyntaxKind.SemicolonToken:
// class c { method() { } | }
case SyntaxKind.CloseBraceToken:
if (isClassLike(location)) {
return location;
}
break;
}
}

// class c { method() { } | method2() { } }
if (location && location.kind === SyntaxKind.SyntaxList && isClassLike(location.parent)) {
return location.parent;
}
return undefined;
}

function tryGetContainingJsxElement(contextToken: Node): JsxOpeningLikeElement {
if (contextToken) {
const parent = contextToken.parent;
Expand Down Expand Up @@ -1306,6 +1373,21 @@ namespace ts.Completions {
});
}

const classMemberKeywordCompletions = filter(keywordCompletions, entry => {
switch (stringToToken(entry.name)) {
case SyntaxKind.PublicKeyword:
case SyntaxKind.ProtectedKeyword:
case SyntaxKind.PrivateKeyword:
case SyntaxKind.AbstractKeyword:
case SyntaxKind.StaticKeyword:
case SyntaxKind.ConstructorKeyword:
case SyntaxKind.ReadonlyKeyword:
case SyntaxKind.GetKeyword:
case SyntaxKind.SetKeyword:
return true;
}
});

function isEqualityExpression(node: Node): node is BinaryExpression {
return isBinaryExpression(node) && isEqualityOperatorKind(node.operatorToken.kind);
}
Expand Down
142 changes: 142 additions & 0 deletions tests/cases/fourslash/completionEntryForClassMembers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
///<reference path="fourslash.ts" />

////abstract class B {
//// abstract getValue(): number;
//// /*abstractClass*/
////}
////class C extends B {
//// /*classThatIsEmptyAndExtendingAnotherClass*/
////}
////class D extends B {
//// /*classThatHasAlreadyImplementedAnotherClassMethod*/
//// getValue() {
//// return 10;
//// }
//// /*classThatHasAlreadyImplementedAnotherClassMethodAfterMethod*/
////}
////class E {
//// /*classThatDoesNotExtendAnotherClass*/
////}
////class F extends B {
//// public /*classThatHasWrittenPublicKeyword*/
////}
////class G extends B {
//// static /*classElementContainingStatic*/
////}
////class H extends B {
//// prop/*classThatStartedWritingIdentifier*/
////}
////class I extends B {
//// prop0: number
//// /*propDeclarationWithoutSemicolon*/
//// prop: number;
//// /*propDeclarationWithSemicolon*/
//// prop1 = 10;
//// /*propAssignmentWithSemicolon*/
//// prop2 = 10
//// /*propAssignmentWithoutSemicolon*/
//// method(): number
//// /*methodSignatureWithoutSemicolon*/
//// method2(): number;
//// /*methodSignatureWithSemicolon*/
//// method3() {
//// /*InsideMethod*/
//// }
//// /*methodImplementation*/
//// get c()
//// /*accessorSignatureWithoutSemicolon*/
//// set c()
//// {
//// }
//// /*accessorSignatureImplementation*/
////}
////class J extends B {
//// get /*classThatHasWrittenGetKeyword*/
////}
////class K extends B {
//// set /*classThatHasWrittenSetKeyword*/
////}
////class J extends B {
//// get identi/*classThatStartedWritingIdentifierOfGetAccessor*/
////}
////class K extends B {
//// set identi/*classThatStartedWritingIdentifierOfSetAccessor*/
////}
////class L extends B {
//// public identi/*classThatStartedWritingIdentifierAfterModifier*/
////}
////class L extends B {
//// static identi/*classThatStartedWritingIdentifierAfterStaticModifier*/
////}

const allowedKeywords = [
"public",
"private",
"protected",
"static",
"abstract",
"readonly",
"get",
"set",
"constructor"
];

const allowedKeywordCount = allowedKeywords.length;
function verifyAllowedKeyWords() {
for (const keyword of allowedKeywords) {
verify.completionListContains(keyword, keyword, /*documentation*/ undefined, "keyword");
}
}

const nonClassElementMarkers = [
"InsideMethod"
];
for (const marker of nonClassElementMarkers) {
goTo.marker(marker);
verify.not.completionListContains("getValue");
verify.not.completionListIsEmpty();
}

// Only keywords allowed at this position since they dont extend the class
const onlyClassElementKeywordLocations = [
"abstractClass",
"classThatDoesNotExtendAnotherClass"
];
for (const marker of onlyClassElementKeywordLocations) {
goTo.marker(marker);
verifyAllowedKeyWords();
verify.completionListCount(allowedKeywordCount);
}

// Base members and class member keywords allowed
const classElementCompletionLocations = [
"classThatIsEmptyAndExtendingAnotherClass",
"classThatHasAlreadyImplementedAnotherClassMethod",
"classThatHasAlreadyImplementedAnotherClassMethodAfterMethod",
// TODO should we give completion for these keywords
//"classThatHasWrittenPublicKeyword",
//"classElementContainingStatic",
"classThatStartedWritingIdentifier",
"propDeclarationWithoutSemicolon",
"propDeclarationWithSemicolon",
"propAssignmentWithSemicolon",
"propAssignmentWithoutSemicolon",
"methodSignatureWithoutSemicolon",
"methodSignatureWithSemicolon",
"methodImplementation",
"accessorSignatureWithoutSemicolon",
"accessorSignatureImplementation",
// TODO should we give completion for these keywords
//"classThatHasWrittenGetKeyword",
//"classThatHasWrittenSetKeyword",
//"classThatStartedWritingIdentifierOfGetAccessor",
//"classThatStartedWritingIdentifierOfSetAccessor",
//"classThatStartedWritingIdentifierAfterModifier",
//"classThatStartedWritingIdentifierAfterStaticModifier"
];
for (const marker of classElementCompletionLocations) {
goTo.marker(marker);
verify.completionListContains("getValue", "(method) B.getValue(): number", /*documentation*/ undefined, "method");
verifyAllowedKeyWords();
verify.completionListCount(allowedKeywordCount + 1);
}
4 changes: 2 additions & 2 deletions tests/cases/fourslash/completionListInNamedClassExpression.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
///<reference path="fourslash.ts" />
///<reference path="fourslash.ts" />

//// var x = class myClass {
//// getClassName (){
Expand All @@ -11,4 +11,4 @@ goTo.marker("0");
verify.completionListContains("myClass", "(local class) myClass", /*documentation*/ undefined, "local class");

goTo.marker("1");
verify.completionListContains("myClass", "(local class) myClass", /*documentation*/ undefined, "local class");
verify.not.completionListContains("myClass", "(local class) myClass", /*documentation*/ undefined, "local class");
Loading

0 comments on commit 945f675

Please sign in to comment.