Fix: Enhance W0236 checker to detect return type mismatches in overridden methods #10355
Conversation
| @@ -1510,6 +1510,19 @@ def _check_invalid_overridden_method( | |||
| node=function_node, | |||
| ) | |||
|
|
|||
| # Handle return type compatibility check | |||
| try: | |||
There was a problem hiding this comment.
I think the try except is not needed because this is already handled by the safe_infer utility function
There was a problem hiding this comment.
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.
Pierre-Sassoulas
added
the
False Negative 🦋
There was a problem hiding this comment.
Thank you for opening a PR @pavan-msys, do you mind adding a functional test in tests/functional/ please ? The example from the issue would work, but feel free to introduce some edge case scenarios.
|
I'm currently working on the codebase to address edge cases and ensure all test scenarios pass successfully. I’ll provide a detailed update once the changes are complete. Thank you! |
|
Hi @Pierre-Sassoulas, @Julfried I have updated the codebase. Could you please review the code and let me know if any changes are required? Thanks! |
|
Hey @pavan-msys thank you for adding the functional tests. I launched the tests in the continuous integration, it seems some issue appeared. You can launch the tests locally yourself with either pytest or tox see https://pylint.readthedocs.io/en/stable/development_guide/contributor_guide/tests/launching_test.html |
for more information, see https://pre-commit.ci
|
I have updated the codebase to handle additional edge cases and have verified the changes through local testing. Could you please review the changes and let me know if any further improvements are needed? Thank you! |
.gitignore
Outdated
| @@ -27,3 +27,4 @@ build-stamp | |||
| .pytest_cache/ | |||
| .mypy_cache/ | |||
| .benchmarks/ | |||
| venv/ | |||
There was a problem hiding this comment.
You should add that in your global gitignore not in each project's gitignore https://stackoverflow.com/a/7335487/2519059
|
The pypy3.10 job fails everywhere, don't mind it. |
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
|
Thank you for the suggestion. The test file |
There was a problem hiding this comment.
Hey @pavan-msys, you deleted the .gitignore completely, you should have reverted the changes instead :)
There's no way that this much new code was already tested. I would expect new tests that would fail without the new code, or at the very least the sample code from the original issue.
Also, are you using a LLM to generate the code for this PR ? The style seems to be one of a LLM (with the leading comments to explain what the code does in great details). It would also explains that you're able to create a lot of code without any sample test code to run it on. If that's the case please do not use LLM like this, I could fix the problem faster by thinking about it myself and making the fix than by reviewing the output of your LLM and then telling you how to fix the slop.
|
Thank you for the feedback @Pierre-Sassoulas I added the current code and comments to get a clearer understanding of the issue—this is not the final fix. I still need some time to investigate further and identify the root cause. To avoid confusion, I’ll go ahead and close this PR for now. Once I have a proper solution, I’ll raise a new PR with the complete fix. Apologies for the extra effort this may have caused, and thank you for your understanding and patience. |
Type of Changes
Description
This PR improves the invalid-overridden-method (W0236) check in Pylint to detect return type mismatches when a method in a subclass overrides a method in its base class.
Previously, Pylint would not catch cases where the overridden method had an incompatible return type compared to the base method.
Changes: