microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.27k stars 12.39k forks source link

Typo in `String#match` (lib.es2015.symbol.wellknown) #36299

Open G-Rath opened 4 years ago

G-Rath commented 4 years ago

TypeScript Version: 3.0.0-dev.201xxxxx

Search Terms: typos, string, lib, es2015.symbol.wellknown


I've wiped the majority of the template since this is a typo report. I was originally going to just make a PR but the template had:

  • [ ] There is an associated issue in the Backlog milestone (required)

The typo itself is that String#match in lib.es2015.symbol.wellknown has it's param description pasted in the middle of it's method description:

https://github.com/microsoft/TypeScript/blob/d2c5d54242c69effbed0ebb27033047e0c995589/src/lib/es2015.symbol.wellknown.d.ts#L209-L212

My PR replaced it with "with a regular expression" - Let me know what the correct way to proceed with this is πŸ™‚

G-Rath commented 4 years ago

I've just found a similar typo in the RegExp interface in es5.lib:

https://github.com/microsoft/TypeScript/blob/d2c5d54242c69effbed0ebb27033047e0c995589/src/lib/es5.d.ts#L912-L913

The last part of the comment of the comment doesn't seem relevant:

The regExp argument is a Regular expression object. It can be a variable name or a literal.

My guess is that is the description for the regexp param taken by #match, which reads very similar:

https://github.com/microsoft/TypeScript/blob/d2c5d54242c69effbed0ebb27033047e0c995589/src/lib/es5.d.ts#L416-L420

sebmjoll commented 4 years ago

I have found another comment issue in replace():

/**
 * Replaces text in a string, using an object that supports replacement within a string.
 * @param searchValue A object can search for and replace matches within a string.
 * @param replaceValue A string containing the text to replace for every successful match of searchValue in this string.
 */
 replace(searchValue: { [Symbol.replace](string: string, replaceValue: string): string; }, replaceValue: string): string;

This comment is misleading because this function replaces all occurrences when the pattern is a regular expression but only the first occurrence when it is a literal as explained in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace :

The replace() method returns a new string with some or all matches of a pattern replaced by a replacement. The pattern can be a string or a RegExp, and the replacement can be a string or a function to be called for each match. If pattern is a string, only the first occurrence will be replaced.

which is confirmed by running the following code:

console.log("foo".replace("o", "O")); // => fOo
console.log("foo".replace(/o/g, "O")); // => fOO