Skip to content

Fix fields_desc type hint to allow Packet class references#5020

Open
T3pp31 wants to merge 1 commit into
secdev:masterfrom
T3pp31:fix/5018-fields-desc-type
Open

Fix fields_desc type hint to allow Packet class references#5020
T3pp31 wants to merge 1 commit into
secdev:masterfrom
T3pp31:fix/5018-fields-desc-type

Conversation

@T3pp31

@T3pp31 T3pp31 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • widen Packet.fields_desc type annotation to ClassVar[List[Union[AnyField, Type[Packet]]]]
  • add # type: ignore at runtime usage sites where mypy cannot infer the post-metaclass type
  • no change to build/dissect behavior

Closes #5018

Approach

Per reviewer feedback, the fix follows the original issue's suggestion: just
update the one typing that's incorrect and use # type: ignore where absolutely
necessary elsewhere. No helper function is added.

At runtime, Packet_metaclass.__new__ resolves Packet references to Field
instances before any instance method runs. The widened annotation causes mypy
to treat self.fields_desc iteration as possibly containing Type[Packet],
so # type: ignore is added at the 17 call sites where mypy reports errors.
Two backslash-continued lines were converted to parenthesized form to allow
# type: ignore on the continued expression.

Why no new UTScapy tests

Per CONTRIBUTING.md, this is a typing-only fix with no runtime behavior change:

  • build/dissect/show paths are unchanged
  • the Packet-in-fields_desc pattern is already exercised by existing tests, e.g.
    test/contrib/loraphy2wan.uts (LinkADRReq inlines DataRate_TXPower and
    Redundancy Packet classes in fields_desc)
  • regression coverage for this change is provided by tox -e mypy and tox -e flake8

Test plan

  • tox -e flake8
  • tox -e mypy (Python 3.12, linux and win32)

@codecov

codecov Bot commented Jun 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 80.21%. Comparing base (14cc726) to head (f031589).
⚠️ Report is 12 commits behind head on master.

Files with missing lines Patch % Lines
scapy/packet.py 95.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5020      +/-   ##
==========================================
- Coverage   80.34%   80.21%   -0.13%     
==========================================
  Files         386      387       +1     
  Lines       96012    96446     +434     
==========================================
+ Hits        77137    77366     +229     
- Misses      18875    19080     +205     
Files with missing lines Coverage Δ
scapy/packet.py 84.70% <95.00%> (ø)

... and 19 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gpotter2

Copy link
Copy Markdown
Member

Hi & thanks for the PR.
The fix provided in the original issue looked more correct: don't add a function, just update the one typing that's incorrect and use # type: ignore if absolutely necessary elsewhere.

Packet.fields_desc accepts Field instances and Packet subclass
references at class definition time. Align the type annotation with
Packet_metaclass resolution behavior and add # type: ignore at
runtime usage sites where mypy cannot infer the post-metaclass type.

Closes secdev#5018

AI-Assisted: yes (Cursor Composer)
@T3pp31 T3pp31 force-pushed the fix/5018-fields-desc-type branch from 9c7d09c to f031589 Compare July 6, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Packet.fields_desc type annotation is inconsistent with runtime behavior

2 participants