Skip to content

Commit

Permalink
Change path exclusion logic
Browse files Browse the repository at this point in the history
Fixes: #3477
  • Loading branch information
ssbarnea committed May 25, 2023
1 parent 7bdb009 commit 2dde029
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 46 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ jobs:
WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:GITHUB_STEP_SUMMARY
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 801
PYTEST_REQPASS: 807
steps:
- name: Activate WSL1
if: "contains(matrix.shell, 'wsl')"
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ repos:
types: [file, yaml]
entry: yamllint --strict
- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: "v0.0.269"
rev: "v0.0.270"
hooks:
- id: ruff
args: [--fix, --exit-non-zero-on-fix]
Expand Down
2 changes: 2 additions & 0 deletions examples/bug-3477/.ansible-lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
exclude_paths:
- playbooks/generic/common.yml
2 changes: 2 additions & 0 deletions examples/bug-3477/playbooks/generic/common.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
- import_playbook: this-file-does-not-exist.yml
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ python_files = [
xfail_strict = true

[tool.ruff]
required-version = "0.0.270"
ignore = [
"E501", # we use black
"ERA001", # auto-removal of commented out code affects development and vscode integration
Expand Down
62 changes: 47 additions & 15 deletions src/ansiblelint/file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from __future__ import annotations

import copy
import fnmatch
import logging
import os
import subprocess
Expand Down Expand Up @@ -489,24 +490,55 @@ def discover_lintables(options: Options) -> dict[str, Any]:

# Applying exclude patterns
if not out:
out = set(".")

exclude_pattern = "|".join(str(x) for x in options.exclude_paths)
_logger.info("Looking up for files, excluding %s ...", exclude_pattern)
# remove './' prefix from output of WcMatch
out = {
strip_dotslash_prefix(fname)
for fname in wcmatch.wcmatch.WcMatch(
".",
exclude_pattern=exclude_pattern,
flags=wcmatch.wcmatch.RECURSIVE,
limit=256,
).match()
}

_logger.info("Looking up for files using glob ...")
out = {str(file) for file in Path(".").glob("**/*.*")}

before_count = len(out)
out = {file for file in out if not is_excluded(Path(file), opts=options)}
after_count = len(out)
if before_count != after_count:
exclude_pattern = ", ".join(str(x) for x in options.exclude_paths)
_logger.info(
"Filtered out %d/%d files due to exclude patterns: %s ...",
before_count - after_count,
before_count,
exclude_pattern,
)
return OrderedDict.fromkeys(sorted(out))


def is_excluded(path: Path, opts: Options = options) -> bool:
"""Verify if a file path should be excluded."""
abs_path = str(path.resolve())
if opts.project_dir and not abs_path.startswith(
os.path.abspath(opts.project_dir),
):
_logger.debug(
"Skipping %s as it is outside of the project directory.",
abs_path,
)
return True

# fnmatch does not work as excepted so we need to ensure the following
# logic:
# - a perfect match -> excluded
# - a match with `/**` appended to the pattern -> excluded
for pattern in opts.exclude_paths:
pattern2 = pattern + "**" if pattern.endswith("/") else pattern + "/**"
if (
abs_path.startswith(pattern)
or fnmatch.fnmatch(str(path), pattern)
or fnmatch.fnmatch(str(path), pattern2)
):
_logger.debug(
"Skipped %s as it matches exclude pattern '%s'",
path,
pattern,
)
return True
return False


def strip_dotslash_prefix(fname: str) -> str:
"""Remove ./ leading from filenames."""
return fname[2:] if fname.startswith("./") else fname
Expand Down
28 changes: 3 additions & 25 deletions src/ansiblelint/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import os
import warnings
from dataclasses import dataclass
from fnmatch import fnmatch
from typing import TYPE_CHECKING, Any

from ansible_compat.runtime import AnsibleWarning
Expand All @@ -17,7 +16,7 @@
from ansiblelint._internal.rules import LoadingFailureRule, WarningRule
from ansiblelint.constants import States
from ansiblelint.errors import LintWarning, MatchError, WarnSource
from ansiblelint.file_utils import Lintable, expand_dirs_in_lintables
from ansiblelint.file_utils import Lintable, expand_dirs_in_lintables, is_excluded
from ansiblelint.rules.syntax_check import AnsibleSyntaxCheckRule

if TYPE_CHECKING:
Expand Down Expand Up @@ -68,7 +67,7 @@ def __init__(
for item in lintables:
if not isinstance(item, Lintable):
item = Lintable(item)
item.explicit = True
item.explicit = True
self.lintables.add(item)

# Expand folders (roles) to their components
Expand All @@ -95,32 +94,11 @@ def _update_exclude_paths(self, exclude_paths: list[str]) -> None:

def is_excluded(self, lintable: Lintable) -> bool:
"""Verify if a file path should be excluded."""
# Any will short-circuit as soon as something returns True, but will
# be poor performance for the case where the path under question is
# not excluded.

# Exclusions should be evaluated only using absolute paths in order
# to work correctly.

# Explicit lintables are never excluded
if lintable.explicit:
return False

abs_path = str(lintable.abspath)
if self.project_dir and not abs_path.startswith(self.project_dir):
_logger.debug(
"Skipping %s as it is outside of the project directory.",
abs_path,
)
return True

return any(
abs_path.startswith(path)
or lintable.path.match(path)
or fnmatch(str(abs_path), path)
or fnmatch(str(lintable), path)
for path in self.exclude_paths
)
return is_excluded(lintable.path)

def run(self) -> list[MatchError]:
"""Execute the linting process."""
Expand Down
21 changes: 19 additions & 2 deletions test/test_file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,19 @@
from pathlib import Path
from typing import TYPE_CHECKING, Any

# pylint: disable=preferred-module
import pytest

from ansiblelint import cli, file_utils
from ansiblelint.__main__ import initialize_logger
from ansiblelint.config import Options
from ansiblelint.file_utils import (
Lintable,
cwd,
expand_path_vars,
expand_paths_vars,
find_project_root,
is_excluded,
normpath,
normpath_path,
)
Expand All @@ -27,7 +30,6 @@
from _pytest.logging import LogCaptureFixture
from _pytest.monkeypatch import MonkeyPatch

from ansiblelint.config import Options
from ansiblelint.constants import FileType
from ansiblelint.rules import RulesCollection

Expand Down Expand Up @@ -119,7 +121,7 @@ def test_discover_lintables_silent(
caplog.set_level(logging.FATAL)
options = cli.get_config([])
test_dir = Path(__file__).resolve().parent
lint_path = test_dir / ".." / "examples" / "roles" / "test-role"
lint_path = (test_dir / ".." / "examples" / "roles" / "test-role").resolve()
if not is_in_git:
monkeypatch.setenv("GIT_DIR", "")

Expand Down Expand Up @@ -579,3 +581,18 @@ def test_bug_2513(
results = Runner(filename, rules=default_rules_collection).run()
assert len(results) == 1
assert results[0].rule.id == "name"


@pytest.mark.parametrize(
("path", "exclude_paths", "result"),
(
pytest.param("foo/some.yml", ["foo"], True, id="0"),
pytest.param("foo/bar/some.yml", ["foo"], True, id="1"),
pytest.param("foo/bar/some.yml", ["foo/"], True, id="2"),
pytest.param("foo/bar/some.yml", ["foo/**"], True, id="3"),
),
)
def test_is_excluded(path: str, exclude_paths: list[str], result: bool) -> None:
"""Test logic for excluding files."""
custom_options = Options(exclude_paths=exclude_paths)
assert is_excluded(Path(path), opts=custom_options) == result
10 changes: 8 additions & 2 deletions test/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@
0,
id="contains_secrets",
),
pytest.param(
Path("examples/playbooks/deep"),
["examples/playbooks/deep/empty.yml"],
0,
id="excludes",
),
),
)
def test_runner(
Expand Down Expand Up @@ -99,8 +105,8 @@ def test_runner_exclude_globs(
)

matches = runner.run()
# we expect to find one 2 matches from the very few .yaml file we have there (most of them have .yml extension)
assert len(matches) == 2
# we expect to find one match from a .yaml (most of them have .yml extension)
assert len(matches) == 1


@pytest.mark.parametrize(
Expand Down

0 comments on commit 2dde029

Please sign in to comment.