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

Pylint plugin to check that return type annotation for __init__ functions is -> None #978

Merged
merged 7 commits into from
Jul 2, 2024

Conversation

natibek
Copy link
Contributor

@natibek natibek commented Jul 1, 2024

The return type annotation for __init__ is inconsistently enforced by mypy. The plugin will warn when there is no explicit return type annotation and when the return type in the annotation is not None.

@natibek natibek requested a review from richrines1 July 1, 2024 21:38
@natibek natibek self-assigned this Jul 1, 2024
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we rename this to make it obvious it's a pylint plugin? e.g. something like pylint_init_return.py

@@ -0,0 +1,58 @@
"""Check type type annotation of __init__ functions"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""Check type type annotation of __init__ functions"""
"""Check type annotation of __init__ functions"""

Comment on lines 35 to 36
"""Called for function and method definitions (def).
Checks if the return type annotation for __init__ is explicity None or incorrect.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""Called for function and method definitions (def).
Checks if the return type annotation for __init__ is explicity None or incorrect.
"""Called for function and method definitions (def).
Checks if the return type annotation for __init__ is explicitly None or incorrect.


name = "init-return-check"
msgs = {
"W9022": (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity - where did the codes come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The largest code I found was W9021 which was being used for one of the docstring pylint checks. The next available one was W9022.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok thanks. is there any risk that a future pylint update might introduce a new warning conflicting with this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is a future pylint change that leads to a new warning with the same code, pylint will throw an error whenever it is called and it will state the conflicting codes. I can check if there are conventions for what the codes should be for custom plugins so that they don't cause conflicts or if there are ways of checking for free warning codes.

Checks if the return type annotation for __init__ is explicity None or incorrect.

Args:
node: Node for a function or method definition in the AST
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
node: Node for a function or method definition in the AST
node: Node for a function or method definition in the AST.



def register(linter: PyLinter) -> None:
"""Registers plugin to be accessed by pylint
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""Registers plugin to be accessed by pylint
"""Registers plugin to be accessed by pylint.

"""Registers plugin to be accessed by pylint

Args:
linter: The base pylinter which the custom checker will inherit from
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
linter: The base pylinter which the custom checker will inherit from
linter: The base pylinter which the custom checker will inherit from.

@@ -58,7 +58,7 @@ def __init__(
ibmq_instance: str | None = None,
ibmq_channel: str | None = None,
**kwargs: Any,
):
) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's one way to find out it works!

Copy link
Contributor

@richrines1 richrines1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! this is great, thanks :)

@natibek natibek merged commit fe59e64 into main Jul 2, 2024
19 checks passed
@natibek natibek deleted the pylint-plugin-init-return branch July 2, 2024 17:38
natibek added a commit that referenced this pull request Jul 11, 2024
Changes include: 
- Remove Monthly Benchmark Action (#972)
- Integration test inconsistencies fix (#974)
- Pylint plugin to check that return type annotation for __init__ functions is -> None (#978)
- Add mulitcore support for pytest (#976)
- support both <x>_test.py and test_<x>.py for test discovery (#985)
- drop qiskit-ibm-provider from qss (#986)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants