Skip to content

[ty] Add mdtest suite for typing.Concatenate#23554

Merged
dhruvmanila merged 3 commits intomainfrom
dhruv/concatenate-tests
Mar 3, 2026
Merged

[ty] Add mdtest suite for typing.Concatenate#23554
dhruvmanila merged 3 commits intomainfrom
dhruv/concatenate-tests

Conversation

@dhruvmanila
Copy link
Member

Summary

This PR adds a comprehensive test suite for typing.Concatenate in preparation for astral-sh/ty#1535 and to help the review process in #23119.

Test Plan

Run mdtest.

@dhruvmanila dhruvmanila requested a review from carljm as a code owner February 25, 2026 11:51
@dhruvmanila dhruvmanila added the testing Related to testing Ruff itself label Feb 25, 2026
@dhruvmanila dhruvmanila added the ty Multi-file analysis & type inference label Feb 25, 2026
Comment on lines -267 to -270
def _(c: Callable[[Concatenate[int, str, ...], int], int]):
# TODO: Should reveal the correct signature
reveal_type(c) # revealed: (...) -> int
```
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is removed because I don't think it's true that Concatenate is allowed within the parameter list of a Callable annotation.

@carljm carljm removed their request for review February 25, 2026 20:32
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brilliant, thank you!

from typing import Callable, Concatenate

def _(c: Callable[Concatenate[int, str, ...], bool]):
# TODO: Should reveal `(int, str, /, ...) -> bool`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or (int, str, /, *args: Unknown, **kwargs: Unknown) -> bool since, per the spec, *args: Unknown, **kwargs: Unknown is equivalent to ..., even in a signature that has other parameters besides *args, **kwargs. And I think I'd prefer spelling this type as (int, str, /, *args: Unknown, **kwargs: Unknown) -> bool -- it has the advantage that it's valid Python syntax for a function declaration, which is nice!

Copy link
Member Author

@dhruvmanila dhruvmanila Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's correct but we've been using ... to display the gradual form everywhere:

from typing import Callable

def _(x: Callable[..., None]):
    reveal_type(x)  # revealed: (...) -> None

I think mypy does spell it out although without the parameter names (def (*Any, **Any)) and we do store the gradual form as *args: Any, **kwargs: Any internally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's correct but we've been using ... to display the gradual form everywhere:

Right, and that feels intuitive to me — but combining the... with other parameters in the list feels like a sorta confusing hybrid of valid syntax and invalid syntax 😆

Anyway, no need to spend time on this now since it's just a TODO comment for now anyway, and display implementations are easy to change later as well!

Comment on lines +178 to +187
from typing import Concatenate

# error: [invalid-type-form] "`typing.Concatenate` requires at least two arguments when used in a type expression"
def _(x: Concatenate): ...

# TODO: Should be an error - Concatenate is not a valid standalone type
def invalid1(x: Concatenate[int, ...]) -> None: ...

# TODO: Should be an error - Concatenate is not a valid standalone type
def invalid2() -> Concatenate[int, ...]: ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the missing validation here -- we should be able to use the same (or if not, similar) hooks to the ones I'm adding in #23625, I think

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think so, thanks!

@dhruvmanila dhruvmanila force-pushed the dhruv/concatenate-tests branch from 8b9d2d5 to acb4e24 Compare March 3, 2026 09:44
@dhruvmanila dhruvmanila force-pushed the dhruv/concatenate-tests branch from acb4e24 to ec9a00d Compare March 3, 2026 09:45
@dhruvmanila dhruvmanila enabled auto-merge (squash) March 3, 2026 09:45
@dhruvmanila dhruvmanila merged commit a91591e into main Mar 3, 2026
46 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/concatenate-tests branch March 3, 2026 09:51
carljm added a commit that referenced this pull request Mar 3, 2026
* main:
  [ty] Apply narrowing to walrus values (#23687)
  [`perflint`] Extend `PERF102` to comprehensions and generators (#23473)
  [ty] Fix GitHub-annotations mdtest output format (#23694)
  [ty] Reduce the number of potentially-flaky projects (#23698)
  [`pydocstyle`] Fix numpy section ordering (`D420`) (#23685)
  [ty] Move method-related types to a submodule (#23691)
  [ty] Avoid the mandatory "ecosystem-analyzer workflow run cancelled" notification every time you make a PR (#23695)
  [ty] Move `Type::subtyping_is_always_reflexive` to `types::relation` (#23692)
  Update conformance suite commit hash (#23693)
  [ty] Add mdtest suite for `typing.Concatenate` (#23554)
  [ty] filter out pre-loop bindings from loop headers (#23536)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Related to testing Ruff itself ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants