From 3ba68ac5eb79fa12996c0e1faba94efeba9b9f52 Mon Sep 17 00:00:00 2001 From: Tim Date: Mon, 31 Aug 2026 13:48:29 -0700 Subject: [PATCH 1/2] fix: annotate the query views with the classes they return `BodyView.blocks()` only ever appends a `BlockView`, `attributes()` only an `AttributeView`, and `BlockView.body` is always a `BodyView`, but all of them were annotated `NodeView`. Callers under a strict type checker could not reach `block_type`, `labels`, `name_labels` or `AttributeView.name` without an `isinstance` narrowing or a cast for a runtime type that is never anything else. Narrow the annotations on `DocumentView`, `BodyView` and `BlockView`. The view classes stay imported inside the method bodies -- the cycle is real -- with `TYPE_CHECKING` imports added for the annotations alone, so there is no runtime change of any kind. The new tests assert the annotations rather than the runtime types: a runtime check passed before this change too, which is why nothing caught it. --- CHANGELOG.md | 4 +- hcl2/query/blocks.py | 14 ++-- hcl2/query/body.py | 22 +++--- test/unit/query/test_view_annotations.py | 88 ++++++++++++++++++++++++ 4 files changed, 113 insertions(+), 15 deletions(-) create mode 100644 test/unit/query/test_view_annotations.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 61dc143a..f9e94075 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## \[Unreleased\] -- Nothing yet. +### Changed + +- `blocks()` and `attributes()` on the query views are annotated as returning `List[BlockView]` and `List[AttributeView]` rather than `List[NodeView]`, and `BlockView.body` as `BodyView`. Each only ever returns the concrete class; the wider annotation put `block_type`, `labels`, `name_labels` and `AttributeView.name` behind an `isinstance` narrowing or a cast for callers under a strict type checker. Annotation-only, no runtime change. ## \[8.1.3\] - 2026-08-26 diff --git a/hcl2/query/blocks.py b/hcl2/query/blocks.py index 269f2209..4f623d9d 100644 --- a/hcl2/query/blocks.py +++ b/hcl2/query/blocks.py @@ -1,6 +1,6 @@ """BlockView facade.""" -from typing import Any, List, Optional +from typing import TYPE_CHECKING, Any, List, Optional from hcl2.const import COMMENTS_KEY from hcl2.query._base import NodeView, register_view @@ -10,6 +10,10 @@ from hcl2.rules.strings import StringRule from hcl2.utils import SerializationOptions +if TYPE_CHECKING: # imported at runtime inside the methods, to break the cycle + from hcl2.query.attributes import AttributeView + from hcl2.query.body import BodyView + def _label_to_str(label) -> str: """Convert a block label (IdentifierRule or StringRule) to a plain string.""" @@ -54,7 +58,7 @@ def name_labels(self) -> List[str]: return self.labels[1:] @property - def body(self) -> "NodeView": + def body(self) -> "BodyView": """Return the block body as a BodyView.""" from hcl2.query.body import BodyView @@ -76,21 +80,21 @@ def to_dict(self, options: Optional[SerializationOptions] = None) -> Any: result[COMMENTS_KEY] = self._adjacent_comments + existing return result - def blocks(self, block_type: Optional[str] = None, *labels: str) -> List["NodeView"]: + def blocks(self, block_type: Optional[str] = None, *labels: str) -> List["BlockView"]: """Delegate to body.""" from hcl2.query.body import BodyView node: BlockRule = self._node # type: ignore[assignment] return BodyView(node.body).blocks(block_type, *labels) - def attributes(self, name: Optional[str] = None) -> List["NodeView"]: + def attributes(self, name: Optional[str] = None) -> List["AttributeView"]: """Delegate to body.""" from hcl2.query.body import BodyView node: BlockRule = self._node # type: ignore[assignment] return BodyView(node.body).attributes(name) - def attribute(self, name: str) -> Optional["NodeView"]: + def attribute(self, name: str) -> Optional["AttributeView"]: """Delegate to body.""" from hcl2.query.body import BodyView diff --git a/hcl2/query/body.py b/hcl2/query/body.py index b9f2ce54..dd7d7b13 100644 --- a/hcl2/query/body.py +++ b/hcl2/query/body.py @@ -1,11 +1,15 @@ """DocumentView and BodyView facades.""" -from typing import List, Optional +from typing import TYPE_CHECKING, List, Optional from hcl2.query._base import NodeView, register_view from hcl2.rules.base import AttributeRule, BlockRule, BodyRule, StartRule from hcl2.rules.whitespace import NewLineOrCommentRule +if TYPE_CHECKING: # imported at runtime inside the methods, to break the cycle + from hcl2.query.attributes import AttributeView + from hcl2.query.blocks import BlockView + def _collect_leading_comments(body: BodyRule, child_index: int) -> List[dict]: """Collect comments from NewLineOrCommentRule siblings preceding *child_index*. @@ -59,15 +63,15 @@ def body(self) -> "BodyView": node: StartRule = self._node # type: ignore[assignment] return BodyView(node.body) - def blocks(self, block_type: Optional[str] = None, *labels: str) -> List["NodeView"]: + def blocks(self, block_type: Optional[str] = None, *labels: str) -> List["BlockView"]: """Return matching blocks, delegating to body.""" return self.body.blocks(block_type, *labels) - def attributes(self, name: Optional[str] = None) -> List["NodeView"]: + def attributes(self, name: Optional[str] = None) -> List["AttributeView"]: """Return matching attributes, delegating to body.""" return self.body.attributes(name) - def attribute(self, name: str) -> Optional["NodeView"]: + def attribute(self, name: str) -> Optional["AttributeView"]: """Return a single attribute by name, or None.""" return self.body.attribute(name) @@ -76,12 +80,12 @@ def attribute(self, name: str) -> Optional["NodeView"]: class BodyView(NodeView): """View over an HCL2 body (BodyRule).""" - def blocks(self, block_type: Optional[str] = None, *labels: str) -> List["NodeView"]: + def blocks(self, block_type: Optional[str] = None, *labels: str) -> List["BlockView"]: """Return blocks, optionally filtered by type and labels.""" from hcl2.query.blocks import BlockView node: BodyRule = self._node # type: ignore[assignment] - results: List[NodeView] = [] + results: List["BlockView"] = [] for child in node.children: if not isinstance(child, BlockRule): continue @@ -98,12 +102,12 @@ def blocks(self, block_type: Optional[str] = None, *labels: str) -> List["NodeVi results.append(block_view) return results - def attributes(self, name: Optional[str] = None) -> List["NodeView"]: + def attributes(self, name: Optional[str] = None) -> List["AttributeView"]: """Return attributes, optionally filtered by name.""" from hcl2.query.attributes import AttributeView node: BodyRule = self._node # type: ignore[assignment] - results: List[NodeView] = [] + results: List["AttributeView"] = [] for child in node.children: if not isinstance(child, AttributeRule): continue @@ -114,7 +118,7 @@ def attributes(self, name: Optional[str] = None) -> List["NodeView"]: results.append(attr_view) return results - def attribute(self, name: str) -> Optional["NodeView"]: + def attribute(self, name: str) -> Optional["AttributeView"]: """Return a single attribute by name, or None.""" attrs = self.attributes(name) return attrs[0] if attrs else None diff --git a/test/unit/query/test_view_annotations.py b/test/unit/query/test_view_annotations.py new file mode 100644 index 00000000..ed7fc37c --- /dev/null +++ b/test/unit/query/test_view_annotations.py @@ -0,0 +1,88 @@ +# pylint: disable=C0103,C0114,C0115,C0116 +"""The query views' return annotations name the class they actually return. + +`blocks()` only ever appends a `BlockView` and `attributes()` only ever an +`AttributeView`, but both were annotated `List[NodeView]`. Under a strict type +checker that put `block_type`, `labels`, `name_labels` and `AttributeView.name` +out of reach without an `isinstance` narrowing or a cast for a runtime type +that is never anything else. + +These assert the annotations rather than the runtime types, because a runtime +check passes either way -- it is only the declaration that was wrong. +""" + +from typing import List, Optional, get_type_hints +from unittest import TestCase + +from hcl2.query.attributes import AttributeView +from hcl2.query.blocks import BlockView +from hcl2.query.body import BodyView, DocumentView + +# `blocks()` and `attributes()` import their view classes inside the method to +# break an import cycle, so the annotations resolve only against this mapping. +_NAMESPACE = { + "AttributeView": AttributeView, + "BlockView": BlockView, + "BodyView": BodyView, +} + + +def _returns(method): + return get_type_hints(method, localns=_NAMESPACE)["return"] + + +class TestBodyViewAnnotations(TestCase): + def test_blocks_returns_block_views(self): + self.assertEqual(_returns(BodyView.blocks), List[BlockView]) + + def test_attributes_returns_attribute_views(self): + self.assertEqual(_returns(BodyView.attributes), List[AttributeView]) + + def test_attribute_returns_an_optional_attribute_view(self): + self.assertEqual(_returns(BodyView.attribute), Optional[AttributeView]) + + +class TestDocumentViewAnnotations(TestCase): + """The document-level methods delegate to the body and must not re-widen.""" + + def test_blocks_returns_block_views(self): + self.assertEqual(_returns(DocumentView.blocks), List[BlockView]) + + def test_attributes_returns_attribute_views(self): + self.assertEqual(_returns(DocumentView.attributes), List[AttributeView]) + + def test_attribute_returns_an_optional_attribute_view(self): + self.assertEqual(_returns(DocumentView.attribute), Optional[AttributeView]) + + +class TestBlockViewAnnotations(TestCase): + def test_blocks_returns_block_views(self): + self.assertEqual(_returns(BlockView.blocks), List[BlockView]) + + def test_attributes_returns_attribute_views(self): + self.assertEqual(_returns(BlockView.attributes), List[AttributeView]) + + def test_attribute_returns_an_optional_attribute_view(self): + self.assertEqual(_returns(BlockView.attribute), Optional[AttributeView]) + + def test_body_returns_a_body_view(self): + self.assertEqual(_returns(BlockView.body.fget), BodyView) + + +class TestAnnotationsMatchRuntime(TestCase): + """The declarations above are only worth having if they stay true.""" + + SOURCE = 'resource "aws_instance" "web" {\n ami = "ami-1"\n}\n' + + def test_blocks_are_block_views(self): + doc = DocumentView.parse(self.SOURCE) + self.assertTrue(all(isinstance(block, BlockView) for block in doc.blocks())) + + def test_attributes_are_attribute_views(self): + doc = DocumentView.parse(self.SOURCE) + block = doc.blocks("resource")[0] + self.assertTrue(all(isinstance(attr, AttributeView) for attr in block.attributes())) + + def test_block_body_is_a_body_view(self): + doc = DocumentView.parse(self.SOURCE) + self.assertIsInstance(doc.blocks("resource")[0].body, BodyView) From 8c8f64b09abf594ae9655e10d6ea0e2d3e6056e7 Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 1 Sep 2026 16:04:47 -0700 Subject: [PATCH 2/2] fix: bind the view classes so their annotations resolve at runtime The narrowed return annotations named `BlockView`, `BodyView` and `AttributeView` as forward references while the classes were imported inside each method. `typing.get_type_hints` reads a function's own globals, so every one of those annotations raised `NameError` for any caller that introspected it -- pydantic, a documentation builder, a runtime validator -- even though the classes were importable. Before the narrowing the annotations named `NodeView`, which is imported at module level, so this was a regression rather than a pre-existing gap. `AttributeView` has no cycle to break and moves to a plain top-level import. `BodyView` and `BlockView` do name each other, so each module imports the other at the bottom, after its own classes exist: the name lands in module globals, which is what resolution needs, and the cycle still cannot bite because neither import runs before the classes are defined. Verified under all three import orders. The tests asked for the hints with a hand-built `localns`, which supplied exactly the names that were missing and so could not see this. They now call `get_type_hints` bare, the way a consumer does. --- CHANGELOG.md | 2 +- hcl2/query/blocks.py | 21 ++++------ hcl2/query/body.py | 19 ++++----- test/unit/query/test_view_annotations.py | 50 +++++++++++++++++++----- 4 files changed, 60 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9e94075..ad3e6564 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Changed -- `blocks()` and `attributes()` on the query views are annotated as returning `List[BlockView]` and `List[AttributeView]` rather than `List[NodeView]`, and `BlockView.body` as `BodyView`. Each only ever returns the concrete class; the wider annotation put `block_type`, `labels`, `name_labels` and `AttributeView.name` behind an `isinstance` narrowing or a cast for callers under a strict type checker. Annotation-only, no runtime change. +- `blocks()` and `attributes()` on the query views are annotated as returning `List[BlockView]` and `List[AttributeView]` rather than `List[NodeView]`, and `BlockView.body` as `BodyView`. Each only ever returns the concrete class; the wider annotation put `block_type`, `labels`, `name_labels` and `AttributeView.name` behind an `isinstance` narrowing or a cast for callers under a strict type checker. The view classes are now imported at module level rather than inside each method, so `typing.get_type_hints` can resolve the annotations the way any consumer reads them; the values returned are unchanged. ## \[8.1.3\] - 2026-08-26 diff --git a/hcl2/query/blocks.py b/hcl2/query/blocks.py index 4f623d9d..260de9ac 100644 --- a/hcl2/query/blocks.py +++ b/hcl2/query/blocks.py @@ -1,19 +1,16 @@ """BlockView facade.""" -from typing import TYPE_CHECKING, Any, List, Optional +from typing import Any, List, Optional from hcl2.const import COMMENTS_KEY from hcl2.query._base import NodeView, register_view +from hcl2.query.attributes import AttributeView from hcl2.rules.abstract import LarkElement from hcl2.rules.base import BlockRule from hcl2.rules.literal_rules import IdentifierRule from hcl2.rules.strings import StringRule from hcl2.utils import SerializationOptions -if TYPE_CHECKING: # imported at runtime inside the methods, to break the cycle - from hcl2.query.attributes import AttributeView - from hcl2.query.body import BodyView - def _label_to_str(label) -> str: """Convert a block label (IdentifierRule or StringRule) to a plain string.""" @@ -60,8 +57,6 @@ def name_labels(self) -> List[str]: @property def body(self) -> "BodyView": """Return the block body as a BodyView.""" - from hcl2.query.body import BodyView - node: BlockRule = self._node # type: ignore[assignment] return BodyView(node.body) @@ -82,21 +77,21 @@ def to_dict(self, options: Optional[SerializationOptions] = None) -> Any: def blocks(self, block_type: Optional[str] = None, *labels: str) -> List["BlockView"]: """Delegate to body.""" - from hcl2.query.body import BodyView - node: BlockRule = self._node # type: ignore[assignment] return BodyView(node.body).blocks(block_type, *labels) def attributes(self, name: Optional[str] = None) -> List["AttributeView"]: """Delegate to body.""" - from hcl2.query.body import BodyView - node: BlockRule = self._node # type: ignore[assignment] return BodyView(node.body).attributes(name) def attribute(self, name: str) -> Optional["AttributeView"]: """Delegate to body.""" - from hcl2.query.body import BodyView - node: BlockRule = self._node # type: ignore[assignment] return BodyView(node.body).attribute(name) + + +# See the note in `hcl2/query/body.py`: the two modules name each other in their +# annotations, and binding the name here rather than inside each method is what +# lets `typing.get_type_hints` resolve them. +from hcl2.query.body import BodyView # noqa: E402 pylint: disable=wrong-import-position,cyclic-import diff --git a/hcl2/query/body.py b/hcl2/query/body.py index dd7d7b13..a3415d17 100644 --- a/hcl2/query/body.py +++ b/hcl2/query/body.py @@ -1,15 +1,12 @@ """DocumentView and BodyView facades.""" -from typing import TYPE_CHECKING, List, Optional +from typing import List, Optional from hcl2.query._base import NodeView, register_view +from hcl2.query.attributes import AttributeView from hcl2.rules.base import AttributeRule, BlockRule, BodyRule, StartRule from hcl2.rules.whitespace import NewLineOrCommentRule -if TYPE_CHECKING: # imported at runtime inside the methods, to break the cycle - from hcl2.query.attributes import AttributeView - from hcl2.query.blocks import BlockView - def _collect_leading_comments(body: BodyRule, child_index: int) -> List[dict]: """Collect comments from NewLineOrCommentRule siblings preceding *child_index*. @@ -82,8 +79,6 @@ class BodyView(NodeView): def blocks(self, block_type: Optional[str] = None, *labels: str) -> List["BlockView"]: """Return blocks, optionally filtered by type and labels.""" - from hcl2.query.blocks import BlockView - node: BodyRule = self._node # type: ignore[assignment] results: List["BlockView"] = [] for child in node.children: @@ -104,8 +99,6 @@ def blocks(self, block_type: Optional[str] = None, *labels: str) -> List["BlockV def attributes(self, name: Optional[str] = None) -> List["AttributeView"]: """Return attributes, optionally filtered by name.""" - from hcl2.query.attributes import AttributeView - node: BodyRule = self._node # type: ignore[assignment] results: List["AttributeView"] = [] for child in node.children: @@ -122,3 +115,11 @@ def attribute(self, name: str) -> Optional["AttributeView"]: """Return a single attribute by name, or None.""" attrs = self.attributes(name) return attrs[0] if attrs else None + + +# `BlockView` subclasses nothing here but names `BodyView` in its own annotations, +# so the two modules refer to each other. Importing at the bottom -- after both +# classes exist -- breaks the cycle while still binding the name in this module's +# globals, which is where `typing.get_type_hints` looks. Deferring it into the +# methods instead would leave the public annotations unresolvable to any caller. +from hcl2.query.blocks import BlockView # noqa: E402 pylint: disable=wrong-import-position,cyclic-import diff --git a/test/unit/query/test_view_annotations.py b/test/unit/query/test_view_annotations.py index ed7fc37c..fccaf648 100644 --- a/test/unit/query/test_view_annotations.py +++ b/test/unit/query/test_view_annotations.py @@ -9,26 +9,24 @@ These assert the annotations rather than the runtime types, because a runtime check passes either way -- it is only the declaration that was wrong. + +They resolve them the way a consumer does: a bare `get_type_hints`, with no +namespace supplied. Passing one would hide a name the annotation cannot reach +on its own, which is the failure mode a forward reference invites. """ from typing import List, Optional, get_type_hints from unittest import TestCase +from hcl2.query import blocks as blocks_module +from hcl2.query import body as body_module from hcl2.query.attributes import AttributeView from hcl2.query.blocks import BlockView from hcl2.query.body import BodyView, DocumentView -# `blocks()` and `attributes()` import their view classes inside the method to -# break an import cycle, so the annotations resolve only against this mapping. -_NAMESPACE = { - "AttributeView": AttributeView, - "BlockView": BlockView, - "BodyView": BodyView, -} - def _returns(method): - return get_type_hints(method, localns=_NAMESPACE)["return"] + return get_type_hints(method)["return"] class TestBodyViewAnnotations(TestCase): @@ -86,3 +84,37 @@ def test_attributes_are_attribute_views(self): def test_block_body_is_a_body_view(self): doc = DocumentView.parse(self.SOURCE) self.assertIsInstance(doc.blocks("resource")[0].body, BodyView) + + +class TestAnnotationsResolveUnaided(TestCase): + """The names the annotations use have to live in the defining module. + + `get_type_hints` reads a function's own globals. While the view classes were + imported inside the methods, every one of these annotations raised + `NameError` for anyone who introspected them -- pydantic, a documentation + builder, a runtime validator -- even though the classes were importable. + """ + + def test_body_module_binds_block_view(self): + self.assertIs(body_module.BlockView, BlockView) + + def test_blocks_module_binds_body_view(self): + self.assertIs(blocks_module.BodyView, BodyView) + + def test_every_annotated_member_resolves_without_a_namespace(self): + members = [ + BodyView.blocks, + BodyView.attributes, + BodyView.attribute, + DocumentView.blocks, + DocumentView.attributes, + DocumentView.attribute, + DocumentView.body.fget, + BlockView.blocks, + BlockView.attributes, + BlockView.attribute, + BlockView.body.fget, + ] + for member in members: + with self.subTest(member=member.__qualname__): + self.assertIn("return", get_type_hints(member))