Skip to content

Commit 89cfeb0

Browse files
authored
fix(warnings): avoid MissingSpecAttributeWarning if property type is missing or optional (#258)
Fixes #257
1 parent 1654d65 commit 89cfeb0

File tree

5 files changed

+118
-11
lines changed

5 files changed

+118
-11
lines changed

.github/actions/setup/action.yml

+4-4
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ inputs:
77
default: "3.12"
88
poetry-version:
99
description: "Poetry version to install"
10-
default: "1.7.0"
10+
default: "2.1.1"
1111
cache:
1212
description: "Cache directory"
1313
default: "${{ runner.temp }}/cache"
@@ -17,19 +17,19 @@ runs:
1717
steps:
1818
- name: "Set up Python"
1919
id: setup-python
20-
uses: actions/setup-python@v4
20+
uses: actions/setup-python@v5
2121
with:
2222
python-version: ${{ inputs.python-version }}
2323
update-environment: false
2424

2525
- name: "Set up Python 3.12 for Poetry"
2626
id: setup-poetry-python
27-
uses: actions/setup-python@v4
27+
uses: actions/setup-python@v5
2828
with:
2929
python-version: 3.12
3030

3131
- name: "Set up dependency cache"
32-
uses: actions/cache@v3
32+
uses: actions/cache@v4
3333
with:
3434
key: ${{ runner.os }}-${{ steps.setup-poetry-python.outputs.python-version }}-${{ steps.setup-python.outputs.python-version }}-${{ inputs.poetry-version }}-${{ hashFiles('poetry.lock') }}
3535
path: ${{ inputs.cache }}

.github/workflows/ci.yml

+17-4
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,22 @@ on:
1010
jobs:
1111
test:
1212
name: "Test Python ${{ matrix.python-version }} on ${{ matrix.os }}"
13-
runs-on: ${{ matrix.os }}-latest
13+
runs-on: ${{ matrix.os }}
1414
strategy:
15+
fail-fast: false
1516
matrix:
16-
os: [Ubuntu, Windows, macOS]
17-
python-version: ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12"]
17+
os: [ubuntu-latest, windows-latest, macos-latest]
18+
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"]
19+
include:
20+
- os: ubuntu-22.04
21+
python-version: "3.7"
22+
- os: windows-latest
23+
python-version: "3.7"
24+
- os: macos-13
25+
python-version: "3.7"
26+
permissions:
27+
id-token: write
28+
1829
steps:
1930
- name: "Check out repository"
2031
uses: actions/checkout@v4
@@ -28,7 +39,9 @@ jobs:
2839
run: poetry run poe test-ci
2940

3041
- name: "Upload coverage report"
31-
uses: codecov/codecov-action@v3
42+
uses: codecov/codecov-action@v5
43+
with:
44+
use_oidc: true
3245

3346
check:
3447
name: "Lint and type checks"

decoy/spy_core.py

+38-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,17 @@
22
import inspect
33
import functools
44
import warnings
5-
from typing import Any, Dict, NamedTuple, Optional, Tuple, Type, Union, get_type_hints
5+
from typing import (
6+
Any,
7+
Dict,
8+
NamedTuple,
9+
Optional,
10+
Tuple,
11+
Type,
12+
Union,
13+
Sequence,
14+
get_type_hints,
15+
)
616

717
from .spy_events import SpyInfo
818
from .warnings import IncorrectCallWarning, MissingSpecAttributeWarning
@@ -112,12 +122,15 @@ def create_child_core(self, name: str, is_async: bool) -> "SpyCore":
112122
source = self._source
113123
child_name = f"{self._name}.{name}"
114124
child_source = None
125+
child_found = False
115126

116127
if inspect.isclass(source):
117128
# use type hints to get child spec for class attributes
118129
child_hint = _get_type_hints(source).get(name)
119130
# use inspect to get child spec for methods and properties
120131
child_source = inspect.getattr_static(source, name, child_hint)
132+
# record whether a child was found before we make modifications
133+
child_found = child_source is not None
121134

122135
if isinstance(child_source, property):
123136
child_source = _get_type_hints(child_source.fget).get("return")
@@ -136,7 +149,9 @@ def create_child_core(self, name: str, is_async: bool) -> "SpyCore":
136149
# signature reporting by wrapping it in a partial
137150
child_source = functools.partial(child_source, None)
138151

139-
if child_source is None and source is not None:
152+
child_source = _unwrap_optional(child_source)
153+
154+
if source is not None and child_found is False:
140155
# stacklevel: 4 ensures warning is linked to call location
141156
warnings.warn(
142157
MissingSpecAttributeWarning(f"{self._name} has no attribute '{name}'"),
@@ -215,3 +230,24 @@ def _get_type_hints(obj: Any) -> Dict[str, Any]:
215230
return get_type_hints(obj)
216231
except Exception:
217232
return {}
233+
234+
235+
def _unwrap_optional(source: Any) -> Any:
236+
"""Return the source's base type if it's a optional.
237+
238+
If the type is a union of more than just T | None,
239+
bail out and return None to avoid potentially false warnings.
240+
"""
241+
origin = getattr(source, "__origin__", None)
242+
args: Sequence[Any] = getattr(source, "__args__", ())
243+
244+
# TODO(mc, 2025-03-19): support larger unions? might be a lot of work for little payoff
245+
if origin is Union:
246+
if len(args) == 2 and args[0] is type(None):
247+
return args[1]
248+
if len(args) == 2 and args[1] is type(None):
249+
return args[0]
250+
251+
return None
252+
253+
return source

tests/fixtures.py

+21-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"""Common test fixtures."""
22
from functools import lru_cache
3-
from typing import Any, Generic, TypeVar
3+
from typing import Any, Generic, TypeVar, Optional, Union
44

55

66
class SomeClass:
@@ -28,6 +28,11 @@ def primitive_property(self) -> str:
2828
"""Get a primitive computed property."""
2929
raise NotImplementedError()
3030

31+
@property
32+
def mystery_property(self): # type: ignore[no-untyped-def] # noqa: ANN201
33+
"""Get a property without type annotations."""
34+
raise NotImplementedError()
35+
3136
@lru_cache(maxsize=None) # noqa: B019
3237
def some_wrapped_method(self, val: str) -> str:
3338
"""Get a thing through a wrapped method."""
@@ -48,6 +53,21 @@ def child(self) -> SomeClass:
4853
"""Get the child instance."""
4954
raise NotImplementedError()
5055

56+
@property
57+
def optional_child(self) -> Optional[SomeClass]:
58+
"""Get the child instance."""
59+
raise NotImplementedError()
60+
61+
@property
62+
def union_none_child(self) -> Union[None, SomeClass]:
63+
"""Get the child instance."""
64+
raise NotImplementedError()
65+
66+
@property
67+
def union_child(self) -> Union[SomeClass, "SomeAsyncClass"]:
68+
"""Get the child instance."""
69+
raise NotImplementedError()
70+
5171

5272
class SomeAsyncClass:
5373
"""Async testing class."""

tests/test_spy_core.py

+38
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,7 @@ def test_warn_if_called_incorrectly() -> None:
445445
def test_warn_if_spec_does_not_have_method() -> None:
446446
"""It should trigger a warning if bound_args is called incorrectly."""
447447
class_subject = SpyCore(source=SomeClass, name=None)
448+
nested_class_subject = SpyCore(source=SomeNestedClass, name=None)
448449
func_subject = SpyCore(source=some_func, name=None)
449450
specless_subject = SpyCore(source=None, name="anonymous")
450451

@@ -458,6 +459,29 @@ def test_warn_if_spec_does_not_have_method() -> None:
458459
warnings.simplefilter("error")
459460
class_subject.create_child_core("foo", False)
460461

462+
# property access without types should not warn
463+
with warnings.catch_warnings():
464+
warnings.simplefilter("error")
465+
class_subject.create_child_core("mystery_property", False)
466+
467+
# property access should be allowed through optionals
468+
with warnings.catch_warnings():
469+
warnings.simplefilter("error")
470+
parent = nested_class_subject.create_child_core("optional_child", False)
471+
parent.create_child_core("primitive_property", False)
472+
473+
# property access should be allowed through None unions
474+
with warnings.catch_warnings():
475+
warnings.simplefilter("error")
476+
parent = nested_class_subject.create_child_core("union_none_child", False)
477+
parent.create_child_core("primitive_property", False)
478+
479+
# property access should not be checked through unions
480+
with warnings.catch_warnings():
481+
warnings.simplefilter("error")
482+
parent = nested_class_subject.create_child_core("union_child", False)
483+
parent.create_child_core("who_knows", False)
484+
461485
# incorrect class usage should warn
462486
with pytest.warns(
463487
MissingSpecAttributeWarning, match="has no attribute 'this_is_wrong'"
@@ -469,3 +493,17 @@ def test_warn_if_spec_does_not_have_method() -> None:
469493
MissingSpecAttributeWarning, match="has no attribute 'this_is_wrong'"
470494
):
471495
func_subject.create_child_core("this_is_wrong", False)
496+
497+
# incorrect nested property usage should warn
498+
with pytest.warns(
499+
MissingSpecAttributeWarning, match="has no attribute 'this_is_wrong'"
500+
):
501+
parent = nested_class_subject.create_child_core("optional_child", False)
502+
parent.create_child_core("this_is_wrong", False)
503+
504+
# incorrect nested property usage should warn
505+
with pytest.warns(
506+
MissingSpecAttributeWarning, match="has no attribute 'this_is_wrong'"
507+
):
508+
parent = nested_class_subject.create_child_core("union_none_child", False)
509+
parent.create_child_core("this_is_wrong", False)

0 commit comments

Comments
 (0)