Skip to content

Commit

Permalink
Ensure that symlink-arguments are resolved
Browse files Browse the repository at this point in the history
- Fixes bug where a giving a symlinked directory as argument would
  fail to lint the entire role, basically because the linter failed
  to resolve the link before processing all lintables within it.
- Ensure we resolve symlink in Lintable constructor
- Add normalization function that returns a Path
  • Loading branch information
ssbarnea committed Sep 21, 2022
1 parent e00891b commit 2d314b9
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ jobs:
WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:TOX_PARALLEL_NO_SPINNER
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 698
PYTEST_REQPASS: 702

steps:
- name: Activate WSL1
Expand Down
4 changes: 4 additions & 0 deletions .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,7 @@ disable =
[TYPECHECK]
# pylint is unable to detect Namespace attributes and will throw a E1101
generated-members=options.*

[SUMMARY]
# We don't need the score spamming console, as we either pass or fail
score=n
3 changes: 2 additions & 1 deletion src/ansiblelint/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,8 @@ def get_config(arguments: list[str]) -> Namespace:

if options.project_dir == ".":
project_dir = guess_project_dir(options.config_file)
options.project_dir = normpath(project_dir)
options.project_dir = os.path.expanduser(normpath(project_dir))

if not options.project_dir or not os.path.exists(options.project_dir):
raise RuntimeError(
f"Failed to determine a valid project_dir: {options.project_dir}"
Expand Down
65 changes: 57 additions & 8 deletions src/ansiblelint/file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,54 @@ def normpath(path: str | BasePathLike) -> str:
if not path:
path = "."
# conversion to string in order to allow receiving non string objects
cwd = os.getcwd()
relpath = os.path.relpath(str(path))
path_absolute = os.path.abspath(str(path))
if path_absolute.startswith(cwd):
return relpath
if path_absolute.startswith(os.path.expanduser("~")):
return path_absolute.replace(os.path.expanduser("~"), "~")
# we avoid returning relative paths that end-up at root level
if path_absolute in relpath:
return path_absolute
return relpath


# That is needed for compatibility with py38, later was added to Path class
def is_relative_to(path: Path, *other: Any) -> bool:
"""Return True if the path is relative to another path or False."""
try:
path.resolve().absolute().relative_to(*other)
return True
except ValueError:
return False


def normpath_path(path: str | BasePathLike) -> Path:
"""Normalize a path in order to provide a more consistent output.
- Any symlinks are resolved.
- Any paths outside the CWD are resolved to their absolute path.
- Any absolute path within current user home directory is compressed to
make use of '~', so it is easier to read and more portable.
"""
if not isinstance(path, Path):
path = Path(path)

is_relative = is_relative_to(path, path.cwd())
path = path.resolve()
if is_relative:
path = path.relative_to(path.cwd())

# Compress any absolute path within current user home directory
if path.is_absolute():
home = Path.home()
if is_relative_to(path, home):
path = Path("~") / path.relative_to(home)

return path


@contextmanager
def cwd(path: str | BasePathLike) -> Iterator[None]:
"""Context manager for temporary changing current working directory."""
Expand Down Expand Up @@ -133,6 +173,8 @@ class Lintable:
Providing file content when creating the object allow creation of in-memory
instances that do not need files to be present on disk.
When symlinks are given, they will always be resolved to their target.
"""

def __init__(
Expand All @@ -142,20 +184,23 @@ def __init__(
kind: FileType | None = None,
):
"""Create a Lintable instance."""
# Filename is effective file on disk, for stdin is a namedtempfile
self.filename: str = str(name)
self.dir: str = ""
self.kind: FileType | None = None
self.stop_processing = False # Set to stop other rules from running
self._data: Any = None
self.line_skips: dict[int, set[str]] = defaultdict(set)

if isinstance(name, str):
self.name = normpath(name)
self.path = Path(self.name)
else:
self.name = str(name)
self.path = name
name = Path(name)
is_relative = is_relative_to(name, str(name.cwd()))
name = name.resolve()
if is_relative:
name = name.relative_to(name.cwd())
name = normpath_path(name)
self.path = name
# Filename is effective file on disk, for stdin is a namedtempfile
self.name = self.filename = str(name)

self._content = self._original_content = content
self.updated = False

Expand Down Expand Up @@ -312,7 +357,11 @@ def data(self) -> Any:

# pylint: disable=redefined-outer-name
def discover_lintables(options: Namespace) -> dict[str, Any]:
"""Find all files that we know how to lint."""
"""Find all files that we know how to lint.
Return format is normalized, relative for stuff below cwd, ~/ for stuff
under current user, and absolete for everything else.
"""
# git is preferred as it also considers .gitignore
git_command_present = [
"git",
Expand Down
19 changes: 18 additions & 1 deletion test/test_file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
expand_paths_vars,
guess_project_dir,
normpath,
normpath_path,
)

from .conftest import cwd
Expand Down Expand Up @@ -200,7 +201,7 @@ def test_default_kinds(monkeypatch: MonkeyPatch, path: str, kind: FileType) -> N

# pylint: disable=unused-argument
def mockreturn(options: Namespace) -> dict[str, Any]:
return {path: kind}
return {normpath(path): kind}

# assert Lintable is able to determine file type
lintable_detected = Lintable(path)
Expand Down Expand Up @@ -411,3 +412,19 @@ def test_lintable_content_deleter(
assert tmp_updated_lintable.content == updated_content
del tmp_updated_lintable.content
assert tmp_updated_lintable.content == content


@pytest.mark.parametrize(
("path", "result"),
(
pytest.param("foo", "foo", id="rel"),
pytest.param(os.path.expanduser("~/xxx"), "~/xxx", id="rel-to-home"),
pytest.param("/a/b/c", "/a/b/c", id="absolute"),
pytest.param(
"examples/playbooks/roles", "examples/roles", id="resolve-symlink"
),
),
)
def test_normpath_path(path: str, result: str) -> None:
"""Tests behavior of normpath."""
assert normpath_path(path) == Path(result)

0 comments on commit 2d314b9

Please sign in to comment.