From dd09fadf29bb867d44f6cb6838cc4a2cc63baaf3 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Sun, 5 Apr 2026 09:30:06 +0000 Subject: [PATCH] fix(exceptions): replace Any with str | os.PathLike | None for FileSystemError.path Narrow the path parameter type hint in FileSystemError.__init__ from Any to str | os.PathLike[str] | None. This improves static analysis accuracy since path is semantically a filesystem path, not an arbitrary value. Changes: - Add import os to exceptions.py - Replace path: Any = None with path: str | os.PathLike[str] | None = None - Reformat __init__ signature to multi-line for line-length compliance - Audit call-sites (context_service.py, project_service.py): both pass pathlib.Path which implements os.PathLike[str], no changes needed - Add BDD feature file covering str, pathlib.Path, None, and annotation check - Add corresponding step definitions ISSUES CLOSED: #3034 --- features/filesystem_error_type_hint.feature | 29 ++++ .../steps/filesystem_error_type_hint_steps.py | 128 ++++++++++++++++++ src/cleveragents/core/exceptions.py | 8 +- 3 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 features/filesystem_error_type_hint.feature create mode 100644 features/steps/filesystem_error_type_hint_steps.py diff --git a/features/filesystem_error_type_hint.feature b/features/filesystem_error_type_hint.feature new file mode 100644 index 000000000..2786ec882 --- /dev/null +++ b/features/filesystem_error_type_hint.feature @@ -0,0 +1,29 @@ +Feature: FileSystemError path parameter type hint + As a developer using the CleverAgents exception hierarchy + I want FileSystemError to accept str, pathlib.Path, and None for the path parameter + So that static analysis tools can catch callers passing incompatible types + + Background: + Given the FileSystemError class is imported from cleveragents.core.exceptions + + Scenario: Construct FileSystemError with a str path + When I construct a FileSystemError with message "file not found" and path "/tmp/foo.txt" + Then the FileSystemError is created successfully + And the path attribute equals "/tmp/foo.txt" + + Scenario: Construct FileSystemError with a pathlib.Path path + When I construct a FileSystemError with message "permission denied" and a pathlib.Path path + Then the FileSystemError is created successfully + And the path attribute is the pathlib.Path instance + + Scenario: Construct FileSystemError with None path + When I construct a FileSystemError with message "unknown fs error" and no path + Then the FileSystemError is created successfully + And the path attribute is None + + Scenario: FileSystemError path type annotation is str or os.PathLike or None + When I inspect the type annotation of the path parameter in FileSystemError.__init__ + Then the annotation is not Any + And the annotation accepts str values + And the annotation accepts os.PathLike values + And the annotation accepts None diff --git a/features/steps/filesystem_error_type_hint_steps.py b/features/steps/filesystem_error_type_hint_steps.py new file mode 100644 index 000000000..cef3a8467 --- /dev/null +++ b/features/steps/filesystem_error_type_hint_steps.py @@ -0,0 +1,128 @@ +"""Step definitions for FileSystemError path parameter type hint tests.""" + +from __future__ import annotations + +import os +from pathlib import Path +from typing import Any, get_args, get_origin, get_type_hints + +from behave import given, then, when +from behave.runner import Context + +from cleveragents.core.exceptions import FileSystemError + + +@given("the FileSystemError class is imported from cleveragents.core.exceptions") +def step_import_filesystem_error(context: Context) -> None: + """Ensure FileSystemError is available.""" + context.FileSystemError = FileSystemError + + +@when('I construct a FileSystemError with message "{message}" and path "{path}"') +def step_construct_with_str_path(context: Context, message: str, path: str) -> None: + """Construct FileSystemError with a str path.""" + context.error = FileSystemError(message=message, path=path) + context.path_value = path + + +@when('I construct a FileSystemError with message "{message}" and a pathlib.Path path') +def step_construct_with_pathlib_path(context: Context, message: str) -> None: + """Construct FileSystemError with a pathlib.Path path.""" + path = Path("/tmp/test_dir/file.txt") + context.error = FileSystemError(message=message, path=path) + context.path_value = path + + +@when('I construct a FileSystemError with message "{message}" and no path') +def step_construct_with_none_path(context: Context, message: str) -> None: + """Construct FileSystemError with no path (defaults to None).""" + context.error = FileSystemError(message=message) + context.path_value = None + + +@when("I inspect the type annotation of the path parameter in FileSystemError.__init__") +def step_inspect_annotation(context: Context) -> None: + """Inspect the type annotation of the path parameter.""" + hints = get_type_hints(FileSystemError.__init__) + context.path_annotation = hints.get("path") + + +@then("the FileSystemError is created successfully") +def step_error_created(context: Context) -> None: + """Verify the FileSystemError was created without error.""" + assert isinstance(context.error, FileSystemError), ( + f"Expected FileSystemError instance, got {type(context.error)}" + ) + + +@then('the path attribute equals "{expected_path}"') +def step_path_equals_str(context: Context, expected_path: str) -> None: + """Verify the path attribute equals the expected string.""" + assert context.error.path == expected_path, ( + f"Expected path '{expected_path}', got '{context.error.path}'" + ) + + +@then("the path attribute is the pathlib.Path instance") +def step_path_is_pathlib(context: Context) -> None: + """Verify the path attribute is the pathlib.Path instance.""" + assert isinstance(context.error.path, Path), ( + f"Expected pathlib.Path instance, got {type(context.error.path)}" + ) + assert context.error.path == context.path_value, ( + f"Expected path '{context.path_value}', got '{context.error.path}'" + ) + + +@then("the path attribute is None") +def step_path_is_none(context: Context) -> None: + """Verify the path attribute is None.""" + assert context.error.path is None, ( + f"Expected path to be None, got '{context.error.path}'" + ) + + +@then("the annotation is not Any") +def step_annotation_not_any(context: Context) -> None: + """Verify the path annotation is not Any.""" + assert context.path_annotation is not Any, ( + "path parameter should not be typed as Any" + ) + + +@then("the annotation accepts str values") +def step_annotation_accepts_str(context: Context) -> None: + """Verify the annotation includes str.""" + annotation = context.path_annotation + args = get_args(annotation) + assert str in args, f"Expected str in annotation args, got {args}" + + +@then("the annotation accepts os.PathLike values") +def step_annotation_accepts_pathlike(context: Context) -> None: + """Verify the annotation includes os.PathLike.""" + annotation = context.path_annotation + args = get_args(annotation) + # Check for os.PathLike or os.PathLike[str] in the union args. + # Three checks are needed because generic alias representation varies across + # Python versions: + # - get_origin() works for parameterized generics (os.PathLike[str]) + # - Direct identity check handles the bare os.PathLike case + # - __origin__ attribute is a fallback for older typing internals + pathlike_present = any( + ( + get_origin(a) is os.PathLike + or a is os.PathLike + or (hasattr(a, "__origin__") and a.__origin__ is os.PathLike) + ) + for a in args + ) + assert pathlike_present, f"Expected os.PathLike in annotation args, got {args}" + + +@then("the annotation accepts None") +def step_annotation_accepts_none(context: Context) -> None: + """Verify the annotation includes None (NoneType).""" + annotation = context.path_annotation + args = get_args(annotation) + assert type(None) in args, f"Expected NoneType in annotation args, got {args}" diff --git a/src/cleveragents/core/exceptions.py b/src/cleveragents/core/exceptions.py index 4ca01d81f..b149f7c86 100644 --- a/src/cleveragents/core/exceptions.py +++ b/src/cleveragents/core/exceptions.py @@ -4,6 +4,7 @@ Based on ADR-005: Error Handling Hierarchy and Exception Strategy. All exceptions inherit from CleverAgentsError and follow fail-fast principles. """ +import os from typing import Any @@ -234,8 +235,11 @@ class FileSystemError(CleverAgentsError): """File system operation failures.""" def __init__( - self, message: str, path: Any = None, details: dict[str, Any] | None = None - ): + self, + message: str, + path: str | os.PathLike[str] | None = None, + details: dict[str, Any] | None = None, + ) -> None: """Initialize with path information. Args: -- 2.52.0