Merge pull request #432 from gauge-sh/separate-interfaces-from-depend…

…ency-checks

Separate interface checks from dependency checks

README.md

@@ -87,7 +87,7 @@ tach check
 You will see:
 
 ```bash
-✅ All module dependencies validated!
+✅ All modules validated!
 ```
 
 You can validate that Tach is working by either:

docs/getting-started/getting-started.mdx

@@ -52,7 +52,7 @@ tach check
 You will see:
 
 ```bash
-✅ All module dependencies validated!
+✅ All modules validated!
 ```
 
 You can validate that Tach is working by either:

docs/usage/deprecate.mdx

@@ -32,7 +32,7 @@ get_data()
 This import won't fail `tach check`, instead you'll see:
 ```shell
 ‼️ parsing.py[L1]: Import 'core.get_data' is deprecated. 'parsing' should not depend on 'core'.
-✅ All module dependencies validated!
+✅ All modules validated!
 ```
 
 Note that we still see that all module dependencies are valid! To fail on the dependency, simply remove it from the `depends_on` key.

docs/usage/interfaces.mdx

@@ -40,7 +40,7 @@ DataModel.objects.all()
 This import would **fail** `tach check` with the following error:
 
 ```shell
-❌ domain.py[L1]: Module 'core' is in strict mode. Only imports from the public interface of this module are allowed. The import 'core.main.DataModel' (in module 'parsing') is not public.
+❌ domain.py[L1]: Module 'core' has a public interface. Only imports from the public interface of this module are allowed. The import 'core.main.DataModel' (in module 'parsing') is not public.
 ```
 
 In this case, there is a public interface defined in `tach.toml` which includes a service method to use instead.
@@ -54,5 +54,5 @@ get_data()
 `tach check` will now pass!
 
 ```bash
-✅ All module dependencies validated!
+✅ All modules validated!
 ```

python/tach/cli.py

@@ -64,11 +64,11 @@ def build_error_message(error: BoundaryError, source_roots: list[Path]) -> str:
         )
 
     error_template = (
-        f"{icons.FAIL} {BCOLORS.FAIL}{error_location}{BCOLORS.ENDC}{BCOLORS.FAIL}: "
+        f"{icons.FAIL}  {BCOLORS.FAIL}{error_location}{BCOLORS.ENDC}{BCOLORS.WARNING}: "
         f"{{message}} {BCOLORS.ENDC}"
     )
     warning_template = (
-        f"{icons.WARNING} {BCOLORS.FAIL}{error_location}{BCOLORS.ENDC}{BCOLORS.WARNING}: "
+        f"{icons.WARNING}  {BCOLORS.FAIL}{error_location}{BCOLORS.ENDC}{BCOLORS.WARNING}: "
         f"{{message}} {BCOLORS.ENDC}"
     )
     error_info = error.error_info
@@ -85,17 +85,45 @@ def print_warnings(warning_list: list[str]) -> None:
 def print_errors(error_list: list[BoundaryError], source_roots: list[Path]) -> None:
     if not error_list:
         return
-    sorted_results = sorted(error_list, key=lambda e: e.file_path)
-    for error in sorted_results:
+
+    interface_errors: list[BoundaryError] = []
+    dependency_errors: list[BoundaryError] = []
+    for error in sorted(error_list, key=lambda e: e.file_path):
+        if error.error_info.is_interface_error():
+            interface_errors.append(error)
+        else:
+            dependency_errors.append(error)
+
+    if interface_errors:
+        print(f"{BCOLORS.FAIL}Interface Errors:{BCOLORS.ENDC}", file=sys.stderr)
+        for error in interface_errors:
+            print(
+                build_error_message(error, source_roots=source_roots),
+                file=sys.stderr,
+            )
         print(
-            build_error_message(error, source_roots=source_roots),
+            f"{BCOLORS.WARNING}\nIf you intended to change an interface, edit the '[[interfaces]]' section of {CONFIG_FILE_NAME}.toml."
+            f"\nOtherwise, remove any disallowed imports and consider refactoring.\n{BCOLORS.ENDC}",
             file=sys.stderr,
         )
-    if not all(error.error_info.is_deprecated() for error in sorted_results):
-        print(
-            f"{BCOLORS.WARNING}\nIf you intended to add a new dependency, run 'tach sync' to update your module configuration."
-            f"\nOtherwise, remove any disallowed imports and consider refactoring.\n{BCOLORS.ENDC}"
-        )
+
+    if dependency_errors:
+        print(f"{BCOLORS.FAIL}Dependency Errors:{BCOLORS.ENDC}", file=sys.stderr)
+        has_real_errors = False
+        for error in dependency_errors:
+            if not error.error_info.is_deprecated():
+                has_real_errors = True
+            print(
+                build_error_message(error, source_roots=source_roots),
+                file=sys.stderr,
+            )
+        print(file=sys.stderr)
+        if has_real_errors:
+            print(
+                f"{BCOLORS.WARNING}If you intended to add a new dependency, run 'tach sync' to update your module configuration."
+                f"\nOtherwise, remove any disallowed imports and consider refactoring.\n{BCOLORS.ENDC}",
+                file=sys.stderr,
+            )
 
 
 def print_unused_dependencies(
@@ -254,7 +282,17 @@ def build_parser() -> argparse.ArgumentParser:
     check_parser.add_argument(
         "--exact",
         action="store_true",
-        help="Raise errors if any dependency constraints are unused.",
+        help="When checking dependencies, raise errors if any dependencies are unused.",
+    )
+    check_parser.add_argument(
+        "--dependencies",
+        action="store_true",
+        help="Check dependency constraints between modules. When present, all checks must be explicitly enabled.",
+    )
+    check_parser.add_argument(
+        "--interfaces",
+        action="store_true",
+        help="Check interface implementations. When present, all checks must be explicitly enabled.",
     )
     add_base_arguments(check_parser)
 
@@ -528,6 +566,8 @@ def extend_and_validate(
 def tach_check(
     project_root: Path,
     exact: bool = False,
+    dependencies: bool = True,
+    interfaces: bool = True,
     exclude_paths: list[str] | None = None,
 ):
     logger.info(
@@ -554,6 +594,8 @@ def tach_check(
         check_result = check(
             project_root=project_root,
             project_config=project_config,
+            dependencies=dependencies,
+            interfaces=interfaces,
             exclude_paths=exclude_paths,
         )
 
@@ -564,22 +606,14 @@ def tach_check(
             project_root / source_root for source_root in project_config.source_roots
         ]
 
-        if check_result.deprecated_warnings:
-            print_errors(
-                check_result.deprecated_warnings,
-                source_roots=source_roots,
-            )
-        exit_code = 0
-
-        if check_result.errors:
-            print_errors(
-                check_result.errors,
-                source_roots=source_roots,
-            )
-            exit_code = 1
+        print_errors(
+            check_result.errors + check_result.deprecated_warnings,
+            source_roots=source_roots,
+        )
+        exit_code = 1 if len(check_result.errors) > 0 else 0
 
-        # If we're checking in strict mode, we want to verify that pruning constraints has no effect
-        if exact:
+        # If we're checking in exact mode, we want to verify that pruning constraints has no effect
+        if dependencies and exact:
             pruned_config = sync_dependency_constraints(
                 project_root=project_root,
                 project_config=project_config,
@@ -601,9 +635,7 @@ def tach_check(
         sys.exit(1)
 
     if exit_code == 0:
-        print(
-            f"{icons.SUCCESS} {BCOLORS.OKGREEN}All module dependencies validated!{BCOLORS.ENDC}"
-        )
+        print(f"{icons.SUCCESS} {BCOLORS.OKGREEN}All modules validated!{BCOLORS.ENDC}")
     sys.exit(exit_code)
 
 
@@ -1100,9 +1132,18 @@ def main() -> None:
     elif args.command == "sync":
         tach_sync(project_root=project_root, add=args.add, exclude_paths=exclude_paths)
     elif args.command == "check":
-        tach_check(
-            project_root=project_root, exact=args.exact, exclude_paths=exclude_paths
-        )
+        if args.dependencies or args.interfaces:
+            tach_check(
+                project_root=project_root,
+                dependencies=args.dependencies,
+                interfaces=args.interfaces,
+                exact=args.exact,
+                exclude_paths=exclude_paths,
+            )
+        else:
+            tach_check(
+                project_root=project_root, exact=args.exact, exclude_paths=exclude_paths
+            )
     elif args.command == "check-external":
         tach_check_external(project_root=project_root, exclude_paths=exclude_paths)
     elif args.command == "install":

python/tach/extension.pyi

@@ -60,6 +60,8 @@ def dump_project_config_to_toml(project_config: ProjectConfig) -> str: ...
 def check(
     project_root: Path,
     project_config: ProjectConfig,
+    dependencies: bool,
+    interfaces: bool,
     exclude_paths: list[str],
 ) -> CheckResult: ...
 def sync_dependency_constraints(
@@ -83,6 +85,7 @@ class TachVisibilityError(ValueError):
 
 class ErrorInfo:
     def is_dependency_error(self) -> bool: ...
+    def is_interface_error(self) -> bool: ...
     def to_pystring(self) -> str: ...
     def is_deprecated(self) -> bool: ...
 

python/tach/icons.py

@@ -15,14 +15,12 @@ def test_unicode_encoding():
 SUPPORTS_UNICODE = test_unicode_encoding()
 
 
-### Icons which don't depend on Unicode support
-WARNING = "‼️"
-
-
 ### Icons which depend on Unicode support
 if SUPPORTS_UNICODE:
     SUCCESS = "✅"
+    WARNING = "⚠️"
     FAIL = "❌"
 else:
     SUCCESS = "[OK]"
+    WARNING = "[WARN]"
     FAIL = "[FAIL]"

python/tests/test_check.py

@@ -3,6 +3,7 @@
 import pytest
 
 from tach.cli import tach_check
+from tach.icons import SUCCESS, WARNING
 
 
 def test_valid_example_dir(example_dir, capfd):
@@ -11,8 +12,8 @@ def test_valid_example_dir(example_dir, capfd):
         tach_check(project_root=project_root)
     assert exc_info.value.code == 0
     captured = capfd.readouterr()
-    assert "✅" in captured.out  # success state
-    assert "‼️" in captured.err  # deprecated warning
+    assert SUCCESS in captured.out  # success state
+    assert WARNING in captured.err  # deprecated warning
 
 
 def test_valid_example_dir_monorepo(example_dir):

python/tests/test_cli.py

@@ -13,9 +13,18 @@
 @dataclass
 class ErrorInfo:
     exception_message: str
+    dependency_error: bool = False
+    interface_error: bool = False
+    deprecated: bool = False
+
+    def is_dependency_error(self) -> bool:
+        return self.dependency_error
+
+    def is_interface_error(self) -> bool:
+        return self.interface_error
 
     def is_deprecated(self) -> bool:
-        return False
+        return self.deprecated
 
     def to_pystring(self) -> str:
         return self.exception_message
@@ -62,7 +71,7 @@ def test_execute_with_config(capfd, mock_check, mock_project_config):
     captured = capfd.readouterr()
     assert sys_exit.value.code == 0
     assert "✅" in captured.out
-    assert "All module dependencies validated!" in captured.out
+    assert "All modules validated!" in captured.out
 
 
 def test_execute_with_error(capfd, mock_check, mock_project_config):
@@ -109,4 +118,4 @@ def test_execute_with_valid_exclude(capfd, mock_check, mock_project_config):
     captured = capfd.readouterr()
     assert sys_exit.value.code == 0
     assert "✅" in captured.out
-    assert "All module dependencies validated!" in captured.out
+    assert "All modules validated!" in captured.out