Skip to content

#501 left behind: 6 phantom Raises: clauses and 11 Args: name mismatches #519

Description

@JarryShaw

#501 fixed "Extractor's exception mismatch and 40 phantom/stale Args: labels". A full AST sweep of 496 files / 3334 functions (2237 with docstrings) shows the class is not exhausted. Every finding below was reproduced by two independently written scanners.

Phantom Raises: — 6 remaining

#501 fixed exactly one (Extractor.__iter__, IterableErrorCallableError).

pcapkit/protocols/application/httpv2.py gives an unusually clean proof: of its 11 _read_http_* methods, all 11 document Raises: ProtocolError; 6 genuinely raise it, 5 cannot.

  • :413 _read_http_none — and the mechanism is visible: :417 is #raise ProtocolError(f'HTTP/2: [Type {frame}] invalid format'), commented out and replaced by warn(…, ProtocolWarning) at :418
  • :459 _read_http_data
  • :514 _read_http_headers
  • :822 _read_http_goaway
  • :903 _read_http_continuation

Plus pcapkit/protocols/transport/tcp.py:904 _read_mode_sack, which documents ProtocolError: If length is **NOT** multiply of 8 plus 2. — the body at :907-917 only builds Data_SACK, with no length check anywhere.

All 22 commented-out raise statements in the package were swept; only the httpv2:417 case coincides with a docstring still claiming it.

Note these split into two different fixes: the five httpv2 ones look like stale copy-paste where the docstring should go, whereas _read_mode_sack's absent length check looks like the validation is what is missing.

Args: naming a parameter that does not exist — 11

location docstring says signature has
pcapkit/foundation/registry/foundation.py:74 engine: name
pcapkit/protocols/misc/pcap/frame.py:132 module: protocol
pcapkit/protocols/schema/application/httpv2.py:133 schema: only packet
pcapkit/protocols/schema/misc/pcapng.py:214 pkt: packet
pcapkit/protocols/schema/misc/pcapng.py:235 pkt: packet
pcapkit/vendor/mh/binding_ack_flag.py:136 soup: data
pcapkit/vendor/mh/binding_update_flag.py:135 soup: data
pcapkit/vendor/mh/handover_ack_flag.py:131 soup: data
pcapkit/vendor/mh/handover_initiate_flag.py:131 soup: data
pcapkit/vendor/pcapng/option_type.py:272 data: CSV data. soup (parsed HTML — wrong name and wrong description)
pcapkit/vendor/vlan/priority_level.py:57 data: soup

The six vendor/** cases all carry # pylint: disable=arguments-renamed, so the rename was known and only the docstring was missed. pcapkit/vendor/default.py uses data for process/count/context, so data is canonical. The labels look swapped between the process/context pair.

pcapkit/protocols/misc/pcap/frame.py:132 is the most consequential: Frame.register has no **kwargs, so module= is a hard TypeError.

Three of this class were already fixed under separate work (foundation/reassembly/reassembly.py:499 and foundation/traceflow/traceflow.py:430 documenting name where the parameter is protocol; plus registry/foundation.py).

A further 5 sites document self: as a parameterpcapkit/dumpkit/common.py:128,166,185, pcapkit/protocols/misc/pcap/frame.py:403, pcapkit/protocols/misc/pcapng.py:1100 — which napoleon renders as :param self:. Real but cosmetic.

Args: completeness — the other blind spot

42 functions have an Args: section that omits a real parameter. 21 of the 42 are in pcapkit/protocols/internet/hip.py. Two are demonstrably unintended because their own siblings prove the intent:

  • hip.py:3905 _make_param_reg_response documents only code/param/version but takes lifetime, reg_response, reg_response_default/_namespace/_reversed and **kwargs. Its sibling _make_param_reg_failed at :3945 documents all of them.
  • hip.py:4287 _make_param_route_dst omits symmetric, must_follow, hit; sibling _make_param_route_via at :4658 documents all three.

And a clean house-style measurement: of 450 functions having both **kwargs and an Args: section, 438 document **kwargs and 12 omit it — all 12 in hip.py.

Not defects, recorded so the count stays honest

  • pcapkit/corekit/multidict.py:176 documents KeyError while raising MissingKeyError — which is a KeyError subclass, so the docstring is literally true. Naming the real class would be nicer; it is not drift.
  • pcapkit/protocols/internet/esp.py:735 documents cryptography.exceptions.InvalidTag — a scanner false positive; it genuinely propagates from the third-party AEAD finalize().
  • 387 functions raise without any Raises: section at all. A real systemic gap (only 278 docstrings have one), but a blanket fix is a very large mechanical change with little reader benefit. The 6 wrong clauses matter far more than the absent ones.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions