Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions oida/checkers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,20 @@
from .components import ComponentIsolationChecker
from .config import ConfigChecker
from .imports import RelativeImportsChecker
from .models import ComponentModelIsolationChecker

__all__ = [
"Code",
"ComponentIsolationChecker",
"ComponentModelIsolationChecker",
"ConfigChecker",
"RelativeImportsChecker",
"Violation",
]

ALL_CHECKERS = (
ComponentIsolationChecker,
ComponentModelIsolationChecker,
ConfigChecker,
RelativeImportsChecker,
)
Expand Down
1 change: 1 addition & 0 deletions oida/checkers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class Code(int, enum.Enum):
ODA003 = 3 # Invalid ALLOWED_IMPORTS statement in config
ODA004 = 4 # Invalid ALLOWED_FOREIGN_KEYS statement in config
ODA005 = 5 # Private attribute referenced
ODA006 = 6 # Related field to different app


class Violation(NamedTuple):
Expand Down
197 changes: 197 additions & 0 deletions oida/checkers/models.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
import ast
from contextlib import contextmanager
from functools import reduce
from typing import Any, Iterator

from ..config import ComponentConfig, ProjectConfig
from .base import Checker, Code

RELATED_FIELD_CLASSES = (
"django.db.models.ForeignKey",
"django.db.models.OneToOneField",
"tienda.core.fields.OneToOneOrNoneField",
)


class ComponentModelIsolationChecker(Checker):
"""
TODO: classdoc
"""

slug = "component-model-isolation"

def __init__(
self,
module: str | None,
name: str,
component_config: ComponentConfig | None,
project_config: ProjectConfig,
) -> None:
super().__init__(module, name, component_config, project_config)
self.scopes: list[dict[str, str]] = [{}]
# This checker will collect any relations it sees, regardless of any
# config. This is used to automatically generate component configs with
# allowed relations based on current violations.
self.seen_relations: set[str] = set()

@property
def current_scope(self) -> dict[str, str]:
return self.scopes[-1]

@property
def scope(self) -> dict[str, str]:
return reduce(dict.__or__, self.scopes)

@contextmanager
def push_scope(self) -> Iterator[None]:
self.scopes.append({})
try:
yield
finally:
self.scopes.pop()

def is_same_app(self, app_a: str, app_b: str) -> bool:
if app_a == app_b:
return True

return False

def is_violation_silenced(self, path: str) -> bool:
"""Check if a relation that's a violation is ignored by component config"""
allowed_foreign_keys: tuple[str, ...] = (
getattr(self.component_config, "allowed_foreign_keys", None) or ()
)

if path in allowed_foreign_keys:
return True

return False

def maybe_report_violation(self, *, field_path: str, node: ast.AST) -> None:
"""Check a fully qualified name"""

# If access is not allowed we should record the reference even if the
# violation is silenced
self.seen_relations.add(field_path)

if self.is_violation_silenced(field_path):
return

self.report_violation(
node, Code.ODA006, f"Related field to a different app: {field_path}"
)

def visit_Module(self, node: ast.Module) -> None:
"""Only check the file if the module is not ignored"""

if not self.project_config.is_ignored(self.module, self.name):
super().generic_visit(node)

def visit_ImportFrom(self, node: ast.ImportFrom) -> None:
if node.level > 0 or not self.module or not node.module:
return

for name in node.names:
self.current_scope[
name.asname if name.asname else name.name
] = f"{node.module}.{name.name}"

def visit_Import(self, node: ast.Import) -> Any:

# Ignore files that are not in a module
if self.module is None:
return

for name in node.names:
top_name, *_ = name.name.split(".", 1)

if name.asname:
self.current_scope[name.asname] = name.name
else:
# `import foo.bar` only sets foo in the local scope
self.current_scope[top_name] = top_name

def visit_FunctionDef(self, node: ast.FunctionDef) -> None:
with self.push_scope():
self.generic_visit(node)

def generic_visit(self, node):
# print('generic_visit', node, node.__dict__)
return super().generic_visit(node)

def visit_Assign(self, node: ast.Assign) -> None:

with self.push_scope():
self.current_scope["assign_target"] = getattr(node.targets[0], "id", None)
self.generic_visit(node)

def visit_Call(self, node: ast.Call) -> None:
func = node.func

if not isinstance(func, ast.Attribute | ast.Name):
return

func_path = self.resolve_node_fullpath(func)

if func_path not in RELATED_FIELD_CLASSES:
return self.generic_visit(node)

field_path = (
f"{self.scope.get('current_class')}.{self.scope.get('assign_target')}"
)

# Related fields are usually the first arg, but can also be passed as the "to" kwarg
related_model_node = None
if node.args:
related_model_node = node.args[0]
elif node.keywords:
to_keyword = next((kw for kw in node.keywords if kw.arg == "to"), None)
related_model_node = to_keyword.value

