Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(language-service): infer the type of array-like element. #29811

Closed
wants to merge 3 commits into from

Conversation

@ivanwonder
Copy link
Contributor

ivanwonder commented Apr 10, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

const test: { {[key: string]: number};

current behavior of test.test will report a error;

What is the new behavior?

new behaviour will infer the type number;

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@ivanwonder ivanwonder requested review from angular/fw-compiler as code owners Apr 10, 2019
@googlebot googlebot added the cla: yes label Apr 10, 2019
ivanwonder added 2 commits Apr 10, 2019
@kyliau
kyliau approved these changes Apr 10, 2019
Copy link
Member

kyliau left a comment

LGTM for language service. Thank you for fixing this!

@ayazhafiz

This comment has been minimized.

Copy link
Member

ayazhafiz commented Nov 16, 2019

fixed by 53fc2ed

@ayazhafiz ayazhafiz closed this Nov 16, 2019
@andrius-pra

This comment has been minimized.

Copy link
Contributor

andrius-pra commented Nov 17, 2019

@ayazhafiz, 53fc2ed doesn't cover all functionality from this pull request. I still get Identifier 'firstName' is not defined. '__type' does not contain such a member error message in this example:

import { Component, OnInit } from '@angular/core';
import { FormGroup, FormControl } from '@angular/forms';

@Component({
  selector: 'app-simple-form',
  template: `{{ profileForm.controls.firstName.valid }}`
})
export class SimpleFormComponent {
  profileForm = new FormGroup({
    firstName: new FormControl(''),
    lastName: new FormControl(''),
  });
}
@ayazhafiz

This comment has been minimized.

Copy link
Member

ayazhafiz commented Nov 17, 2019

@andrius-pra Yes, you're totally right.. but the functionality of indexing a string index via dot notation is a little bit different from the index signature functionality. We will need to change how SymbolTableWrapper#get works, as @ivanwonder did in this PR. This can be done pretty easily; I will make a PR unless someone else is interested.

ayazhafiz added a commit to ayazhafiz/angular that referenced this pull request Nov 17, 2019
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
@ivanwonder

This comment has been minimized.

Copy link
Contributor Author

ivanwonder commented Nov 18, 2019

@ayazhafiz have a plan to support this feature?

testTwo: [string, number];

I add a comment here.
* sometimes we need the key of arguments to get the type of the expression(type Example =

@ayazhafiz

This comment has been minimized.

Copy link
Member

ayazhafiz commented Nov 18, 2019

No, it really appears that we closed this PR too soon; sorry about that - we thought it was just concerned with objects having index signatures. Would you like to create a new PR to implement the tuple functionality @ivanwonder?

@ivanwonder

This comment has been minimized.

Copy link
Contributor Author

ivanwonder commented Nov 18, 2019

@andrius-pra Yes, you're totally right.. but the functionality of indexing a string index via dot notation is a little bit different from the index signature functionality. We will need to change how SymbolTableWrapper#get works, as @ivanwonder did in this PR. This can be done pretty easily; I will make a PR unless someone else is interested.

I extend the SymbolTableWrapper because I don't want to add a new function parameter.
https://github.com/ayazhafiz/angular/blob/0fd03938fd006fe1e7e24eb38d4eb756513948ef/packages/language-service/src/typescript_symbols.ts#L459
and the symbols can be got from the type. and It’s a bit strange to me. This is why i add a new classSymbolTableAndStringIndexTypeWrapper

@ivanwonder

This comment has been minimized.

Copy link
Contributor Author

ivanwonder commented Nov 18, 2019

No, it really appears that we closed this PR too soon; sorry about that - we thought it was just concerned with objects having index signatures. Would you like to create a new PR to implement the tuple functionality @ivanwonder?

If I have time, I will try.

ayazhafiz added a commit to ayazhafiz/angular that referenced this pull request Nov 26, 2019
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
mhevery added a commit that referenced this pull request Nov 27, 2019
…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
mhevery added a commit that referenced this pull request Nov 27, 2019
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.