From b77e8006196ee1b0dce000f800f4874d2a446843 Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Wed, 3 Feb 2021 16:09:17 +0000 Subject: [PATCH] Recurse inside given folders When given folders, use the normal detection algorithm to look inside them, making the outcome in sync with user expectations. Fixes: #1241 --- src/ansiblelint/file_utils.py | 67 ++++++++++++++++++++++++++++++++--- src/ansiblelint/runner.py | 7 ++-- src/ansiblelint/utils.py | 40 +-------------------- test/TestCliRolePaths.py | 2 +- test/TestRunner.py | 4 ++- test/TestUtils.py | 8 ++--- 6 files changed, 77 insertions(+), 51 deletions(-) diff --git a/src/ansiblelint/file_utils.py b/src/ansiblelint/file_utils.py index 684dc55f7d..16ff5c46b0 100644 --- a/src/ansiblelint/file_utils.py +++ b/src/ansiblelint/file_utils.py @@ -1,8 +1,13 @@ """Utility functions related to file operations.""" +import copy +import logging import os +import subprocess +from argparse import Namespace +from collections import OrderedDict from contextlib import contextmanager from pathlib import Path -from typing import TYPE_CHECKING, Any, Iterator, List, Optional, Union +from typing import TYPE_CHECKING, Any, Dict, Iterator, List, Optional, Set, Union import wcmatch.pathlib @@ -15,6 +20,8 @@ else: BasePathLike = os.PathLike +_logger = logging.getLogger(__package__) + def normpath(path: Union[str, BasePathLike]) -> str: """ @@ -24,7 +31,12 @@ def normpath(path: Union[str, BasePathLike]) -> str: make this user configurable. """ # convertion to string in order to allow receiving non string objects - return os.path.relpath(str(path)) + relpath = os.path.relpath(str(path)) + abspath = os.path.abspath(str(path)) + # we avoid returning relative paths that endup at root level + if abspath in relpath: + return abspath + return relpath @contextmanager @@ -87,8 +99,8 @@ def __init__( kind: Optional[FileType] = None): """Create a Lintable instance.""" if isinstance(name, str): - self.name = name - self.path = Path(name) + self.name = normpath(name) + self.path = Path(self.name) else: self.name = str(name) self.path = name @@ -137,3 +149,50 @@ def __eq__(self, other: object) -> bool: def __repr__(self) -> str: """Return user friendly representation of a lintable.""" return f"{self.name} ({self.kind})" + + +def get_yaml_files(options: Namespace) -> Dict[str, Any]: + """Find all yaml files.""" + # git is preferred as it also considers .gitignore + git_command = ['git', 'ls-files', '*.yaml', '*.yml'] + _logger.info("Discovering files to lint: %s", ' '.join(git_command)) + + out = None + + try: + out = subprocess.check_output( + git_command, + stderr=subprocess.STDOUT, + universal_newlines=True + ).splitlines() + except subprocess.CalledProcessError as exc: + _logger.warning( + "Failed to discover yaml files to lint using git: %s", + exc.output.rstrip('\n') + ) + except FileNotFoundError as exc: + if options.verbosity: + _logger.warning( + "Failed to locate command: %s", exc + ) + + if out is None: + out = [ + os.path.join(root, name) + for root, dirs, files in os.walk('.') + for name in files + if name.endswith('.yaml') or name.endswith('.yml') + ] + + return OrderedDict.fromkeys(sorted(out)) + + +def expand_dirs_in_lintables(lintables: Set[Lintable]) -> None: + """Return all recognized lintables within given directory.""" + all_files = get_yaml_files(options) + + for item in copy.copy(lintables): + if item.path.is_dir(): + for filename in all_files: + if filename.startswith(str(item.path)): + lintables.add(Lintable(filename)) diff --git a/src/ansiblelint/runner.py b/src/ansiblelint/runner.py index 05a5602f89..d87b47f428 100644 --- a/src/ansiblelint/runner.py +++ b/src/ansiblelint/runner.py @@ -10,7 +10,7 @@ import ansiblelint.utils from ansiblelint._internal.rules import LoadingFailureRule from ansiblelint.errors import MatchError -from ansiblelint.file_utils import Lintable +from ansiblelint.file_utils import Lintable, expand_dirs_in_lintables from ansiblelint.rules.AnsibleSyntaxCheckRule import AnsibleSyntaxCheckRule if TYPE_CHECKING: @@ -52,6 +52,9 @@ def __init__( item = Lintable(item) self.lintables.add(item) + # Expand folders (roles) to their components + expand_dirs_in_lintables(self.lintables) + self.tags = tags self.skip_list = skip_list self._update_exclude_paths(exclude_paths) @@ -126,7 +129,7 @@ def worker(lintable: Lintable) -> List[MatchError]: skip_list=self.skip_list)) # update list of checked files - self.checked_files.update([str(x.path) for x in files]) + self.checked_files.update([str(x.path) for x in self.lintables]) return sorted(set(matches)) diff --git a/src/ansiblelint/utils.py b/src/ansiblelint/utils.py index d0a7cd79d5..1c7dd0bf6e 100644 --- a/src/ansiblelint/utils.py +++ b/src/ansiblelint/utils.py @@ -23,9 +23,7 @@ import inspect import logging import os -import subprocess from argparse import Namespace -from collections import OrderedDict from collections.abc import ItemsView from functools import lru_cache from pathlib import Path @@ -60,7 +58,7 @@ from ansiblelint._internal.rules import AnsibleParserErrorRule, LoadingFailureRule, RuntimeErrorRule from ansiblelint.constants import FileType from ansiblelint.errors import MatchError -from ansiblelint.file_utils import Lintable +from ansiblelint.file_utils import Lintable, get_yaml_files # ansible-lint doesn't need/want to know about encrypted secrets, so we pass a # string as the password to enable such yaml files to be opened and parsed @@ -711,42 +709,6 @@ def is_playbook(filename: str) -> bool: return False -def get_yaml_files(options: Namespace) -> dict: - """Find all yaml files.""" - # git is preferred as it also considers .gitignore - git_command = ['git', 'ls-files', '*.yaml', '*.yml'] - _logger.info("Discovering files to lint: %s", ' '.join(git_command)) - - out = None - - try: - out = subprocess.check_output( - git_command, - stderr=subprocess.STDOUT, - universal_newlines=True - ).splitlines() - except subprocess.CalledProcessError as exc: - _logger.warning( - "Failed to discover yaml files to lint using git: %s", - exc.output.rstrip('\n') - ) - except FileNotFoundError as exc: - if options.verbosity: - _logger.warning( - "Failed to locate command: %s", exc - ) - - if out is None: - out = [ - os.path.join(root, name) - for root, dirs, files in os.walk('.') - for name in files - if name.endswith('.yaml') or name.endswith('.yml') - ] - - return OrderedDict.fromkeys(sorted(out)) - - # pylint: disable=too-many-statements def get_lintables( options: Namespace = Namespace(), diff --git a/test/TestCliRolePaths.py b/test/TestCliRolePaths.py index a3a6b5170a..cfa2625681 100644 --- a/test/TestCliRolePaths.py +++ b/test/TestCliRolePaths.py @@ -129,7 +129,7 @@ def test_run_single_role_path_with_roles_path_env(self): role_path = 'roles/test-role' env = os.environ.copy() - env['ANSIBLE_ROLES_PATH'] = os.path.join(cwd, 'use-as-default-roles-path') + env['ANSIBLE_ROLES_PATH'] = os.path.realpath(os.path.join(cwd, "../examples/roles")) result = run_ansible_lint(role_path, cwd=cwd, env=env) assert 'Use shell only when shell functionality is required' in result.stdout diff --git a/test/TestRunner.py b/test/TestRunner.py index bca0e63f59..325a5de51f 100644 --- a/test/TestRunner.py +++ b/test/TestRunner.py @@ -73,7 +73,9 @@ def test_runner_with_directory(default_rules_collection, directory_name) -> None runner = Runner( directory_name, rules=default_rules_collection) - assert list(runner.lintables)[0].kind == 'role' + + expected = Lintable(name=directory_name, kind="role") + assert expected in runner.lintables def test_files_not_scanned_twice(default_rules_collection) -> None: diff --git a/test/TestUtils.py b/test/TestUtils.py index 708f2d6a2c..4d6d3a6553 100644 --- a/test/TestUtils.py +++ b/test/TestUtils.py @@ -30,7 +30,7 @@ import pytest -from ansiblelint import cli, constants, utils +from ansiblelint import cli, constants, file_utils, utils from ansiblelint.__main__ import initialize_logger from ansiblelint.cli import get_rules_dirs from ansiblelint.constants import FileType @@ -186,10 +186,10 @@ def test_get_yaml_files_git_verbose( options = cli.get_config(['-v']) initialize_logger(options.verbosity) monkeypatch.setenv(reset_env_var, '') - utils.get_yaml_files(options) + file_utils.get_yaml_files(options) expected_info = ( - "ansiblelint.utils", + "ansiblelint", logging.INFO, 'Discovering files to lint: git ls-files *.yaml *.yml') @@ -220,7 +220,7 @@ def test_get_yaml_files_silent(is_in_git, monkeypatch, capsys): ) monkeypatch.chdir(str(lint_path)) - files = utils.get_yaml_files(options) + files = file_utils.get_yaml_files(options) stderr = capsys.readouterr().err assert not stderr, 'No stderr output is expected when the verbosity is off' assert len(files) == yaml_count, (