Skip to content

Commit

Permalink
Merge pull request #563 from gauge-sh/unify-ignore-directives
Browse files Browse the repository at this point in the history
Split tach-ignore handling between `check` and `check-external`
  • Loading branch information
emdoyle authored Jan 23, 2025
2 parents ed526bf + 1964e11 commit e95a139
Show file tree
Hide file tree
Showing 19 changed files with 504 additions and 331 deletions.
58 changes: 12 additions & 46 deletions python/tach/check_external.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
from __future__ import annotations

from dataclasses import dataclass
from typing import TYPE_CHECKING

from tach.errors import TachError
from tach.extension import check_external_dependencies, set_excluded_paths
from tach.utils.external import get_module_mappings, is_stdlib_module
from tach.extension import (
ExternalCheckDiagnostics,
check_external_dependencies,
set_excluded_paths,
)
from tach.utils.external import (
get_module_mappings,
get_stdlib_modules,
)

if TYPE_CHECKING:
from pathlib import Path

from tach.extension import ProjectConfig


@dataclass
class ExternalCheckDiagnostics:
undeclared_dependencies: dict[str, list[str]]
unused_dependencies: dict[str, list[str]]


def extract_module_mappings(rename: list[str]) -> dict[str, list[str]]:
try:
return {
Expand All @@ -35,9 +35,6 @@ def check_external(
project_config: ProjectConfig,
exclude_paths: list[str],
) -> ExternalCheckDiagnostics:
serialized_source_roots = [
str(project_root / source_root) for source_root in project_config.source_roots
]
set_excluded_paths(
project_root=str(project_root),
exclude_paths=exclude_paths,
Expand All @@ -49,42 +46,11 @@ def check_external(
metadata_module_mappings.update(
extract_module_mappings(project_config.external.rename)
)
diagnostics = check_external_dependencies(
return check_external_dependencies(
project_root=str(project_root),
source_roots=serialized_source_roots,
project_config=project_config,
module_mappings=metadata_module_mappings,
ignore_type_checking_imports=project_config.ignore_type_checking_imports,
)
undeclared_dependencies_by_file = diagnostics[0]
unused_dependencies_by_project = diagnostics[1]

excluded_external_modules = set(project_config.external.exclude)
filtered_undeclared_dependencies: dict[str, list[str]] = {}
for filepath, undeclared_dependencies in undeclared_dependencies_by_file.items():
dependencies = set(
filter(
lambda dependency: not is_stdlib_module(dependency)
and dependency not in excluded_external_modules,
undeclared_dependencies,
)
)
if dependencies:
filtered_undeclared_dependencies[filepath] = list(dependencies)
filtered_unused_dependencies: dict[str, list[str]] = {}
for filepath, unused_dependencies in unused_dependencies_by_project.items():
dependencies = set(
filter(
lambda dependency: not is_stdlib_module(dependency)
and dependency not in excluded_external_modules,
unused_dependencies,
)
)
if dependencies:
filtered_unused_dependencies[filepath] = list(dependencies)

return ExternalCheckDiagnostics(
undeclared_dependencies=filtered_undeclared_dependencies,
unused_dependencies=filtered_unused_dependencies,
stdlib_modules=get_stdlib_modules(),
)


Expand Down
56 changes: 35 additions & 21 deletions python/tach/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,14 @@ def print_warnings(warning_list: list[str]) -> None:
print(f"{BCOLORS.WARNING}{warning}{BCOLORS.ENDC}", file=sys.stderr)


def print_errors(error_list: list[BoundaryError], source_roots: list[Path]) -> None:
def print_errors(error_list: list[str]) -> None:
for error in error_list:
print(f"{BCOLORS.FAIL}{error}{BCOLORS.ENDC}", file=sys.stderr)


def print_boundary_errors(
error_list: list[BoundaryError], source_roots: list[Path]
) -> None:
if not error_list:
return

Expand Down Expand Up @@ -242,35 +249,41 @@ def print_visibility_errors(
def print_undeclared_dependencies(
undeclared_dependencies: dict[str, list[str]],
) -> None:
any_undeclared = False
for file_path, dependencies in undeclared_dependencies.items():
if dependencies:
any_undeclared = True
print(
f"{icons.FAIL}: {BCOLORS.FAIL}Undeclared dependencies in {BCOLORS.ENDC}{BCOLORS.WARNING}'{file_path}'{BCOLORS.ENDC}:"
)
for dependency in dependencies:
print(f"\t{BCOLORS.FAIL}{dependency}{BCOLORS.ENDC}")
print(
f"{BCOLORS.WARNING}\nAdd the undeclared dependencies to the corresponding pyproject.toml file, "
f"or consider ignoring the dependencies by adding them to the 'external.exclude' list in {CONFIG_FILE_NAME}.toml.\n{BCOLORS.ENDC}",
file=sys.stderr,
)
if any_undeclared:
print(
f"{BCOLORS.WARNING}\nAdd the undeclared dependencies to the corresponding pyproject.toml file, "
f"or consider ignoring the dependencies by adding them to the 'external.exclude' list in {CONFIG_FILE_NAME}.toml.\n{BCOLORS.ENDC}",
file=sys.stderr,
)


def print_unused_external_dependencies(
unused_dependencies: dict[str, list[str]],
) -> None:
any_unused = False
for pyproject_path, dependencies in unused_dependencies.items():
if dependencies:
any_unused = True
print(
f"{icons.WARNING} {BCOLORS.WARNING}Unused dependencies from project at {BCOLORS.OKCYAN}'{pyproject_path}'{BCOLORS.ENDC}{BCOLORS.ENDC}:"
)
for dependency in dependencies:
print(f"\t{BCOLORS.WARNING}{dependency}{BCOLORS.ENDC}")
print(
f"{BCOLORS.OKCYAN}\nRemove the unused dependencies from the corresponding pyproject.toml file, "
f"or consider ignoring the dependencies by adding them to the 'external.exclude' list in {CONFIG_FILE_NAME}.toml.\n{BCOLORS.ENDC}",
file=sys.stderr,
)
if any_unused:
print(
f"{BCOLORS.OKCYAN}\nRemove the unused dependencies from the corresponding pyproject.toml file, "
f"or consider ignoring the dependencies by adding them to the 'external.exclude' list in {CONFIG_FILE_NAME}.toml.\n{BCOLORS.ENDC}",
file=sys.stderr,
)


def add_base_arguments(parser: argparse.ArgumentParser) -> None:
Expand Down Expand Up @@ -635,7 +648,7 @@ def tach_check(
project_root / source_root for source_root in project_config.source_roots
]

print_errors(
print_boundary_errors(
check_result.errors + check_result.deprecated_warnings,
source_roots=source_roots,
)
Expand Down Expand Up @@ -690,22 +703,23 @@ def tach_check_external(
exclude_paths=exclude_paths,
)

if result.unused_dependencies:
print_unused_external_dependencies(result.unused_dependencies)
print_warnings(result.warnings)
print_errors(result.errors)
print_unused_external_dependencies(result.unused_dependencies)
print_undeclared_dependencies(result.undeclared_dependencies)

if result.undeclared_dependencies:
print_undeclared_dependencies(result.undeclared_dependencies)
if result.errors or result.undeclared_dependencies:
sys.exit(1)
else:
print(
f"{icons.SUCCESS} {BCOLORS.OKGREEN}All external dependencies validated!{BCOLORS.ENDC}"
)
sys.exit(0)

except Exception as e:
print(str(e))
sys.exit(1)

print(
f"{icons.SUCCESS} {BCOLORS.OKGREEN}All external dependencies validated!{BCOLORS.ENDC}"
)
sys.exit(0)


def tach_mod(
project_root: Path,
Expand Down
21 changes: 17 additions & 4 deletions python/tach/extension.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ def get_external_imports(
source_roots: list[str],
file_path: str,
ignore_type_checking_imports: bool,
include_string_imports: bool,
) -> list[tuple[str, int]]: ...
def get_normalized_imports(
source_roots: list[str],
Expand All @@ -24,10 +23,10 @@ def set_excluded_paths(
) -> None: ...
def check_external_dependencies(
project_root: str,
source_roots: list[str],
project_config: ProjectConfig,
module_mappings: dict[str, list[str]],
ignore_type_checking_imports: bool,
) -> tuple[dict[str, list[str]], dict[str, list[str]]]: ...
stdlib_modules: list[str],
) -> ExternalCheckDiagnostics: ...
def create_dependency_report(
project_root: str,
project_config: ProjectConfig,
Expand Down Expand Up @@ -95,6 +94,20 @@ class CheckDiagnostics:

def serialize_json(self, pretty_print: bool = False) -> str: ...

class ExternalCheckDiagnostics:
undeclared_dependencies: dict[str, list[str]]
unused_dependencies: dict[str, list[str]]
errors: list[str]
warnings: list[str]

def __new__(
cls,
undeclared_dependencies: dict[str, list[str]],
unused_dependencies: dict[str, list[str]],
errors: list[str],
warnings: list[str],
) -> ExternalCheckDiagnostics: ...

class DependencyConfig:
path: str
deprecated: bool
Expand Down
3 changes: 0 additions & 3 deletions python/tach/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,12 @@ def get_external_dependencies(
source_roots: list[str],
file_path: str,
ignore_type_checking_imports: bool,
include_string_imports: bool = False,
excluded_modules: set[str] | None = None,
) -> list[ExternalDependency]:
external_imports = get_external_imports(
source_roots=source_roots,
file_path=file_path,
ignore_type_checking_imports=ignore_type_checking_imports,
include_string_imports=include_string_imports,
)

excluded_modules = excluded_modules or set()
Expand Down Expand Up @@ -196,7 +194,6 @@ def external_dependency_report(
file_path=str(path.resolve()),
excluded_modules=set(project_config.external.exclude),
ignore_type_checking_imports=project_config.ignore_type_checking_imports,
include_string_imports=project_config.include_string_imports,
)
return render_external_dependency_report(path, external_dependencies, raw=raw)

Expand Down
14 changes: 14 additions & 0 deletions python/tach/utils/external.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,20 @@ def is_stdlib_module(module: str) -> bool:
return in_stdlib(module) # type: ignore


def get_stdlib_modules() -> list[str]:
if sys.version_info >= (3, 10):
modules = set(sys.builtin_module_names)
modules.update(sys.stdlib_module_names)
modules.update(KNOWN_MODULE_SPECIAL_CASES)
return sorted(modules)
else:
from stdlib_list import stdlib_list # type: ignore

modules: set[str] = set(stdlib_list()) # type: ignore
modules.update(KNOWN_MODULE_SPECIAL_CASES)
return list(sorted(modules))


def _get_installed_modules(dist: Any) -> list[str]:
# This method is best-effort, and is only used for Python < 3.10
module_names: set[str] = set()
Expand Down
Loading

0 comments on commit e95a139

Please sign in to comment.