-
Notifications
You must be signed in to change notification settings - Fork 0
Clone issue 13171 #13
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: master
Are you sure you want to change the base?
Changes from all commits
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -100,10 +100,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
FineGrainedDeferredNodeType: _TypeAlias = Union[FuncDef, MypyFile, OverloadedFuncDef] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# A node which is postponed to be processed during the next pass. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# In normal mode one can defer functions and methods (also decorated and/or overloaded) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# and lambda expressions. Nested functions can't be deferred -- only top-level functions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# and methods of classes not defined within a function can be deferred. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class DeferredNode(NamedTuple): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
node: DeferredNodeType | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# And its TypeInfo (for semantic analysis self type handling | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -2426,16 +2423,7 @@ def check_assignment(self, lvalue: Lvalue, rvalue: Expression, infer_lvalue_type | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Special case: only non-abstract non-protocol classes can be assigned to | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# variables with explicit type Type[A], where A is protocol or abstract. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rvalue_type = get_proper_type(rvalue_type) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lvalue_type = get_proper_type(lvalue_type) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (isinstance(rvalue_type, CallableType) and rvalue_type.is_type_obj() and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(rvalue_type.type_object().is_abstract or | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rvalue_type.type_object().is_protocol) and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
isinstance(lvalue_type, TypeType) and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
isinstance(lvalue_type.item, Instance) and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(lvalue_type.item.type.is_abstract or | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lvalue_type.item.type.is_protocol)): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.msg.concrete_only_assign(lvalue_type, rvalue) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if not self.check_concrete_only_assign(lvalue_type, lvalue, rvalue_type, rvalue): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if rvalue_type and infer_lvalue_type and not isinstance(lvalue_type, PartialType): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Don't use type binder for definitions of special forms, like named tuples. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -2453,6 +2441,48 @@ def check_assignment(self, lvalue: Lvalue, rvalue: Expression, infer_lvalue_type | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.infer_variable_type(inferred, lvalue, rvalue_type, rvalue) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.check_assignment_to_slots(lvalue) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def check_concrete_only_assign(self, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lvalue_type: Optional[Type], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lvalue: Expression, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rvalue_type: Type, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rvalue: Expression) -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+2444
to
+2448
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. Parameter Organization InconsistencyParameter ordering mixes types and expressions inconsistently reducing method signature readability. Grouping related parameters together would improve comprehension and follow consistent organization patterns. Current ordering makes parameter relationships unclear. Standards
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (isinstance(lvalue, NameExpr) and isinstance(rvalue, NameExpr) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
and lvalue.node == rvalue.node): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# This means that we reassign abstract class to itself. Like `A = A` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+2449
to
+2451
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. Self Assignment DetectionSelf-assignment detection relies on node identity comparison which may not handle all edge cases like aliased references or complex expressions. While the current implementation prevents false positives for direct self-assignment, it could miss security-relevant reassignments through aliases. Consider expanding validation to handle indirect references that could bypass abstract class assignment restrictions. Standards
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Types should already be proper types from the caller | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Skip processing for PartialType to avoid assertion failures | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if isinstance(lvalue_type, PartialType): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+2456
to
+2457
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. PartialType Crash PreventionEarly return for PartialType prevents assertion failures in get_proper_type() calls that would crash the type checker. This addresses critical reliability issue where partially inferred types during branch analysis could cause complete checker failure. Performance impact includes avoiding expensive crash recovery and maintaining system stability.
Commitable Suggestion
Suggested change
Standards
Comment on lines
+2454
to
+2457
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. Redundant Type ProcessingComment indicates types should already be proper types from caller, yet original code performed get_proper_type() calls that were removed. This violates organization guideline requiring code reuse and redundancy avoidance. The refactoring assumes caller responsibility without enforcing the contract through preconditions. Standards
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if not ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
isinstance(rvalue_type, CallableType) and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rvalue_type.is_type_obj() and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(rvalue_type.type_object().is_abstract or | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rvalue_type.type_object().is_protocol)): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lvalue_is_a_type = ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
isinstance(lvalue_type, TypeType) and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
isinstance(lvalue_type.item, Instance) and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(lvalue_type.item.type.is_abstract or | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lvalue_type.item.type.is_protocol) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lvalue_is_a_callable = False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if isinstance(lvalue_type, CallableType): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ret_type = get_proper_type(lvalue_type.ret_type) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lvalue_is_a_callable = ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
isinstance(ret_type, Instance) and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(ret_type.type.is_abstract or ret_type.type.is_protocol) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+2472
to
+2478
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. This block can be slightly refactored to improve readability by nesting the
Suggested change
Comment on lines
+2472
to
+2478
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. Callable Logic EnhancementCallable validation logic only checks return type for abstract/protocol status but ignores callable parameter types. Complete callable validation should verify all parameter types for abstract/protocol constraints. This creates incomplete validation coverage for callable type assignments. Standards
Comment on lines
+2472
to
+2478
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. Incomplete Callable ValidationCallable validation logic only examines return type for abstract/protocol constraints but ignores parameter types. Complete validation requires checking all parameter types for abstract/protocol status. This creates logical gaps where callable assignments with abstract parameter types bypass validation checks.
Commitable Suggestion
Suggested change
Standards
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+2472
to
+2479
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. Incomplete Validation LogicCallable validation only checks return type for abstract/protocol status but ignores parameter types. Complete validation should verify all parameter types for abstract/protocol constraints. This creates incomplete coverage for callable type assignments. Standards
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if lvalue_is_a_type or lvalue_is_a_callable: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# `lvalue_type` here is either `TypeType` or `CallableType`: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.msg.concrete_only_assign(cast(Type, lvalue_type), rvalue) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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. Type Cast SafetyType cast to Type without runtime validation could cause ClassCastException if lvalue_type is not actually a Type instance. The cast assumes type safety based on conditional checks but doesn't handle edge cases where type system assumptions fail. Adding runtime type validation before cast would prevent potential runtime failures. Standards
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. Unsafe Type CastType cast to Type without runtime validation assumes type safety based on conditional checks but doesn't handle edge cases where type system assumptions fail. The cast could cause runtime failures if lvalue_type is not actually a Type instance despite passing the conditional guards. Standards
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. Unsafe Type CastType cast to Type without runtime validation assumes type safety based on conditional checks but doesn't handle edge cases where type system assumptions fail. The cast could cause runtime failures if lvalue_type is not actually a Type instance despite passing conditional guards.
Commitable Suggestion
Suggested change
Standards
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. Unsafe Type CastType cast to Type without runtime validation assumes type safety based on conditional checks. The cast could cause runtime failures if lvalue_type is not actually a Type instance despite passing conditional guards. Logic assumes perfect type system alignment without defensive validation. Standards
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+2444
to
+2484
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. Restore alias handling before concrete-only enforcement Factoring the concrete-only check into from typing import Type, TypeAlias
from abc import ABC
class Base(ABC): ...
Concrete = type("Concrete", (Base,), {})
AbstractType: TypeAlias = Type[Base]
x: AbstractType = Base # used to error, now passes To keep the previous behaviour, normalize both sides after the PartialType guard so aliases (and other wrappers) are unwrapped before the if isinstance(lvalue_type, PartialType):
return True
- if not (
- isinstance(rvalue_type, CallableType) and
- rvalue_type.is_type_obj() and
- (rvalue_type.type_object().is_abstract or
- rvalue_type.type_object().is_protocol)):
+ proper_rvalue_type = get_proper_type(rvalue_type)
+ if not (
+ isinstance(proper_rvalue_type, CallableType) and
+ proper_rvalue_type.is_type_obj() and
+ (proper_rvalue_type.type_object().is_abstract or
+ proper_rvalue_type.type_object().is_protocol)):
return True
- lvalue_is_a_type = (
- isinstance(lvalue_type, TypeType) and
- isinstance(lvalue_type.item, Instance) and
- (lvalue_type.item.type.is_abstract or
- lvalue_type.item.type.is_protocol)
+ proper_lvalue_type = get_proper_type(lvalue_type)
+
+ lvalue_is_a_type = (
+ isinstance(proper_lvalue_type, TypeType) and
+ isinstance(proper_lvalue_type.item, Instance) and
+ (proper_lvalue_type.item.type.is_abstract or
+ proper_lvalue_type.item.type.is_protocol)
)
lvalue_is_a_callable = False
- if isinstance(lvalue_type, CallableType):
- ret_type = get_proper_type(lvalue_type.ret_type)
+ if isinstance(proper_lvalue_type, CallableType):
+ ret_type = get_proper_type(proper_lvalue_type.ret_type)
lvalue_is_a_callable = (
isinstance(ret_type, Instance) and
(ret_type.type.is_abstract or ret_type.type.is_protocol)
)
if lvalue_is_a_type or lvalue_is_a_callable:
# `lvalue_type` here is either `TypeType` or `CallableType`:
- self.msg.concrete_only_assign(cast(Type, lvalue_type), rvalue)
+ self.msg.concrete_only_assign(cast(Type, proper_lvalue_type), rvalue)
return False That reinstates the original guard for alias-backed annotations. 🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# (type, operator) tuples for augmented assignments supported with partial types | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
partial_type_augmented_ops: Final = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
('builtins.list', '+'), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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.
Method Parameter Ordering
Parameter ordering mixes types and expressions inconsistently (lvalue_type, lvalue, rvalue_type, rvalue). Grouping related parameters together (types together, expressions together) would improve method signature readability and follow consistent parameter organization patterns.
Standards