Clone issue 13171 #13
Conversation
Summary of ChangesHello @visz11, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors and enhances the type checker's logic concerning the assignment of abstract classes and protocols. It consolidates the existing validation into a new, more modular method, Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new helper TypeChecker.check_concrete_only_assign and updates check_assignment to delegate concrete-only assignment validation; introduces unit tests for abstract classes and protocols with Callable-returned types. The patch also accidentally includes a duplicate definition of the new helper. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CA as check_assignment
participant TC as TypeChecker
participant CCOA as check_concrete_only_assign
participant ERR as error_emitter
CA->>TC: determine lvalue_type, rvalue_type
CA->>CCOA: check_concrete_only_assign(lvalue_type, lvalue, rvalue_type, rvalue)
alt concrete-only violation
CCOA->>ERR: emit concrete_only_assign error
CCOA-->>CA: False (stop further processing)
else no violation
CCOA-->>CA: True (continue)
CA->>CA: proceed with remaining assignment checks
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Fix concrete_only_assign check for Callable return typesTL;DR: Extends the concrete-only assignment check to handle Callable return types with abstract classes or protocols. Refacto PR SummaryRefactors the concrete-only assignment check into a dedicated method that also handles Callable return types. Change HighlightsClick to expand
Sequence DiagramsequenceDiagram
participant TC as TypeChecker
participant CCOA as check_concrete_only_assign
participant MSG as MessageBuilder
TC->>CCOA: lvalue_type, lvalue, rvalue_type, rvalue
Note over CCOA: Check for self-assignment case
CCOA->>CCOA: Check if rvalue is abstract/protocol
CCOA->>CCOA: Check if lvalue is TypeType or CallableType
alt lvalue is TypeType or CallableType with abstract/protocol
CCOA->>MSG: concrete_only_assign error
CCOA-->>TC: Return False (error)
else
CCOA-->>TC: Return True (valid)
end
Testing GuideClick to expand
|
|
/refacto-test |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the logic for checking assignments of abstract or protocol classes into a new check_concrete_only_assign method. This new method extends the check to also cover assignments to variables of type Callable[..., A] where A is an abstract class or protocol. Additionally, it correctly allows self-assignments like A = A. The changes are well-implemented and include corresponding tests for both abstract classes and protocols. I have one minor suggestion to improve the readability of the new method.
| 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) | ||
| ) |
There was a problem hiding this comment.
This block can be slightly refactored to improve readability by nesting the isinstance check. This makes the logic a little more explicit and avoids a long boolean expression.
| 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) | |
| ) | |
| lvalue_is_a_callable = False | |
| if isinstance(lvalue_type, CallableType): | |
| ret_type = get_proper_type(lvalue_type.ret_type) | |
| if isinstance(ret_type, Instance): | |
| lvalue_is_a_callable = ( | |
| ret_type.type.is_abstract or ret_type.type.is_protocol | |
| ) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
mypy/checker.py(2 hunks)test-data/unit/check-abstract.test(1 hunks)test-data/unit/check-protocols.test(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
mypy/checker.py (3)
mypy/types.py (13)
Type(210-249)get_proper_type(2561-2561)get_proper_type(2563-2563)get_proper_type(2566-2583)CallableType(1473-1854)is_type_obj(1273-1273)is_type_obj(1633-1634)is_type_obj(1882-1885)type_object(1276-1276)type_object(1636-1644)type_object(1887-1890)TypeType(2454-2524)Instance(1091-1258)mypy/nodes.py (2)
Expression(212-218)NameExpr(1656-1674)mypy/messages.py (1)
concrete_only_assign(1360-1362)
🔇 Additional comments (2)
test-data/unit/check-abstract.test (1)
253-284: Solid coverage for callable-returning abstracts.Thanks for exercising callable-typed vars against abstract/concrete class assignments, aliases, and self-rebinds—this mirrors the existing
Type[...]scenarios perfectly and guards against regressions in the new helper.test-data/unit/check-protocols.test (1)
1622-1651: Nice parity for protocol callables.This adds the protocol counterpart to the abstract-class tests, ensuring callable variables, aliases, and self-assignments behave consistently after the refactor.
|
/refacto-visz |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
Code Review: Type Assignment Refactoring👍 Well Done
📁 Selected files for review (3)
🎯 Custom Instructions
📝 Additional Comments
|
![refacto-visz[bot]](https://avatars.githubusercontent.com/u/196302493?s=60&v=4){kind=small}
mypy/checker.py
Outdated
| rvalue_type = get_proper_type(rvalue_type) | ||
| lvalue_type = get_proper_type(lvalue_type) |
There was a problem hiding this comment.
Redundant Type Processing
Duplicate get_proper_type() calls on same variables that were already processed at lines 2429. This creates redundant type processing overhead in assignment checking hot path. The refactored method performs the same operations twice for identical inputs.
Standards
- Org-Guideline-Reuse code wherever possible and avoid redundant code by refactoring with utils and static method across application
- ISO-IEC-25010-Performance-Efficiency-Resource-Utilization
- Optimization-Pattern-Redundancy-Elimination
mypy/checker.py
Outdated
| rvalue_type = get_proper_type(rvalue_type) | ||
| lvalue_type = get_proper_type(lvalue_type) |
There was a problem hiding this comment.
Code Duplication Opportunity
The get_proper_type() calls are duplicated from the original code that was refactored. This creates maintenance overhead where the same type processing logic exists in both locations. Consider extracting this common pattern into a shared utility method to follow DRY principles.
Standards
- Clean-Code-Functions
- SOLID-SRP
- Refactoring-Extract-Method
- Org-Guideline-Reuse code wherever possible and avoid redundant code by refactoring with utils and static method across application
| 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) | ||
| ) |
There was a problem hiding this comment.
Callable Logic Enhancement
Callable 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
- Algorithm-Correctness-Completeness
- Business-Rule-Type-Validation
- Logic-Verification-Coverage
|
|
||
| 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.
Type Cast Safety
Type 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
- ISO-IEC-25010-Reliability-Fault-Tolerance
- ISO-IEC-25010-Functional-Correctness-Appropriateness
- DbC-Preconditions
| def check_concrete_only_assign(self, | ||
| lvalue_type: Optional[Type], | ||
| lvalue: Expression, | ||
| rvalue_type: Type, | ||
| rvalue: Expression) -> bool: |
There was a problem hiding this comment.
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
- Clean-Code-Functions
- Maintainability-Quality-Readability
| 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` |
There was a problem hiding this comment.
Self Assignment Detection
Self-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
- CWE-670
- OWASP-A04
|
/refacto-visz |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
Code Review: Type Processing Refactoring👍 Well Done
📁 Selected files for review (3)
🎯 Custom Instructions
📝 Additional Comments
|
mypy/checker.py
Outdated
| rvalue_type = get_proper_type(rvalue_type) | ||
| lvalue_type = get_proper_type(lvalue_type) |
There was a problem hiding this comment.
Redundant Type Processing
Duplicate get_proper_type() calls violate organization guideline requiring code reuse and redundancy avoidance. The same type processing operations were already performed at lines 2426 in the original code. This creates unnecessary computational overhead in the assignment checking hot path and maintenance burden through code duplication.
# Types should already be proper types from the caller
Commitable Suggestion
| rvalue_type = get_proper_type(rvalue_type) | |
| lvalue_type = get_proper_type(lvalue_type) | |
| # Types should already be proper types from the caller |
Standards
- Org-Guideline-Reuse code wherever possible and avoid redundant code by refactoring with utils and static method across application
- ISO-IEC-25010-Performance-Efficiency-Resource-Utilization
|
|
||
| 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.
Unsafe Type Cast
Type 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
- Algorithm-Correctness-Type-Safety
- Logic-Verification-Runtime-Safety
- Business-Rule-Defensive-Programming
| def check_concrete_only_assign(self, | ||
| lvalue_type: Optional[Type], | ||
| lvalue: Expression, | ||
| rvalue_type: Type, | ||
| rvalue: Expression) -> bool: |
There was a problem hiding this comment.
Parameter Organization Inconsistency
Parameter 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
- Clean-Code-Functions
- Maintainability-Quality-Readability
| 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) | ||
| ) | ||
|
|
There was a problem hiding this comment.
Incomplete Validation Logic
Callable 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
- SOLID-ISP
- Clean-Code-Functions
- Maintainability-Quality-Completeness
Co-authored-by: refacto-visz[bot] <230139763+refacto-visz[bot]@users.noreply.github.com>
|
/refacto-visz |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
Code Review: Type System Refactoring👍 Well Done
📁 Selected files for review (3)
🎯 Custom Instructions
📝 Additional Comments
|
|
|
||
| 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.
Unsafe Type Cast
Type 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.
# Handle each type case separately to avoid unsafe casting
if lvalue_is_a_type:
self.msg.concrete_only_assign(lvalue_type, rvalue)
elif lvalue_is_a_callable:
self.msg.concrete_only_assign(lvalue_type, rvalue)
Commitable Suggestion
| self.msg.concrete_only_assign(cast(Type, lvalue_type), rvalue) | |
| # Handle each type case separately to avoid unsafe casting | |
| if lvalue_is_a_type: | |
| self.msg.concrete_only_assign(lvalue_type, rvalue) | |
| elif lvalue_is_a_callable: | |
| self.msg.concrete_only_assign(lvalue_type, rvalue) |
Standards
- CWE-704
- CWE-843
- OWASP-A04
| if isinstance(lvalue_type, PartialType): | ||
| return True |
There was a problem hiding this comment.
PartialType Crash Prevention
Early 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.
# Skip processing for PartialType to avoid assertion failures
if isinstance(lvalue_type, PartialType) or lvalue_type is None:
return True
Commitable Suggestion
| if isinstance(lvalue_type, PartialType): | |
| return True | |
| # Skip processing for PartialType to avoid assertion failures | |
| if isinstance(lvalue_type, PartialType) or lvalue_type is None: | |
| return True |
Standards
- ISO-IEC-25010-Reliability-Fault-Tolerance
- ISO-IEC-25010-Performance-Efficiency-Time-Behavior
- Algorithmic-Complexity-Early-Exit-Optimization
|
|
||
| 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.
Unsafe Type Cast
Type 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
- Algorithm-Correctness-Type-Safety
- Logic-Verification-Runtime-Safety
- Business-Rule-Defensive-Programming
| 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) | ||
| ) |
There was a problem hiding this comment.
Incomplete Callable Validation
Callable 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.
lvalue_is_a_callable = False
if isinstance(lvalue_type, CallableType):
ret_type = get_proper_type(lvalue_type.ret_type)
# Check return type for abstract/protocol status
lvalue_is_a_callable = (
isinstance(ret_type, Instance) and
(ret_type.type.is_abstract or ret_type.type.is_protocol)
)
# Also check parameter types for abstract/protocol status
if not lvalue_is_a_callable:
for arg_type in lvalue_type.arg_types:
arg_type = get_proper_type(arg_type)
if (isinstance(arg_type, Instance) and
(arg_type.type.is_abstract or arg_type.type.is_protocol)):
lvalue_is_a_callable = True
break
Commitable Suggestion
| 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) | |
| ) | |
| lvalue_is_a_callable = False | |
| if isinstance(lvalue_type, CallableType): | |
| ret_type = get_proper_type(lvalue_type.ret_type) | |
| # Check return type for abstract/protocol status | |
| lvalue_is_a_callable = ( | |
| isinstance(ret_type, Instance) and | |
| (ret_type.type.is_abstract or ret_type.type.is_protocol) | |
| ) | |
| # Also check parameter types for abstract/protocol status | |
| if not lvalue_is_a_callable: | |
| for arg_type in lvalue_type.arg_types: | |
| arg_type = get_proper_type(arg_type) | |
| if (isinstance(arg_type, Instance) and | |
| (arg_type.type.is_abstract or arg_type.type.is_protocol)): | |
| lvalue_is_a_callable = True | |
| break |
Standards
- Business-Rule-Validation-Completeness
- Logic-Verification-Coverage
- Algorithm-Correctness-Completeness
| # Types should already be proper types from the caller | ||
| # Skip processing for PartialType to avoid assertion failures | ||
| if isinstance(lvalue_type, PartialType): | ||
| return True |
There was a problem hiding this comment.
Redundant Type Processing
Comment 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
- Org-Guideline-Reuse code wherever possible and avoid redundant code by refactoring with utils and static method across application
- ISO-IEC-25010-Reliability-Maturity
- DbC-Preconditions
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mypy/checker.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
mypy/checker.py (3)
mypy/types.py (14)
Type(210-249)PartialType(2399-2433)CallableType(1473-1854)is_type_obj(1273-1273)is_type_obj(1633-1634)is_type_obj(1882-1885)type_object(1276-1276)type_object(1636-1644)type_object(1887-1890)TypeType(2454-2524)Instance(1091-1258)get_proper_type(2561-2561)get_proper_type(2563-2563)get_proper_type(2566-2583)mypy/nodes.py (2)
Expression(212-218)NameExpr(1656-1674)mypy/messages.py (1)
concrete_only_assign(1360-1362)
| def check_concrete_only_assign(self, | ||
| lvalue_type: Optional[Type], | ||
| lvalue: Expression, | ||
| rvalue_type: Type, | ||
| rvalue: Expression) -> bool: | ||
| 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` | ||
| 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 | ||
| 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) | ||
| ) | ||
|
|
||
| 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) | ||
| return False | ||
| return True |
There was a problem hiding this comment.
Restore alias handling before concrete-only enforcement
Factoring the concrete-only check into check_concrete_only_assign() dropped the get_proper_type() normalization we used to do in-line. As a result, when the lvalue (or rvalue) type arrives wrapped in a TypeAliasType, the new helper never recognizes it as a TypeType/CallableType, so the concrete-only guard silently stops running. For example:
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 passesTo keep the previous behaviour, normalize both sides after the PartialType guard so aliases (and other wrappers) are unwrapped before the isinstance checks:
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 FalseThat reinstates the original guard for alias-backed annotations.
🤖 Prompt for AI Agents
In mypy/checker.py around lines 2444 to 2484, the new helper dropped
normalization with get_proper_type(), so TypeAliasType (and other wrappers) can
bypass the concrete-only enforcement; after the PartialType guard, call
get_proper_type() on both lvalue_type and rvalue_type (e.g. assign lvalue_type =
get_proper_type(lvalue_type) and rvalue_type = get_proper_type(rvalue_type))
before the isinstance checks so TypeAliasType/other wrappers are unwrapped and
the subsequent TypeType/CallableType logic runs as before.
2 participants
(Explain how this PR changes mypy.)
Summary by CodeRabbit