Skip to content

Commit

Permalink
fix(language-service): determine index types accessed using dot notation
Browse files Browse the repository at this point in the history
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 angular#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 angular#29811
Closes angular/vscode-ng-language-service#126
  • Loading branch information
ayazhafiz committed Nov 26, 2019
1 parent 5de7960 commit d737166
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 28 deletions.
50 changes: 39 additions & 11 deletions packages/language-service/src/typescript_symbols.ts
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,12 @@ 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, private context: TypeContext, private _tsType?: ts.Type) {
this.symbol = symbol && context && (symbol.flags & ts.SymbolFlags.Alias) ?
context.checker.getAliasedSymbol(symbol) :
symbol;
Expand Down Expand Up @@ -340,7 +337,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 +451,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 +470,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
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
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 d737166

Please sign in to comment.