Skip to content
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

RET501 should exempt overriding class methods #12198

Open
epenet opened this issue Jul 5, 2024 · 4 comments
Open

RET501 should exempt overriding class methods #12198

epenet opened this issue Jul 5, 2024 · 4 comments
Labels
accepted Ready for implementation rule Implementing or modifying a lint rule type-inference Requires more advanced type inference.

Comments

@epenet
Copy link
Contributor

epenet commented Jul 5, 2024

If a parent class defines a return type hint for a method, then it makes sense to keep the return None explicit.
This is similar to #3704/#3705, but it involves looking also at the parent classes.

class BaseClass:
    def get_value(self) -> str | None:
        return "a value"

class Class(BaseClass):
    def get_value(self) -> None:
        return None
        # RET501 [*] Do not explicitly `return None` in function if it is the only possible return value

Based on home-assistant/core#115031

@AlexWaygood
Copy link
Member

This seems reasonable to me. PR welcome.

@AlexWaygood AlexWaygood added rule Implementing or modifying a lint rule help wanted Contributions especially welcome accepted Ready for implementation labels Jul 8, 2024
@epenet
Copy link
Contributor Author

epenet commented Jul 9, 2024

Note: it appears this would be severely limited by the multifile-analysis restrictions.
#7447

@MichaReiser
Copy link
Member

Yes, this is a good point. I don't think we can implement this today without multifile-analysis

@MichaReiser MichaReiser added multifile-analysis and removed help wanted Contributions especially welcome labels Jul 9, 2024
@Avasam
Copy link
Contributor

Avasam commented Jul 25, 2024

With the @override decorator this can be done w/o multi-fine analysis.

@MichaReiser MichaReiser added type-inference Requires more advanced type inference. and removed multifile-analysis labels Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation rule Implementing or modifying a lint rule type-inference Requires more advanced type inference.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants