Skip to content

Ensure method calls on unions get accurate types #4309

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.

Sign up for GitHub

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

Open
KimlikDAO-bot wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Ensure method calls on unions get accurate types #4309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions externs/es3.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -676,8 +676,8 @@ ReadonlyArray.prototype.join = function(opt_separator) {};
/**
* Extracts a section of an array and returns a new array.
*
* @param {?number=} begin Zero-based index at which to begin extraction.
* @param {?number=} end Zero-based index at which to end extraction. slice
* @param {number=} begin Zero-based index at which to begin extraction.
* @param {number=} end Zero-based index at which to end extraction. slice
* extracts up to but not including end.
* @return {!Array<T>}
* @this {IArrayLike<T>|string}
Expand Down Expand Up @@ -906,8 +906,8 @@ Array.prototype.shift = function() {};
/**
* Extracts a section of an array and returns a new array.
*
* @param {?number=} begin Zero-based index at which to begin extraction.
* @param {?number=} end Zero-based index at which to end extraction. slice
* @param {number=} begin Zero-based index at which to begin extraction.
* @param {number=} end Zero-based index at which to end extraction. slice
* extracts up to but not including end.
* @return {!Array<T>}
* @this {IArrayLike<T>|string}
Expand Down Expand Up @@ -1188,13 +1188,13 @@ Number.prototype.toPrecision = function(opt_precision) {};
/**
* Returns a string representing the number.
* @this {Number|number}
* @param {(number|Number)=} opt_radix An optional radix.
* @param {number=} radix An optional radix.
* @return {string}
* @nosideeffects
* @see http://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toString
* @override
*/
Number.prototype.toString = function(opt_radix) {};
Number.prototype.toString = function(radix) {};

// Properties.
/**
Expand Down Expand Up @@ -2178,13 +2178,13 @@ String.prototype.search = function(pattern) {};

/**
* @this {String|string}
* @param {number} begin
* @param {number=} opt_end
* @param {number=} begin
* @param {number=} end
* @return {string}
* @nosideeffects
* @see http://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/slice
*/
String.prototype.slice = function(begin, opt_end) {};
String.prototype.slice = function(begin, end) {};

/**
* @this {String|string}
Expand Down
6 changes: 3 additions & 3 deletions externs/es6.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,12 +389,12 @@ function ArrayBuffer(length) {}
ArrayBuffer.prototype.byteLength;

/**
* @param {number} begin
* @param {number=} opt_end
* @param {number=} begin
* @param {number=} end
* @return {!ArrayBuffer}
* @nosideeffects
*/
ArrayBuffer.prototype.slice = function(begin, opt_end) {};
ArrayBuffer.prototype.slice = function(begin, end) {};

/**
* @param {*} arg
Expand Down
168 changes: 156 additions & 12 deletions src/com/google/javascript/jscomp/testing/TestExternsBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,87 @@ function BigInt(arg) {}
BigInt.prototype.valueOf = function() {};
""";

private static final String NUMBER_EXTERNS =
"""
/**
* @constructor
* @param {*=} arg
* @return {number}
*/
function Number(arg) {}

/**
* @this {Number|number}
* @param {number=} radix
* @return {string}
*/
Number.prototype.toString = function(radix) {};

/**
* @return {number}
*/
Number.prototype.valueOf = function() {};
""";

private static final String UINT8ARRAY_EXTERNS =
"""
/**
* @constructor
* @implements {IArrayLike<number>}
* @param {*=} arg
* @param {number=} opt_byteOffset
* @param {number=} opt_length
*/
function Uint8Array(arg, opt_byteOffset, opt_length) {}

/** @type {number} */
Uint8Array.prototype.length;

/**
* @param {number=} begin
* @param {number=} end
* @return {!Uint8Array}
* @nosideeffects
*/
Uint8Array.prototype.slice = function(begin, end) {};

/**
* @param {number} searchElement
* @param {number=} opt_fromIndex
* @return {number}
* @nosideeffects
*/
Uint8Array.prototype.indexOf = function(searchElement, opt_fromIndex) {};

/**
* @param {number} searchElement
* @param {number=} opt_fromIndex
* @return {boolean}
* @nosideeffects
*/
Uint8Array.prototype.includes = function(searchElement, opt_fromIndex) {};
""";

private static final String ARRAYBUFFER_EXTERNS =
"""
/**
* @constructor
* @param {number} length
*/
function ArrayBuffer(length) {}

/** @type {number} */
ArrayBuffer.prototype.byteLength;

/**
* @param {number=} begin
* @param {number=} end
* @return {!ArrayBuffer}
* @nosideeffects
*/
ArrayBuffer.prototype.slice = function(begin, end) {};
""";

private static final String ITERABLE_EXTERNS =
"""
// Symbol is needed for Symbol.iterator
Expand Down Expand Up @@ -274,17 +355,22 @@ function String(arg) {}
String.prototype[Symbol.iterator] = function() {};
/** @type {number} */
String.prototype.length;
/** @param {number} sliceArg */
String.prototype.slice = function(sliceArg) {};
/**
* @this {string|!String}
* @this {!String|string}
* @param {number=} begin
* @param {number=} end
* @return {string}
*/
String.prototype.slice = function(begin, end) {};
/**
* @this {!String|string}
* @param {*=} opt_separator
* @param {number=} opt_limit
* @return {!Array<string>}
*/
String.prototype.split = function(opt_separator, opt_limit) {};
/**
* @this {string|!String}
* @this {!String|string}
* @param {string} search_string
* @param {number=} opt_position
* @return {boolean}
Expand All @@ -305,6 +391,12 @@ function String(arg) {}
* @return {string}
*/
String.prototype.charAt = function(index) {};
/**
* @this {!String|string}
* @param {...*} var_args
* @return {string}
*/
String.prototype.concat = function(var_args) {};
/**
* @this {!String|string}
* @param {*} regexp
Expand All @@ -316,22 +408,27 @@ function String(arg) {}
* @return {string}
*/
String.prototype.toLowerCase = function() {};

/**
* @param {number} count
* @this {!String|string}
* @return {string}
* @nosideeffects
*/
String.prototype.repeat = function(count) {};

/**
* @param {string} searchString
* @param {number=} position
* @return {boolean}
* @nosideeffects
*/
String.prototype.includes = function(searchString, position) {};
/**
* @this {!String|string}
* @param {string|null} searchValue
* @param {(number|null)=} fromIndex
* @return {number}
*/
String.prototype.indexOf = function(searchValue, fromIndex) {};
""";
private static final String FUNCTION_EXTERNS =
"""
Expand Down Expand Up @@ -521,21 +618,27 @@ function ReadonlyArray(var_args) {}
*/
ReadonlyArray.prototype.concat;
/**
* @param {?number=} begin Zero-based index at which to begin extraction.
* @param {?number=} end Zero-based index at which to end extraction. slice
* @param {number=} begin Zero-based index at which to begin extraction.
* @param {number=} end Zero-based index at which to end extraction. slice
* extracts up to but not including end.
* @return {!Array<T>}
* @this {!IArrayLike<T>|string}
* @template T
* @nosideeffects
*/
ReadonlyArray.prototype.slice;

/**
* @param {T} elem
* @param {number=} fromIndex
* @return {number}
* @this {!IArrayLike<T>|string}
* @template T
*/
ReadonlyArray.prototype.indexOf = function(elem, fromIndex) {};
/**
* @return {!IteratorIterable<T>}
*/
ReadonlyArray.prototype.values;

/**
* @param {T} searchElement
* @param {number=} fromIndex
Expand Down Expand Up @@ -614,8 +717,8 @@ function Array(var_args) {}
Array.prototype.concat = function(var_args) {};
/**
* @override
* @param {?number=} begin Zero-based index at which to begin extraction.
* @param {?number=} end Zero-based index at which to end extraction. slice
* @param {number=} begin Zero-based index at which to begin extraction.
* @param {number=} end Zero-based index at which to end extraction. slice
* extracts up to but not including end.
* @return {!Array<T>}
* @this {!IArrayLike<T>|string}
Expand All @@ -640,6 +743,16 @@ function Array(var_args) {}
* @nosideeffects
*/
Array.prototype.includes = function(searchElement, fromIndex) {};

/**
* @param {T} elem
* @param {number=} fromIndex
* @return {number}
* @this {!IArrayLike<T>|string}
* @template T
* @override
*/
Array.prototype.indexOf = function(elem, fromIndex) {};
""";
private static final String MAP_EXTERNS =
"""
Expand Down Expand Up @@ -1060,6 +1173,9 @@ function Polymer_LegacyElementMixin(){}
""";

private boolean includeBigIntExterns = false;
private boolean includeNumberExterns = false;
private boolean includeUint8ArrayExterns = false;
private boolean includeArrayBufferExterns = false;
private boolean includeIterableExterns = false;
private boolean includeStringExterns = false;
private boolean includeFunctionExterns = false;
Expand Down Expand Up @@ -1091,6 +1207,25 @@ public TestExternsBuilder addBigInt() {
return this;
}

@CanIgnoreReturnValue
public TestExternsBuilder addNumber() {
includeNumberExterns = true;
return this;
}

@CanIgnoreReturnValue
public TestExternsBuilder addUint8Array() {
includeUint8ArrayExterns = true;
addArray(); // Uint8Array implements IArrayLike<number>
return this;
}

@CanIgnoreReturnValue
public TestExternsBuilder addArrayBuffer() {
includeArrayBufferExterns = true;
return this;
}

@CanIgnoreReturnValue
public TestExternsBuilder addIterable() {
includeIterableExterns = true;
Expand Down Expand Up @@ -1273,6 +1408,9 @@ public String build() {
if (includeBigIntExterns) {
externSections.add(BIGINT_EXTERNS);
}
if (includeNumberExterns) {
externSections.add(NUMBER_EXTERNS);
}
if (includeIterableExterns) {
externSections.add(ITERABLE_EXTERNS);
}
Expand All @@ -1288,6 +1426,12 @@ public String build() {
if (includeArrayExterns) {
externSections.add(ARRAY_EXTERNS);
}
if (includeUint8ArrayExterns) {
externSections.add(UINT8ARRAY_EXTERNS);
}
if (includeArrayBufferExterns) {
externSections.add(ARRAYBUFFER_EXTERNS);
}
if (includeMapExterns) {
externSections.add(MAP_EXTERNS);
}
Expand Down
4 changes: 2 additions & 2 deletions test/com/google/javascript/jscomp/PeepholeReplaceKnownMethodsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -764,13 +764,13 @@ public void testReplaceWithCharAt() {
foldSame(
"""
/** @constructor */ function A() {};
A.prototype.substring = function(begin$jscomp$1, end$jscomp$1) {};
A.prototype.substring = function(begin$jscomp$2, end$jscomp$2) {};
function f(/** !A */ a) { a.substring(0, 1); }
""");
foldSame(
"""
/** @constructor */ function A() {};
A.prototype.slice = function(begin$jscomp$1, end$jscomp$1) {};
A.prototype.slice = function(begin$jscomp$2, end$jscomp$2) {};
function f(/** !A */ a) { a.slice(0, 1); }
""");

Expand Down
11 changes: 7 additions & 4 deletions test/com/google/javascript/jscomp/SymbolTableTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1626,12 +1626,15 @@ public void testSymbolForScopeOfNatives() {
SymbolTable table = createSymbolTableWithDefaultExterns("");

// From the externs.
Symbol sliceArg = getLocalVar(table, "sliceArg");
assertThat(sliceArg).isNotNull();
Symbol slice = getGlobalVar(table, "String.prototype.slice");
assertThat(slice).isNotNull();

Symbol scope = table.getSymbolForScope(table.getScope(sliceArg));
Symbol begin = table.getParameterInFunction(slice, "begin");
assertThat(begin).isNotNull();

Symbol scope = table.getSymbolForScope(table.getScope(begin));
assertThat(scope).isNotNull();
assertThat(getGlobalVar(table, "String.prototype.slice")).isEqualTo(scope);
assertThat(slice).isEqualTo(scope);

Symbol proto = getGlobalVar(table, "String.prototype");
assertThat(proto.getDeclaration().getNode().getSourceFileName()).isEqualTo("externs1");
Expand Down
2 changes: 1 addition & 1 deletion test/com/google/javascript/jscomp/TypeCheckCovarianceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1462,7 +1462,7 @@ function MyArray() {}
found : MyArray<number,number>
required: ReadonlyArray<*>
missing : []
mismatch: [includes,takes]
mismatch: [includes,indexOf,takes]
""")
.addDiagnostic(
// this diagnostic is correct - ReadonlyArray<number>.returns() returns a number, while
Expand Down
Loading
⚡