Skip to content

Commit 2799ffa

Browse files
mnsaglammnsaglam
committed
Ensure method calls on unions get accurate types
- Ensure that Array, ArrayBuffer, ReadonlyArray, String, TypedArray have consistent slice() signatures so method calls over the union gets typed correctly. - Ensure that BigInt.prototype.toString and Number.prototype.toString are typed consistently so (bigint|number).toString() gets inferred correctly. - Add tests for indexOf, includes, slice, toString so that the signatures of these cognate methods do not drift by accident. Note: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/slice claims that String.prototype.slice() first argument is required at the beginning of the doc, but later admits "If indexStart is omitted, undefined, or cannot be converted to a number, it's treated as 0." Making both params of String.prototype.slice is consistent with the runtime and common TypeScript typings.
1 parent 79f2de5 commit 2799ffa

7 files changed

Lines changed: 278 additions & 31 deletions

File tree

‎externs/es3.js‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -676,8 +676,8 @@ ReadonlyArray.prototype.join = function(opt_separator) {};
676676
/**
677677
* Extracts a section of an array and returns a new array.
678678
*
679-
* @param {?number=} begin Zero-based index at which to begin extraction.
680-
* @param {?number=} end Zero-based index at which to end extraction. slice
679+
* @param {number=} begin Zero-based index at which to begin extraction.
680+
* @param {number=} end Zero-based index at which to end extraction. slice
681681
* extracts up to but not including end.
682682
* @return {!Array<T>}
683683
* @this {IArrayLike<T>|string}
@@ -906,8 +906,8 @@ Array.prototype.shift = function() {};
906906
/**
907907
* Extracts a section of an array and returns a new array.
908908
*
909-
* @param {?number=} begin Zero-based index at which to begin extraction.
910-
* @param {?number=} end Zero-based index at which to end extraction. slice
909+
* @param {number=} begin Zero-based index at which to begin extraction.
910+
* @param {number=} end Zero-based index at which to end extraction. slice
911911
* extracts up to but not including end.
912912
* @return {!Array<T>}
913913
* @this {IArrayLike<T>|string}
@@ -1188,13 +1188,13 @@ Number.prototype.toPrecision = function(opt_precision) {};
11881188
/**
11891189
* Returns a string representing the number.
11901190
* @this {Number|number}
1191-
* @param {(number|Number)=} opt_radix An optional radix.
1191+
* @param {number=} radix An optional radix.
11921192
* @return {string}
11931193
* @nosideeffects
11941194
* @see http://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toString
11951195
* @override
11961196
*/
1197-
Number.prototype.toString = function(opt_radix) {};
1197+
Number.prototype.toString = function(radix) {};
11981198

11991199
// Properties.
12001200
/**
@@ -2178,13 +2178,13 @@ String.prototype.search = function(pattern) {};
21782178

21792179
/**
21802180
* @this {String|string}
2181-
* @param {number} begin
2182-
* @param {number=} opt_end
2181+
* @param {number=} begin
2182+
* @param {number=} end
21832183
* @return {string}
21842184
* @nosideeffects
21852185
* @see http://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/slice
21862186
*/
2187-
String.prototype.slice = function(begin, opt_end) {};
2187+
String.prototype.slice = function(begin, end) {};
21882188

21892189
/**
21902190
* @this {String|string}

‎externs/es6.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -389,12 +389,12 @@ function ArrayBuffer(length) {}
389389
ArrayBuffer.prototype.byteLength;
390390

391391
/**
392-
* @param {number} begin
393-
* @param {number=} opt_end
392+
* @param {number=} begin
393+
* @param {number=} end
394394
* @return {!ArrayBuffer}
395395
* @nosideeffects
396396
*/
397-
ArrayBuffer.prototype.slice = function(begin, opt_end) {};
397+
ArrayBuffer.prototype.slice = function(begin, end) {};
398398

399399
/**
400400
* @param {*} arg

‎src/com/google/javascript/jscomp/testing/TestExternsBuilder.java‎

Lines changed: 156 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,87 @@ function BigInt(arg) {}
183183
BigInt.prototype.valueOf = function() {};
184184
""";
185185

186+
private static final String NUMBER_EXTERNS =
187+
"""
188+
/**
189+
* @constructor
190+
* @param {*=} arg
191+
* @return {number}
192+
*/
193+
function Number(arg) {}
194+
195+
/**
196+
* @this {Number|number}
197+
* @param {number=} radix
198+
* @return {string}
199+
*/
200+
Number.prototype.toString = function(radix) {};
201+
202+
/**
203+
* @return {number}
204+
*/
205+
Number.prototype.valueOf = function() {};
206+
""";
207+
208+
private static final String UINT8ARRAY_EXTERNS =
209+
"""
210+
/**
211+
* @constructor
212+
* @implements {IArrayLike<number>}
213+
* @param {*=} arg
214+
* @param {number=} opt_byteOffset
215+
* @param {number=} opt_length
216+
*/
217+
function Uint8Array(arg, opt_byteOffset, opt_length) {}
218+
219+
/** @type {number} */
220+
Uint8Array.prototype.length;
221+
222+
/**
223+
* @param {number=} begin
224+
* @param {number=} end
225+
* @return {!Uint8Array}
226+
* @nosideeffects
227+
*/
228+
Uint8Array.prototype.slice = function(begin, end) {};
229+
230+
/**
231+
* @param {number} searchElement
232+
* @param {number=} opt_fromIndex
233+
* @return {number}
234+
* @nosideeffects
235+
*/
236+
Uint8Array.prototype.indexOf = function(searchElement, opt_fromIndex) {};
237+
238+
/**
239+
* @param {number} searchElement
240+
* @param {number=} opt_fromIndex
241+
* @return {boolean}
242+
* @nosideeffects
243+
*/
244+
Uint8Array.prototype.includes = function(searchElement, opt_fromIndex) {};
245+
""";
246+
247+
private static final String ARRAYBUFFER_EXTERNS =
248+
"""
249+
/**
250+
* @constructor
251+
* @param {number} length
252+
*/
253+
function ArrayBuffer(length) {}
254+
255+
/** @type {number} */
256+
ArrayBuffer.prototype.byteLength;
257+
258+
/**
259+
* @param {number=} begin
260+
* @param {number=} end
261+
* @return {!ArrayBuffer}
262+
* @nosideeffects
263+
*/
264+
ArrayBuffer.prototype.slice = function(begin, end) {};
265+
""";
266+
186267
private static final String ITERABLE_EXTERNS =
187268
"""
188269
// Symbol is needed for Symbol.iterator
@@ -274,17 +355,22 @@ function String(arg) {}
274355
String.prototype[Symbol.iterator] = function() {};
275356
/** @type {number} */
276357
String.prototype.length;
277-
/** @param {number} sliceArg */
278-
String.prototype.slice = function(sliceArg) {};
279358
/**
280-
* @this {string|!String}
359+
* @this {!String|string}
360+
* @param {number=} begin
361+
* @param {number=} end
362+
* @return {string}
363+
*/
364+
String.prototype.slice = function(begin, end) {};
365+
/**
366+
* @this {!String|string}
281367
* @param {*=} opt_separator
282368
* @param {number=} opt_limit
283369
* @return {!Array<string>}
284370
*/
285371
String.prototype.split = function(opt_separator, opt_limit) {};
286372
/**
287-
* @this {string|!String}
373+
* @this {!String|string}
288374
* @param {string} search_string
289375
* @param {number=} opt_position
290376
* @return {boolean}
@@ -305,6 +391,12 @@ function String(arg) {}
305391
* @return {string}
306392
*/
307393
String.prototype.charAt = function(index) {};
394+
/**
395+
* @this {!String|string}
396+
* @param {...*} var_args
397+
* @return {string}
398+
*/
399+
String.prototype.concat = function(var_args) {};
308400
/**
309401
* @this {!String|string}
310402
* @param {*} regexp
@@ -316,22 +408,27 @@ function String(arg) {}
316408
* @return {string}
317409
*/
318410
String.prototype.toLowerCase = function() {};
319-
320411
/**
321412
* @param {number} count
322413
* @this {!String|string}
323414
* @return {string}
324415
* @nosideeffects
325416
*/
326417
String.prototype.repeat = function(count) {};
327-
328418
/**
329419
* @param {string} searchString
330420
* @param {number=} position
331421
* @return {boolean}
332422
* @nosideeffects
333423
*/
334424
String.prototype.includes = function(searchString, position) {};
425+
/**
426+
* @this {!String|string}
427+
* @param {string|null} searchValue
428+
* @param {(number|null)=} fromIndex
429+
* @return {number}
430+
*/
431+
String.prototype.indexOf = function(searchValue, fromIndex) {};
335432
""";
336433
private static final String FUNCTION_EXTERNS =
337434
"""
@@ -521,21 +618,27 @@ function ReadonlyArray(var_args) {}
521618
*/
522619
ReadonlyArray.prototype.concat;
523620
/**
524-
* @param {?number=} begin Zero-based index at which to begin extraction.
525-
* @param {?number=} end Zero-based index at which to end extraction. slice
621+
* @param {number=} begin Zero-based index at which to begin extraction.
622+
* @param {number=} end Zero-based index at which to end extraction. slice
526623
* extracts up to but not including end.
527624
* @return {!Array<T>}
528625
* @this {!IArrayLike<T>|string}
529626
* @template T
530627
* @nosideeffects
531628
*/
532629
ReadonlyArray.prototype.slice;
533-
630+
/**
631+
* @param {T} elem
632+
* @param {number=} fromIndex
633+
* @return {number}
634+
* @this {!IArrayLike<T>|string}
635+
* @template T
636+
*/
637+
ReadonlyArray.prototype.indexOf = function(elem, fromIndex) {};
534638
/**
535639
* @return {!IteratorIterable<T>}
536640
*/
537641
ReadonlyArray.prototype.values;
538-
539642
/**
540643
* @param {T} searchElement
541644
* @param {number=} fromIndex
@@ -614,8 +717,8 @@ function Array(var_args) {}
614717
Array.prototype.concat = function(var_args) {};
615718
/**
616719
* @override
617-
* @param {?number=} begin Zero-based index at which to begin extraction.
618-
* @param {?number=} end Zero-based index at which to end extraction. slice
720+
* @param {number=} begin Zero-based index at which to begin extraction.
721+
* @param {number=} end Zero-based index at which to end extraction. slice
619722
* extracts up to but not including end.
620723
* @return {!Array<T>}
621724
* @this {!IArrayLike<T>|string}
@@ -640,6 +743,16 @@ function Array(var_args) {}
640743
* @nosideeffects
641744
*/
642745
Array.prototype.includes = function(searchElement, fromIndex) {};
746+
747+
/**
748+
* @param {T} elem
749+
* @param {number=} fromIndex
750+
* @return {number}
751+
* @this {!IArrayLike<T>|string}
752+
* @template T
753+
* @override
754+
*/
755+
Array.prototype.indexOf = function(elem, fromIndex) {};
643756
""";
644757
private static final String MAP_EXTERNS =
645758
"""
@@ -1060,6 +1173,9 @@ function Polymer_LegacyElementMixin(){}
10601173
""";
10611174

10621175
private boolean includeBigIntExterns = false;
1176+
private boolean includeNumberExterns = false;
1177+
private boolean includeUint8ArrayExterns = false;
1178+
private boolean includeArrayBufferExterns = false;
10631179
private boolean includeIterableExterns = false;
10641180
private boolean includeStringExterns = false;
10651181
private boolean includeFunctionExterns = false;
@@ -1091,6 +1207,25 @@ public TestExternsBuilder addBigInt() {
10911207
return this;
10921208
}
10931209

1210+
@CanIgnoreReturnValue
1211+
public TestExternsBuilder addNumber() {
1212+
includeNumberExterns = true;
1213+
return this;
1214+
}
1215+
1216+
@CanIgnoreReturnValue
1217+
public TestExternsBuilder addUint8Array() {
1218+
includeUint8ArrayExterns = true;
1219+
addArray(); // Uint8Array implements IArrayLike<number>
1220+
return this;
1221+
}
1222+
1223+
@CanIgnoreReturnValue
1224+
public TestExternsBuilder addArrayBuffer() {
1225+
includeArrayBufferExterns = true;
1226+
return this;
1227+
}
1228+
10941229
@CanIgnoreReturnValue
10951230
public TestExternsBuilder addIterable() {
10961231
includeIterableExterns = true;
@@ -1273,6 +1408,9 @@ public String build() {
12731408
if (includeBigIntExterns) {
12741409
externSections.add(BIGINT_EXTERNS);
12751410
}
1411+
if (includeNumberExterns) {
1412+
externSections.add(NUMBER_EXTERNS);
1413+
}
12761414
if (includeIterableExterns) {
12771415
externSections.add(ITERABLE_EXTERNS);
12781416
}
@@ -1288,6 +1426,12 @@ public String build() {
12881426
if (includeArrayExterns) {
12891427
externSections.add(ARRAY_EXTERNS);
12901428
}
1429+
if (includeUint8ArrayExterns) {
1430+
externSections.add(UINT8ARRAY_EXTERNS);
1431+
}
1432+
if (includeArrayBufferExterns) {
1433+
externSections.add(ARRAYBUFFER_EXTERNS);
1434+
}
12911435
if (includeMapExterns) {
12921436
externSections.add(MAP_EXTERNS);
12931437
}

‎test/com/google/javascript/jscomp/PeepholeReplaceKnownMethodsTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -764,13 +764,13 @@ public void testReplaceWithCharAt() {
764764
foldSame(
765765
"""
766766
/** @constructor */ function A() {};
767-
A.prototype.substring = function(begin$jscomp$1, end$jscomp$1) {};
767+
A.prototype.substring = function(begin$jscomp$2, end$jscomp$2) {};
768768
function f(/** !A */ a) { a.substring(0, 1); }
769769
""");
770770
foldSame(
771771
"""
772772
/** @constructor */ function A() {};
773-
A.prototype.slice = function(begin$jscomp$1, end$jscomp$1) {};
773+
A.prototype.slice = function(begin$jscomp$2, end$jscomp$2) {};
774774
function f(/** !A */ a) { a.slice(0, 1); }
775775
""");
776776

‎test/com/google/javascript/jscomp/SymbolTableTest.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1626,12 +1626,15 @@ public void testSymbolForScopeOfNatives() {
16261626
SymbolTable table = createSymbolTableWithDefaultExterns("");
16271627

16281628
// From the externs.
1629-
Symbol sliceArg = getLocalVar(table, "sliceArg");
1630-
assertThat(sliceArg).isNotNull();
1629+
Symbol slice = getGlobalVar(table, "String.prototype.slice");
1630+
assertThat(slice).isNotNull();
16311631

1632-
Symbol scope = table.getSymbolForScope(table.getScope(sliceArg));
1632+
Symbol begin = table.getParameterInFunction(slice, "begin");
1633+
assertThat(begin).isNotNull();
1634+
1635+
Symbol scope = table.getSymbolForScope(table.getScope(begin));
16331636
assertThat(scope).isNotNull();
1634-
assertThat(getGlobalVar(table, "String.prototype.slice")).isEqualTo(scope);
1637+
assertThat(slice).isEqualTo(scope);
16351638

16361639
Symbol proto = getGlobalVar(table, "String.prototype");
16371640
assertThat(proto.getDeclaration().getNode().getSourceFileName()).isEqualTo("externs1");

‎test/com/google/javascript/jscomp/TypeCheckCovarianceTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1462,7 +1462,7 @@ function MyArray() {}
14621462
found : MyArray<number,number>
14631463
required: ReadonlyArray<*>
14641464
missing : []
1465-
mismatch: [includes,takes]
1465+
mismatch: [includes,indexOf,takes]
14661466
""")
14671467
.addDiagnostic(
14681468
// this diagnostic is correct - ReadonlyArray<number>.returns() returns a number, while

0 commit comments

Comments
 (0)