-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
(feat) Add type annotations checker #10749
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Alvaro Frias <[email protected]>
Signed-off-by: Alvaro Frias <[email protected]>
Signed-off-by: Alvaro Frias <[email protected]>
Signed-off-by: Alvaro Frias <[email protected]>
Signed-off-by: Alvaro Frias <[email protected]>
Signed-off-by: Alvaro Frias <[email protected]>
This comment has been minimized.
This comment has been minimized.
Pierre-Sassoulas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this PR, this looks pretty refined already.
| with ``--enable=missing-return-type-annotation``. | ||
|
|
||
| The check automatically skips: | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can also add function and methods starting with test_ for return-type ? I always found adding -> None in all tests to be rather pointless. But then mypy would disagree and we probably want to agree with mypy. (Btw just gave me an idea of a checker to check that function starting with test_ should not return anything).
Signed-off-by: Alvaro Frias <[email protected]>
for more information, see https://pre-commit.ci
Signed-off-by: Alvaro Frias <[email protected]>
β¦nt into feature/require-typeannotation
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Alvaro Frias <[email protected]>
for more information, see https://pre-commit.ci
Signed-off-by: Alvaro Frias <[email protected]>
β¦nt into feature/require-typeannotation
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10749 +/- ##
==========================================
+ Coverage 95.98% 95.99% +0.01%
==========================================
Files 176 177 +1
Lines 19540 19623 +83
==========================================
+ Hits 18755 18838 +83
Misses 785 785
π New features to boost your workflow:
|
Signed-off-by: Alvaro Frias <[email protected]>
Signed-off-by: Alvaro Frias <[email protected]>
cdce8p
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the benefit of having this be a pylint check? Most type checkers already implement these and likely do a better job at detecting all edge cases.
| class TypeAnnotationChecker(checkers.BaseChecker): | ||
| """Checker for enforcing type annotations on functions and methods. | ||
| This checker verifies that functions and methods have appropriate | ||
| type annotations for return values and parameters. | ||
| """ | ||
|
|
||
| name = "type-annotation" | ||
| msgs = { | ||
| "C3801": ( | ||
| "Missing return type annotation for function %r", | ||
| "missing-return-type-annotation", | ||
| "Used when a function or method does not have a return type annotation. " | ||
| "Type annotations improve code readability and help with static type checking.", | ||
| {"default_enabled": False}, | ||
| ), | ||
| "C3802": ( | ||
| "Missing type annotation for parameter %r in function %r", | ||
| "missing-param-type-annotation", | ||
| "Used when a function or method parameter does not have a type annotation. " | ||
| "Type annotations improve code readability and help with static type checking.", | ||
| {"default_enabled": False}, | ||
| ), | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of setting default_enabled: False it might be better to move the whole checker to an optional extension.
|
The reasoning was #3853 (comment)
|
Not sure I agree with it. Adding type annotations only make sense if they are also checked for correctness with a type checker and most type checkers implement checks for it already, in particular mypy and pyright. In the end this is just duplicate code with will need to be maintained. If someone really wants to do it, then I guess an extension is probably fine. Though I don't really think it makes sense pylint. |
|
I wouldn't use it personally but there's 16 upvote for the issue so there seem to be a demand for it. I agree it should be an extension or a plugin though. Concretely @qequ it means it should move from |
Type of Changes
Description
This PR implements a new type annotation checker for Pylint that helps enforce the presence of type annotations in Python code. As discussed in #3853, type annotations improve code readability and enable better static analysis.
What's New
Two New Convention-Level Checkers
C2901:
missing-return-type-annotationDetects functions and methods without return type annotations.
C2902:
missing-param-type-annotationDetects function/method parameters without type annotations.
Key Features
*args,**kwargs)selfandclsparameters (automatically skipped)__init__methods (return type check skipped)@abstractmethod,@propertydecorators@typing.overloadstub definitionsFuture Enhancements
Following Issue discussion and the Google Python Style Guide model, which requires annotations only for public APIs, it could be added different checks for private/public methods:
Other possible future enhancements can be Variable Annotations, and configurable options in .pylintrc
Closes #3853