-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Unsafe subtype: datetime/date #20448
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
Open
m-aciek
wants to merge
11
commits into
python:master
Choose a base branch
from
m-aciek:unsafe-subtype
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+372
−0
Open
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
5a9efed
Initial plan
Copilot 126ffab
Implement unsafe subtype checking for datetime/date comparisons
Copilot 7ff17b2
Fix docstring indentation and line continuation style
Copilot 857f0fa
Flag date vs date comparisons as unsafe when datetime subclass exists
Copilot ffc3592
Add detailed documentation for supertype comparison checking
Copilot 901cdec
Block datetime/date inheritance when unsafe-subtype is enabled
Copilot cb83a2a
Move errorcodes import to module level for performance
Copilot 1b55f52
Remove date/date comparison warnings as inheritance blocking makes th…
Copilot 2d231f5
Remove redundant comparison check code in visit_comparison_expr
Copilot 5ee1d68
Add documentation for unsafe-subtype error code
Copilot 427f31c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |||||||||||||||||||||||
| import mypy.applytype | ||||||||||||||||||||||||
| import mypy.constraints | ||||||||||||||||||||||||
| import mypy.typeops | ||||||||||||||||||||||||
| from mypy import errorcodes as codes | ||||||||||||||||||||||||
| from mypy.checker_state import checker_state | ||||||||||||||||||||||||
| from mypy.erasetype import erase_type | ||||||||||||||||||||||||
| from mypy.expandtype import ( | ||||||||||||||||||||||||
|
|
@@ -84,6 +85,12 @@ | |||||||||||||||||||||||
| IS_VAR: Final = 4 | ||||||||||||||||||||||||
| IS_EXPLICIT_SETTER: Final = 5 | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Known unsafe subtyping relationships that should trigger warnings. | ||||||||||||||||||||||||
| # Each tuple is (subclass_fullname, superclass_fullname). | ||||||||||||||||||||||||
| # These are cases where a class is a subclass at runtime but treating it | ||||||||||||||||||||||||
| # as a subtype can cause runtime errors. | ||||||||||||||||||||||||
| UNSAFE_SUBTYPING_PAIRS: Final = [("datetime.datetime", "datetime.date")] | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| TypeParameterChecker: _TypeAlias = Callable[[Type, Type, int, bool, "SubtypeContext"], bool] | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
@@ -528,6 +535,15 @@ def visit_instance(self, left: Instance) -> bool: | |||||||||||||||||||||||
| if left.type.alt_promote and left.type.alt_promote.type is right.type: | ||||||||||||||||||||||||
| return True | ||||||||||||||||||||||||
| rname = right.type.fullname | ||||||||||||||||||||||||
| lname = left.type.fullname | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Check if this is an unsafe subtype relationship that should be blocked | ||||||||||||||||||||||||
| if self.options and codes.UNSAFE_SUBTYPE in self.options.enabled_error_codes: | ||||||||||||||||||||||||
| # Block unsafe subtyping relationships when the error code is enabled | ||||||||||||||||||||||||
| for subclass, superclass in UNSAFE_SUBTYPING_PAIRS: | ||||||||||||||||||||||||
| if lname == subclass and rname == superclass: | ||||||||||||||||||||||||
| return False | ||||||||||||||||||||||||
|
Comment on lines
+541
to
+545
Collaborator
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.
Suggested change
If I'm not mistaken, you don't actually have to iterate manually for this |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Always try a nominal check if possible, | ||||||||||||||||||||||||
| # there might be errors that a user wants to silence *once*. | ||||||||||||||||||||||||
| # NamedTuples are a special case, because `NamedTuple` is not listed | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,276 @@ | ||
| [case testDatetimeVsDateComparison] | ||
| # flags: --enable-error-code unsafe-subtype | ||
| from datetime import date, datetime | ||
|
|
||
| dt: datetime | ||
| d: date | ||
|
|
||
| if dt < d: # E: Unsupported operand types for < ("datetime" and "date") | ||
| pass | ||
|
|
||
| if d > dt: # E: Unsupported operand types for < ("datetime" and "date") | ||
| pass | ||
|
|
||
| if dt == d: | ||
| pass | ||
|
|
||
| if dt != d: | ||
| pass | ||
|
|
||
| if dt <= d: # E: Unsupported operand types for <= ("datetime" and "date") | ||
| pass | ||
|
|
||
| if dt >= d: # E: Unsupported operand types for >= ("datetime" and "date") | ||
| pass | ||
| [builtins fixtures/classmethod.pyi] | ||
| [file datetime.pyi] | ||
| class date: | ||
| def __init__(self, year: int, month: int, day: int) -> None: ... | ||
| @classmethod | ||
| def today(cls) -> date: ... | ||
| def __lt__(self, other: date) -> bool: ... | ||
| def __le__(self, other: date) -> bool: ... | ||
| def __gt__(self, other: date) -> bool: ... | ||
| def __ge__(self, other: date) -> bool: ... | ||
| def __eq__(self, other: object) -> bool: ... | ||
| def __ne__(self, other: object) -> bool: ... | ||
|
|
||
| class datetime(date): | ||
| def __init__(self, year: int, month: int, day: int, hour: int = 0, minute: int = 0, second: int = 0, microsecond: int = 0) -> None: ... | ||
| @classmethod | ||
| def now(cls) -> datetime: ... | ||
| def __lt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __le__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __gt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __ge__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
|
|
||
| [case testDatetimeVsDateComparisonDisabled] | ||
| # No flags, so the error should not appear | ||
| from datetime import date, datetime | ||
|
|
||
| dt: datetime | ||
| d: date | ||
|
|
||
| if dt < d: | ||
| pass | ||
|
|
||
| if d > dt: | ||
| pass | ||
| [builtins fixtures/classmethod.pyi] | ||
| [file datetime.pyi] | ||
| class date: | ||
| def __init__(self, year: int, month: int, day: int) -> None: ... | ||
| @classmethod | ||
| def today(cls) -> date: ... | ||
| def __lt__(self, other: date) -> bool: ... | ||
| def __le__(self, other: date) -> bool: ... | ||
| def __gt__(self, other: date) -> bool: ... | ||
| def __ge__(self, other: date) -> bool: ... | ||
| def __eq__(self, other: object) -> bool: ... | ||
| def __ne__(self, other: object) -> bool: ... | ||
|
|
||
| class datetime(date): | ||
| def __init__(self, year: int, month: int, day: int, hour: int = 0, minute: int = 0, second: int = 0, microsecond: int = 0) -> None: ... | ||
| @classmethod | ||
| def now(cls) -> datetime: ... | ||
| def __lt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __le__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __gt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __ge__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
|
|
||
| [case testDatetimeVsDateComparisonExplicitTypes] | ||
| # flags: --enable-error-code unsafe-subtype | ||
| from datetime import date, datetime | ||
|
|
||
| def compare_datetime_date(dt: datetime, d: date) -> bool: | ||
| return dt < d # E: Unsupported operand types for < ("datetime" and "date") | ||
| [builtins fixtures/classmethod.pyi] | ||
| [file datetime.pyi] | ||
| class date: | ||
| def __init__(self, year: int, month: int, day: int) -> None: ... | ||
| @classmethod | ||
| def today(cls) -> date: ... | ||
| def __lt__(self, other: date) -> bool: ... | ||
| def __le__(self, other: date) -> bool: ... | ||
| def __gt__(self, other: date) -> bool: ... | ||
| def __ge__(self, other: date) -> bool: ... | ||
| def __eq__(self, other: object) -> bool: ... | ||
| def __ne__(self, other: object) -> bool: ... | ||
|
|
||
| class datetime(date): | ||
| def __init__(self, year: int, month: int, day: int, hour: int = 0, minute: int = 0, second: int = 0, microsecond: int = 0) -> None: ... | ||
| @classmethod | ||
| def now(cls) -> datetime: ... | ||
| def __lt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __le__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __gt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __ge__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
|
|
||
| [case testDatetimeComparisonOK] | ||
| # flags: --enable-error-code unsafe-subtype | ||
| from datetime import date, datetime | ||
|
|
||
| dt1: datetime | ||
| dt2: datetime | ||
| d1: date | ||
| d2: date | ||
|
|
||
| # datetime vs datetime is safe | ||
| if dt1 < dt2: | ||
| pass | ||
|
|
||
| # date vs date is now OK since inheritance blocking prevents datetime from being passed as date | ||
| if d1 < d2: | ||
| pass | ||
| [builtins fixtures/classmethod.pyi] | ||
| [file datetime.pyi] | ||
| class date: | ||
| def __init__(self, year: int, month: int, day: int) -> None: ... | ||
| @classmethod | ||
| def today(cls) -> date: ... | ||
| def __lt__(self, other: date) -> bool: ... | ||
| def __le__(self, other: date) -> bool: ... | ||
| def __gt__(self, other: date) -> bool: ... | ||
| def __ge__(self, other: date) -> bool: ... | ||
| def __eq__(self, other: object) -> bool: ... | ||
| def __ne__(self, other: object) -> bool: ... | ||
|
|
||
| class datetime(date): | ||
| def __init__(self, year: int, month: int, day: int, hour: int = 0, minute: int = 0, second: int = 0, microsecond: int = 0) -> None: ... | ||
| @classmethod | ||
| def now(cls) -> datetime: ... | ||
| def __lt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __le__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __gt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __ge__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
|
|
||
| [case testDatetimeVsDateComparisonWithNow] | ||
| # flags: --enable-error-code unsafe-subtype | ||
| from datetime import date, datetime | ||
|
|
||
| if datetime.now() < date.today(): # E: Unsupported operand types for < ("datetime" and "date") | ||
| pass | ||
| [builtins fixtures/classmethod.pyi] | ||
| [file datetime.pyi] | ||
| class date: | ||
| def __init__(self, year: int, month: int, day: int) -> None: ... | ||
| @classmethod | ||
| def today(cls) -> date: ... | ||
| def __lt__(self, other: date) -> bool: ... | ||
| def __le__(self, other: date) -> bool: ... | ||
| def __gt__(self, other: date) -> bool: ... | ||
| def __ge__(self, other: date) -> bool: ... | ||
| def __eq__(self, other: object) -> bool: ... | ||
| def __ne__(self, other: object) -> bool: ... | ||
|
|
||
| class datetime(date): | ||
| def __init__(self, year: int, month: int, day: int, hour: int = 0, minute: int = 0, second: int = 0, microsecond: int = 0) -> None: ... | ||
| @classmethod | ||
| def now(cls) -> datetime: ... | ||
| def __lt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __le__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __gt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __ge__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
|
|
||
| [case testDatetimeVsDateComparisonInExpression] | ||
| # flags: --enable-error-code unsafe-subtype | ||
| from datetime import date, datetime | ||
|
|
||
| dt: datetime | ||
| d: date | ||
|
|
||
| result = dt < d # E: Unsupported operand types for < ("datetime" and "date") | ||
| [builtins fixtures/classmethod.pyi] | ||
| [file datetime.pyi] | ||
| class date: | ||
| def __init__(self, year: int, month: int, day: int) -> None: ... | ||
| @classmethod | ||
| def today(cls) -> date: ... | ||
| def __lt__(self, other: date) -> bool: ... | ||
| def __le__(self, other: date) -> bool: ... | ||
| def __gt__(self, other: date) -> bool: ... | ||
| def __ge__(self, other: date) -> bool: ... | ||
| def __eq__(self, other: object) -> bool: ... | ||
| def __ne__(self, other: object) -> bool: ... | ||
|
|
||
| class datetime(date): | ||
| def __init__(self, year: int, month: int, day: int, hour: int = 0, minute: int = 0, second: int = 0, microsecond: int = 0) -> None: ... | ||
| @classmethod | ||
| def now(cls) -> datetime: ... | ||
| def __lt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __le__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __gt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __ge__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
|
|
||
| [case testDateVsDateMixedWithDatetime] | ||
| # flags: --enable-error-code unsafe-subtype | ||
| from datetime import date, datetime | ||
|
|
||
| def compare_dates(d1: date, d2: date) -> bool: | ||
| # With inheritance blocking, this is now safe - datetime cannot be passed here | ||
| return d1 < d2 | ||
|
|
||
| # Example usage that would fail at runtime: | ||
| dt = datetime.now() | ||
| d = date.today() | ||
| # This now errors because datetime is not assignable to date | ||
| result = compare_dates(dt, d) # E: Argument 1 to "compare_dates" has incompatible type "datetime"; expected "date" | ||
| [builtins fixtures/classmethod.pyi] | ||
| [file datetime.pyi] | ||
| class date: | ||
| def __init__(self, year: int, month: int, day: int) -> None: ... | ||
| @classmethod | ||
| def today(cls) -> date: ... | ||
| def __lt__(self, other: date) -> bool: ... | ||
| def __le__(self, other: date) -> bool: ... | ||
| def __gt__(self, other: date) -> bool: ... | ||
| def __ge__(self, other: date) -> bool: ... | ||
| def __eq__(self, other: object) -> bool: ... | ||
| def __ne__(self, other: object) -> bool: ... | ||
|
|
||
| class datetime(date): | ||
| def __init__(self, year: int, month: int, day: int, hour: int = 0, minute: int = 0, second: int = 0, microsecond: int = 0) -> None: ... | ||
| @classmethod | ||
| def now(cls) -> datetime: ... | ||
| def __lt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __le__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __gt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __ge__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
|
|
||
| [case testInheritanceBlocking] | ||
| # flags: --enable-error-code unsafe-subtype | ||
| from datetime import date, datetime | ||
|
|
||
| # Assignment should be blocked | ||
| d: date = datetime.now() # E: Incompatible types in assignment (expression has type "datetime", variable has type "date") | ||
|
|
||
| # Function parameters should be blocked | ||
| def accept_date(d: date) -> None: | ||
| pass | ||
|
|
||
| accept_date(datetime.now()) # E: Argument 1 to "accept_date" has incompatible type "datetime"; expected "date" | ||
|
|
||
| # But date to date should still work | ||
| d2: date = date.today() # OK | ||
| accept_date(date.today()) # OK | ||
| [builtins fixtures/classmethod.pyi] | ||
| [file datetime.pyi] | ||
| class date: | ||
| def __init__(self, year: int, month: int, day: int) -> None: ... | ||
| @classmethod | ||
| def today(cls) -> date: ... | ||
| def __lt__(self, other: date) -> bool: ... | ||
| def __le__(self, other: date) -> bool: ... | ||
| def __gt__(self, other: date) -> bool: ... | ||
| def __ge__(self, other: date) -> bool: ... | ||
| def __eq__(self, other: object) -> bool: ... | ||
| def __ne__(self, other: object) -> bool: ... | ||
|
|
||
| class datetime(date): | ||
| def __init__(self, year: int, month: int, day: int, hour: int = 0, minute: int = 0, second: int = 0, microsecond: int = 0) -> None: ... | ||
| @classmethod | ||
| def now(cls) -> datetime: ... | ||
| def __lt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __le__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __gt__(self, other: datetime) -> bool: ... # type: ignore[override] | ||
| def __ge__(self, other: datetime) -> bool: ... # type: ignore[override] |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be a set, but I'm not sure whether it is actually faster than a single-element list