Skip to content

Commit

Permalink
refactor: improve violation finder (#655)
Browse files Browse the repository at this point in the history
* refactor(violations): link violation class to finders

* refactor(violations): move finder to dedicated module
  • Loading branch information
mkniewallner committed Apr 2, 2024
1 parent a40f47b commit af87217
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 82 deletions.
58 changes: 4 additions & 54 deletions python/deptry/core.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

import logging
import operator
import sys
from dataclasses import dataclass
from typing import TYPE_CHECKING
Expand All @@ -13,22 +12,12 @@
from deptry.python_file_finder import get_all_python_files_in
from deptry.reporters import JSONReporter, TextReporter
from deptry.stdlibs import STDLIBS_PYTHON
from deptry.violations import (
DEP001MissingDependenciesFinder,
DEP001MissingDependencyViolation,
DEP002UnusedDependenciesFinder,
DEP002UnusedDependencyViolation,
DEP003TransitiveDependenciesFinder,
DEP003TransitiveDependencyViolation,
DEP004MisplacedDevDependenciesFinder,
DEP004MisplacedDevDependencyViolation,
)
from deptry.violations.finder import find_violations

if TYPE_CHECKING:
from collections.abc import Mapping
from pathlib import Path

from deptry.dependency import Dependency
from deptry.dependency_getter.base import DependenciesExtract
from deptry.violations import Violation

Expand Down Expand Up @@ -89,7 +78,9 @@ def run(self) -> None:
if not module_with_locations.module.standard_library
]

violations = self._find_violations(imported_modules_with_locations, dependencies_extract.dependencies)
violations = find_violations(
imported_modules_with_locations, dependencies_extract.dependencies, self.ignore, self.per_rule_ignores
)
TextReporter(violations, use_ansi=not self.no_ansi).report()

if self.json_output:
Expand All @@ -110,47 +101,6 @@ def _find_python_files(self) -> list[Path]:

return python_files

def _find_violations(
self, imported_modules_with_locations: list[ModuleLocations], dependencies: list[Dependency]
) -> list[Violation]:
violations = []

if DEP001MissingDependencyViolation.error_code not in self.ignore:
violations.extend(
DEP001MissingDependenciesFinder(
imported_modules_with_locations, dependencies, self.per_rule_ignores.get("DEP001", ())
).find()
)

if DEP002UnusedDependencyViolation.error_code not in self.ignore:
violations.extend(
DEP002UnusedDependenciesFinder(
imported_modules_with_locations, dependencies, self.per_rule_ignores.get("DEP002", ())
).find()
)

if DEP003TransitiveDependencyViolation.error_code not in self.ignore:
violations.extend(
DEP003TransitiveDependenciesFinder(
imported_modules_with_locations, dependencies, self.per_rule_ignores.get("DEP003", ())
).find()
)

if DEP004MisplacedDevDependencyViolation.error_code not in self.ignore:
violations.extend(
DEP004MisplacedDevDependenciesFinder(
imported_modules_with_locations, dependencies, self.per_rule_ignores.get("DEP004", ())
).find()
)

return self._get_sorted_violations(violations)

@staticmethod
def _get_sorted_violations(violations: list[Violation]) -> list[Violation]:
return sorted(
violations, key=operator.attrgetter("location.file", "location.line", "location.column", "error_code")
)

def _get_local_modules(self) -> set[str]:
"""
Get all local Python modules from the source directories and `known_first_party` list.
Expand Down
4 changes: 2 additions & 2 deletions python/deptry/violations/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations

from deptry.violations.base import Violation
from deptry.violations.base import Violation, ViolationsFinder
from deptry.violations.dep001_missing.finder import DEP001MissingDependenciesFinder
from deptry.violations.dep001_missing.violation import DEP001MissingDependencyViolation
from deptry.violations.dep002_unused.finder import DEP002UnusedDependenciesFinder
Expand All @@ -20,5 +20,5 @@
"DEP003TransitiveDependenciesFinder",
"DEP004MisplacedDevDependenciesFinder",
"Violation",
"ViolationFinder",
"ViolationsFinder",
)
1 change: 1 addition & 0 deletions python/deptry/violations/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class ViolationsFinder(ABC):
"""

violation: ClassVar[type[Violation]]
imported_modules_with_locations: list[ModuleLocations]
dependencies: list[Dependency]
ignored_modules: tuple[str, ...] = ()
Expand Down
4 changes: 3 additions & 1 deletion python/deptry/violations/dep001_missing/finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ class DEP001MissingDependenciesFinder(ViolationsFinder):
Given a list of imported modules and a list of project dependencies, determine which ones are missing.
"""

violation = DEP001MissingDependencyViolation

def find(self) -> list[Violation]:
logging.debug("\nScanning for missing dependencies...")
missing_dependencies: list[Violation] = []
Expand All @@ -29,7 +31,7 @@ def find(self) -> list[Violation]:

if self._is_missing(module):
for location in module_with_locations.locations:
missing_dependencies.append(DEP001MissingDependencyViolation(module, location))
missing_dependencies.append(self.violation(module, location))

return missing_dependencies

Expand Down
6 changes: 3 additions & 3 deletions python/deptry/violations/dep002_unused/finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ class DEP002UnusedDependenciesFinder(ViolationsFinder):
even if `matplotlib` itself is not imported anywhere.
"""

violation = DEP002UnusedDependencyViolation

def find(self) -> list[Violation]:
logging.debug("\nScanning for unused dependencies...")
unused_dependencies: list[Violation] = []
Expand All @@ -35,9 +37,7 @@ def find(self) -> list[Violation]:
logging.debug("Scanning module %s...", dependency.name)

