[Hinty] Type hinting automaton.py #3391

Davidy22 · 2021-10-06T10:16:15Z

Used ansmachine.py as initial reference, Looked at some other files that I grepped into along the way.

Some notes taken while adding the hints:
add_breakpoints() returns None
Unsure of object type that could have fileno() on 609
645 assumed from rd and wr from the above class

Part of the long series of commits to work towards completion of #2158

Tests fail on my branch, but the error message seems unrelated to the file I changed, and I tried committing just a copy of master and that fails too so I guess it's just some teething issue in master

codecov · 2021-10-06T10:58:37Z

Codecov Report

Merging #3391 (5e0cf48) into master (d1d2ff1) will decrease coverage by 0.00%.
The diff coverage is 83.64%.

@@            Coverage Diff             @@
##           master    #3391      +/-   ##
==========================================
- Coverage   85.28%   85.27%   -0.01%     
==========================================
  Files         277      277              
  Lines       57135    57192      +57     
==========================================
+ Hits        48728    48771      +43     
- Misses       8407     8421      +14

Impacted Files	Coverage Δ
scapy/automaton.py	`75.43% <82.43%> (+0.33%)`	⬆️
scapy/compat.py	`95.73% <100.00%> (+0.02%)`	⬆️
scapy/contrib/isotp/isotp_soft_socket.py	`32.08% <100.00%> (ø)`
scapy/pipetool.py	`86.12% <100.00%> (+0.03%)`	⬆️
scapy/sendrecv.py	`80.92% <100.00%> (ø)`
scapy/supersocket.py	`58.78% <100.00%> (ø)`
scapy/utils.py	`77.36% <100.00%> (ø)`
scapy/arch/windows/__init__.py	`67.73% <0.00%> (-0.57%)`	⬇️

Davidy22 · 2021-10-06T11:21:07Z

How did the % on files that I didn't change get affected?

polybassa · 2021-10-06T11:29:09Z

Don't care about it. Codecov has some instability.

gpotter2 · 2021-10-09T19:29:39Z

You need to add automaton.py to .config/mypy/mypy_enabled.txt

Davidy22 · 2021-10-10T02:25:29Z

Added, noticed flake had some things to say about my hints too.

Davidy22 · 2021-10-13T10:47:20Z

Cleared out a couple hundred errors that mypy threw out, was nice to learn that there was a thing that would help with checking the type annotations. Made some notes along the way because there's some particularly hairy ones that I haven't gotten to resolving yet, some of which might involve changing other files. Pasting below:

WINDOWS and sys.platform == "win32" duplicate functionality, but sys.platform == "win32" is needed to make mypy stop whining about windows specific things.
mypy does not support using base class six.with_metaclass
state_wrapper being a function with attributes stapled to it throughout the program causes some issues in type checking. Followed a workaround here but there's some errors to work out still.
NewStateRequested being declared not in the top level causes some trouble for type checking.
Other files that reference things in this file are passing in some very unusual compound types.

gpotter2 · 2021-10-13T16:34:16Z

Thats some very solid work thanks a lot !

I have a trick for the WINDOWS vs sys.platform issue in the bags, because we're going to need it at some point to type arch/ anyways. I'mma try to commit it ASAP

Used ansmachine.py as initial reference, grepped into some other files along the way. Some notes taken while adding the hints: add_breakpoints returns None Unsure of object type that could have fileno() on 609 645 assumed from rd and wr from the above class

polydroi · 2021-11-01T21:08:41Z

test/testsocket.py

@@ -21,7 +21,7 @@
 open_test_sockets = list()  # type: List[TestSocket]


-class TestSocket(ObjectPipe, object):
+class TestSocket(ObjectPipe[Packet], SuperSocket):


I think I've tested this, but than decided to implement the Supersocket functions directly, since I had problems with the close function. Maybe that's fixed now, I just wanted to mention this, in case of strange bugs.

This was indeed super buggy but I think I figured it out, it needed an overwrite of the recv() function

Sorry for the previous ping btw 😅

polybassa · 2021-11-02T05:10:33Z

Your welcome. I was searching for the question, but couldn't find it. -- Viele Grüße Nils Weiß On November 1, 2021 10:48:16 PM GMT+01:00, gpotter2 ***@***.***> wrote: ***@***.*** commented on this pull request.>

…

> > > > @@ -21,7 +21,7 @@> open_test_sockets = list() # type: List[TestSocket]> > > -class TestSocket(ObjectPipe, object):> +class TestSocket(ObjectPipe[Packet], SuperSocket):> > Sorry for the previous ping btw 😅 > > -- > You are receiving this because you were mentioned.> Reply to this email directly or view it on GitHub:> #3391 (comment)

polybassa · 2021-11-02T05:12:00Z

Also, select or select_objects made a problem. But again, maybe it's all fixed now. Just to keep in mind. -- Viele Grüße Nils Weiß On November 1, 2021 10:47:54 PM GMT+01:00, gpotter2 ***@***.***> wrote: ***@***.*** commented on this pull request.

