Add checkers for typing.final for Python version 3.8 or later #5133
Conversation
- overridden-final-method - subclassed-final-class Closes pylint-dev#3197
pylint/checkers/classes.py
Outdated
| @@ -874,10 +885,11 @@ def _check_consistent_mro(self, node): | |||
| # Old style class, there's no mro so don't do anything. | |||
| pass | |||
|
|
|||
| def _check_proper_bases(self, node): | |||
| def _check_proper_bases(self, node: nodes.ClassDef) -> None: | |||
There was a problem hiding this comment.
Thank you for adding the typing here. Could we create another function _check_typing_final instead, please ?
There was a problem hiding this comment.
Absolutely, this will also fix the failing test because of the lambda case. I'll put this back to the way it was and move it to its own thing.
I have one doubt aside from this - Is it best to include the changes mentioned in the issue for typing.Final in this merge-request, or would it be preferable to create a separate merge-request for that?
- Move checker logic to its own method
- tuple -> list to be consistent with the existing module style
- Doc formatting tweak
Closes pylint-dev#3197
- Re-add dot in docstring Closes pylint-dev#3197
Pull Request Test Coverage Report for Build 1325185515
💛 - Coveralls |
- Remove accidentally committed file Closes pylint-dev#3197
- Return early if not PY38 Closes pylint-dev#3197
| @@ -898,6 +910,23 @@ def _check_proper_bases(self, node): | |||
| "useless-object-inheritance", args=node.name, node=node | |||
| ) | |||
|
|
|||
| def _check_typing_final(self, node: nodes.ClassDef) -> None: | |||
| """Detect that a class does not subclass a class decorated with `typing.final`""" | |||
| if not PY38_PLUS: | |||
There was a problem hiding this comment.
@Pierre-Sassoulas Sorry for responding to this after this has been merged, but:
Tell me if I'm wrong, but I don't think this is how this check is intended right? Taking the consider-f-string example: if the python version is < 3.8 it shouldn't suggest to use an f-string as they are not available. The suggestion that the checker provides is nonsensical for 3.6 or 3.7.
This checker on the other hand should be used irrespective of the python: whenever somebody uses Final it should never be overwritten. There is room for a using-final-in-unsupported-version checker, which could check if Final is being used while the version is set to 3.6 or 3.7 but I don't think this checker should be stopped in that case.
Put differently, this line says that on 3.6 or 3.7 Final can be overwritten (which is not true) instead of warning that Final is being used in an unsupported version (which could be a checker of its own).
Please let me know if I misunderstood this setting, but if not I'm happy to provide a PR to remove these two lines.
There was a problem hiding this comment.
I think there's no messages that can be emitted and we don't do any check for python < 3.8 (we exit before checking decorated_with(ancestor, typing.final"]) and just return for 3.6 and 3.7. So I don't think we have a false positive, but you're right that it would be better to check anyway and warn for using-final-in-unsupported-version. II don't know how often this mistake happen but I'd merge a check for that, as python 3.7 will be available for some time.
Closes #3197
Type of Changes
Description
Add two new checkers Python versions of 3.8 and up:
overridden-final-methodsubclassed-final-classCloses #3197