Remove extra coverage comments

Author: dan-blanchard

ninja/signature/details.py

@@ -393,32 +393,14 @@ def detect_collection_fields(
 
                 # check union types
                 if get_origin(annotation_or_field) in UNION_TYPES:
-                    found = False
-                    # This for loop is unreachable in practice because union types with missing fields
-                    # should be handled earlier in the validation process
-                    for arg in get_args(annotation_or_field):
-                        # This continue path is unreachable because NoneType handling is done earlier in union processing
-                        if arg is type(None):
-                            continue  # Skip NoneType
-                        if hasattr(arg, "model_fields"):
-                            found_field = next(
-                                (
-                                    a
-                                    for a in arg.model_fields.values()
-                                    if a.alias == attr
-                                ),
-                                arg.model_fields.get(attr),
-                            )
-                            if found_field is not None:
-                                annotation_or_field = found_field
-                                found = True
-                                break
-                    # This error condition is unreachable because union fields are pre-validated
-                    # and all union members should have compatible field structures
-                    if not found:
-                        # No suitable field found in any union member, skip this path
-                        annotation_or_field = None
-                        break  # Break out of the attr loop
+                    for arg in get_args(annotation_or_field):  # pragma: no branch
+                        found_field = next(
+                            (a for a in arg.model_fields.values() if a.alias == attr),
+                            arg.model_fields.get(attr),
+                        )
+                        if found_field is not None:
+                            annotation_or_field = found_field
+                            break
                 else:
                     annotation_or_field = next(
                         (
@@ -433,15 +415,7 @@ def detect_collection_fields(
                     annotation_or_field, "outer_type_", annotation_or_field
                 )
 
-            # This condition is unreachable because union processing failures are handled above
-            # and annotation_or_field should never be None at this point in normal operation
-            if annotation_or_field is None:
-                continue
-
-            # This condition is unreachable because annotation access is handled in the union processing
-            # and should not require additional annotation unwrapping at this point
-            if hasattr(annotation_or_field, "annotation"):
-                annotation_or_field = annotation_or_field.annotation
+            annotation_or_field = annotation_or_field.annotation
 
             if is_collection_type(annotation_or_field):
                 result.append(path[-1])

tests/test_models.py

@@ -51,9 +51,9 @@ class ComplexUnionModel(BaseModel):
     simple_union: Union[str, int] = "default"
 
 
-# Model with non-optional union of pydantic models to cover lines 253-254
+# Model with non-optional union of pydantic models
 class MultiModelUnion(BaseModel):
-    # Union of multiple pydantic models without None - should trigger lines 253-254
+    # Union of multiple pydantic models without None -
     models: Union[SomeModel, OtherModel]  # No default, no None
 
 
@@ -121,37 +121,34 @@ def view_union_body(request, union_body: Union[SomeModel, OtherModel]):
 
 
 @router.post("/test-optional-union")
-def view_optional_union(optional_model: Union[SomeModel, None] = Query(None)):
+def view_optional_union(request, optional_model: Union[SomeModel, None] = Query(None)):
     if optional_model is None:
         return {"result": "none"}
     return {"result": optional_model}
 
 
 @router.post("/test-nested-union")
-def view_nested_union(data: NestedUnionModel):
+def view_nested_union(request, data: NestedUnionModel):
     return data.model_dump()
 
 
 @router.post("/test-complex-union")
-def view_complex_union(data: ComplexUnionModel = Query(...)):
+def view_complex_union(request, data: ComplexUnionModel = Query(...)):
     return data
 
 
-# Test direct union parameter to cover lines 227-230 in _model_flatten_map
+# Test direct union parameter to cover _model_flatten_map
 @router.post("/test-direct-union")
 def view_direct_union(request, model: Union[SomeModel, OtherModel] = Query(...)):
     return model
 
 
-# Test union of pydantic models to cover lines 253-254
+# Test union of pydantic models
 @router.post("/test-multi-model-union")
-def view_multi_model_union(data: MultiModelUnion):
+def view_multi_model_union(request, data: MultiModelUnion):
     return data.model_dump()
 
 
-# Test edge cases to improve coverage
-
-
 # Create models that will cause field name collisions during flattening
 class ModelA(BaseModel):
     name: str
@@ -161,7 +158,7 @@ class ModelB(BaseModel):
     name: str  # Same field name as ModelA
 
 
-# Test case to trigger model field collision error (lines 207-210)
+# Test case to trigger model field collision error
 try:
     router_collision = Router()
 
@@ -177,7 +174,7 @@ def view_model_collision(
     pass
 
 
-# Test to trigger ConfigError on line 197 - duplicate name collision in union with Query(None)
+# Test to trigger ConfigError - duplicate name collision in union with Query(None)
 def test_union_query_name_collision():
     """Test that duplicate union parameter names with Query(None) raise ConfigError."""
 
@@ -186,7 +183,7 @@ def test_union_query_name_collision():
         api = NinjaAPI()
         router_test = Router()
 
-        # This should trigger the ConfigError on line 197 when both parameters
+        # This should trigger the ConfigError when both parameters
         # have the same alias and are processed as Union[Model, None] = Query(None)
         @router_test.post("/collision-test")
         def collision_endpoint(
@@ -204,19 +201,18 @@ def collision_endpoint(
         assert False, "Expected ConfigError for duplicate name collision"  # noqa: B011
 
     except ConfigError as e:
-        # This is the expected behavior - line 197 should be hit
+        # This is the expected behavior
         assert "Duplicated name" in str(e)
         assert "person" in str(e)
 
 
-# Test to trigger line 229 and other missing lines
 @router.post("/test-union-with-none")
 def view_union_with_none(request, optional: Union[str, None] = Query(None)):
-    """Test Union[str, None] to trigger line 229 (continue for NoneType)."""
+    """Test Union[str, None]"""
     return {"optional": optional}
 
 
-# Test union field with multiple pydantic models (lines 253-254)
+# Test union field with multiple pydantic models
 class UnionFieldTestModel(BaseModel):
     choice: Union[SomeModel, OtherModel]
 
@@ -239,9 +235,8 @@ def view_collection_union(request, data: CollectionUnionModel):
     return data.model_dump()
 
 
-# Additional model for testing complex nested union fields (lines 253-254)
+# Additional model for testing complex nested union fields
 class ComplexUnionField(BaseModel):
-    # This should trigger lines 253-254 since it's a non-optional union of pydantic models
     model_choice: Union[SomeModel, OtherModel]  # No None, no default
     name: str = "test"
 
@@ -252,20 +247,19 @@ def view_complex_union_field(request, complex_data: ComplexUnionField):
     return complex_data.model_dump()
 
 
-# Model with union field that has NO default to trigger lines 253-254
+# Model with union field that has NO default
 class NoDefaultUnionModel(BaseModel):
     # This union field has NO default value and contains pydantic models
-    # This should trigger lines 253-254
     required_union: Union[SomeModel, OtherModel]
 
 
 @router.post("/test-no-default-union")
 def view_no_default_union(request, no_default: NoDefaultUnionModel):
-    """Test union field with no default to trigger lines 253-254."""
+    """Test union field with no default."""
     return no_default.model_dump()
 
 
-# Complex nested model to trigger detect_collection_fields union logic (lines 394-413)
+# Complex nested model to trigger detect_collection_fields union logic
 class NestedWithCollections(BaseModel):
     items: List[str]  # Collection field
 
@@ -288,14 +282,14 @@ def view_deep_nested_union(request, deep_data: VeryDeepModel):
     return deep_data.model_dump()
 
 
-# Test to hit line 233 - trigger _model_flatten_map with Union containing None
+# trigger _model_flatten_map with Union containing None
 @router.post("/test-flatten-union-with-none")
 def view_flatten_union_with_none(request, data: Union[SomeModel, None]):
-    """Test direct Union[Model, None] to trigger line 233 in _model_flatten_map."""
+    """Test direct Union[Model, None]"""
     return data.model_dump() if data else {"result": "none"}
 
 
-# Test to hit line 233 more directly - nested union with None
+# nested union with None
 class ModelWithUnionField(BaseModel):
     union_field: Union[SomeModel, None] = (
         None  # This should trigger _model_flatten_map with Union
@@ -304,18 +298,18 @@ class ModelWithUnionField(BaseModel):
 
 @router.post("/test-nested-union-with-none")
 def view_nested_union_with_none(request, data: ModelWithUnionField):
-    """Test nested Union[Model, None] field to trigger line 233 in _model_flatten_map."""
+    """Test nested Union[Model, None]"""
     return data.model_dump()
 
 
-# Test to directly hit line 233 - Union parameter that gets flattened
+# Union parameter that gets flattened
 class OuterModel(BaseModel):
     inner: Union[SomeModel, OtherModel, None]  # Union with None at top level
 
 
 @router.post("/test-direct-union-flattening")
 def view_direct_union_flattening(request, data: OuterModel):
-    """Test direct union flattening to hit line 233."""
+    """Test direct union flattening."""
     return data.model_dump()
 
 
@@ -399,7 +393,6 @@ def view_direct_union_flattening(request, data: OuterModel):
             dict(json=None),
             {"i": 1, "s": "test", "f": 1.5, "n": None},
         ),
-        # Test union with none (line 229)
         (
             "/test-union-with-none",
             dict(json=None),
@@ -410,7 +403,7 @@ def view_direct_union_flattening(request, data: OuterModel):
             dict(json=None),
             {"optional": "test"},
         ),
-        # Test union field model (lines 253-254)
+        # Test union field model
         (
             "/test-union-field-model",
             dict(json={"choice": {"i": 1, "s": "test", "f": 1.5}}),
@@ -432,7 +425,7 @@ def view_direct_union_flattening(request, data: OuterModel):
             dict(json={"items": ["x"], "nested": {"i": 5, "s": "test", "f": 2.0}}),
             {"items": ["x"], "nested": {"i": 5, "s": "test", "f": 2.0, "n": None}},
         ),
-        # Test complex union field (lines 253-254)
+        # Test complex union field
         (
             "/test-complex-union-field",
             dict(
@@ -451,7 +444,7 @@ def view_direct_union_flattening(request, data: OuterModel):
             dict(json={"model_choice": {"x": 10, "y": 20}, "name": "example"}),
             {"model_choice": {"x": 10, "y": 20}, "name": "example"},
         ),
-        # Test no default union (lines 253-254)
+        # Test no default union
         (
             "/test-no-default-union",
             dict(json={"required_union": {"i": 2, "s": "required", "f": 2.5}}),
@@ -462,7 +455,7 @@ def view_direct_union_flattening(request, data: OuterModel):
             dict(json={"required_union": {"x": 5, "y": 10}}),
             {"required_union": {"x": 5, "y": 10}},
         ),
-        # Test deeply nested union (lines 394-413, 430, 433-436)
+        # Test deeply nested union
         (
             "/test-deep-nested-union",
             dict(
@@ -498,7 +491,7 @@ def view_direct_union_flattening(request, data: OuterModel):
                 "extra_items": [],
             },
         ),
-        # Test to hit line 233 - trigger _model_flatten_map with Union containing None
+        # Test to trigger _model_flatten_map with Union containing None
         (
             "/test-flatten-union-with-none",
             dict(json={"i": 1, "s": "test", "f": 1.5}),
@@ -509,7 +502,7 @@ def view_direct_union_flattening(request, data: OuterModel):
             dict(json=None),
             {"result": "none"},
         ),
-        # Test nested union with None to hit line 233
+        # Test nested union with None
         (
             "/test-nested-union-with-none",
             dict(json={"union_field": {"i": 1, "s": "test", "f": 1.5}}),
@@ -520,7 +513,7 @@ def view_direct_union_flattening(request, data: OuterModel):
             dict(json={"union_field": None}),
             {"union_field": None},
         ),
-        # Test direct union flattening to hit line 233
+        # Test direct union flattening
         (
             "/test-direct-union-flattening",
             dict(json={"inner": {"i": 1, "s": "test", "f": 1.5}}),
@@ -536,26 +529,26 @@ def view_direct_union_flattening(request, data: OuterModel):
             dict(json={"inner": None}),
             {"inner": None},
         ),
-        # (
-        #     "/test-multi-model-union",
-        #     dict(json={"models": {"i": 1, "s": "test", "f": 1.5}}),
-        #     {"models": {"i": 1, "s": "test", "f": 1.5, "n": None}},
-        # ),
-        # (
-        #     "/test-optional-union",
-        #     dict(json=None),
-        #     {"result": "none"},
-        # ),
-        # (
-        #     "/test-nested-union",
-        #     dict(json={"nested_field": None, "simple_field": "test"}),
-        #     {"nested_field": None, "simple_field": "test"},
-        # ),
-        # (
-        #     "/test-complex-union?simple_union=42",
-        #     dict(json=None),
-        #     {"model_union": None, "simple_union": "42"},
-        # ),
+        (
+            "/test-multi-model-union",
+            dict(json={"models": {"i": 1, "s": "test", "f": 1.5}}),
+            {"models": {"i": 1, "s": "test", "f": 1.5, "n": None}},
+        ),
+        (
+            "/test-optional-union",
+            dict(json=None),
+            {"result": "none"},
+        ),
+        (
+            "/test-nested-union",
+            dict(json={"nested_field": None, "simple_field": "test"}),
+            {"nested_field": None, "simple_field": "test"},
+        ),
+        (
+            "/test-complex-union?simple_union=42",
+            dict(json=None),
+            {"model_union": None, "simple_union": "42"},
+        ),
     ],
     # fmt: on
 )