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

[Hinty] Add type hints to Scapy #2158

Closed
gpotter2 opened this issue Jul 21, 2019 · 29 comments
Closed

[Hinty] Add type hints to Scapy #2158

gpotter2 opened this issue Jul 21, 2019 · 29 comments
Labels
Hinty major Major changes
Milestone

Comments

@gpotter2
Copy link
Member

gpotter2 commented Jul 21, 2019

Project "Hinty" aims at adding Type hints to Scapy. It will help discover bugs, improve the API, and make Scapy up-to-date with the high standards of Python libraries.

Implementation

We use mypy to ensure automatic testing of the work that has already been completed. PRs that fall under project Hinty will process one (or a few) files and register them into the checks. The file .config/mypy/mypy_enabled.txt lists the files in which mypy checks are enabled. This process has been added by #2162

Because we support Python 2.7, the format of the Type hints should be the following:
https://mypy.readthedocs.io/en/latest/cheat_sheet.html

Guide on how to contribute

  1. 💿 Make sure you've "git clonned" the latest version of scapy and are working from within it !

  2. ✏️ Pick a file you want to work on. If you're new on scapy, pick a file from within the layers/ or contrib/ folder that is small in size and complexity: at this point in time, the remaining layers in Scapy's core are often hard to type.

  3. ✏️ Add that file to .config/mypy/mypy_enabled.txt. This will enable it in our test suite.

  4. ➡️ pip install pyannotate cryptography tox mock (install tox, pyannotate and cryptography)

  5. ➡️ Run the tests for your file: tox --sitepackages -- -x -K tcpdump -K manufdb -K wireshark -K tshark -K brotli -K zstd -K ci_only -K not_pyannotate -N
    You shall specify a -e py[38]-[linux]_non_root (fill the brackets with your OS infos) option before the --sitepackages option if you have multiple versions of python installed. To see all available tox tags (the whole py...non_root string), type tox -l. The -x option tells our test suite to use pyannotate.
    This will create test/pyannotate_results

  6. ➡️ pyannotate --type-info test/pyannotate_results -w [the file.py you are working on]

  7. ✏️ The file has been automatically processed. Now, edit it to fix the mistakes, and check your work with tox -e mypy. This might take time

    • ℹ️ Remember that you can use cast(type, obj) from typing to help mypy without affecting the code.
    • ⚠️ Import any types you might need from scapy.compat instead of typing. It provides a fallback if typing isn't installed (python < 3.5). I know it's annoying.
    • ℹ️ If a class extends Field directly (very few do) it will require to be passed the internal and machine representation of the field. See this if you don't know what I'm talking about. For instance: class subField(Field[float, int]): is a field where i=float and m=int. In most cases, it should be [int, int] (a normal integer field).
  8. ⚠️ If you get error: No library stub file for module ..., add an exception for it in .config/mypy/mypy.ini (follow the existing format)

  9. ✅ Check that the files still pass PEP8: tox -e flake8

  10. 🥇 When everything passes, submit a PR. 😄

Active hinty related issues

Documentation for advanced typing (that I always lose the links for..)

Hinty status

MyPy Support: 25.44%

Module Typed code (lines) Typed files
[core] 100.00% 33/33
arch 100.00% 13/13
layers 3.06% 2/92
contrib 14.28% 27/167
libs 23.30% 6/8
asn1 100.00% 4/4
tools 11.08% 1/9
modules 0.00% 0/8
@gpotter2 gpotter2 changed the title Sanitary cleanup Docstrings sanitary cleanup Jul 21, 2019
@guedou

This comment has been minimized.

@gpotter2

This comment has been minimized.

@guedou

This comment has been minimized.

@gpotter2 gpotter2 changed the title Docstrings sanitary cleanup Type hints / Docstrings cleanup Jul 22, 2019
@gpotter2

This comment has been minimized.

@guedou

This comment has been minimized.

@gpotter2 gpotter2 changed the title Type hints / Docstrings cleanup [Hinty] Add type hints to Scapy Jul 25, 2019
@JaninePrukop

This comment has been minimized.

@gpotter2

This comment has been minimized.

@nnrepos

This comment has been minimized.

@nnrepos

This comment has been minimized.

@nnrepos

This comment has been minimized.

@gpotter2

This comment has been minimized.

@akshaycbor

This comment has been minimized.

@guedou

This comment has been minimized.

@akshaycbor

This comment has been minimized.

@gpotter2

This comment has been minimized.

akshaycbor pushed a commit to akshaycbor/scapy that referenced this issue Dec 2, 2019
guedou pushed a commit that referenced this issue Dec 9, 2019
* #2158 - Add Hinty for plist.py

* Import MATPLOTLIB from extlib.py

* Add Line2D and update for review comments

* Add regression test changes for new matplotlib.lines import

* Remove unnecessary space

* Add new line to restart appveyer build
@gpotter2 gpotter2 mentioned this issue Apr 7, 2020
@fennellkyle

This comment has been minimized.

@gpotter2

This comment has been minimized.

@fennellkyle

This comment has been minimized.

@gpotter2
Copy link
Member Author

gpotter2 commented May 28, 2020

OSX is based on BSD 👍 there is a bsd.uts

@luizbarcelos
Copy link

Wanted to start helping on this issue, it's been a while since it was last updated though. Should I just follow the tutorial @gpotter2 ? Perhaps there is a higher priority issue for beginners on this repo, please let me know.

@gpotter2
Copy link
Member Author

gpotter2 commented Jun 3, 2021

@luizbarcelos Hi and thanks for your interest. I try to keep this page updated so feel free to help

@Davidy22
Copy link
Contributor

Davidy22 commented Oct 5, 2021

Hello, just grabbed the repo to take a shot at this and I opened up automaton.py to start working, then opened up ansmachine.py to see some examples and I noticed the way that type hinting is being done currently, with

    def __init__(self, **kargs):
        # type: (Any) -> None

While the type hints that I was aware of look something like:

    def __init__(self, **kargs: Any) -> None:

I'm wriitng automaton.py in the style that I'm seeing in ansmachine.py right now, but is there a reason why it's written like this instead of in PEP 484 style?

@polybassa
Copy link
Contributor

polybassa commented Oct 5, 2021 via email

@guedou
Copy link
Member

guedou commented Dec 10, 2022

With v2.5.0 being closer, can this issue be closed?

@gpotter2
Copy link
Member Author

gpotter2 commented Dec 10, 2022

Technically there's still arch/bpf. I was secretly thinking I could finish it before 2.5.0 but it's not blocking at all.
I'll close it once it's done

@KhazAkar
Copy link
Contributor

KhazAkar commented Jan 16, 2023

Hi,
Since there's plan to drop python 2.x support after 2.5.0, will functions etc get signatures not in comment-style, but directly in the future?

@gpotter2
Copy link
Member Author

Yes ! that's planned

@rohanvp
Copy link

rohanvp commented Nov 14, 2023

I am currently working on project hinty on file scapy/contrib/carp.py . The mypy documentation says it no longer supports python2 versions. The link https://mypy.readthedocs.io/en/latest/cheat_sheet.html from the page #2158 is going to non-existent page.

What is my way forward?

Secondly, is the following a correct process to contribute to hinty?

I have picked a file that I am working on (scapy/contrib/carp.py) . I run that file using "mypy scapy/contrib/carp.py .
I get multiple errors(e.g. [name-defined] , [attr-defined] etc). I start resolving those errors one by one.
Once there are no errors I continue with step 2 mentioned in #2158 .
Please advise if this is the correct way.

@gpotter2
Copy link
Member Author

gpotter2 commented Nov 28, 2023

Typing of the core is now fully achieved. I'd consider this 'completed'.

With Python 2.7 being dropped, this issue is also now obsolete. Closing

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

No branches or pull requests

12 participants