Skip to content

Fix allcaptures()/allspans() return type in regex stubs - #16319

Open
afonsojanu wants to merge 1 commit into
python:mainfrom
afonsojanu:fix/regex-allcaptures-allspans-variable-tuple-566
Open

Fix allcaptures()/allspans() return type in regex stubs#16319
afonsojanu wants to merge 1 commit into
python:mainfrom
afonsojanu:fix/regex-allcaptures-allspans-variable-tuple-566

Conversation

@afonsojanu

Copy link
Copy Markdown

Fixes #566

allcaptures() and allspans() were typed as fixed one-element tuples (tuple[list[AnyStr]] and tuple[list[tuple[int, int]]]), but at runtime they always return one entry per capture group in the pattern, plus the whole match, which is a count that varies from pattern to pattern. A fixed-length-1 tuple is exactly wrong for that shape: a type checker refuses to unpack or index past the first element even though the real return value routinely has more than one.

>>> import regex
>>> regex.match(r"(\w+) (\w+)", "hello world").allcaptures()
(['hello world'], ['hello'], ['world'])

Changed both to the variable-length tuple form, matching what captures() right above them in the same stub already uses for the identical shape.

Added a regression test asserting the corrected static type on both methods, and confirmed it fails with the exact error the issue reports (an assert-type mismatch against the old fixed-length type) when the stub change is reverted.

  • python tests/runtests.py --run-stubtest stubs/regex: pre-commit, structure check, Pyright, ty, pyrefly, mypy, stubtest, and both regression tests all pass.

Both were typed as fixed one-element tuples, tuple[list[AnyStr]] and
tuple[list[tuple[int, int]]], even though the runtime always returns
one entry per capture group, a count that varies with the pattern.
A type checker refuses to unpack or index past the first element of a
fixed-length tuple, so any code doing that against the real, variable
return value failed to type-check even though it ran correctly.

Changed both to the variable-length tuple form, matching what the
neighboring captures() method already uses for the same shape.

Fixes python#566
@github-actions

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@donbarbos

Copy link
Copy Markdown
Contributor

Thank you! LGTM, but could you remove the tests?

In typeshed, we only add regression tests for functions and classes which are known to have caused complex problems in the past, or where stubs are difficult to get right. 100% test coverage for typeshed is neither necessary nor desirable, as it would lead to code duplication.

See tests/REGRESSION.md for more information.

And for the record, the issue number refers to the following problem in the library itself: mrabarnett/mrab-regex#566

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants