Skip to content

Commit

Permalink
fix(language-service): determine index types accessed using dot notat…
Browse files Browse the repository at this point in the history 
…ion (#33884)

Commit 53fc2ed added support for
determining index types accessed using index signatures, but did not
include support for index types accessed using dot notation:

```typescript
const obj<T>: { [key: string]: T };
obj['stringKey']. // gets `T.` completions
obj.stringKey.    // did not peviously get `T.` completions
```

This adds support for determining an index type accessed via dot
notation by rigging an object's symbol table to return the string index
signature type a property access refers to, if that property does not
explicitly exist on the object. This is very similar to @ivanwonder's
work in #29811.

`SymbolWrapper` now takes an additional parameter to explicitly set the
type of the symbol wrapped. This is done because
`SymbolTableWrapper#get` only has access to the symbol of the index
type, _not_ the index signature symbol itself. An attempt to get the
type of the index type will give an error.

Closes #29811
Closes angular/vscode-ng-language-service#126

PR Close #33884
  • Loading branch information
@ayazhafiz @mhevery
ayazhafiz authored and mhevery committed Nov 26, 2019
1 parent 6d7d2e4 commit 49804fe
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 28 deletions.
56 changes: 45 additions & 11 deletions packages/language-service/src/typescript_symbols.ts
View file
Expand Up @@ -275,7 +275,7 @@ class TypeWrapper implements Symbol {
// the former includes properties on the base class whereas the latter does
// not. This provides properties like .bind(), .call(), .apply(), etc for
// functions.
return new SymbolTableWrapper(this.tsType.getApparentProperties(), this.context);
return new SymbolTableWrapper(this.tsType.getApparentProperties(), this.context, this.tsType);
}

signatures(): Signature[] { return signaturesOf(this.tsType, this.context); }
Expand All @@ -302,15 +302,18 @@ class TypeWrapper implements Symbol {

class SymbolWrapper implements Symbol {
private symbol: ts.Symbol;
// TODO(issue/24571): remove '!'.
private _tsType !: ts.Type;
// TODO(issue/24571): remove '!'.
private _members !: SymbolTable;
private _members?: SymbolTable;

public readonly nullable: boolean = false;
public readonly language: string = 'typescript';

constructor(symbol: ts.Symbol, private context: TypeContext) {
constructor(
symbol: ts.Symbol,
/** TypeScript type context of the symbol. */
private context: TypeContext,
/** Type of the TypeScript symbol, if known. If not provided, the type of the symbol
* will be determined dynamically; see `SymbolWrapper#tsType`. */
private _tsType?: ts.Type) {
this.symbol = symbol && context && (symbol.flags & ts.SymbolFlags.Alias) ?
context.checker.getAliasedSymbol(symbol) :
symbol;
Expand Down Expand Up @@ -340,7 +343,7 @@ class SymbolWrapper implements Symbol {
const typeWrapper = new TypeWrapper(declaredType, this.context);
this._members = typeWrapper.members();
} else {
this._members = new SymbolTableWrapper(this.symbol.members !, this.context);
this._members = new SymbolTableWrapper(this.symbol.members !, this.context, this.tsType);
}
}
return this._members;
Expand Down Expand Up @@ -454,8 +457,16 @@ function toSymbols(symbolTable: ts.SymbolTable | undefined): ts.Symbol[] {
class SymbolTableWrapper implements SymbolTable {
private symbols: ts.Symbol[];
private symbolTable: ts.SymbolTable;

constructor(symbols: ts.SymbolTable|ts.Symbol[]|undefined, private context: TypeContext) {
private stringIndexType?: ts.Type;

/**
* Creates a queryable table of symbols belonging to a TypeScript entity.
* @param symbols symbols to query belonging to the entity
* @param context program context
* @param type original TypeScript type of entity owning the symbols, if known
*/
constructor(
symbols: ts.SymbolTable|ts.Symbol[], private context: TypeContext, private type?: ts.Type) {
symbols = symbols || [];

if (Array.isArray(symbols)) {
Expand All @@ -465,18 +476,41 @@ class SymbolTableWrapper implements SymbolTable {
this.symbols = toSymbols(symbols);
this.symbolTable = symbols;
}

if (type) {
this.stringIndexType = type.getStringIndexType();
}
}

get size(): number { return this.symbols.length; }

get(key: string): Symbol|undefined {
const symbol = getFromSymbolTable(this.symbolTable, key);
return symbol ? new SymbolWrapper(symbol, this.context) : undefined;
if (symbol) {
return new SymbolWrapper(symbol, this.context);
}

if (this.stringIndexType) {
// If the key does not exist as an explicit symbol on the type, it may be accessing a string
// index signature using dot notation:
//
// const obj<T>: { [key: string]: T };
// obj.stringIndex // equivalent to obj['stringIndex'];
//
// In this case, return the type indexed by an arbitrary string key.
const symbol = this.stringIndexType.getSymbol();
if (symbol) {
return new SymbolWrapper(symbol, this.context, this.stringIndexType);
}
}

return undefined;
}

has(key: string): boolean {
const table: any = this.symbolTable;
return (typeof table.has === 'function') ? table.has(key) : table[key] != null;
return ((typeof table.has === 'function') ? table.has(key) : table[key] != null) ||
this.stringIndexType !== undefined;
}

values(): Symbol[] { return this.symbols.map(s => new SymbolWrapper(s, this.context)); }
Expand Down
33 changes: 22 additions & 11 deletions packages/language-service/test/completions_spec.ts
View file
Expand Up @@ -117,18 +117,29 @@ describe('completions', () => {
expectContain(completions, CompletionKind.PROPERTY, ['id', 'name']);
});

it('should be able to get property completions for members in an array', () => {
mockHost.override(TEST_TEMPLATE, `{{ heroes[0].~{heroes-number-index}}}`);
const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'heroes-number-index');
const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start);
expectContain(completions, CompletionKind.PROPERTY, ['id', 'name']);
});
describe('property completions for members of an indexed type', () => {
it('should work with numeric index signatures (arrays)', () => {
mockHost.override(TEST_TEMPLATE, `{{ heroes[0].~{heroes-number-index}}}`);
const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'heroes-number-index');
const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start);
expectContain(completions, CompletionKind.PROPERTY, ['id', 'name']);
});

it('should be able to get property completions for members in an indexed type', () => {
mockHost.override(TEST_TEMPLATE, `{{ heroesByName['Jacky'].~{heroes-string-index}}}`);
const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'heroes-string-index');
const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start);
expectContain(completions, CompletionKind.PROPERTY, ['id', 'name']);
describe('with string index signatures', () => {
it('should work with index notation', () => {
mockHost.override(TEST_TEMPLATE, `{{ heroesByName['Jacky'].~{heroes-string-index}}}`);
const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'heroes-string-index');
const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start);
expectContain(completions, CompletionKind.PROPERTY, ['id', 'name']);
});

it('should work with dot notation', () => {
mockHost.override(TEST_TEMPLATE, `{{ heroesByName.jacky.~{heroes-string-index}}}`);
const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'heroes-string-index');
const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start);
expectContain(completions, CompletionKind.PROPERTY, ['id', 'name']);
});
});
});

