diff --git a/src/rejuvenation/cli.py b/src/rejuvenation/cli.py index 5c9d6728..425353e4 100644 --- a/src/rejuvenation/cli.py +++ b/src/rejuvenation/cli.py @@ -9,6 +9,9 @@ from renaissance.recipes.python_refactoring import PythonRefactoring from renaissance.syntax_tree import ASTShower +# TODO: is this file working as intended? +# See https://github.com/TNO/Renaissance.Py/issues/196 + def refactor(): """AI: Run the `refactor`/`extract`/`inspect` CLI subcommands based on `sys.argv`.""" diff --git a/src/renaissance/project/project_scanner.py b/src/renaissance/project/project_scanner.py index 0c4c88df..87728eaa 100644 --- a/src/renaissance/project/project_scanner.py +++ b/src/renaissance/project/project_scanner.py @@ -1,87 +1,104 @@ -"""AI: Scanners that discover project source files from a compilation database or file system.""" +"""Language-specific source file scanners used to feed recipes with the files to process.""" import json -from os import system +import subprocess from pathlib import Path class ProjectScanner: - """AI: Base scanner interface for discovering a project's source files.""" + """Base class for language-specific source file scanners.""" def find_sources(self) -> list[str]: - """AI: Discover and return this project's source file paths.""" + """Return the paths of source files found by this scanner. Implemented by subclasses.""" raise NotImplementedError class CppScanner(ProjectScanner): - """AI: Scanner that discovers C/C++ sources from a compilation database.""" + """Scan a C/C++ project's compile_commands.json for source files.""" - def __init__(self, compile_commands_path: str = "compile_commands.json"): - """AI: Configure a scanner that discovers C/C++ sources from a compilation database.""" + def __init__(self, compile_commands_path: str = "compile_commands.json") -> None: + """Store the path to the compile_commands.json to read sources from.""" self.compile_commands_path = compile_commands_path def find_sources(self) -> list[str]: - """AI: Discover C/C++ source files listed in the compilation database.""" + """Return every file entry listed in compile_commands.json, sorted and deduplicated.""" if not Path(self.compile_commands_path).exists(): - raise FileNotFoundError("compile_commands.json not found") + message = "compile_commands.json not found" + raise FileNotFoundError(message) with Path(self.compile_commands_path).open() as f: commands = json.load(f) - return sorted(set(entry["file"] for entry in commands if "file" in entry)) + return sorted({entry["file"] for entry in commands if "file" in entry}) class JavaScanner(ProjectScanner): - """AI: Scanner that discovers Java sources under a root directory.""" + """Scan a directory tree for Java source files.""" - def __init__(self, root_dir: str = "."): - """AI: Configure a scanner that discovers Java sources under a root directory.""" + def __init__(self, root_dir: str = ".") -> None: + """Store the root directory to scan for .java files.""" self.root_dir = root_dir def find_sources(self) -> list[str]: - """AI: Discover Java source files recursively under the root directory.""" + """Return every .java file under root_dir, sorted.""" + # TODO: is this correct? does this filter out files correctly? + # See https://github.com/TNO/Renaissance.Py/issues/200 + java_files = Path(self.root_dir).rglob("*.java") return sorted(str(f) for f in java_files) class PythonScanner(ProjectScanner): - """AI: Scanner that discovers Python sources under known package directories.""" + """Scan a directory tree for Python source files.""" + + EXCLUDED_DIRS = frozenset({".git", "__pycache__", ".venv", "venv"}) + # TODO: incomplete list, extend this list with more files/directories that should always be ignored - def __init__(self, root_dir: str = ".", package_dirs: list[str] | None = None): - """AI: Configure a scanner that discovers Python sources under known package directories.""" - # return (file_path for file_path in current_dir.iterdir() if is_python_file) + def __init__(self, root_dir: str = ".", package_dirs: list[str] | None = None) -> None: + """Store the scan root and optional package_dirs narrowing. + package_dirs, when given, narrows the scan to those subdirectories of root_dir. + Left as None (the default), the whole of root_dir is scanned instead of assuming a + src/lib/test layout, since that assumption silently skipped real third-party layouts. + """ self.root_dir = root_dir - self.package_dirs = package_dirs or ["src", "lib", "test"] - # TODO: Why this hardcoded default heuristic? - # Why not what Python by default enforces or what is derived from the project config? + self.package_dirs = package_dirs def find_sources(self) -> list[str]: - """AI: Discover Python source files under each configured package directory.""" - files = [] - - for d in self.package_dirs: - file_path = Path(self.root_dir) / d - if file_path.exists(): - files.extend(file_path.glob("**/*.py")) - return sorted(files) + """Return every .py file under root_dir (or package_dirs, if given), sorted, excluding EXCLUDED_DIRS.""" + path = Path(self.root_dir) + if not path.exists(): + message = f"root_dir does not exist: {self.root_dir}" + raise FileNotFoundError(message) + if not path.is_dir(): + message = f"root_dir is not a directory: {self.root_dir}" + raise NotADirectoryError(message) + + roots = [Path(self.root_dir) / d for d in self.package_dirs] if self.package_dirs else [Path(self.root_dir)] + files: list[Path] = [] + for root in roots: + if not root.exists(): + continue + files.extend(path for path in root.rglob("*.py") if not any(part in self.EXCLUDED_DIRS for part in path.parts)) + return sorted(str(path) for path in files) class BearCppScanner(CppScanner): - """AI: Scanner that generates a compilation database via Bear before discovering sources.""" + """CppScanner that generates compile_commands.json via Bear when it's missing.""" - def __init__(self, build_dir: str = ".", compile_commands_path: str = "compile_commands.json"): - """AI: Configure a scanner that generates a compilation database via Bear before discovering sources.""" + def __init__(self, build_dir: str = ".", compile_commands_path: str = "compile_commands.json") -> None: + """Store the build directory to run Bear in, alongside the compile_commands.json path.""" super().__init__(compile_commands_path) self.build_dir = build_dir - def run_bear(self): - """AI: Regenerate the compilation database by running Bear over the configured build.""" - print("Running Bear to generate compile_commands.json...") - result = system(f"bear -- make -C {self.build_dir}") - if result != 0: - raise RuntimeError("Bear failed to run or make failed.") + def run_bear(self) -> None: + """Run Bear to generate compile_commands.json by wrapping the project's make invocation.""" + # bear is resolved via PATH intentionally; build_dir isn't attacker-controlled here. + result = subprocess.run(["bear", "--", "make", "-C", self.build_dir], check=False) # noqa: S603, S607 + if result.returncode != 0: + message = "Bear failed to run or make failed." + raise RuntimeError(message) def find_sources(self) -> list[str]: - """AI: Generate the compilation database via Bear if missing, then discover C/C++ sources from it.""" + """Generate compile_commands.json via Bear if missing, then return its listed sources.""" if not Path(self.compile_commands_path).exists(): self.run_bear() return super().find_sources() diff --git a/test/project/test_project_scanner.py b/test/project/test_project_scanner.py index 58395212..7b2387b1 100644 --- a/test/project/test_project_scanner.py +++ b/test/project/test_project_scanner.py @@ -1,6 +1,8 @@ """Tests for the project source-file scanners.""" import json +from pathlib import Path +from unittest.mock import Mock import pytest from hamcrest import assert_that, calling, contains_inanyorder, empty, equal_to, is_, raises @@ -196,15 +198,69 @@ def test_searches_multiple_package_dirs(self, tmp_path): ) def test_default_package_dirs(self): - """AI: Assert PythonScanner defaults package_dirs to ["src", "lib", "test"].""" + """Assert PythonScanner defaults package_dirs to None.""" scanner = PythonScanner() - assert_that(scanner.package_dirs, equal_to(["src", "lib", "test"])) + assert_that(scanner.package_dirs, is_(None)) def test_default_root_dir(self): """AI: Assert PythonScanner defaults root_dir to ".".""" scanner = PythonScanner() assert_that(scanner.root_dir, is_(".")) + def test_raises_when_root_dir_does_not_exist(self, tmp_path): + """Assert PythonScanner.find_sources raises FileNotFoundError when root_dir doesn't exist.""" + scanner = PythonScanner(str(tmp_path / "does-not-exist")) + + assert_that(calling(scanner.find_sources), raises(FileNotFoundError)) + + def test_raises_when_root_dir_is_not_a_directory(self, tmp_path): + """Assert PythonScanner.find_sources raises NotADirectoryError when root_dir is a file.""" + root_file = tmp_path / "not_a_dir.py" + root_file.write_text("") + scanner = PythonScanner(str(root_file)) + + assert_that(calling(scanner.find_sources), raises(NotADirectoryError)) + + @pytest.mark.parametrize("excluded_dir", sorted(PythonScanner.EXCLUDED_DIRS)) + def test_excludes_known_noise_dirs_in_default_whole_tree_scan(self, tmp_path, excluded_dir): + """Assert PythonScanner.find_sources excludes known noise directories during a default whole-tree scan.""" + noise_dir = tmp_path / excluded_dir + noise_dir.mkdir() + (noise_dir / "ignored.py").write_text("") + (tmp_path / "kept.py").write_text("") + + scanner = PythonScanner(str(tmp_path)) + result = [Path(p).name for p in scanner.find_sources()] + + assert_that(result, equal_to(["kept.py"])) + + @pytest.mark.parametrize("excluded_dir", sorted(PythonScanner.EXCLUDED_DIRS)) + def test_excludes_known_noise_dirs_within_explicit_package_dirs(self, tmp_path, excluded_dir): + """Assert PythonScanner.find_sources excludes known noise directories within an explicit package_dirs.""" + src = tmp_path / "src" + src.mkdir() + noise_dir = src / excluded_dir + noise_dir.mkdir() + (noise_dir / "ignored.py").write_text("") + (src / "kept.py").write_text("") + + scanner = PythonScanner(str(tmp_path), package_dirs=["src"]) + result = [Path(p).name for p in scanner.find_sources()] + + assert_that(result, equal_to(["kept.py"])) + + def test_default_package_dirs_scans_whole_root_dir(self, tmp_path): + """Assert PythonScanner scans the whole root_dir by default when package_dirs is not given.""" + # Motivating case: source living outside src/lib/test (e.g. redis-py's redis/ layout). + redis_like = tmp_path / "redis" + redis_like.mkdir() + (redis_like / "client.py").write_text("") + + scanner = PythonScanner(str(tmp_path)) + result = [str(p) for p in scanner.find_sources()] + + assert_that(result, equal_to([str(redis_like / "client.py")])) + # --------------------------------------------------------------------------- # BearCppScanner @@ -249,14 +305,14 @@ def test_find_sources_does_not_call_run_bear_when_compile_commands_exists(self, def test_run_bear_raises_on_nonzero_exit(self, mocker): """AI: Assert BearCppScanner.run_bear raises RuntimeError when the Bear subprocess exits non-zero.""" scanner = BearCppScanner() - mocker.patch("renaissance.project.project_scanner.system", return_value=1) + mocker.patch("renaissance.project.project_scanner.subprocess.run", return_value=Mock(returncode=1)) assert_that(calling(scanner.run_bear), raises(RuntimeError)) def test_run_bear_succeeds_on_zero_exit(self, mocker): """AI: Assert BearCppScanner.run_bear does not raise when the Bear subprocess exits zero.""" scanner = BearCppScanner() - mocker.patch("renaissance.project.project_scanner.system", return_value=0) + mocker.patch("renaissance.project.project_scanner.subprocess.run", return_value=Mock(returncode=0)) # Should not raise scanner.run_bear()