if isinstance(related_model_node, ast.Constant):
# We don't really know which app we currently are in, as it might not be named
# the same as the folder, and we might have different depths of apps in different
# contexts. E.g. if the project is componentized or not.

# TODO: This assumed app name based on the path
module_app = self.module.split(".")[1]
import_app = related_model_node.value.split(".")[0]

elif isinstance(related_model_node, ast.Attribute | ast.Name):
related_path = self.resolve_node_fullpath(related_model_node)

module_app = ".".join(self.module.split(".")[:2])
import_app = ".".join(related_path.split(".")[:2])

else:
print("WARNING")
return

if self.is_same_app(module_app, import_app):
# print("same app", self.module, field_path)
pass
else:
self.maybe_report_violation(field_path=field_path, node=node)

def resolve_node_fullpath(self, node: ast.Attribute | ast.Name) -> str:
"""
Takes in an ast expression, and tries to resolve the full name of the reference.
Currently only works for ast.Attribute and ast.Name.
"""

parts = []

while isinstance(node, ast.Attribute):
parts.insert(0, node.attr)
node = node.value

if isinstance(node, ast.Name):
parts.insert(0, self.scope.get(node.id))

return ".".join([part for part in parts if part is not None])

def visit_ClassDef(self, node):
self.current_scope[node.name] = self.module
with self.push_scope():
self.current_scope["current_class"] = node.name
self.generic_visit(node)
5 changes: 3 additions & 2 deletions oida/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import sys
from dataclasses import dataclass, field
from itertools import zip_longest
from typing import Iterable

if sys.version_info >= (3, 11):
Expand All @@ -25,8 +26,8 @@ def is_ignored(self, module: str | None, name: str) -> bool:

for ignore in self.ignored_modules:
if all(
name == "*" or len(path) < i or name == path[i]
for i, name in enumerate(ignore.split("."))
ignore_part == "*" or path_part == ignore_part or ignore_part is None
for path_part, ignore_part in zip_longest(path, ignore.split("."))
):
return True

Expand Down
112 changes: 112 additions & 0 deletions tests/test_model_isolation_violations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import pytest

from oida.checkers import Code, ComponentModelIsolationChecker, Violation

pytestmark = pytest.mark.module(name="models", module="project.component.app")


@pytest.mark.module(
"""\
from django.db import models

class MyModel(models.Model):
fk = models.ForeignKey("other.OtherModel")
"""
)
def test_foreignkey_to_other_app_1(
checker: ComponentModelIsolationChecker, violations: list[Violation]
) -> None:
"""Test that private function calls at the module level is caught"""
assert violations == [
Violation(
line=4,
column=9,
code=Code.ODA006,
message="Related field to a different app: MyModel.fk",
)
]


@pytest.mark.module(
"""\
from django.db.models import ForeignKey

class MyModel(models.Model):
fk = ForeignKey("other.OtherModel")
"""
)
def test_foreignkey_to_other_app_2(
checker: ComponentModelIsolationChecker, violations: list[Violation]
) -> None:
"""Test that private function calls at the module level is caught"""
assert violations == [
Violation(
line=4,
column=9,
code=Code.ODA006,
message="Related field to a different app: MyModel.fk",
)
]


@pytest.mark.module(
"""\
import django.db.models
from project.other.models import OtherModel

class MyModel(models.Model):
fk = django.db.models.ForeignKey(OtherModel)
"""
)
def test_foreignkey_to_other_app_3(
checker: ComponentModelIsolationChecker, violations: list[Violation]
) -> None:
"""Test that private function calls at the module level is caught"""
assert violations == [
Violation(
line=5,
column=9,
code=Code.ODA006,
message="Related field to a different app: MyModel.fk",
)
]


@pytest.mark.module(
"""\
from django.db import models
from project.other import models as other_models

class MyModel(models.Model):
fk = models.OneToOneField(to=other_models.OtherModel)
"""
)
def test_foreignkey_to_other_app_4(
checker: ComponentModelIsolationChecker, violations: list[Violation]
) -> None:
"""Test that private function calls at the module level is caught"""
assert violations == [
Violation(
line=5,
column=9,
code=Code.ODA006,
message="Related field to a different app: MyModel.fk",
)
]


@pytest.mark.component_config(allowed_foreign_keys=["MyModel.fk"])
@pytest.mark.module(
"""\
from django.db import models
from project.other import models as other_models

class MyModel(models.Model):
fk = models.OneToOneField(to=other_models.OtherModel)
"""
)
def test_foreignkey_to_other_app_5(
checker: ComponentModelIsolationChecker, violations: list[Violation]
) -> None:
"""Test that private function calls at the module level is caught"""
assert not violations