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

Add Typing Stubs #2020

Open
syheliel opened this issue Jan 19, 2022 · 5 comments
Open

Add Typing Stubs #2020

syheliel opened this issue Jan 19, 2022 · 5 comments

Comments

@syheliel
Copy link

Nowadays many tools(like mypy or pyright) start to provide type check. Would this project add typing stubs in the future?
image

@Arusekk
Copy link
Member

Arusekk commented Jan 19, 2022

While definitely cool, this is (AFAIK) strictly infeasible, since type annotations are incompatible with python 2. And pwntools is still compatible with python 2, for reasons I myself do not like. One day, sure, but this is way beyond the horizon from the current perspective.

@Skinner927
Copy link

It appears it is actually not infeasible to do it inline with 2.7 compatibility: https://peps.python.org/pep-0484/#suggested-syntax-for-python-2-7-and-straddling-code

Stub files are also an option but can rapidly become out of date as they have to be maintained separately.

There are a few key methods which would greatly benefit from type annotations: microsoft/python-type-stubs#203

Would there be an objection to an MR with (with at least some of) this compatible type hinting?

@syheliel
Copy link
Author

syheliel commented Jun 4, 2022

It appears it is actually not infeasible to do it inline with 2.7 compatibility: https://peps.python.org/pep-0484/#suggested-syntax-for-python-2-7-and-straddling-code

Yes, the inline type annotation is not supported in python2.7, but you can add them in comments like this:

def embezzle(self, account, funds=1000000, *fake_receipts):
    # type: (str, int, *str) -> None
    """Embezzle funds from account using fake receipts."""
    <code goes here>

The type annotation in comments is in the PEP484, so you don't need to worry about the compatibility for type checker tools.

Would there be an objection to an MR with (with at least some of) this compatible type hinting?

I think it's promising, but now we may decide which part of the codes should be type hinted first

@Snowflake8
Copy link

Snowflake8 commented Oct 12, 2022

This actually causes some code to incorrectly be detected as unreachable in some environments, as mentioned here:
microsoft/python-type-stubs#203

The type analyzer determines that the tube.sendline method always raises an exception. That's because it calls the tube.send method which calls tube.send_raw, which raises the exception EOFError('Not implemented'). The method tube.send_raw has no type annotation, is not decorated with @abc.abstractmethod and is not raising a NotImplementedError. Those are the heuristics that pylance uses to determine whether a method always raises an exception or whether it's likely to be overridden with another implementation.

Ideally, pwn library would include type information. Even minimal type annotations (e.g. a return type annotation for the tube.send_raw method) in this case would address the issue. Switching from EOFError to NotIimplementedError would also address the problem.

This is particularly annoying since it happens after the first send or sendline (which is where I encountered this issue)

@peace-maker
Copy link
Member

This actually causes some code to incorrectly be detected as unreachable in some environments, as mentioned here: microsoft/python-type-stubs#203
This is particularly annoying since it happens after the first send or sendline (which is where I encountered this issue)

That particular issue is already fixed in dev #2069

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants