Skip to content

Commit dcac1df

Browse files
sheetalkamatsheetalkamat
committed
Filter symbols based on the meaning at the location
1 parent 5e36756 commit dcac1df

34 files changed

+563
-261
lines changed

‎Jakefile.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -959,7 +959,7 @@ var nodeServerInFile = "tests/webTestServer.ts";
959959
compileFile(nodeServerOutFile, [nodeServerInFile], [builtLocalDirectory, tscFile], [], /*useBuiltCompiler:*/ true, { noOutFile: true, lib: "es6" });
960960

961961
desc("Runs browserify on run.js to produce a file suitable for running tests in the browser");
962-
task("browserify", ["tests", builtLocalDirectory, nodeServerOutFile], function() {
962+
task("browserify", ["tests", run, builtLocalDirectory, nodeServerOutFile], function() {
963963
var cmd = 'browserify built/local/run.js -t ./scripts/browserify-optional -d -o built/local/bundle.js';
964964
exec(cmd);
965965
}, { async: true });

‎src/compiler/checker.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14590,12 +14590,31 @@ namespace ts {
1459014590
? (<PropertyAccessExpression>node).expression
1459114591
: (<QualifiedName>node).left;
1459214592

14593-
const type = checkExpression(left);
14593+
return isValidPropertyAccessWithType(node, left, propertyName, getWidenedType(checkExpression(left)));
14594+
}
14595+
14596+
function isValidPropertyAccessWithType(
14597+
node: PropertyAccessExpression | QualifiedName,
14598+
left: LeftHandSideExpression | QualifiedName,
14599+
propertyName: string,
14600+
type: Type): boolean {
14601+
1459414602
if (type !== unknownType && !isTypeAny(type)) {
14595-
const prop = getPropertyOfType(getWidenedType(type), propertyName);
14603+
const prop = getPropertyOfType(type, propertyName);
1459614604
if (prop) {
1459714605
return checkPropertyAccessibility(node, left, type, prop);
1459814606
}
14607+
14608+
// In js files properties of unions are allowed in completion
14609+
if (isInJavaScriptFile(left) && (type.flags & TypeFlags.Union)) {
14610+
for (const elementType of (<UnionType>type).types) {
14611+
if (isValidPropertyAccessWithType(node, left, propertyName, elementType)) {
14612+
return true;
14613+
}
14614+
}
14615+
}
14616+
14617+
return false;
1459914618
}
1460014619
return true;
1460114620
}

‎src/services/completions.ts

Lines changed: 98 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,9 @@ namespace ts.Completions {
578578
isMemberCompletion = true;
579579
isNewIdentifierLocation = false;
580580

581+
// Since this is qualified name check its a type node location
582+
const isTypeLocation = isPartOfTypeNode(node.parent);
583+
const isRhsOfImportDeclaration = isInRightSideOfInternalImportEqualsDeclaration(node);
581584
if (node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.QualifiedName || node.kind === SyntaxKind.PropertyAccessExpression) {
582585
let symbol = typeChecker.getSymbolAtLocation(node);
583586

@@ -589,16 +592,24 @@ namespace ts.Completions {
589592
if (symbol && symbol.flags & SymbolFlags.HasExports) {
590593
// Extract module or enum members
591594
const exportedSymbols = typeChecker.getExportsOfModule(symbol);
595+
const isValidValueAccess = (symbol: Symbol) => typeChecker.isValidPropertyAccess(<PropertyAccessExpression>(node.parent), symbol.name);
596+
const isValidTypeAccess = (symbol: Symbol) => symbolCanbeReferencedAtTypeLocation(symbol);
597+
const isValidAccess = isRhsOfImportDeclaration ?
598+
// Any kind is allowed when dotting off namespace in internal import equals declaration
599+
(symbol: Symbol) => isValidTypeAccess(symbol) || isValidValueAccess(symbol) :
600+
isTypeLocation ? isValidTypeAccess : isValidValueAccess;
592601
forEach(exportedSymbols, symbol => {
593-
if (typeChecker.isValidPropertyAccess(<PropertyAccessExpression>(node.parent), symbol.name)) {
602+
if (isValidAccess(symbol)) {
594603
symbols.push(symbol);
595604
}
596605
});
597606
}
598607
}
599608

600-
const type = typeChecker.getTypeAtLocation(node);
601-
addTypeProperties(type);
609+
if (!isTypeLocation) {
610+
const type = typeChecker.getTypeAtLocation(node);
611+
addTypeProperties(type);
612+
}
602613
}
603614

604615
function addTypeProperties(type: Type) {
@@ -706,13 +717,88 @@ namespace ts.Completions {
706717
isStatement(scopeNode);
707718
}
708719

709-
/// TODO filter meaning based on the current context
710720
const symbolMeanings = SymbolFlags.Type | SymbolFlags.Value | SymbolFlags.Namespace | SymbolFlags.Alias;
711-
symbols = typeChecker.getSymbolsInScope(scopeNode, symbolMeanings);
721+
symbols = filterGlobalCompletion(typeChecker.getSymbolsInScope(scopeNode, symbolMeanings));
712722

713723
return true;
714724
}
715725

726+
function filterGlobalCompletion(symbols: Symbol[]) {
727+
return filter(symbols, symbol => {
728+
if (!isSourceFile(location)) {
729+
// export = /**/ here we want to get all meanings, so any symbol is ok
730+
if (isExportAssignment(location.parent)) {
731+
return true;
732+
}
733+
734+
// This is an alias, follow what it aliases
735+
if (symbol && symbol.flags & SymbolFlags.Alias) {
736+
symbol = typeChecker.getAliasedSymbol(symbol);
737+
}
738+
739+
// import m = /**/ <-- It can only access namespace (if typing import = x. this would get member symbols and not namespace)
740+
if (isInRightSideOfInternalImportEqualsDeclaration(location)) {
741+
return !!(symbol.flags & SymbolFlags.Namespace);
742+
}
743+
744+
if (!isContextTokenValueLocation(contextToken) &&
745+
(isPartOfTypeNode(location) || isContextTokenTypeLocation(contextToken))) {
746+
// Its a type, but you can reach it by namespace.type as well
747+
return symbolCanbeReferencedAtTypeLocation(symbol);
748+
}
749+
}
750+
751+
// expressions are value space (which includes the value namespaces)
752+
return !!(symbol.flags & SymbolFlags.Value);
753+
});
754+
}
755+
756+
function isContextTokenValueLocation(contextToken: Node) {
757+
if (contextToken) {
758+
const parentKind = contextToken.parent.kind;
759+
switch (contextToken.kind) {
760+
case SyntaxKind.TypeOfKeyword:
761+
return parentKind === SyntaxKind.TypeQuery;
762+
}
763+
}
764+
}
765+
766+
function isContextTokenTypeLocation(contextToken: Node) {
767+
if (contextToken) {
768+
const parentKind = contextToken.parent.kind;
769+
switch (contextToken.kind) {
770+
case SyntaxKind.ColonToken:
771+
return parentKind === SyntaxKind.PropertyDeclaration ||
772+
parentKind === SyntaxKind.PropertySignature ||
773+
parentKind === SyntaxKind.Parameter ||
774+
parentKind === SyntaxKind.VariableDeclaration ||
775+
isFunctionLikeKind(parentKind);
776+
777+
case SyntaxKind.EqualsToken:
778+
return parentKind === SyntaxKind.TypeAliasDeclaration;
779+
780+
case SyntaxKind.AsKeyword:
781+
return parentKind === SyntaxKind.AsExpression;
782+
}
783+
}
784+
}
785+
786+
function symbolCanbeReferencedAtTypeLocation(symbol: Symbol): boolean {
787+
// This is an alias, follow what it aliases
788+
if (symbol && symbol.flags & SymbolFlags.Alias) {
789+
symbol = typeChecker.getAliasedSymbol(symbol);
790+
}
791+
792+
if (symbol.flags & (SymbolFlags.ValueModule | SymbolFlags.NamespaceModule)) {
793+
const exportedSymbols = typeChecker.getExportsOfModule(symbol);
794+
// If the exported symbols contains type,
795+
// symbol can be referenced at locations where type is allowed
796+
return forEach(exportedSymbols, symbolCanbeReferencedAtTypeLocation);
797+
}
798+
799+
return !!(symbol.flags & (SymbolFlags.NamespaceModule | SymbolFlags.Type));
800+
}
801+
716802
/**
717803
* Finds the first node that "embraces" the position, so that one may
718804
* accurately aggregate locals from the closest containing scope.
@@ -1140,21 +1226,6 @@ namespace ts.Completions {
11401226
return undefined;
11411227
}
11421228

1143-
function isFunction(kind: SyntaxKind): boolean {
1144-
if (!isFunctionLikeKind(kind)) {
1145-
return false;
1146-
}
1147-
1148-
switch (kind) {
1149-
case SyntaxKind.Constructor:
1150-
case SyntaxKind.ConstructorType:
1151-
case SyntaxKind.FunctionType:
1152-
return false;
1153-
default:
1154-
return true;
1155-
}
1156-
}
1157-
11581229
/**
11591230
* @returns true if we are certain that the currently edited location must define a new location; false otherwise.
11601231
*/
@@ -1166,7 +1237,7 @@ namespace ts.Completions {
11661237
containingNodeKind === SyntaxKind.VariableDeclarationList ||
11671238
containingNodeKind === SyntaxKind.VariableStatement ||
11681239
containingNodeKind === SyntaxKind.EnumDeclaration || // enum a { foo, |
1169-
isFunction(containingNodeKind) ||
1240+
isFunctionLikeButNotConstructor(containingNodeKind) ||
11701241
containingNodeKind === SyntaxKind.ClassDeclaration || // class A<T, |
11711242
containingNodeKind === SyntaxKind.ClassExpression || // var C = class D<T, |
11721243
containingNodeKind === SyntaxKind.InterfaceDeclaration || // interface A<T, |
@@ -1184,7 +1255,7 @@ namespace ts.Completions {
11841255

11851256
case SyntaxKind.OpenParenToken:
11861257
return containingNodeKind === SyntaxKind.CatchClause ||
1187-
isFunction(containingNodeKind);
1258+
isFunctionLikeButNotConstructor(containingNodeKind);
11881259

11891260
case SyntaxKind.OpenBraceToken:
11901261
return containingNodeKind === SyntaxKind.EnumDeclaration || // enum a { |
@@ -1202,7 +1273,7 @@ namespace ts.Completions {
12021273
containingNodeKind === SyntaxKind.ClassExpression || // var C = class D< |
12031274
containingNodeKind === SyntaxKind.InterfaceDeclaration || // interface A< |
12041275
containingNodeKind === SyntaxKind.TypeAliasDeclaration || // type List< |
1205-
isFunction(containingNodeKind);
1276+
isFunctionLikeKind(containingNodeKind);
12061277

12071278
case SyntaxKind.StaticKeyword:
12081279
return containingNodeKind === SyntaxKind.PropertyDeclaration && !isClassLike(contextToken.parent.parent);
@@ -1271,6 +1342,10 @@ namespace ts.Completions {
12711342
return false;
12721343
}
12731344

1345+
function isFunctionLikeButNotConstructor(kind: SyntaxKind) {
1346+
return isFunctionLikeKind(kind) && kind !== SyntaxKind.Constructor;
1347+
}
1348+
12741349
function isDotOfNumericLiteral(contextToken: Node): boolean {
12751350
if (contextToken.kind === SyntaxKind.NumericLiteral) {
12761351
const text = contextToken.getFullText();

‎src/services/utilities.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ namespace ts {
8282
else if (node.parent.kind === SyntaxKind.ExportAssignment) {
8383
return SemanticMeaning.All;
8484
}
85-
else if (isInRightSideOfImport(node)) {
85+
else if (isInRightSideOfInternalImportEqualsDeclaration(node)) {
8686
return getMeaningFromRightHandSideOfImportEquals(node);
8787
}
8888
else if (isDeclarationName(node)) {
@@ -118,7 +118,7 @@ namespace ts {
118118
return SemanticMeaning.Namespace;
119119
}
120120

121-
function isInRightSideOfImport(node: Node) {
121+
export function isInRightSideOfInternalImportEqualsDeclaration(node: Node) {
122122
while (node.parent.kind === SyntaxKind.QualifiedName) {
123123
node = node.parent;
124124
}

‎tests/cases/fourslash/commentsInheritance.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ verify.quickInfos({
346346
});
347347

348348
goTo.marker('16');
349-
verify.completionListContains("i1", "interface i1", "i1 is interface with properties");
349+
verify.not.completionListContains("i1", "interface i1", "i1 is interface with properties");
350350
verify.completionListContains("i1_i", "var i1_i: i1", "");
351351
verify.completionListContains("c1", "class c1", "");
352352
verify.completionListContains("c1_i", "var c1_i: c1", "");
@@ -603,9 +603,9 @@ verify.quickInfos({
603603
});
604604

605605
goTo.marker('51');
606-
verify.completionListContains("i2", "interface i2", "");
606+
verify.not.completionListContains("i2", "interface i2", "");
607607
verify.completionListContains("i2_i", "var i2_i: i2", "");
608-
verify.completionListContains("i3", "interface i3", "");
608+
verify.not.completionListContains("i3", "interface i3", "");
609609
verify.completionListContains("i3_i", "var i3_i: i3", "");
610610

611611
goTo.marker('51i');

‎tests/cases/fourslash/commentsInterface.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,11 +163,11 @@ verify.currentParameterHelpArgumentDocCommentIs("");
163163
verify.quickInfoAt("33q", "(method) i2.nc_fnfoo(b: number): string");
164164

165165
goTo.marker('34');
166-
verify.completionListContains("i1", "interface i1", "this is interface 1");
166+
verify.not.completionListContains("i1", "interface i1", "this is interface 1");
167167
verify.completionListContains("i1_i", "var i1_i: i1", "");
168-
verify.completionListContains("nc_i1", "interface nc_i1", "");
168+
verify.not.completionListContains("nc_i1", "interface nc_i1", "");
169169
verify.completionListContains("nc_i1_i", "var nc_i1_i: nc_i1", "");
170-
verify.completionListContains("i2", "interface i2", "this is interface 2 with memebers");
170+
verify.not.completionListContains("i2", "interface i2", "this is interface 2 with memebers");
171171
verify.completionListContains("i2_i", "var i2_i: i2", "");
172172
verify.completionListContains("i2_i_x", "var i2_i_x: number", "");
173173
verify.completionListContains("i2_i_foo", "var i2_i_foo: (b: number) => string", "");
@@ -194,7 +194,7 @@ verify.completionListContains("a", "(parameter) a: number", "i3_i a");
194194

195195
verify.quickInfoAt("40q", "var i3_i: i3");
196196
goTo.marker('40');
197-
verify.completionListContains("i3", "interface i3", "");
197+
verify.not.completionListContains("i3", "interface i3", "");
198198
verify.completionListContains("i3_i", "var i3_i: i3", "");
199199

200200
goTo.marker('41');

‎tests/cases/fourslash/commentsOverloads.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -295,13 +295,13 @@ verify.completionListContains('f3', 'function f3(a: number): number (+1 overload
295295
verify.completionListContains('f4', 'function f4(a: number): number (+1 overload)', 'this is signature 4 - with number parameter');
296296

297297
goTo.marker('18');
298-
verify.completionListContains('i1', 'interface i1', '');
298+
verify.not.completionListContains('i1', 'interface i1', '');
299299
verify.completionListContains('i1_i', 'var i1_i: new i1(b: number) => any (+1 overload)', '');
300-
verify.completionListContains('i2', 'interface i2', '');
300+
verify.not.completionListContains('i2', 'interface i2', '');
301301
verify.completionListContains('i2_i', 'var i2_i: new i2(a: string) => any (+1 overload)', '');
302-
verify.completionListContains('i3', 'interface i3', '');
302+
verify.not.completionListContains('i3', 'interface i3', '');
303303
verify.completionListContains('i3_i', 'var i3_i: new i3(a: string) => any (+1 overload)', 'new 1');
304-
verify.completionListContains('i4', 'interface i4', '');
304+
verify.not.completionListContains('i4', 'interface i4', '');
305305
verify.completionListContains('i4_i', 'var i4_i: new i4(a: string) => any (+1 overload)', '');
306306

307307
goTo.marker('19');

‎tests/cases/fourslash/completionListAfterFunction2.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@ goTo.marker("1");
99
verify.not.completionListContains("a");
1010

1111
goTo.marker("2");
12+
verify.not.completionListContains("b");
13+
edit.insert("typeof ");
1214
verify.completionListContains("b");
13-

‎tests/cases/fourslash/completionListCladule.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
//// export var x: number;
99
////}
1010
////Foo/*c1*/; // should get "x", "prototype"
11-
////var s: Foo/*c2*/; // should get "x" and "prototype"
11+
////var s: Foo/*c2*/; // no types, in Foo, so shouldnt have anything
1212
////var f = new Foo();
1313
////f/*c3*/;
1414

@@ -20,9 +20,7 @@ verify.completionListContains("staticMethod");
2020

2121
goTo.marker("c2");
2222
edit.insert(".");
23-
verify.completionListContains("x");
24-
verify.completionListContains("staticMethod");
25-
verify.completionListContains("prototype");
23+
verify.completionListIsEmpty();
2624

2725
goTo.marker("c3");
2826
edit.insert(".");

‎tests/cases/fourslash/completionListInClassExpressionWithTypeParameter.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
1-
///<reference path="fourslash.ts" />
1+
///<reference path="fourslash.ts" />
22

33
//// var x = class myClass <TypeParam> {
44
//// getClassName (){
55
//// /*0*/
6+
//// var tmp: /*0Type*/;
67
//// }
78
//// prop: Ty/*1*/
89
//// }
910

1011
goTo.marker("0");
12+
verify.not.completionListContains("TypeParam", "(type parameter) TypeParam in myClass<TypeParam>", /*documentation*/ undefined, "type parameter");
13+
goTo.marker("0Type");
1114
verify.completionListContains("TypeParam", "(type parameter) TypeParam in myClass<TypeParam>", /*documentation*/ undefined, "type parameter");
1215

1316
goTo.marker("1");

0 commit comments

Comments
 (0)