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

Fixes for type parameter name resolution in JS #26830

Merged
merged 3 commits into from
Sep 4, 2018

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Aug 31, 2018

1. check for expando initializers in resolveEntityName

... when resolving type parameters in a prototype property assignment
declaration. For example, this already works:

/** @template T */
function f(x) { this.x = x }
/** @returns {T} */
f.protototype.m = function () { return this.x }

This now works too:

/** @template T */
var f = function (x) { this.x = x }
/** @returns {T} */
f.prototype.m = function () { return this.x }

Fixes #26826

2. Lookup type parameters on prototype-assignment methods

…In the same way that they're looked up on prototype-property methods.

That is, this previously worked:

/** @template T */
function f() { }
/** @param {T} p */
f.prototype.m = function () { }

And this now works too:

/** @template T */
function f() { }
f.prototype = {
  /** @param {T} p */
  m() { }
}

Note that the baselines still have errors; I filed #26831 to follow up on those.

Fixes #24230

when resolving type parameters in a prototype property assignment
declaration. For example, this already works:

```js
/** @template T */
function f(x) { this.x = x }
/** @returns {T} */
f.protototype.m = function () { return this.x }
```

This now works too:

```js
/** @template T */
var f = function (x) { this.x = x }
/** @returns {T} */
f.prototype.m = function () { return this.x }
```

Fixes #26826
In the same way that they're looked up on prototype-property methods.

That is, this previously worked:

```js
/** @template T */
function f() { }
/** @param {T} p */
f.prototype.m = function () { }
```

And this now works too:

```js
/** @template T */
function f() { }
f.prototype = {
  /** @param {T} p */
  m() { }
}
```

Note that the baselines still have errors; I'll file a followup bug for
them.
@sandersn sandersn requested review from RyanCavanaugh and a user August 31, 2018 23:22
@ghost
Copy link

ghost commented Sep 4, 2018

This isn't working when calling the method:

/**
 * Should work for function declarations
 * @constructor
 * @template {string} K
 * @template V
 */
function Multimap() {
    /** @type {Object<string, V>} TODO: Remove the prototype from the fresh object */
    this._map = {};
};
 /**
 * @param {K} key the key ok
 * @returns {V} the value ok
 */
Multimap.prototype.get = function (key) {
    return this._map[key + ''];
}

/** @type {Multimap<"a" | "b", number>} */
const map = new Multimap();
const n = map.get("a");

At the last line, there's an error that "a" isn't assignable to K, and n is of type V instead of number.

* @param {K} key the key ok
* @returns {V} the value ok
*/
get(key) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only works for methods, not properties with function values. That is, get: function(key) ... doesn't work

@sandersn
Copy link
Member Author

sandersn commented Sep 4, 2018

@andy-ms Oof, yeah, that's a bigger limitation of the types we create for JSDoc generics. They're not really generics, and aren't instantiated using the normal instantiation code. That's not related to this fix — you'll see that the type is always just Multimap, although it means that the overall scenario doesn't work. Given that I'll be on vacation and DefinitelyTyped most of the rest of this release, I'm going to create a separate issue to track fixing generic types.

(Note that the 3.0 behaviour is that instances of Multimap are completely unaware of type parameters, so that n: any in your example.)

Filed #26883 to track this.

@sandersn sandersn merged commit 64ac5a5 into master Sep 4, 2018
@sandersn sandersn deleted the js/fix-type-parameter-lookup branch September 4, 2018 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant