Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that symlinked arguments are resolved #2476

Merged
merged 1 commit into from
Sep 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
64 changes: 56 additions & 8 deletions src/ansiblelint/file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,51 @@ def normpath(path: str | BasePathLike) -> str:
# conversion to string in order to allow receiving non string objects
relpath = os.path.relpath(str(path))
path_absolute = os.path.abspath(str(path))
if path_absolute.startswith(os.getcwd()):
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 +172,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 +183,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 +356,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 content
under current user and absolute 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)