Skip to content

Fix: Enhance W0236 checker to detect return type mismatches in overridden methods #10355

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

Closed
wants to merge 8 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 0 additions & 29 deletions .gitignore

This file was deleted.

5 changes: 5 additions & 0 deletions doc/whatsnew/fragments/10351.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Enhanced W0236 (invalid-overridden-method) to detect return type mismatches in overridden methods.

Before this fix, Pylint did not catch return type mismatches when a method in a subclass overrode a method from a base class with a different return type.

Closes #10351
73 changes: 73 additions & 0 deletions pylint/checkers/classes/class_checker.py
Original file line number Diff line number Diff line change
@@ -1465,6 +1465,9 @@
function_node: nodes.FunctionDef,
parent_function_node: nodes.FunctionDef,
) -> None:
if _is_trivial_super_delegation(function_node):
return

parent_is_property = decorated_with_property(
parent_function_node
) or is_property_setter_or_deleter(parent_function_node)
@@ -1510,6 +1513,76 @@
node=function_node,
)

if function_node.returns is None or parent_function_node.returns is None:
return

inferred_return = safe_infer(function_node.returns)
inferred_parent_return = safe_infer(parent_function_node.returns)

# Skip type checking if we couldn't infer one or both return types
if inferred_return is None or inferred_parent_return is None:
return

# Handle special cases where return type differences are allowed
# Parent returns Any - allow any return type in child
if (
isinstance(inferred_parent_return, nodes.Name)
and inferred_parent_return.name == "Any"
):
return

# Parent returns None - allow any return type in child.
if (
isinstance(inferred_parent_return, nodes.Const)
and inferred_parent_return.value is None
):
return

# Validation: Skip if parent has no explicit return type annotation
if parent_function_node.returns is None:
return

# Validation: Check if child return type is a subtype of parent return type
try:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the try except is not needed because this is already handled by the safe_infer utility function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback! @Julfried

I've updated the PR by removing the redundant try-except block, as the safe_infer function already handles those exceptions.

if hasattr(utils, "is_subtype") and utils.is_subtype(

Check failure on line 1547 in pylint/checkers/classes/class_checker.py

GitHub Actions / pylint

E1101

Module 'pylint.checkers.utils' has no 'is_subtype' member
inferred_return, inferred_parent_return
):
return
except astroid.InferenceError:
pass

# Validation: Compare qualified names for class types
if hasattr(inferred_return, "qname") and hasattr(
inferred_parent_return, "qname"
):
if inferred_return.qname() == inferred_parent_return.qname():
return

# Validation: Same name for builtin types
if hasattr(inferred_return, "name") and hasattr(inferred_parent_return, "name"):
if inferred_return.name == inferred_parent_return.name:
return

return_name = getattr(inferred_return, "name", str(inferred_return))
parent_return_name = getattr(
inferred_parent_return, "name", str(inferred_parent_return)
)

# Only report if types are definitely different
if (
return_name != parent_return_name
or inferred_return != inferred_parent_return
):
self.add_message(
"invalid-overridden-method",
args=(
function_node.name,
parent_return_name,
return_name,
),
node=function_node,
)

def _check_functools_or_not(self, decorator: nodes.Attribute) -> bool:
if decorator.attrname != "cached_property":
return False
Loading