diff --git a/doc/whatsnew/fragments/10351.false_negative b/doc/whatsnew/fragments/10351.false_negative new file mode 100644 index 0000000000..d6c2e22413 --- /dev/null +++ b/doc/whatsnew/fragments/10351.false_negative @@ -0,0 +1,5 @@ +Enhanced `invalid-overridden-method` checker to detect return type mismatches when overriding methods. +The checker now properly identifies when subclass methods have incompatible return types compared to their parent methods, +including support for covariant return types and proper `typing.Any` handling. + +Closes #10351 diff --git a/pylint/checkers/classes/class_checker.py b/pylint/checkers/classes/class_checker.py index b493a1ba73..c988e78150 100644 --- a/pylint/checkers/classes/class_checker.py +++ b/pylint/checkers/classes/class_checker.py @@ -33,6 +33,7 @@ is_iterable, is_property_setter, is_property_setter_or_deleter, + is_subclass_of, node_frame_class, only_required_for_messages, safe_infer, @@ -1510,6 +1511,38 @@ def _check_invalid_overridden_method( node=function_node, ) + # Check for return type mismatches + parent_returns = parent_function_node.returns + current_returns = function_node.returns + + # Only check if both methods have return type annotations + if parent_returns is not None and current_returns is not None: + parent_return_type = safe_infer(parent_returns) + current_return_type = safe_infer(current_returns) + + if ( + parent_return_type + and current_return_type + and isinstance(parent_return_type, nodes.ClassDef) + and isinstance(current_return_type, nodes.ClassDef) + ): + # Check for compatibility + compatible = self._are_return_types_compatible( + current_return_type, parent_return_type + ) + + # Emit error if not compatible + if not compatible: + self.add_message( + "invalid-overridden-method", + args=( + function_node.name, + f"return type '{parent_return_type.name}'", + f"return type '{current_return_type.name}'", + ), + node=function_node, + ) + def _check_functools_or_not(self, decorator: nodes.Attribute) -> bool: if decorator.attrname != "cached_property": return False @@ -2056,6 +2089,7 @@ def _check_accessed_members( node.local_attr(attr) # yes, stop here continue + except astroid.NotFoundError: pass # is it an instance attribute of a parent class ? @@ -2393,6 +2427,34 @@ def _check_signature( "signature-differs", args=(class_type, method1.name), node=method1 ) + def _are_return_types_compatible( + self, current_type: nodes.ClassDef, parent_type: nodes.ClassDef + ) -> bool: + """Check if current_type is compatible with parent_type for return type + checking. + + Compatible means: + 1. Same type (qname match) + 2. current_type is a subclass of parent_type (covariance) + 3. parent_type is typing.Any (any type is compatible with Any) + """ + # Same type check + if current_type.qname() == parent_type.qname(): + return True + + # Special case: Any type accepts anything + if parent_type.qname() == "typing.Any": + return True + + # Covariant check - current is subclass of parent + if is_subclass_of(current_type, parent_type): + return True + + # For built-in types, also check by qualified name in ancestors + parent_qname = parent_type.qname() + current_ancestors = {ancestor.qname() for ancestor in current_type.ancestors()} + return parent_qname in current_ancestors + def _uses_mandatory_method_param( self, node: nodes.Attribute | nodes.Assign | nodes.AssignAttr ) -> bool: diff --git a/tests/functional/i/invalid/invalid_overridden_method.py b/tests/functional/i/invalid/invalid_overridden_method.py index d81a7882bd..9922849d6b 100644 --- a/tests/functional/i/invalid/invalid_overridden_method.py +++ b/tests/functional/i/invalid/invalid_overridden_method.py @@ -1,5 +1,6 @@ # pylint: disable=missing-docstring,too-few-public-methods,disallowed-name,invalid-name,unused-argument import abc +from io import TextIOWrapper, BytesIO class SuperClass(metaclass=abc.ABCMeta): @@ -125,3 +126,25 @@ def bar(self): # [invalid-overridden-method] @multiple_returns def bar2(self): # [invalid-overridden-method] return False + + +# https://github.com/pylint-dev/pylint/issues/10351 +# Test case for return type mismatch +class ReturnType(TextIOWrapper): + pass + +class BaseClass(abc.ABC): + @abc.abstractmethod + def read_file(self, path: str) -> TextIOWrapper: + """Abstract method that should return a TextIOWrapper.""" + raise NotImplementedError("Method must be implemented by subclass") + +class ChildClass1(BaseClass): + def read_file(self, path: str) -> BytesIO: # [invalid-overridden-method] + """Implementation returns BytesIO instead of TextIOWrapper.""" + return BytesIO(b"content") + +class ChildClass2(BaseClass): + def read_file(self, path: str) -> ReturnType: + """Implementation returns BytesIO instead of TextIOWrapper.""" + return ReturnType(BytesIO(b"content")) diff --git a/tests/functional/i/invalid/invalid_overridden_method.txt b/tests/functional/i/invalid/invalid_overridden_method.txt index 5506109df0..edc5bb5af2 100644 --- a/tests/functional/i/invalid/invalid_overridden_method.txt +++ b/tests/functional/i/invalid/invalid_overridden_method.txt @@ -1,6 +1,7 @@ -invalid-overridden-method:38:4:38:12:InvalidDerived.prop:Method 'prop' was expected to be 'property', found it instead as 'method':UNDEFINED -invalid-overridden-method:41:4:41:20:InvalidDerived.async_method:Method 'async_method' was expected to be 'async', found it instead as 'non-async':UNDEFINED -invalid-overridden-method:45:4:45:16:InvalidDerived.method_a:Method 'method_a' was expected to be 'method', found it instead as 'property':UNDEFINED -invalid-overridden-method:48:4:48:22:InvalidDerived.method_b:Method 'method_b' was expected to be 'non-async', found it instead as 'async':UNDEFINED -invalid-overridden-method:122:4:122:11:B.bar:Method 'bar' was expected to be 'property', found it instead as 'method':UNDEFINED -invalid-overridden-method:126:4:126:12:B.bar2:Method 'bar2' was expected to be 'property', found it instead as 'method':UNDEFINED +invalid-overridden-method:39:4:39:12:InvalidDerived.prop:Method 'prop' was expected to be 'property', found it instead as 'method':UNDEFINED +invalid-overridden-method:42:4:42:20:InvalidDerived.async_method:Method 'async_method' was expected to be 'async', found it instead as 'non-async':UNDEFINED +invalid-overridden-method:46:4:46:16:InvalidDerived.method_a:Method 'method_a' was expected to be 'method', found it instead as 'property':UNDEFINED +invalid-overridden-method:49:4:49:22:InvalidDerived.method_b:Method 'method_b' was expected to be 'non-async', found it instead as 'async':UNDEFINED +invalid-overridden-method:123:4:123:11:B.bar:Method 'bar' was expected to be 'property', found it instead as 'method':UNDEFINED +invalid-overridden-method:127:4:127:12:B.bar2:Method 'bar2' was expected to be 'property', found it instead as 'method':UNDEFINED +invalid-overridden-method:143:4:143:17:ChildClass1.read_file:Method 'read_file' was expected to be "return type 'TextIOWrapper'", found it instead as "return type 'BytesIO'":UNDEFINED