Trim comment essays and hoist function-local test imports

esphome/arduino/library.py

@@ -1,25 +1,12 @@
 """Arduino-core backend for the shared PlatformIO library converter.
 
-Turns the libraries registered via ``cg.add_library()`` into build inputs for
-a native Arduino build. Bare names that exist under the framework's bundled
-``libraries/`` directory (ESP8266WiFi, Wire, SPI, ...) are read straight from
-the framework tree; everything else goes through the shared
-resolution/download pipeline in ``esphome.platformio.library``. Nothing here
-is core-specific: the caller names the PlatformIO platform, MCU, and cache
-key of the Arduino core it builds.
+Bundled names build straight from the framework tree; everything else goes
+through ``esphome.platformio.library``. Mirrors ``lib_ldf_mode=off``: each
+library builds its own archive; all include dirs join one global path.
 
-Known deviations: flat-layout (``library.properties``, no ``src/``)
-libraries get the recursive default source filter rather than PlatformIO's
-root-only Arduino-1.0 filter (no bundled library is affected), and the
-Arduino ``dot_a_linkage`` property is honored even though PlatformIO
-ignores it. Bundled libraries never run a manifest ``extraScript`` (a
-warning names the library if one declares it). Manifest ``-I`` build
-flags join the global include path rather than staying private to the
-library's own sources as under PlatformIO.
-
-Mirrors PlatformIO's ``lib_ldf_mode=off`` behavior: each library builds into
-its own static archive and every library's include dir joins one global
-include path.
+Deviations from PlatformIO: flat-layout libraries get the recursive default
+source filter; ``dot_a_linkage`` is honored; bundled libraries never run a
+manifest ``extraScript``; manifest ``-I`` flags join the global include path.
 """
 
 from __future__ import annotations
@@ -101,12 +88,8 @@ def _warn_properties_depends(name: str, data: object) -> None:
 
 
 def _manifest_build(name: str, data: object) -> dict:
-    """The manifest's ``build`` section, validated by name.
-
-    A bare json.load imposes no shape; a malformed manifest must name the
-    library instead of an AttributeError deep in a traceback (and must do so
-    before apply_extra_script dereferences the same section).
-    """
+    """The manifest's ``build`` section; a malformed manifest must fail
+    naming the library, not with an AttributeError."""
     build = data.get("build", {}) if isinstance(data, dict) else None
     if not isinstance(build, dict):
         raise EsphomeError(f"Library {name} has a malformed manifest")
@@ -119,9 +102,8 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary:
 
     # PIO's source-dir resolution: manifest srcDir, else src/Src, else the root
     if "srcDir" in build:
-        # An explicitly declared srcDir (falsy included) that does not
-        # resolve is unambiguously a manifest/tree error; a silently empty
-        # source set would surface as link errors far from the cause
+        # A declared srcDir (falsy included) that does not resolve is a
+        # manifest error
         src_dir = build["srcDir"]
         if not (
             isinstance(src_dir, str) and src_dir and (read_path / src_dir).is_dir()
@@ -138,11 +120,8 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary:
     # PlatformIO shell-lexes each build.flags entry
     flag_tokens = lex_build_flags(build.get("flags", []), f"library {name}")
 
-    # build.libArchive is PIO behavior; dot_a_linkage is honored as a
-    # deliberate extra (Arduino IDE's property, which PIO ignores) so
-    # properties-only libraries can opt out of archiving too. Both parse
-    # through the same strict table: bool("false") is True, and a typo'd
-    # value must not silently change link semantics.
+    # dot_a_linkage (Arduino IDE's property, ignored by PIO) is a deliberate
+    # extra. Strict parse: bool("false") is True.
     def _parse_archive(key: str, raw: object) -> bool:
         if isinstance(raw, bool):
             return raw
@@ -233,9 +212,7 @@ def _bundled_library(framework_path: Path, name: str) -> ArduinoLibrary:
         manifest = lib_dir / "library.properties"
         data = parse_library_properties(manifest) if manifest.is_file() else {}
     if isinstance(data, dict):
-        # The dependency walk never runs for bundled libraries (a no-op for
-        # the ESP8266 core, whose bundled manifests declare none); on a core
-        # where one does, the skip must be visible before link errors
+        # Bundled manifest deps are never walked; make the skip visible
         if data.get("dependencies"):
             _LOGGER.warning(
                 "Bundled library %s declares dependencies, which are not "
@@ -296,10 +273,8 @@ def resolve_libraries(
     # PlatformIO's lib_ignore covers framework-bundled libraries too; the
     # shared converter only filters the registry/git ones.
     lib_ignore = lib_ignore_set()
-    # One memoized answer to "does the framework bundle this name?" for the
-    # classification loop, the provides hook, and the dependency walk: the
-    # safety guard and the dir probe must stay fused (path traversal), and
-    # common names (Wire, SPI) are asked repeatedly
+    # Memoized "does the framework bundle this name?"; the safety guard and
+    # dir probe must stay fused (path traversal)
     _provided = functools.cache(
         lambda name: (
             _is_safe_library_name(name)
@@ -309,16 +284,11 @@ def resolve_libraries(
     for library in CORE.platformio_libraries.values():
         if is_lib_ignored(library.name, lib_ignore):
             continue
-        # Only a bare name with a matching framework directory is bundled: a
-        # version pin means a registry package ("pngle@1.1.0"), and a bare
-        # name without the directory resolves from the registry at the
-        # latest version, matching PlatformIO (a typo fails loudly as a
-        # registry lookup error).
+        # Bundled only for a bare name with a matching framework dir; pinned
+        # or unmatched names resolve from the registry, as under PlatformIO.
         if not library.repository and not library.version and _provided(library.name):
-            # A bundled library's own manifest dependencies are not walked.
-            # PlatformIO would walk them even under lib_ldf_mode=off, but no
-            # library bundled with the ESP8266 core declares any, so the walk
-            # is a no-op there; core add_library() calls list what they need.
+            # Bundled libraries' own manifest deps are not walked (none of
+            # the ESP8266 core's declare any; _bundled_library warns if one does)
             bundled.append(_bundled_library(framework_path, library.name))
         else:
             external.append(library)
@@ -328,9 +298,8 @@ def resolve_libraries(
     converted_manifest_names: set[str] = set()
     # Ordered set of bundled dependency names to add once conversion is done
     pending_bundled: dict[str, None] = {}
-    # Short names of the separately-requested externals: a manifest
-    # dependency matching one is already in the build, not a bundled name to
-    # add (a duplicate archive shows up as duplicate-symbol link errors)
+    # Deps matching a separately-requested external are already in the build
+    # (a duplicate archive means duplicate-symbol link errors)
     external_short_names = {
         _external_short_name(lib.name) for lib in external if lib.name
     }
@@ -360,18 +329,14 @@ def resolve_libraries(
             ):
                 continue
             if dep.get("owner") or not _provided(name):
-                # The converter resolves owner-qualified and non-bundled
-                # names from the registry; an owner-less name that exists in
-                # the framework tree prefers the bundled copy ({"Wire": "*"}
-                # normalizes to version="*"), matching PlatformIO's
-                # process_dependencies. The shared walk's post-emit
-                # reconciliation reports any real drops.
+                # Owner-less names in the framework tree prefer the bundled
+                # copy (PIO's process_dependencies); everything else resolves
+                # via the converter, and the walk reports any real drops
                 continue
             if not dependency_is_usable(dep, pio_platform, "arduino", component.name):
                 continue
-            # Deferred: a later-emitted converted library may satisfy this
-            # name (its manifest name is only known at its own emit), and
-            # adding the bundled copy too would double the archive
+            # Deferred: a later-emitted library's manifest name may satisfy
+            # this; adding now could double the archive
             pending_bundled.setdefault(name)
 
     def _emit(component: ConvertedLibrary) -> None:
@@ -388,9 +353,6 @@ def resolve_libraries(
         _add_bundled_dependencies(component)
 
     if external:
-        # Every converter drop path raises (an incompatible top-level is a
-        # RuntimeError, resolution and download failures raise), so the
-        # return needs no re-verification here.
         convert_libraries(
             external,
             LibraryBackend(
@@ -398,10 +360,8 @@ def resolve_libraries(
                 framework="arduino",
                 emit=_emit,
                 cache_key=cache_key,
-                # The graph walk must not resolve a bundled name from the
-                # registry ({"Wire": "*"} in a manifest); the bundled copy
-                # is added by _add_bundled_dependencies after emit. Unsafe
-                # names are simply not provided.
+                # The walk must not resolve bundled names from the registry;
+                # _add_bundled_dependencies adds them after emit
                 provides=_provided,
             ),
         )

esphome/build_gen/build_tool.py

@@ -21,14 +21,9 @@ def main() -> int:
         # Remove first: ``ar rc`` replaces members but never drops ones whose
         # source was removed from the build, which would leak stale objects.
         Path(archive).unlink(missing_ok=True)
-        # Expand the response file here instead of passing @rspfile: GNU ar
-        # treats backslashes in response files as escapes, corrupting Windows
-        # paths ("sub\a.o" -> "suba.o").
-        # One path per line (rspfile_content = $in_newline). ninja shell-quotes
-        # a path containing specials, so undo a simple surrounding quote per
-        # line. Expanding into argv trades away the OS command-line length
-        # limit rspfiles dodge; the relative object paths here stay far
-        # below it.
+        # GNU ar treats backslashes in response files as escapes (corrupts
+        # Windows paths), so expand the rspfile into argv, stripping the
+        # simple surrounding quote ninja adds to special paths.
         objects = [
             line[1:-1]
             if len(line) >= 2 and line[0] == line[-1] and line[0] in "'\""

esphome/platformio/library.py

@@ -298,10 +298,9 @@ class LibraryBackend:
     framework: str
     emit: Callable[["ConvertedLibrary"], None]
     cache_key: str
-    # When set, an owner-less manifest dependency this returns True for is
-    # skipped by the graph walk: the backend provides it outside the
-    # registry (e.g. a library bundled with the Arduino core), mirroring
-    # PlatformIO's process_dependencies preference for bundled builders.
+    # Owner-less dependency names this returns True for are skipped by the
+    # walk; the backend supplies them outside the registry (e.g. core-bundled
+    # libraries).
     provides: Callable[[str], bool] | None = None
 
 
@@ -630,13 +629,8 @@ def join_flag_args(tokens: Iterable[str], owner: str) -> list[str]:
 def dependency_is_usable(
     dep: dict, platform: str | None, framework: str, requester: str
 ) -> bool:
-    """Whether a manifest dependency passes the compatibility filter.
-
-    The routine cross-platform skip logs at debug; any other
-    ``InvalidLibrary`` cause is a dropped dependency and warns naming the
-    requester (unreachable from ``check_library_data`` today, which raises
-    only for the platform filter).
-    """
+    """Compatibility filter for a manifest dependency: platform mismatches
+    skip at debug, any other ``InvalidLibrary`` warns naming the requester."""
     try:
         check_library_data(dep, platform, framework)
     except IncompatiblePlatform as e:
@@ -696,9 +690,7 @@ def normalize_dependencies(
                 continue
             normalized.append(entry)
         elif isinstance(entry, str) and entry:
-            # PIO also accepts a bare list of names ("dependencies":
-            # ["Wire"]); dropping them here would hide a real dependency
-            # from every caller's visibility warning
+            # PIO also accepts a bare list of names ("dependencies": ["Wire"])
             normalized.append({"name": entry})
         else:
             _LOGGER.warning(
@@ -1022,10 +1014,8 @@ def convert_libraries(
             component.data.get("dependencies"), component.name
         ):
             if "name" not in dependency or "version" not in dependency:
-                # Version-less deps cannot resolve from the registry.
-                # Deferred: only the final resolution set can tell a real
-                # drop from a name another manifest resolves later, so the
-                # reconciliation after emit owns the warning
+                # Version-less deps cannot resolve from the registry; the
+                # post-emit reconciliation owns the drop warning
                 _LOGGER.debug(
                     "Skip version-less dependency %r of %s",
                     dependency.get("name"),
@@ -1053,9 +1043,8 @@ def convert_libraries(
                 # The backend adds it from its own tree; resolving it here
                 # would fetch a same-named registry package instead
                 if (pin := dependency.get("version")) and pin != "*":
-                    # The declared constraint is discarded for the bundled
-                    # copy; a too-old bundled library must not surface as
-                    # link errors with no stated cause
+                    # The version pin is discarded for the bundled copy; make
+                    # the substitution visible
                     _LOGGER.warning(
                         "Dependency %s pins version %s; using the library "
                         "bundled with the framework instead",
@@ -1123,11 +1112,9 @@ def convert_libraries(
     for component in components.values():
         backend.emit(component)
 
-    # A version-less dependency is satisfied when its request key resolved,
-    # a resolved component's manifest name matches, or the backend provides
-    # it from its own tree (e.g. the arduino bundled libraries, added by the
-    # backend after emit). Anything else is a real drop that would otherwise
-    # surface as link errors far from the cause.
+    # Warn for version-less deps nothing satisfied (request key, manifest
+    # name, or backend provides()); a silent drop surfaces as link errors
+    # far from the cause.
     resolved_manifest_names = {c.data.get("name") for c in components.values()}
     warned: set[str] = set()
     for dep_name, dep_owner, requester in skipped_versionless:

tests/unit_tests/build_gen/test_build_tool.py

@@ -3,6 +3,8 @@
 from __future__ import annotations
 
 from pathlib import Path
+import subprocess
+import sys
 from unittest.mock import MagicMock, patch
 
 import pytest
@@ -50,8 +52,6 @@ def test_unknown_mode(capsys: pytest.CaptureFixture[str]) -> None:
 
 def test_runs_as_script(tmp_path: Path) -> None:
     """The ninja rules invoke the file as a plain script."""
-    import subprocess
-    import sys
 
     src = tmp_path / "a.bin"
     src.write_text("x")

tests/unit_tests/test_arduino_library.py

@@ -12,7 +12,13 @@ import pytest
 from esphome.arduino import library as component
 from esphome.const import KEY_CORE, KEY_TARGET_PLATFORM, PLATFORM_ESP8266
 from esphome.core import CORE, EsphomeError, Library
-from esphome.platformio.library import ConvertedLibrary, LibraryBackend
+import esphome.platformio.library as pio_library
+from esphome.platformio.library import (
+    ConvertedLibrary,
+    IncompatiblePlatform,
+    InvalidLibrary,
+    LibraryBackend,
+)
 
 
 @pytest.fixture(autouse=True)
@@ -212,9 +218,7 @@ def test_resolve_libraries_bundled(tmp_path: Path) -> None:
 def test_resolve_libraries_registry_name_is_external(
     tmp_path: Path, version: str | None
 ) -> None:
-    """A name that is not bundled reaches the converter: bare resolves from
-    the registry at the latest version (matching PlatformIO and the
-    documented libraries: key) and a version pin is a registry package."""
+    """A name that is not bundled reaches the converter, bare or pinned."""
     framework = _make_framework(tmp_path)
     _add_library("pngle", version)
     with patch.object(component, "convert_libraries", return_value=[]) as mock_convert:
@@ -410,12 +414,8 @@ def test_bundled_dependency_nonplatform_rejection_warns(
     tmp_path: Path, caplog: pytest.LogCaptureFixture
 ) -> None:
     """An InvalidLibrary whose cause is not the platform filter is visible."""
-    from esphome.platformio.library import InvalidLibrary
-
     framework = _make_framework(tmp_path)
     converted = _webserver(tmp_path, {"build": {}, "dependencies": [{"name": "Wire"}]})
-    import esphome.platformio.library as pio_library
-
     with (
         _emitting_converter(converted),
         patch.object(
@@ -466,9 +466,7 @@ def test_library_info_lib_archive_parse(
 
 
 def test_bundled_dependency_dict_shorthand_prefers_bundled(tmp_path: Path) -> None:
-    """The {"Wire": "*"} dict shorthand (version="*", no owner) must resolve
-    to the bundled library, matching PIO's process_dependencies, instead of
-    being routed to the registry."""
+    """The {"Wire": "*"} dict shorthand resolves to the bundled library."""
     framework = _make_framework(tmp_path)
     converted = _webserver(tmp_path, {"build": {}, "dependencies": {"Wire": "*"}})
     with _emitting_converter(converted):
@@ -481,12 +479,8 @@ def test_bundled_dependency_platform_rejection_is_debug(
 ) -> None:
     """The typed IncompatiblePlatform (the routine cross-platform skip)
     stays at debug regardless of message wording."""
-    from esphome.platformio.library import IncompatiblePlatform
-
     framework = _make_framework(tmp_path)
     converted = _webserver(tmp_path, {"build": {}, "dependencies": [{"name": "Wire"}]})
-    import esphome.platformio.library as pio_library
-
     with (
         _emitting_converter(converted),
         patch.object(
@@ -626,11 +620,8 @@ def test_bundled_library_non_dict_manifest_skips_probes_and_raises(
 def test_dict_shorthand_dependency_skips_registry_through_real_converter(
     tmp_path: Path, monkeypatch: pytest.MonkeyPatch
 ) -> None:
-    """{"Wire": "*"} in a real manifest must never reach the registry: the
-    graph walk skips backend-provided names and the bundled copy is added
-    after emit (no converter mock; a registry touch fails the test)."""
-    import esphome.platformio.library as pio_library
-
+    """{"Wire": "*"} resolves to the bundled copy without touching the
+    registry (real converter)."""
     framework = _make_framework(tmp_path)
     _local_lib(tmp_path, {"Wire": "*"})
     # Pin the component cache to tmp_path (data_dir honors an ambient
@@ -702,8 +693,6 @@ def test_pinned_bundled_dependency_substitution_warns(
 ) -> None:
     """A non-* version pin on a backend-provided dependency is discarded
     for the bundled copy; the substitution must be visible."""
-    import esphome.platformio.library as pio_library
-
     framework = _make_framework(tmp_path)
     _local_lib(tmp_path, {"Wire": "^2.0.0"})
     monkeypatch.setenv("ESPHOME_DATA_DIR", str(tmp_path / ".esphome"))
@@ -720,9 +709,7 @@ def test_pinned_bundled_dependency_substitution_warns(
 def test_transitively_resolved_dependency_does_not_warn(
     tmp_path: Path, caplog: pytest.LogCaptureFixture
 ) -> None:
-    """A version-less dependency the walk resolved as another library's
-    registry dependency is present in the build; the skipping warning must
-    stay quiet for it."""
+    """A dependency the walk already resolved does not warn."""
     framework = _make_framework(tmp_path)
     _add_library("ESP32Async/ESPAsyncWebServer", "3.9.6")
     ws, tcp = _ws_tcp_pair(tmp_path)
@@ -754,9 +741,8 @@ def test_external_short_name(spec: str, expected: str) -> None:
 def test_converted_manifest_name_suppresses_bundled_dependency(
     tmp_path: Path,
 ) -> None:
-    """A dependency name a converted library's manifest provides is not
-    also added from the framework tree (a duplicate archive would surface
-    as duplicate-symbol link errors), even when the provider emits later."""
+    """A name a converted library's manifest provides is not also added
+    from the framework tree, even when the provider emits later."""
     framework = _make_framework(tmp_path)
     _add_library("ESP32Async/ESPAsyncWebServer", "3.9.6")
     # Requested under a different short name; only the manifest says "Wire"
@@ -814,8 +800,6 @@ def test_versionless_dependency_with_provider_stays_quiet(
 ) -> None:
     """With a provides backend the version-less skip is routine (debug) and
     the bundled copy is picked up after emit."""
-    import esphome.platformio.library as pio_library
-
     framework = _make_framework(tmp_path)
     _local_lib(tmp_path, [{"name": "Wire"}])
     monkeypatch.setenv("ESPHOME_DATA_DIR", str(tmp_path / ".esphome"))

tests/unit_tests/test_platformio_library.py

@@ -679,9 +679,8 @@ def test_walk_warns_for_nonplatform_invalid_library(
 def test_versionless_owner_qualified_dependency_warns_despite_provides(
     tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture
 ) -> None:
-    """The backend's provides() only covers owner-less names (the walk's
-    backend-provided skip has the same guard), so an owner-qualified
-    version-less dependency that nobody adds must still warn."""
+    """An owner-qualified version-less dependency is not satisfied by
+    provides(); it must still warn."""
     _patch_download_with_manifests(
         monkeypatch,
         tmp_path,
@@ -756,9 +755,8 @@ def test_versionless_url_ish_dependency_name_warns_cleanly(
 def test_versionless_dependency_matching_resolved_manifest_name_stays_quiet(
     tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture
 ) -> None:
-    """A bare dependency name satisfied by a component requested under an
-    owner-qualified spec (manifest names match) is not a drop; a nameless
-    entry is skipped without a reconciliation warning."""
+    """A bare name satisfied by an owner-qualified component's manifest
+    name is not a drop."""
     _patch_download_with_manifests(
         monkeypatch,
         tmp_path,