[pyupgrade] Fix false negative for TypeVar with default argument in non-pep695-generic-class (UP046)#20660
Conversation
|
ntBre
left a comment
There was a problem hiding this comment.
Thank you, this is looking good. I guess I expected this to be more complicated to have left the TODO around for so long 😆 I thought we would need an explicit Python 3.13 check somewhere, but it looks like the default argument to the typing.TypeVar constructors was only added in 3.13 too. So it seems safe to assume we're on 3.13+ if we find one.
As I noted in the inline comments, I think we need to make this a preview change and also check on UP040 test cases. I think all three of UP040, UP046, and UP047 share this same infrastructure.
| == vars.len()) | ||
| .then_some(vars) | ||
| // can't predict what the user wanted. | ||
| (vars.iter().unique_by(|tvar| tvar.name).count() == vars.len()).then_some(vars) |
There was a problem hiding this comment.
We need to preview gate this as I mentioned on the issue. I think this might be the easiest place to do that, assuming every usage goes through this code path.
There was a problem hiding this comment.
As demonstrated here, this change also affects UP047. I think it should also affect UP040, so we should make sure we have test coverage for that as well.
ntBre
left a comment
There was a problem hiding this comment.
The code changes look good, but now that this is a preview change, we also need to run the test cases in preview mode.
ntBre
left a comment
There was a problem hiding this comment.
Thanks!
I just pushed a few commits adding more of the test cases to the preview tests and using the new assert_diagnostics_diff macro. I also cleaned up some of the resolved TODOs in the tests.
…rable * origin/main: Don't use codspeed or depot runners in CI jobs on forks (#20894) [ty] cache Type::is_redundant_with (#20477) Fix run-away for mutually referential instance attributes (#20645) [ty] Limit shown import paths to at most 5 unless ty runs with `-v` (#20912) [ty] Use field-specifier return type as the default type for the field (#20915) [ty] Do not assume that `field`s have a default value (#20914) [ty] Fix match pattern value narrowing to use equality semantics (#20882) Update setup instructions for Zed 0.208.0+ (#20902) Move TOML indent size config (#20905) [syntax-errors]: implement F702 as semantic syntax error (#20869) [ty] Heterogeneous unpacking support for unions (#20377) [ty] refactor `Place` (#20871) Auto-accept snapshot changes as part of typeshed-sync PRs (#20892) [`airflow`] Add warning to `airflow.datasets.DatasetEvent` usage (`AIR301`) (#20551) [`flake8-pyi`] Fix operator precedence by adding parentheses when needed (`PYI061`) (#20508) [`pyupgrade`] Fix false negative for `TypeVar` with default argument in `non-pep695-generic-class` (`UP046`) (#20660) Update parser snapshots (#20893) Fix syntax error false positives for escapes and quotes in f-strings (#20867)
…nt-sets * dcreager/non-non-inferable: Don't use codspeed or depot runners in CI jobs on forks (#20894) [ty] cache Type::is_redundant_with (#20477) Fix run-away for mutually referential instance attributes (#20645) [ty] Limit shown import paths to at most 5 unless ty runs with `-v` (#20912) [ty] Use field-specifier return type as the default type for the field (#20915) [ty] Do not assume that `field`s have a default value (#20914) [ty] Fix match pattern value narrowing to use equality semantics (#20882) Update setup instructions for Zed 0.208.0+ (#20902) Move TOML indent size config (#20905) [syntax-errors]: implement F702 as semantic syntax error (#20869) [ty] Heterogeneous unpacking support for unions (#20377) [ty] refactor `Place` (#20871) Auto-accept snapshot changes as part of typeshed-sync PRs (#20892) [`airflow`] Add warning to `airflow.datasets.DatasetEvent` usage (`AIR301`) (#20551) [`flake8-pyi`] Fix operator precedence by adding parentheses when needed (`PYI061`) (#20508) [`pyupgrade`] Fix false negative for `TypeVar` with default argument in `non-pep695-generic-class` (`UP046`) (#20660) Update parser snapshots (#20893) Fix syntax error false positives for escapes and quotes in f-strings (#20867)
Summary
Fixes #20656