refactor(corekit)!: move chardet into corekit (#1712)

docs/source/pcapkit/corekit/chardet.rst

@@ -0,0 +1,18 @@
+Character Set Detection
+=======================
+
+.. module:: pcapkit.corekit.chardet
+
+:mod:`pcapkit.corekit.chardet` wraps `chardet`_ with a bounded cache, for
+turning the bytes of a text field into a :obj:`str`. It is shared by
+:meth:`StringField.post_process
+<pcapkit.corekit.fields.strings.StringField.post_process>` and
+:meth:`ProtocolBase.decode <pcapkit.protocols.protocol.Protocol.decode>`,
+which is why it sits directly under :mod:`pcapkit.corekit` -- below both of
+them -- rather than in :mod:`pcapkit.corekit.fields`.
+
+.. _chardet: https://chardet.readthedocs.io
+
+.. autofunction:: pcapkit.corekit.chardet.detect
+
+.. autodata:: pcapkit.corekit.chardet.DETECT_CACHE_SIZE

docs/source/pcapkit/corekit/index.rst

@@ -19,6 +19,7 @@ inherits from.
    :maxdepth: 2
 
    fields/index
+   chardet
    context
    enum
    infoclass

docs/source/pcapkit/utilities/index.rst

@@ -17,7 +17,6 @@ several user-refined exceptions and warnings.
    exceptions
    warnings
    logging
-   chardet
 
 Version Compatibility
 =====================

pcapkit/all.py

@@ -143,7 +143,7 @@
                                                             # Scapy engine
 
     # pcapkit.utilities -- unlisted: its names stay reachable as attributes, but
-    # ``import *`` leaves out generic helpers such as ``warn``, ``reset``,
-    # ``configure`` and ``detect``
+    # ``import *`` leaves out generic helpers such as ``warn``, ``reset`` and
+    # ``configure``
     #'beholder_ng', 'seekset_ng',                            # Decorators
 ]

pcapkit/utilities/chardet.py → pcapkit/corekit/chardet.py

@@ -2,9 +2,9 @@
 """Character Set Detection
 =============================
 
-.. module:: pcapkit.utilities.chardet
+.. module:: pcapkit.corekit.chardet
 
-:mod:`pcapkit.utilities.chardet` wraps `chardet`_ with a bounded cache, for
+:mod:`pcapkit.corekit.chardet` wraps `chardet`_ with a bounded cache, for
 turning the bytes of a text field into a :obj:`str`.
 
 .. _chardet: https://chardet.readthedocs.io

pcapkit/corekit/fields/strings.py

@@ -4,8 +4,8 @@
 import urllib.parse as urllib_parse
 from typing import TYPE_CHECKING, Any, Generic, TypeVar
 
+from pcapkit.corekit.chardet import detect
 from pcapkit.corekit.fields.field import NO_VALUE, Field
-from pcapkit.utilities.chardet import detect
 from pcapkit.utilities.compat import Dict, unquote
 from pcapkit.utilities.exceptions import FieldValueError
 

pcapkit/protocols/application/ftp.py

@@ -19,14 +19,14 @@
 
 from pcapkit.const.ftp.command import Command as Enum_Command
 from pcapkit.const.ftp.return_code import ReturnCode as Enum_ReturnCode
+from pcapkit.corekit.chardet import detect
 from pcapkit.corekit.enum import EnumLookup
 from pcapkit.protocols.application.application import Application
 from pcapkit.protocols.data.application.ftp import FTP as Data_FTP
 from pcapkit.protocols.data.application.ftp import Request as Data_Request
 from pcapkit.protocols.data.application.ftp import Response as Data_Response
 from pcapkit.protocols.misc.raw import Raw
 from pcapkit.protocols.schema.application.ftp import FTP as Schema_FTP
-from pcapkit.utilities.chardet import detect
 from pcapkit.utilities.compat import StrEnum, auto
 from pcapkit.utilities.exceptions import ProtocolError, UnsupportedCall
 

pcapkit/protocols/application/httpv1.py

@@ -32,6 +32,7 @@
 
 from pcapkit.const.http.method import Method as Enum_Method
 from pcapkit.const.http.status_code import StatusCode as Enum_StatusCode
+from pcapkit.corekit.chardet import detect
 from pcapkit.corekit.enum import EnumLookup
 from pcapkit.corekit.multidict import OrderedMultiDict
 from pcapkit.protocols.application.http import (_HTTP2_PREFACE_HEADER, _RE_METHOD, _RE_STATUS,
@@ -41,7 +42,6 @@
 from pcapkit.protocols.data.application.httpv1 import RequestHeader as Data_RequestHeader
 from pcapkit.protocols.data.application.httpv1 import ResponseHeader as Data_ResponseHeader
 from pcapkit.protocols.schema.application.httpv1 import HTTP as Schema_HTTP
-from pcapkit.utilities.chardet import detect
 from pcapkit.utilities.compat import StrEnum, auto
 from pcapkit.utilities.exceptions import ProtocolError
 

pcapkit/protocols/protocol.py

@@ -33,6 +33,7 @@
 
 import aenum
 
+from pcapkit.corekit.chardet import detect
 from pcapkit.corekit.context import ContextRegistry
 from pcapkit.corekit.fields.misc import ConditionalField, SchemaField, SwitchField
 from pcapkit.corekit.module import ModuleDescriptor
@@ -45,7 +46,6 @@
 from pcapkit.protocols.data.protocol import Packet as Data_Packet
 from pcapkit.protocols.schema.misc.raw import Raw as Schema_Raw
 from pcapkit.protocols.schema.schema import Schema
-from pcapkit.utilities.chardet import detect
 from pcapkit.utilities.compat import cached_property
 from pcapkit.utilities.decorators import beholder, seekset
 from pcapkit.utilities.exceptions import (BaseError, ProtocolError, ProtocolNotFound,

pcapkit/utilities/__init__.py

@@ -12,12 +12,11 @@
 several user-refined exceptions and warnings.
 
 """
-from pcapkit.utilities.chardet import detect
 from pcapkit.utilities.decorators import beholder, prepare, seekset
 from pcapkit.utilities.exceptions import stacklevel
 from pcapkit.utilities.logging import configure, ensure_output, get_logger, logger, reset
 from pcapkit.utilities.warnings import warn
 
 __all__ = ['logger', 'get_logger', 'configure', 'reset', 'ensure_output',
-           'warn', 'stacklevel', 'detect',
+           'warn', 'stacklevel',
            'beholder', 'prepare', 'seekset']

tests/corekit/test_chardet_housing_1712_unit.py

@@ -0,0 +1,122 @@
+# -*- coding: utf-8 -*-
+"""GitHub issue #1712: the bounded-cache :mod:`chardet` wrapper lives in corekit.
+
+:func:`~pcapkit.corekit.chardet.detect` and
+:data:`~pcapkit.corekit.chardet.DETECT_CACHE_SIZE` moved from
+``pcapkit.utilities.chardet`` to :mod:`pcapkit.corekit.chardet`, and the
+``pcapkit.utilities.detect`` re-export went with them: the owner's ruling on
+#1715, applied to #1712, is that a move updates every import path directly and
+leaves no re-export behind. This pins that the old path no longer imports, that
+every caller uses the new module, and that the move adds no import cycle -- the
+new module imports nothing from :mod:`pcapkit`, and :mod:`pcapkit.utilities`
+still imports nothing from :mod:`pcapkit.corekit`.
+
+"""
+
+import ast
+import importlib
+import json
+import pathlib
+import subprocess  # nosec: B404
+import sys
+import unittest
+
+from tests._support import reimport_once_per_class
+from tests.corekit._roundtrip import skip_without_runtime
+
+ROOT = pathlib.Path(__file__).resolve().parents[2]
+
+NEW_PATH = 'pcapkit.corekit.chardet'
+
+#: Where the wrapper lived before #1712; removed, not re-exported.
+OLD_PATH = 'pcapkit.utilities.chardet'
+
+#: Every module that calls :func:`~pcapkit.corekit.chardet.detect`.
+CALLERS = (
+    'pcapkit.corekit.fields.strings',
+    'pcapkit.protocols.protocol',
+    'pcapkit.protocols.application.ftp',
+    'pcapkit.protocols.application.httpv1',
+)
+
+
+def imported_modules(path: 'pathlib.Path') -> 'list[str]':
+    """Every module ``path`` imports, read with :mod:`ast` rather than imported."""
+    names = []
+    for node in ast.walk(ast.parse(path.read_text(encoding='utf-8'))):
+        if isinstance(node, ast.Import):
+            names.extend(alias.name for alias in node.names)
+        elif isinstance(node, ast.ImportFrom):
+            names.append('.' * node.level + (node.module or ''))
+    return names
+
+
+class ChardetHousingTests(unittest.TestCase):
+    """The wrapper is at :data:`NEW_PATH` and nowhere else."""
+
+    def setUp(self) -> None:
+        skip_without_runtime(self)
+        reimport_once_per_class(self)
+
+    def test_the_old_import_path_no_longer_imports(self) -> None:
+        """No re-export is left at ``pcapkit.utilities.chardet``."""
+        with self.assertRaises(ModuleNotFoundError) as ctx:
+            importlib.import_module(OLD_PATH)
+        self.assertEqual(ctx.exception.name, OLD_PATH)
+
+    def test_detect_is_no_longer_reexported(self) -> None:
+        """``detect`` is gone from :mod:`pcapkit.utilities`, and so from :mod:`pcapkit.all`."""
+        for package in ('pcapkit.utilities', 'pcapkit.all'):
+            with self.subTest(package=package):
+                module = importlib.import_module(package)
+                self.assertNotIn('detect', getattr(module, '__all__', ()))
+                self.assertFalse(hasattr(module, 'detect'))
+
+    def test_the_wrapper_keeps_its_bound(self) -> None:
+        """The cache stays bounded at :data:`~pcapkit.corekit.chardet.DETECT_CACHE_SIZE`."""
+        module = importlib.import_module(NEW_PATH)
+        self.assertEqual(module.__all__, ['detect'])
+        self.assertEqual(module.DETECT_CACHE_SIZE, 1024)
+        self.assertEqual(module.detect.cache_info().maxsize, module.DETECT_CACHE_SIZE)
+
+    def test_every_caller_uses_the_new_module(self) -> None:
+        """Each caller holds the one cached function, so they share its cache."""
+        detect = importlib.import_module(NEW_PATH).detect
+        for name in CALLERS:
+            with self.subTest(module=name):
+                self.assertIs(importlib.import_module(name).detect, detect)
+
+
+class ChardetLayeringTests(unittest.TestCase):
+    """The move adds no edge that could close an import cycle."""
+
+    def test_the_new_module_imports_nothing_from_pcapkit(self) -> None:
+        """A leaf module cannot close a cycle through its importers."""
+        path = ROOT.joinpath(*NEW_PATH.split('.')).with_suffix('.py')
+        names = imported_modules(path)
+        self.assertIn('chardet', names)
+        self.assertEqual([name for name in names if name.startswith(('pcapkit', '.'))], [])
+
+    def test_utilities_imports_nothing_from_corekit(self) -> None:
+        """:mod:`pcapkit.utilities` sits below :mod:`pcapkit.corekit` and does not reach up."""
+        for path in sorted(ROOT.joinpath('pcapkit', 'utilities').glob('*.py')):
+            with self.subTest(path=path.name):
+                self.assertEqual([name for name in imported_modules(path)
+                                  if name.startswith('pcapkit.corekit')], [])
+
+    def test_the_new_module_imports_first_in_a_fresh_interpreter(self) -> None:
+        """Imported first, from the tree under test, with nothing loaded at the old path."""
+        skip_without_runtime(self)
+        script = ('import json, sys, pcapkit.corekit.chardet as m; '
+                  'print(json.dumps([m.__file__, sorted(n for n in sys.modules'
+                  ' if n.startswith("pcapkit.") and n.endswith(".chardet"))]))')
+        proc = subprocess.run([sys.executable, '-c', script], cwd=str(ROOT),  # nosec: B603
+                              capture_output=True, text=True, timeout=120, check=False)
+        self.assertEqual(proc.returncode, 0, proc.stderr)
+        path, loaded = json.loads(proc.stdout.splitlines()[-1])
+        self.assertEqual(pathlib.Path(path).resolve(), ROOT / 'pcapkit' / 'corekit' / 'chardet.py')
+        self.assertEqual(loaded, [NEW_PATH])
+
+
+if __name__ == '__main__':
+    unittest.main()

tests/corekit/test_stringfield_lossless_unit.py

@@ -3,7 +3,7 @@
 
 GitHub issue #1293: :class:`~pcapkit.corekit.fields.strings.StringField` was
 lossy three ways. Without an ``encoding`` it decoded with the charset
-:func:`~pcapkit.utilities.chardet.detect` guessed and packed the text back as
+:func:`~pcapkit.corekit.chardet.detect` guessed and packed the text back as
 UTF-8; an octet the charset could not decode became U+FFFD and was packed as
 U+FFFD's own three octets; and a length of ``-1`` was taken from the *character*
 count, so multi-octet text lost its tail.

tests/project/test_all_exports_unit.py

@@ -24,12 +24,12 @@
 #: Names an aggregated package exports and :mod:`pcapkit.all` deliberately does
 #: not list, keyed by the package. :mod:`pcapkit.utilities` has been unlisted
 #: since ``769a17c78`` (2022-05-29), which commented its group out: its exports
-#: are generic helpers -- ``warn``, ``reset``, ``configure``, ``detect`` -- that
+#: are generic helpers -- ``warn``, ``reset``, ``configure`` -- that
 #: ``import *`` should not drop into the caller's namespace. A name added here is
 #: an API decision.
 DELIBERATE_NON_EXPORTS = {
     'pcapkit.utilities': ('logger', 'get_logger', 'configure', 'reset', 'ensure_output',
-                          'warn', 'stacklevel', 'detect', 'beholder', 'prepare', 'seekset'),
+                          'warn', 'stacklevel', 'beholder', 'prepare', 'seekset'),
 }