Skip to content

design: #514 recorded two rulings it never implemented — the Base/public collapse and the enforcing @final #778

Description

@JarryShaw

#514 closed with "nothing added" — the maintainer ruled the custom-versus-builtin registration check is not wanted. But two substantial rulings were recorded on it before it closed, neither implemented and neither tracked anywhere now that it is shut. This issue exists so they are not lost.

(b) Collapse each XXXBase/public pair, and make name-registration opt-in by keyword. Five pairs: Protocol/ProtocolBase, Engine/EngineBase, Reassembly/ReassemblyBase, TraceFlow/TraceFlowBase, and the Schema pair. Blast radius measured on #514: ≈390 edit points, five PRs. Three hazards no issubclass sweep finds, all verified there:

  • tests/test_base_class_contract.py:75-80 imports both halves of all five pairs, so it does not fail — it fails to import. A rewrite or a deletion, and it is the one file every family touches.
  • tests/_support.py:337 defines a fake class ProtocolBase and injects it into sys.modules['pcapkit.protocols.protocol'] at :354, consumed via protochain.py:72,158. Rename the real one and the fake stops being found; tests/test_support_helpers.py:413 then AttributeErrors.
  • The split is today a runtime-enforced final substitute: ProtocolBase.__init_subclass__ rejects the registration keyword, so a library-style subclass cannot opt in at all. Collapsing to one class downgrades that guarantee to "do not pass the keyword".

The enforcing @final. Recorded ruling: subclassing a FINAL class raises rather than warns, in Info.__init_subclass__ (corekit/infoclass.py:244) and Schema.__init_subclass__ (protocols/schema/schema.py:1065); re-decorating an already-FINAL class raises too. Key it on __finalised__ == FINAL in the class's own __dict__, not a new flag — getattr is wrong because typing.final's __final__ inherits, so every descendant reads final. Measured additive: Info 486 descendants / 453 FINAL / 0 subclassed; Schema 445 / 408 / 0.

These are one decision, not two. (b) is safe with the runtime @final and lossy without it, because the @final is what replaces the guarantee the collapse removes.

Also blocks #682 — the registry is keyed on __name__.upper() (foundation/registry/protocols.py:219), so a user class named HTTP still displaces the built-in, and two distinct HTTP classes are already live in the ProtocolBase descendant tree. Opt-in-by-keyword registration makes that displacement explicit instead of automatic, which is the cheapest available fix for #682.

Needed from the maintainer

Did closing #514 with "nothing added" drop (b) and the @final enforcement as well, or only the custom-versus-builtin check? The rulings above read as decided in principle, but they were recorded on an issue that then closed with nothing added, so I will not start ≈390 edit points on that reading. One of:

  1. Both still wanted — I sequence them together as one programme, five PRs, @final first so the collapse is not lossy.
  2. @final enforcement only — cheap, additive, 0 in-tree breakage, and it does not touch the public API. register_protocol's key space is still not unique: the three HTTP classes share one key (follow-up to #675) #682 then stays open on its own merits.
  3. Both dropped — I close this and register_protocol's key space is still not unique: the three HTTP classes share one key (follow-up to #675) #682 records the __name__.upper() displacement as accepted residual risk.

Nothing else is blocked on the answer, so this is a quiet hold rather than a stall.

No activity

Activity on this issue will appear here.

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

    breakingBreaks public-facing behaviour or API (apply alongside the type label)designA design or decision issue: a pattern being decided rather than a defect or a request

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions