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

[enhancement] Add typing hints for most functions, which makes it easier to use IDEs for programming #117

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

rtpt-romankarwacik
Copy link
Contributor

I added typing hints for most functions, which makes it easier to use IDEs for programming.

@p0dalirius p0dalirius self-assigned this Nov 26, 2024
@p0dalirius p0dalirius added the enhancement New feature or request label Nov 26, 2024
@p0dalirius p0dalirius changed the title Add typing hints [enhancement] Add typing hints for most functions, which makes it easier to use IDEs for programming Nov 26, 2024
@p0dalirius
Copy link
Owner

Hey,

Thank you for this PR. I am not really sure to understand the usecase for this? Can you explain it in more details?

As of now, the code does not pass the linting in my Actions:

https://github.com/p0dalirius/smbclient-ng/actions/runs/12029054251/job/33536128360?pr=117

Run # stop the build if there are Python syntax errors or undefined names
./smbclientng/core/utils.py:460:61: F821 undefined name 'Tuple'
def is_port_open(target: str, port: int, timeout: float) -> Tuple[bool, Optional[str]]:
                                                            ^
1     F821 undefined name 'Tuple'

Can you fix this?

Best regards,

@rtpt-romankarwacik
Copy link
Contributor Author

I am not really sure to understand the usecase for this? Can you explain it in more details?

VSCode and other IDEs can use the types to give hints about the used variables and functions, and can sometimes highlight if None type checks are missing and similar cases, for example:
Screenshot from 2024-11-26 15-16-22
Although impacket does not have much typing, it can help to see the arguments needed of functions without having to look them up manually.

from __future__ import annotations

is needed to not have to put the type annotations in strings, and

from typing import TYPE_CHECKING

if TYPE_CHECKING:

is needed to avoid circular imports when using typing hints.

@p0dalirius
Copy link
Owner

Thank you for the very clear explanation!

Merging,

@p0dalirius p0dalirius merged commit ea343aa into p0dalirius:main Nov 26, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants