Skip to content

Commit

Permalink
feat: Implement basic handling of Alias for breaking changes
Browse files Browse the repository at this point in the history
PR #140: #140
Co-authored-by: Yurii Shnitkovskyi <Yurii.Shnitkovskyi@lseg.com>
Co-authored-by: Timothée Mazzucotelli <pawamoy@pm.me>
  • Loading branch information
Georggi authored Apr 10, 2023
1 parent 3a16e58 commit aa8ce00
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 34 deletions.
56 changes: 42 additions & 14 deletions src/griffe/diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from colorama import Fore, Style

from griffe.dataclasses import Alias, Attribute, Class, Function, Object, ParameterKind
from griffe.exceptions import AliasResolutionError
from griffe.logger import get_logger

POSITIONAL = frozenset((ParameterKind.positional_only, ParameterKind.positional_or_keyword))
Expand Down Expand Up @@ -388,6 +389,24 @@ def _attribute_incompatibilities(old_attribute: Attribute, new_attribute: Attrib
yield AttributeChangedValueBreakage(new_attribute, old_attribute.value, new_attribute.value)


def _alias_incompatibilities(
old_obj: Object | Alias,
new_obj: Object | Alias,
*,
ignore_private: bool,
) -> Iterable[Breakage]:
if not ignore_private:
return
try:
old_member = old_obj.target if old_obj.is_alias else old_obj # type: ignore[union-attr]
new_member = new_obj.target if new_obj.is_alias else new_obj # type: ignore[union-attr]
except AliasResolutionError:
logger.debug(f"API check: {old_obj.path} | {new_obj.path}: skip alias with unknown target")
return

yield from _type_based_yield(old_member, new_member, ignore_private=ignore_private)


def _member_incompatibilities(
old_obj: Object | Alias,
new_obj: Object | Alias,
Expand All @@ -399,10 +418,6 @@ def _member_incompatibilities(
logger.debug(f"API check: {old_obj.path}.{name}: skip private object")
continue

if old_member.is_alias:
logger.debug(f"API check: {old_obj.path}.{name}: skip alias")
continue # TODO

logger.debug(f"API check: {old_obj.path}.{name}")
try:
new_member = new_obj.members[name]
Expand All @@ -411,16 +426,29 @@ def _member_incompatibilities(
yield ObjectRemovedBreakage(old_member, old_member, None) # type: ignore[arg-type]
continue

if new_member.kind != old_member.kind:
yield ObjectChangedKindBreakage(new_member, old_member.kind, new_member.kind) # type: ignore[arg-type]
elif old_member.is_module:
yield from _member_incompatibilities(old_member, new_member, ignore_private=ignore_private) # type: ignore[arg-type]
elif old_member.is_class:
yield from _class_incompatibilities(old_member, new_member, ignore_private=ignore_private) # type: ignore[arg-type]
elif old_member.is_function:
yield from _function_incompatibilities(old_member, new_member) # type: ignore[arg-type]
elif old_member.is_attribute:
yield from _attribute_incompatibilities(old_member, new_member) # type: ignore[arg-type]
yield from _type_based_yield(old_member, new_member, ignore_private=ignore_private)


def _type_based_yield(
old_member: Object | Alias,
new_member: Object | Alias,
*,
ignore_private: bool,
) -> Iterator[Breakage]:
if old_member.is_alias or new_member.is_alias:
# Should be first, since there can be the case where there is an alias and another kind of object, which may
# not be a breaking change
yield from _alias_incompatibilities(old_member, new_member, ignore_private=ignore_private) # type: ignore[arg-type]
elif new_member.kind != old_member.kind:
yield ObjectChangedKindBreakage(new_member, old_member.kind, new_member.kind) # type: ignore[arg-type]
elif old_member.is_module:
yield from _member_incompatibilities(old_member, new_member, ignore_private=ignore_private) # type: ignore[arg-type]
elif old_member.is_class:
yield from _class_incompatibilities(old_member, new_member, ignore_private=ignore_private) # type: ignore[arg-type]
elif old_member.is_function:
yield from _function_incompatibilities(old_member, new_member) # type: ignore[arg-type]
elif old_member.is_attribute:
yield from _attribute_incompatibilities(old_member, new_member) # type: ignore[arg-type]


def _returns_are_compatible(old_function: Function, new_function: Function) -> bool:
Expand Down
64 changes: 52 additions & 12 deletions tests/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@
from importlib import invalidate_caches
from pathlib import Path
from textwrap import dedent
from typing import Iterator
from typing import Iterator, Mapping, Sequence

from griffe.agents.inspector import inspect
from griffe.agents.visitor import visit
from griffe.dataclasses import Module, Object
from griffe.loader import GriffeLoader

TMPDIR_PREFIX = "griffe_"

Expand All @@ -39,24 +40,35 @@ def temporary_pyfile(code: str) -> Iterator[tuple[str, Path]]:


@contextmanager
def temporary_pypackage(package: str, modules: list[str] | None = None, *, init: bool = True) -> Iterator[TmpPackage]:
"""Create a module.py file containing the given code in a temporary directory.
def temporary_pypackage(
package: str,
modules: Sequence[str] | Mapping[str, str] | None = None,
*,
init: bool = True,
) -> Iterator[TmpPackage]:
"""Create a package containing the given modules in a temporary directory.
Parameters:
package: The package name. Example: `"a"` gives
a package named `a`, while `"a/b"` gives a namespace package
named `a` with a package inside named `b`.
If `init` is false, then `b` is also a namespace package.
modules: Additional modules to create in the package,
like '["b.py", "c/d.py", "e/f"]`.
modules: Additional modules to create in the package.
If a list, simply touch the files: `["b.py", "c/d.py", "e/f"]`.
If a dict, keys are the file names and values their contents:
`{"b.py": "b = 1", "c/d.py": "print('hey from c')"}`.
init: Whether to create an `__init__` module in the leaf package.
Yields:
tmp_dir: The temporary directory containing the package.
package_name: The package name, as to dynamically import it.
package_path: The package path.
A named tuple with the following fields:
- `tmp_dir`: The temporary directory containing the package.
- `name`: The package name, as to dynamically import it.
- `path`: The package path.
"""
modules = modules or []
modules = modules or {}
if isinstance(modules, list):
modules = {mod: "" for mod in modules}
mkdir_kwargs = {"parents": True, "exist_ok": True}
with tempfile.TemporaryDirectory(prefix=TMPDIR_PREFIX) as tmpdir:
tmpdirpath = Path(tmpdir)
Expand All @@ -65,18 +77,46 @@ def temporary_pypackage(package: str, modules: list[str] | None = None, *, init:
package_path.mkdir(**mkdir_kwargs)
if init:
package_path.joinpath("__init__.py").touch()
for module in modules:
for module_name, module_contents in modules.items(): # type: ignore[union-attr]
current_path = package_path
for part in Path(module).parts:
for part in Path(module_name).parts:
if part.endswith((".py", ".pyi")):
current_path.joinpath(part).touch()
current_path.joinpath(part).write_text(dedent(module_contents))
else:
current_path /= part
current_path.mkdir(**mkdir_kwargs)
current_path.joinpath("__init__.py").touch()
yield TmpPackage(tmpdirpath, package_name, package_path)


@contextmanager
def temporary_visited_package(
package: str,
modules: Sequence[str] | Mapping[str, str] | None = None,
*,
init: bool = True,
) -> Iterator[Module]:
"""Create and visit a temporary package.
Parameters:
package: The package name. Example: `"a"` gives
a package named `a`, while `"a/b"` gives a namespace package
named `a` with a package inside named `b`.
If `init` is false, then `b` is also a namespace package.
modules: Additional modules to create in the package.
If a list, simply touch the files: `["b.py", "c/d.py", "e/f"]`.
If a dict, keys are the file names and values their contents:
`{"b.py": "b = 1", "c/d.py": "print('hey from c')"}`.
init: Whether to create an `__init__` module in the leaf package.
Yields:
A module.
"""
with temporary_pypackage(package, modules, init=init) as tmp_package:
loader = GriffeLoader(search_paths=[tmp_package.tmpdir])
yield loader.load_module(tmp_package.name)


@contextmanager
def temporary_visited_module(code: str) -> Iterator[Module]:
"""Create and visit a temporary module with the given code.
Expand Down
26 changes: 18 additions & 8 deletions tests/test_diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,26 @@
import pytest

from griffe.diff import Breakage, BreakageKind, find_breaking_changes
from tests.helpers import temporary_visited_module
from tests.helpers import temporary_visited_module, temporary_visited_package


@pytest.mark.skipif(sys.version_info < (3, 8), reason="no positional-only parameters on Python 3.7")
@pytest.mark.parametrize(
("old_code", "new_code", "expected_breakages"),
[
# ),
(
"a = True",
"a = False",
[BreakageKind.ATTRIBUTE_CHANGED_VALUE],
),
(
"class A(int, str): ...",
"class A(int): ...",
"class a(int, str): ...",
"class a(int): ...",
[BreakageKind.CLASS_REMOVED_BASE],
),
(
"A = 0",
"class A: ...",
"a = 0",
"class a: ...",
[BreakageKind.OBJECT_CHANGED_KIND],
),
(
Expand Down Expand Up @@ -173,8 +172,19 @@ def test_diff_griffe(old_code: str, new_code: str, expected_breakages: list[Brea
new_code: Parametrized code of the new module version.
expected_breakages: A list of breakage kinds to expect.
"""
with temporary_visited_module(old_code) as old_module, temporary_visited_module(new_code) as new_module:
breaking = list(find_breaking_changes(old_module, new_module))
# check without any alias
with temporary_visited_module(old_code) as old_package, temporary_visited_module(new_code) as new_package:
breaking = list(find_breaking_changes(old_package, new_package))
assert len(breaking) == len(expected_breakages)
for breakage, expected_kind in zip(breaking, expected_breakages):
assert breakage.kind is expected_kind
# check with aliases
import_a = "from ._mod_a import a"
old_modules = {"__init__.py": import_a, "_mod_a.py": old_code}
new_modules = {"__init__.py": new_code and import_a, "_mod_a.py": new_code}
with temporary_visited_package("package_old", old_modules) as old_package: # noqa: SIM117
with temporary_visited_package("package_new", new_modules) as new_package:
breaking = list(find_breaking_changes(old_package, new_package))
assert len(breaking) == len(expected_breakages)
for breakage, expected_kind in zip(breaking, expected_breakages):
assert breakage.kind is expected_kind

0 comments on commit aa8ce00

Please sign in to comment.