if self._is_unused(dependency):
unused_dependencies.append(
DEP002UnusedDependencyViolation(dependency, Location(dependency.definition_file))
)
unused_dependencies.append(self.violation(dependency, Location(dependency.definition_file)))

return unused_dependencies

Expand Down
4 changes: 3 additions & 1 deletion python/deptry/violations/dep003_transitive/finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class DEP003TransitiveDependenciesFinder(ViolationsFinder):
Then it must be a transitive dependency.
"""

violation = DEP003TransitiveDependencyViolation

def find(self) -> list[Violation]:
logging.debug("\nScanning for transitive dependencies...")
transitive_dependencies: list[Violation] = []
Expand All @@ -37,7 +39,7 @@ def find(self) -> list[Violation]:
if self._is_transitive(module):
# `self._is_transitive` only returns `True` if the package is not None.
for location in module_with_locations.locations:
transitive_dependencies.append(DEP003TransitiveDependencyViolation(module, location))
transitive_dependencies.append(self.violation(module, location))

return transitive_dependencies

Expand Down
4 changes: 3 additions & 1 deletion python/deptry/violations/dep004_misplaced_dev/finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ class DEP004MisplacedDevDependenciesFinder(ViolationsFinder):
This is the case for any development dependency encountered, since files solely used for development purposes should be excluded from scanning.
"""

violation = DEP004MisplacedDevDependencyViolation

def find(self) -> list[Violation]:
"""
In this function, we use 'corresponding_package_name' instead of module.package, since it can happen that a
Expand All @@ -38,7 +40,7 @@ def find(self) -> list[Violation]:

if corresponding_package_name and self._is_development_dependency(module, corresponding_package_name):
for location in module_with_locations.locations:
misplaced_dev_dependencies.append(DEP004MisplacedDevDependencyViolation(module, location))
misplaced_dev_dependencies.append(self.violation(module, location))

return misplaced_dev_dependencies

Expand Down
53 changes: 53 additions & 0 deletions python/deptry/violations/finder.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
from __future__ import annotations

import operator
from typing import TYPE_CHECKING

from deptry.violations import (
DEP001MissingDependenciesFinder,
DEP002UnusedDependenciesFinder,
DEP003TransitiveDependenciesFinder,
DEP004MisplacedDevDependenciesFinder,
)

if TYPE_CHECKING:
from typing import Mapping

from deptry.dependency import Dependency
from deptry.module import ModuleLocations
from deptry.violations import Violation, ViolationsFinder


_VIOLATIONS_FINDERS: tuple[type[ViolationsFinder], ...] = (
DEP001MissingDependenciesFinder,
DEP002UnusedDependenciesFinder,
DEP003TransitiveDependenciesFinder,
DEP004MisplacedDevDependenciesFinder,
)


def find_violations(
imported_modules_with_locations: list[ModuleLocations],
dependencies: list[Dependency],
ignore: tuple[str, ...],
per_rule_ignores: Mapping[str, tuple[str, ...]],
) -> list[Violation]:
violations = []

for violation_finder in _VIOLATIONS_FINDERS:
if violation_finder.violation.error_code not in ignore:
violations.extend(
violation_finder(
imported_modules_with_locations,
dependencies,
per_rule_ignores.get(violation_finder.violation.error_code, ()),
).find()
)

return _get_sorted_violations(violations)


def _get_sorted_violations(violations: list[Violation]) -> list[Violation]:
return sorted(
violations, key=operator.attrgetter("location.file", "location.line", "location.column", "error_code")
)
20 changes: 0 additions & 20 deletions tests/unit/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,6 @@
from tests.utils import create_files, run_within_dir


def test__get_sorted_violations() -> None:
violations = [
DEP004MisplacedDevDependencyViolation(Module("foo"), Location(Path("foo.py"), 1, 0)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("foo.py"), 2, 0)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("foo.py"), 1, 0)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("bar.py"), 3, 1)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("bar.py"), 2, 1)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("bar.py"), 3, 0)),
]

assert Core._get_sorted_violations(violations) == [
DEP001MissingDependencyViolation(Module("foo"), Location(Path("bar.py"), 2, 1)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("bar.py"), 3, 0)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("bar.py"), 3, 1)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("foo.py"), 1, 0)),
DEP004MisplacedDevDependencyViolation(Module("foo"), Location(Path("foo.py"), 1, 0)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("foo.py"), 2, 0)),
]


@pytest.mark.parametrize(
("known_first_party", "root_suffix", "expected"),
[
Expand Down
28 changes: 28 additions & 0 deletions tests/unit/violations/test_finder.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
from __future__ import annotations

from pathlib import Path

from deptry.imports.location import Location
from deptry.module import Module
from deptry.violations import DEP001MissingDependencyViolation, DEP004MisplacedDevDependencyViolation
from deptry.violations.finder import _get_sorted_violations


def test__get_sorted_violations() -> None:
violations = [
DEP004MisplacedDevDependencyViolation(Module("foo"), Location(Path("foo.py"), 1, 0)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("foo.py"), 2, 0)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("foo.py"), 1, 0)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("bar.py"), 3, 1)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("bar.py"), 2, 1)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("bar.py"), 3, 0)),
]

assert _get_sorted_violations(violations) == [
DEP001MissingDependencyViolation(Module("foo"), Location(Path("bar.py"), 2, 1)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("bar.py"), 3, 0)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("bar.py"), 3, 1)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("foo.py"), 1, 0)),
DEP004MisplacedDevDependencyViolation(Module("foo"), Location(Path("foo.py"), 1, 0)),
DEP001MissingDependencyViolation(Module("foo"), Location(Path("foo.py"), 2, 0)),
]

0 comments on commit af87217

Please sign in to comment.