-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix used-before-assignment for PEP 695 type aliases + parameters #10488
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
Fix used-before-assignment for PEP 695 type aliases and parameters. | ||
|
||
Closes #9815 |
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -21,6 +21,7 @@ | |||||||||||
"ext", | ||||||||||||
"regression", | ||||||||||||
"regression_02", | ||||||||||||
"used_02", | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sometimes I wonder if it still makes sense to keep the artificial file limit around. IMO it's just annoying most of the time. Should we remove it? pylint/pylint/testutils/functional/find_functional_tests.py Lines 13 to 17 in 32304fe
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without the file limit it was really annoying to find a functional tests though (scrolling for eternity). It's not going to be annoying for a while if we remove the file limit contraint though. Might be a release time check maybe ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I mostly use My issue here was a more practical one. I wanted to add one more test which should be placed alongside the other
That would probably just delay the issue and make releases more time consuming. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you never just browse the functional tests to see if something match what you want to test ? I agree with the git blame not working well if we move a lot of file. (It's already the case almost everywhere, I did refactor a lot in the past, and I'm not alone in 20 years, some git blame in pylint already make me feel like a cyber-crime forensic. I'm not saying we should make it worse voluntarily though). I'm a little reluctant to remove all rules. One possible solution would be to find another clear organization for the functional tests (like in the extension functional tests directory, probably). One directory per message, avoid testing multiple message in the same file. Or remove constraint on regression directory only : the issue always arise in this directory, right ? Or remove constraint on regression directory but force regression tests to be inside There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Tbh not really. I usually just search for the error message and see which existing tests pop up or which tests fail if I change something 😅
Yeah, it's broken already basically. Thankfully it still mostly works for the checkers themselves which is usually enough to find the right PR.
We mostly do that already but sometimes it's just more practical to test multiple different things in one go. Otherwise we end up duplicating a lot of test code which will make maintaining it just more difficult.
The -- There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, but isn't the discussion's goal to find a solution to the annoyance ;) ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||
} | ||||||||||||
"""Direct parent directories that should be ignored.""" | ||||||||||||
|
||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,23 @@ | ||
"""used-before-assignment re: python 3.12 generic typing syntax (PEP 695)""" | ||
"""Tests for used-before-assignment with Python 3.12 generic typing syntax (PEP 695)""" | ||
# pylint: disable = invalid-name,missing-docstring,too-few-public-methods,unused-argument | ||
|
||
from typing import TYPE_CHECKING, Callable | ||
|
||
from typing import Callable | ||
type Point[T] = tuple[T, ...] | ||
type Alias[*Ts] = tuple[*Ts] | ||
type Alias[**P] = Callable[P] | ||
|
||
# pylint: disable = invalid-name, missing-class-docstring, too-few-public-methods | ||
type Alias2[**P] = Callable[P, None] | ||
|
||
# https://github.com/pylint-dev/pylint/issues/9815 | ||
type IntOrX = int | X # [used-before-assignment] FALSE POSITIVE | ||
type AliasType = int | X | Y | ||
|
||
class X: | ||
pass | ||
|
||
if TYPE_CHECKING: | ||
class Y: ... | ||
|
||
class Good[T: Y]: ... | ||
type OtherAlias[T: Y] = T | None | ||
|
||
# https://github.com/pylint-dev/pylint/issues/9884 | ||
def func[T: Y](x: T) -> None: # [redefined-outer-name] FALSE POSITIVE | ||
... |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
used-before-assignment:11:20:11:21::Using variable 'X' before assignment:HIGH | ||
redefined-outer-name:22:9:22:13:func:Redefining name 'T' from outer scope (line 6):UNDEFINED |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
"""Tests for used-before-assignment with Python 3.13 type var defaults (PEP 696)""" | ||
# pylint: disable=missing-docstring,unused-argument,too-few-public-methods | ||
|
||
from typing import TYPE_CHECKING | ||
|
||
if TYPE_CHECKING: | ||
class Y: ... | ||
|
||
class Good1[T = Y]: ... | ||
class Good2[*Ts = tuple[int, Y]]: ... | ||
class Good3[**P = [int, Y]]: ... | ||
type Alias[T = Y] = T | None | ||
|
||
# https://github.com/pylint-dev/pylint/issues/9884 | ||
def func[T = Y](x: T) -> None: # [redefined-outer-name] FALSE POSITIVE | ||
... |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
[testoptions] | ||
min_pyver=3.13 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
redefined-outer-name:15:9:15:14:func:Redefining name 'T' from outer scope (line 12):UNDEFINED |
Uh oh!
There was an error while loading. Please reload this page.