…

> @@ -21,7 +21,7 @@ open_test_sockets = list() # type: List[TestSocket] -class TestSocket(ObjectPipe, object): +class TestSocket(ObjectPipe[Packet], SuperSocket): This was indeed super buggy but I think I figured it out, it needed an overwrite of the `recv()` function -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #3391 (comment)

gpotter2 · 2021-11-02T12:40:13Z

@guedou This is ready for review.
I'd like to finish typing all of scapy's core soon-ish and proceed with 2.5.0.

There is mostly pipetools and arch left

test/contrib/automotive/ccp.uts

guedou · 2021-11-02T19:57:27Z

scapy/automaton.py


 def select_objects(inputs, remain):
+    # type: (List[Any], Union[float, int, None]) -> List[Any]


Is it possible to use a stricter type than Any here?

It appears python itself isn't sure how to type this better: https://github.com/python/typeshed/blob/994b69ef8f18e76689daca3947879c3d7f76173e/stdlib/select.pyi#L28

* Type hinting automaton.py Used ansmachine.py as initial reference, grepped into some other files along the way. Some notes taken while adding the hints: add_breakpoints returns None Unsure of object type that could have fileno() on 609 645 assumed from rd and wr from the above class * Various typing fixes Co-authored-by: gpotter2 <[email protected]>

gpotter2 added Hinty needs rework/cleanup labels Oct 10, 2021

gpotter2 added the hacktoberfest-accepted When hacktoberfest PRs are good enough label Nov 1, 2021

Type hinting automaton.py

2c4a524

Used ansmachine.py as initial reference, grepped into some other files along the way. Some notes taken while adding the hints: add_breakpoints returns None Unsure of object type that could have fileno() on 609 645 assumed from rd and wr from the above class

gpotter2 removed the needs rework/cleanup label Nov 1, 2021

polydroi reviewed Nov 1, 2021

View reviewed changes

gpotter2 added this to the 2.5.0 milestone Nov 1, 2021

gpotter2 changed the title ~~Type hinting automaton.py~~ [Hinty] Type hinting automaton.py Nov 1, 2021

guedou previously approved these changes Nov 2, 2021

View reviewed changes

Various typing fixes

5e0cf48

gpotter2 dismissed guedou’s stale review via 5e0cf48 November 2, 2021 20:37

This was referenced Nov 2, 2021

[Hinty] Core typing: scapy/pipetools.py & scapy/scapypipes.py #3429

Merged

[Hinty] Add type hints to Scapy #2158

Closed

gpotter2 merged commit e28aa57 into secdev:master Nov 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Hinty] Type hinting automaton.py #3391

[Hinty] Type hinting automaton.py #3391

Davidy22 commented Oct 6, 2021 •

edited

Loading

codecov bot commented Oct 6, 2021 •

edited

Loading

Davidy22 commented Oct 6, 2021

polybassa commented Oct 6, 2021

gpotter2 commented Oct 9, 2021

Davidy22 commented Oct 10, 2021

Davidy22 commented Oct 13, 2021 •

edited

Loading

gpotter2 commented Oct 13, 2021

polydroi Nov 1, 2021

gpotter2 Nov 1, 2021

gpotter2 Nov 1, 2021

polybassa commented Nov 2, 2021 via email

polybassa commented Nov 2, 2021 via email

gpotter2 commented Nov 2, 2021

guedou Nov 2, 2021

gpotter2 Nov 2, 2021

guedou Nov 2, 2021


		def select_objects(inputs, remain):
		# type: (List[Any], Union[float, int, None]) -> List[Any]

[Hinty] Type hinting automaton.py #3391

[Hinty] Type hinting automaton.py #3391

Conversation

Davidy22 commented Oct 6, 2021 • edited Loading

codecov bot commented Oct 6, 2021 • edited Loading

Codecov Report

Davidy22 commented Oct 6, 2021

polybassa commented Oct 6, 2021

gpotter2 commented Oct 9, 2021

Davidy22 commented Oct 10, 2021

Davidy22 commented Oct 13, 2021 • edited Loading

gpotter2 commented Oct 13, 2021

polydroi Nov 1, 2021

Choose a reason for hiding this comment

gpotter2 Nov 1, 2021

Choose a reason for hiding this comment

gpotter2 Nov 1, 2021

Choose a reason for hiding this comment

polybassa commented Nov 2, 2021 via email

polybassa commented Nov 2, 2021 via email

gpotter2 commented Nov 2, 2021

guedou Nov 2, 2021

Choose a reason for hiding this comment

gpotter2 Nov 2, 2021

Choose a reason for hiding this comment

guedou Nov 2, 2021

Choose a reason for hiding this comment

Davidy22 commented Oct 6, 2021 •

edited

Loading

codecov bot commented Oct 6, 2021 •

edited

Loading

Davidy22 commented Oct 13, 2021 •

edited

Loading