Make RegExpMatchArray index and input field non-optional

[TomasHubelbauer]

On MDN, I don't see documentation for any case where either of these could be undefined. This change dates all the way back to 2017 when @jedmao asked @DanielRosenwasser about it, but there was no response:

7bf846a#diff-88c6092c5611bde0c0df8f538cb4334bcdc2860d73616190f1c3d3095ceda301R794

Either this is a mistake or there indeed is a case where either of these can be undefined but if that's the case, can anyone please educate me on that? I will update the MDN page to include information about this as well as add JSDoc description to the fields so that this information appears on hover, too.

The fields being optional is problematic because either one has to add a condition that will never be useful for anything or use the ! operator. The problem with the latter is that it is not obvious why it needs to be there (circling back to the root of this issue) and that TypeScript is also used for type checking JavaScript with JSDoc and JavaScript has no !, so code like this won't work in JavaScript checked with TypeScript:

for (const match of ''.matchAll(/something/g)) {
  results[match.index] = match[0];
}

In a case like this, the only solution is to use @ts-ignore and risk missing other erros on that line or to add the presumable useless condition for checking index is not undefined.

• There is an associated issue in the Backlog milestone (required)
• Code is up-to-date with the main branch
• You've successfully run gulp runtests locally
• There are new or updated unit tests validating the change

[typescript-bot]

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

[TomasHubelbauer]

#36788 is related.

[RyanCavanaugh]

I think e.g. https://github.com/alexrecuenco/TypeScript/commit/95ddd79bb22d3db9e6da9c6c2f5527cc01cbdf78 is the right fix here instead?

[DanielRosenwasser]

So a few things:

First, we've outlined a few rules to make it easier to manage and facilitate discussions. We understand that they add more work for contributors, but they mean that the team can scale out better at managing issues, PRs, and engaging with the community. Making us take the time to say "no, please add tests as we've asked" and disregarding our issue templates is not constructive.

Second, I probably did not see that comment - leaving comments on commits is just less typical for us and typically gets lost in the shuffle of the issue tracker. Apologies to @jedmao.

Third - looking a little closely at the ECMAScript spec, I do think that there is a mismatch between what we have and what gets created in the runtime. RegExpBuiltinExec says that the following are created unconditionally:

• an index property
• an input property
• a property at index 0
• a groups property *that may be undefined

So RegExpMatchArray should look something kind of like

// src/lib/es5.d.ts

interface RegExpMatchArray extends Array<string> {
    // always present
    index: string;

    // always present
    input: string;

    // always present - helpful for '--noUncheckedIndexedAccess'
    0: string;
}

// src/lib/es2018.regexp.d.ts

interface RegExpMatchArray extends Array<string> {
    // always present - helpful for '--exactOptionalPropertyTypes'
    groups: { [key: string]: string } | undefined;
}

[DanielRosenwasser]

Rereading the spec, the global flag ensures that they do not share the same code path, so I was wrong.

https://tc39.es/ecma262/#sec-regexp.prototype-@@match

@RyanCavanaugh found an easy example of when they have very different shapes.

> "blahfoo".match(/(bar)?/g)
< (8) ['', '', '', '', '', '', '', '']

> /(bar)?/g.exec("blahfoo")
< (2) ['', undefined, index: 0, input: 'blahfoo', groups: undefined]