From cca099a146f212d0e9ea2df26c12bfbc59706e80 Mon Sep 17 00:00:00 2001 From: "Augusto W. Andreoli" Date: Sat, 10 Apr 2021 03:39:34 +0200 Subject: [PATCH] fix: don't fail when there is no config/root file (#350) * fix: error when no config file and no root file exist * style: add `nitpick-style.toml` as a root file --- src/nitpick/constants.py | 3 +- src/nitpick/flake8.py | 3 +- src/nitpick/project.py | 90 +++++++++++------------ src/nitpick/violations.py | 4 +- tests/helpers.py | 6 +- tests/{test_config.py => test_project.py} | 77 ++++++++++++++++++- 6 files changed, 127 insertions(+), 56 deletions(-) rename tests/{test_config.py => test_project.py} (70%) diff --git a/src/nitpick/constants.py b/src/nitpick/constants.py index 755427a5..91458ef8 100644 --- a/src/nitpick/constants.py +++ b/src/nitpick/constants.py @@ -20,7 +20,7 @@ SETUP_PY = "setup.py" SETUP_CFG = "setup.cfg" REQUIREMENTS_STAR_TXT = "requirements*.txt" -PIPFILE_STAR = "Pipfile.*" +PIPFILE_STAR = "Pipfile*" ROOT_PYTHON_FILES = ("app.py", "wsgi.py", "autoapp.py") MANAGE_PY = "manage.py" TOX_INI = "tox.ini" @@ -38,6 +38,7 @@ # All root files ROOT_FILES = ( DOT_NITPICK_TOML, + NITPICK_STYLE_TOML, PRE_COMMIT_CONFIG_YAML, PYPROJECT_TOML, SETUP_PY, diff --git a/src/nitpick/flake8.py b/src/nitpick/flake8.py index 5e3de764..80e0654c 100644 --- a/src/nitpick/flake8.py +++ b/src/nitpick/flake8.py @@ -13,6 +13,7 @@ from nitpick.core import Nitpick from nitpick.enums import OptionEnum from nitpick.exceptions import QuitComplainingError +from nitpick.project import find_main_python_file from nitpick.typedefs import Flake8Error from nitpick.violations import Fuss @@ -49,7 +50,7 @@ def collect_errors(self) -> Iterator[Fuss]: nit = Nitpick.singleton() current_python_file = Path(self.filename) - main_python_file: Path = nit.project.find_main_python_file() + main_python_file: Path = find_main_python_file(nit.project.root) if current_python_file.absolute() != main_python_file.absolute(): # Only report warnings once, for the main Python file of this project. logger.debug("Ignoring other Python file: {}", self.filename) diff --git a/src/nitpick/project.py b/src/nitpick/project.py index e2d78b01..5b2cd4d3 100644 --- a/src/nitpick/project.py +++ b/src/nitpick/project.py @@ -40,9 +40,6 @@ def climb_directory_tree(starting_path: PathOrStr, file_patterns: Iterable[str]) -> Set[Path]: # TODO: add unit test """Climb the directory tree looking for file patterns.""" current_dir: Path = Path(starting_path).absolute() - if current_dir.is_file(): - current_dir = current_dir.parent - while current_dir.anchor != str(current_dir): for root_file in file_patterns: found_files = list(current_dir.glob(root_file)) @@ -52,7 +49,20 @@ def climb_directory_tree(starting_path: PathOrStr, file_patterns: Iterable[str]) return set() -# TODO: add unit tests with tmp_path https://docs.pytest.org/en/stable/tmpdir.html +def find_starting_dir(current_dir: PathOrStr) -> Path: + """Find the starting dir from the current dir.""" + logger.debug(f"Searching root from current dir: {str(current_dir)!r}") + all_files_dirs = list(Path(current_dir).glob("*")) + logger.debug("All files/dirs in the current dir:\n{}", "\n".join(str(file) for file in all_files_dirs)) + + # Don't fail if the current dir is empty + starting_file = str(all_files_dirs[0]) if all_files_dirs else "" + if starting_file: + return Path(starting_file).parent.absolute() + + return Path(current_dir).absolute() + + def find_root(current_dir: Optional[PathOrStr] = None) -> Path: """Find the root dir of the Python project (the one that has one of the ``ROOT_FILES``). @@ -61,16 +71,8 @@ def find_root(current_dir: Optional[PathOrStr] = None) -> Path: root_dirs: Set[Path] = set() seen: Set[Path] = set() - if not current_dir: - current_dir = Path.cwd() - logger.debug(f"Searching root from current dir: {str(current_dir)!r}") - all_files_dirs = list(Path(current_dir).glob("*")) - logger.debug("All files/dirs in the current dir:\n{}", "\n".join(str(file) for file in all_files_dirs)) - - # Don't fail if the current dir is empty - starting_file = str(all_files_dirs[0]) if all_files_dirs else "" - starting_dir = Path(starting_file).parent.absolute() - while True: + starting_dir = find_starting_dir(current_dir or Path.cwd()) + while starting_dir: # pragma: no cover # starting_dir will always have a value on the first run logger.debug(f"Climbing dir: {starting_dir}") project_files = climb_directory_tree(starting_dir, ROOT_FILES) if project_files and project_files & seen: @@ -81,24 +83,21 @@ def find_root(current_dir: Optional[PathOrStr] = None) -> Path: if not project_files: # If none of the root files were found, try again with manage.py. # On Django projects, it can be in another dir inside the root dir. - project_files = climb_directory_tree(starting_file, [MANAGE_PY]) + project_files = climb_directory_tree(starting_dir, [MANAGE_PY]) if not project_files or project_files & seen: break seen.update(project_files) - logger.debug(f"Project files seen: {project_files}") + logger.debug(f"Django project files seen: {project_files}") for found in project_files: root_dirs.add(found.parent) - if project_files: - logger.debug(f"Root dirs: {str(root_dirs)}") + logger.debug(f"Root dirs: {str(root_dirs)}") # Climb up one directory to search for more project files starting_dir = starting_dir.parent - if not starting_dir: - break if not root_dirs: - logger.error(f"No files found while climbing directory tree from {starting_file}") + logger.error(f"No files found while climbing directory tree from {starting_dir}") raise QuitComplainingError(Reporter().make_fuss(ProjectViolations.NO_ROOT_DIR)) # If multiple roots are found, get the top one (grandparent dir) @@ -107,6 +106,31 @@ def find_root(current_dir: Optional[PathOrStr] = None) -> Path: return top_dir +def find_main_python_file(root_dir: Path) -> Path: + """Find the main Python file in the root dir, the one that will be used to report Flake8 warnings. + + The search order is: + 1. Python files that belong to the root dir of the project (e.g.: ``setup.py``, ``autoapp.py``). + 2. ``manage.py``: they can be on the root or on a subdir (Django projects). + 3. Any other ``*.py`` Python file on the root dir and subdir. + This avoid long recursions when there is a ``node_modules`` subdir for instance. + """ + for the_file in itertools.chain( + # 1. + [root_dir / root_file for root_file in ROOT_PYTHON_FILES], + # 2. + root_dir.glob(f"*/{MANAGE_PY}"), + # 3. + root_dir.glob("*.py"), + root_dir.glob("*/*.py"), + ): + if the_file.exists(): + logger.info(f"Found the file {the_file}") + return Path(the_file) + + raise QuitComplainingError(Reporter().make_fuss(ProjectViolations.NO_PYTHON_FILE, root=str(root_dir))) + + class ToolNitpickSectionSchema(BaseNitpickSchema): """Validation schema for the ``[tool.nitpick]`` section on ``pyproject.toml``.""" @@ -143,30 +167,6 @@ def root(self) -> Path: """Root dir of the project.""" return find_root(self._chosen_root) - def find_main_python_file(self) -> Path: # TODO: add unit tests - """Find the main Python file in the root dir, the one that will be used to report Flake8 warnings. - - The search order is: - 1. Python files that belong to the root dir of the project (e.g.: ``setup.py``, ``autoapp.py``). - 2. ``manage.py``: they can be on the root or on a subdir (Django projects). - 3. Any other ``*.py`` Python file on the root dir and subdir. - This avoid long recursions when there is a ``node_modules`` subdir for instance. - """ - for the_file in itertools.chain( - # 1. - [self.root / root_file for root_file in ROOT_PYTHON_FILES], - # 2. - self.root.glob(f"*/{MANAGE_PY}"), - # 3. - self.root.glob("*.py"), - self.root.glob("*/*.py"), - ): - if the_file.exists(): - logger.info("Found the file {}", the_file) - return Path(the_file) - - raise QuitComplainingError(Reporter().make_fuss(ProjectViolations.NO_PYTHON_FILE, root=str(self.root))) - @mypy_property @lru_cache() def plugin_manager(self) -> PluginManager: diff --git a/src/nitpick/violations.py b/src/nitpick/violations.py index 3070cee0..6e52c816 100644 --- a/src/nitpick/violations.py +++ b/src/nitpick/violations.py @@ -73,8 +73,8 @@ class ProjectViolations(ViolationEnum): NO_ROOT_DIR = ( 101, - "No root directory found for this project!" - f" Create a configuration file on the root ({', '.join(CONFIG_FILES)})." + "No root directory detected." + f" Create a configuration file ({', '.join(CONFIG_FILES)}) manually, or run 'nitpick init'." f" See {READ_THE_DOCS_URL}configuration.html", ) NO_PYTHON_FILE = (102, "No Python file was found on the root dir and subdir of {root!r}") diff --git a/tests/helpers.py b/tests/helpers.py index 6f45951e..369ba6db 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -332,9 +332,9 @@ def cli_run( ) -> "ProjectMock": """Assert the expected CLI output for the chosen command.""" cli_args = [] if apply else ["--check"] - result, actual, expected = self._simulate_cli( - "run", str_or_lines, *cli_args, exit_code=exit_code or 1 if str_or_lines else 0 - ) + if exit_code is None: + exit_code = 1 if str_or_lines else 0 + result, actual, expected = self._simulate_cli("run", str_or_lines, *cli_args, exit_code=exit_code) if exception_class: assert isinstance(result.exception, exception_class) return self diff --git a/tests/test_config.py b/tests/test_project.py similarity index 70% rename from tests/test_config.py rename to tests/test_project.py index c4f98436..aa9b2bbe 100644 --- a/tests/test_config.py +++ b/tests/test_project.py @@ -1,9 +1,23 @@ """Config tests.""" +import os + import pytest -from nitpick.constants import DOT_NITPICK_TOML, PYPROJECT_TOML, SETUP_CFG +from nitpick.constants import ( + DOT_NITPICK_TOML, + GO_MOD, + GO_SUM, + MANAGE_PY, + NITPICK_STYLE_TOML, + PACKAGE_JSON, + PRE_COMMIT_CONFIG_YAML, + PYPROJECT_TOML, + SETUP_CFG, + SETUP_PY, + TOX_INI, +) from nitpick.core import Nitpick -from nitpick.project import Configuration +from nitpick.project import Configuration, find_main_python_file, find_root from nitpick.violations import ProjectViolations from tests.helpers import ProjectMock @@ -19,13 +33,20 @@ def test_singleton(): assert "This class cannot be instantiated directly" in str(err) -def test_no_root_dir(tmp_path): - """No root dir.""" +def test_no_root_dir_with_python_file(tmp_path): + """No root dir with Python file.""" project = ProjectMock(tmp_path, pyproject_toml=False, setup_py=False).create_symlink("hello.py") error = f"NIP101 {ProjectViolations.NO_ROOT_DIR.message}" project.flake8().assert_single_error(error).cli_run(error, exit_code=2).cli_ls(error, exit_code=2) +def test_no_root_dir_no_python_file(tmp_path): + """No root dir, no Python file.""" + project = ProjectMock(tmp_path, pyproject_toml=False, setup_py=False) + error = f"NIP101 {ProjectViolations.NO_ROOT_DIR.message}" + project.cli_run(error, exit_code=2).cli_ls(error, exit_code=2) + + def test_multiple_root_dirs(tmp_path): """Multiple possible "root dirs" found (e.g.: a requirements.txt file inside a docs dir).""" ProjectMock(tmp_path, setup_py=False).touch_file("docs/requirements.txt").touch_file("docs/conf.py").pyproject_toml( @@ -144,3 +165,51 @@ def test_has_multiple_config_files(tmp_path, caplog): ) assert f"Config file: reading from {project.root_dir / DOT_NITPICK_TOML}" in caplog.text assert f"Config file: ignoring existing {project.root_dir / PYPROJECT_TOML}" in caplog.text + + +@pytest.mark.parametrize( + "root_file", + [ + DOT_NITPICK_TOML, + PRE_COMMIT_CONFIG_YAML, + PYPROJECT_TOML, + SETUP_PY, + SETUP_CFG, + "requirements.txt", + "requirements_dev.txt", + "Pipfile", + "Pipfile.lock", + TOX_INI, + PACKAGE_JSON, + "Cargo.toml", + "Cargo.lock", + GO_MOD, + GO_SUM, + NITPICK_STYLE_TOML, + ], +) +def test_find_root_from_sub_dir(tmp_path, root_file): + """Find the root dir from a subdir.""" + root = tmp_path / "deep" / "root" + root.mkdir(parents=True) + (root / root_file).write_text("") + + curdir = root / "going" / "down" / "the" / "rabbit" / "hole" + curdir.mkdir(parents=True) + os.chdir(str(curdir)) + + assert find_root(curdir) == root, root_file + assert find_root(str(curdir)) == root, root_file + + +def test_find_root_django(tmp_path): + """Find Django root with manage.py only: the root is where manage.py is.""" + apps_dir = tmp_path / "apps" + apps_dir.mkdir(parents=True) + (apps_dir / MANAGE_PY).write_text("") + + assert find_root(apps_dir) == apps_dir + + # Search 2 levels of directories + assert find_main_python_file(tmp_path) == apps_dir / MANAGE_PY + assert find_main_python_file(apps_dir) == apps_dir / MANAGE_PY