it('should be able to return attribute names with an incompete attribute', () => {
Expand Down
34 changes: 28 additions & 6 deletions packages/language-service/test/diagnostics_spec.ts
View file
Expand Up @@ -119,13 +119,35 @@ describe('diagnostics', () => {
expect(diagnostics).toEqual([]);
});

it('should produce diagnostics for invalid index type property access', () => {
mockHost.override(TEST_TEMPLATE, `
describe('diagnostics for invalid indexed type property access', () => {
it('should work with numeric index signatures (arrays)', () => {
mockHost.override(TEST_TEMPLATE, `
{{heroes[0].badProperty}}`);
const diags = ngLS.getDiagnostics(TEST_TEMPLATE);
expect(diags.length).toBe(1);
expect(diags[0].messageText)
.toBe(`Identifier 'badProperty' is not defined. 'Hero' does not contain such a member`);
const diags = ngLS.getDiagnostics(TEST_TEMPLATE);
expect(diags.length).toBe(1);
expect(diags[0].messageText)
.toBe(`Identifier 'badProperty' is not defined. 'Hero' does not contain such a member`);
});

describe('with string index signatures', () => {
it('should work with index notation', () => {
mockHost.override(TEST_TEMPLATE, `
{{heroesByName['Jacky'].badProperty}}`);
const diags = ngLS.getDiagnostics(TEST_TEMPLATE);
expect(diags.length).toBe(1);
expect(diags[0].messageText)
.toBe(`Identifier 'badProperty' is not defined. 'Hero' does not contain such a member`);
});

it('should work with dot notation', () => {
mockHost.override(TEST_TEMPLATE, `
{{heroesByName.jacky.badProperty}}`);
const diags = ngLS.getDiagnostics(TEST_TEMPLATE);
expect(diags.length).toBe(1);
expect(diags[0].messageText)
.toBe(`Identifier 'badProperty' is not defined. 'Hero' does not contain such a member`);
});
});
});

it('should not produce errors on function.bind()', () => {
Expand Down

0 comments on commit 49804fe

Please sign in to comment.