From 8673efdba632a4ceb1403aa5fb5088eb709e758e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Verg=C3=A9?= Date: Tue, 6 Feb 2024 17:31:51 +0100 Subject: [PATCH] cli: Cleanly skip broken symlinks that are ignored Before this commit, yamllint would output "[Errno 2] No such file or directory" when running on a directory which contained a broken symbolic link, even if the file is set to be ignored in yamllint configuration. This commit fixes that, and adds corresponding tests. As a side effect this changes `yamllint.linter.run(stream, config)`, so tools that would use this API need to filter ignored files beforehand. Fixes https://github.com/adrienverge/yamllint/issues/399 --- tests/common.py | 31 +++++++++++++++++++++++++++++++ tests/test_cli.py | 40 ++++++++++++---------------------------- tests/test_config.py | 32 +++++++++++++++++++++++++++++++- yamllint/cli.py | 6 +++--- yamllint/linter.py | 3 --- 5 files changed, 77 insertions(+), 35 deletions(-) diff --git a/tests/common.py b/tests/common.py index 1aef9e62..29dcfb9c 100644 --- a/tests/common.py +++ b/tests/common.py @@ -14,8 +14,10 @@ # along with this program. If not, see . import contextlib +from io import StringIO import os import shutil +import sys import tempfile import unittest @@ -54,6 +56,33 @@ def check(self, source, conf, **kwargs): self.assertEqual(real_problems, expected_problems) +class RunContext: + """Context manager for ``cli.run()`` to capture exit code and streams.""" + + def __init__(self, case): + self.stdout = self.stderr = None + self._raises_ctx = case.assertRaises(SystemExit) + + def __enter__(self): + self._raises_ctx.__enter__() + self.old_sys_stdout = sys.stdout + self.old_sys_stderr = sys.stderr + sys.stdout = self.outstream = StringIO() + sys.stderr = self.errstream = StringIO() + return self + + def __exit__(self, *exc_info): + self.stdout = self.outstream.getvalue() + self.stderr = self.errstream.getvalue() + sys.stdout = self.old_sys_stdout + sys.stderr = self.old_sys_stderr + return self._raises_ctx.__exit__(*exc_info) + + @property + def returncode(self): + return self._raises_ctx.exception.code + + def build_temp_workspace(files): tempdir = tempfile.mkdtemp(prefix='yamllint-tests-') @@ -64,6 +93,8 @@ def build_temp_workspace(files): if isinstance(content, list): os.mkdir(path) + elif isinstance(content, str) and content.startswith('symlink://'): + os.symlink(content[10:], path) else: mode = 'wb' if isinstance(content, bytes) else 'w' with open(path, mode) as f: diff --git a/tests/test_cli.py b/tests/test_cli.py index 34dbc5bd..8b817e44 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -23,38 +23,11 @@ import unittest from io import StringIO -from tests.common import build_temp_workspace, temp_workspace +from tests.common import build_temp_workspace, RunContext, temp_workspace from yamllint import cli, config -class RunContext: - """Context manager for ``cli.run()`` to capture exit code and streams.""" - - def __init__(self, case): - self.stdout = self.stderr = None - self._raises_ctx = case.assertRaises(SystemExit) - - def __enter__(self): - self._raises_ctx.__enter__() - self.old_sys_stdout = sys.stdout - self.old_sys_stderr = sys.stderr - sys.stdout = self.outstream = StringIO() - sys.stderr = self.errstream = StringIO() - return self - - def __exit__(self, *exc_info): - self.stdout = self.outstream.getvalue() - self.stderr = self.errstream.getvalue() - sys.stdout = self.old_sys_stdout - sys.stderr = self.old_sys_stderr - return self._raises_ctx.__exit__(*exc_info) - - @property - def returncode(self): - return self._raises_ctx.exception.code - - # Check system's UTF-8 availability def utf8_available(): try: @@ -112,6 +85,9 @@ def setUpClass(cls): 'key: other value\n', # empty dir 'empty-dir': [], + # symbolic link + 'symlinks/file-without-yaml-extension': '42\n', + 'symlinks/link.yaml': 'symlink://file-without-yaml-extension', # non-YAML file 'no-yaml.json': '---\n' 'key: value\n', @@ -152,6 +128,7 @@ def test_find_files_recursively(self): os.path.join(self.wd, 's/s/s/s/s/s/s/s/s/s/s/s/s/s/s/file.yaml'), os.path.join(self.wd, 'sub/directory.yaml/empty.yml'), os.path.join(self.wd, 'sub/ok.yaml'), + os.path.join(self.wd, 'symlinks/link.yaml'), os.path.join(self.wd, 'warn.yaml')], ) @@ -189,6 +166,7 @@ def test_find_files_recursively(self): os.path.join(self.wd, 'en.yaml'), os.path.join(self.wd, 's/s/s/s/s/s/s/s/s/s/s/s/s/s/s/file.yaml'), os.path.join(self.wd, 'sub/ok.yaml'), + os.path.join(self.wd, 'symlinks/link.yaml'), os.path.join(self.wd, 'warn.yaml')] ) @@ -226,6 +204,8 @@ def test_find_files_recursively(self): os.path.join(self.wd, 'sub/directory.yaml/empty.yml'), os.path.join(self.wd, 'sub/directory.yaml/not-yaml.txt'), os.path.join(self.wd, 'sub/ok.yaml'), + os.path.join(self.wd, 'symlinks/file-without-yaml-extension'), + os.path.join(self.wd, 'symlinks/link.yaml'), os.path.join(self.wd, 'warn.yaml')] ) @@ -247,6 +227,8 @@ def test_find_files_recursively(self): os.path.join(self.wd, 'sub/directory.yaml/empty.yml'), os.path.join(self.wd, 'sub/directory.yaml/not-yaml.txt'), os.path.join(self.wd, 'sub/ok.yaml'), + os.path.join(self.wd, 'symlinks/file-without-yaml-extension'), + os.path.join(self.wd, 'symlinks/link.yaml'), os.path.join(self.wd, 'warn.yaml')] ) @@ -711,6 +693,7 @@ def test_run_list_files(self): os.path.join(self.wd, 's/s/s/s/s/s/s/s/s/s/s/s/s/s/s/file.yaml'), os.path.join(self.wd, 'sub/directory.yaml/empty.yml'), os.path.join(self.wd, 'sub/ok.yaml'), + os.path.join(self.wd, 'symlinks/link.yaml'), os.path.join(self.wd, 'warn.yaml')] ) @@ -727,6 +710,7 @@ def test_run_list_files(self): os.path.join(self.wd, 's/s/s/s/s/s/s/s/s/s/s/s/s/s/s/file.yaml'), os.path.join(self.wd, 'sub/directory.yaml/not-yaml.txt'), os.path.join(self.wd, 'sub/ok.yaml'), + os.path.join(self.wd, 'symlinks/link.yaml'), os.path.join(self.wd, 'warn.yaml')] ) diff --git a/tests/test_config.py b/tests/test_config.py index 73f7c01e..eec2ade2 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -20,7 +20,7 @@ import unittest from io import StringIO -from tests.common import build_temp_workspace +from tests.common import build_temp_workspace, RunContext from yamllint import cli, config from yamllint.config import YamlLintConfigError @@ -773,3 +773,33 @@ def test_run_with_ignored_from_file(self): './s/s/ign-trail/s/s/file2.lint-me-anyway.yaml:4:17: ' + trailing, './s/s/ign-trail/s/s/file2.lint-me-anyway.yaml:5:5: ' + hyphen, ))) + + def test_run_with_ignore_with_broken_symlink(self): + wd = build_temp_workspace({ + 'file-without-yaml-extension': '42\n', + 'link.yaml': 'symlink://file-without-yaml-extension', + 'link-404.yaml': 'symlink://file-that-does-not-exist', + }) + backup_wd = os.getcwd() + os.chdir(wd) + + with RunContext(self) as ctx: + cli.run(('-f', 'parsable', '.')) + self.assertNotEqual(ctx.returncode, 0) + + with open(os.path.join(wd, '.yamllint'), 'w') as f: + f.write('extends: default\n' + 'ignore: |\n' + ' *404.yaml\n') + with RunContext(self) as ctx: + cli.run(('-f', 'parsable', '.')) + self.assertEqual(ctx.returncode, 0) + docstart = '[warning] missing document start "---" (document-start)' + out = '\n'.join(sorted(ctx.stdout.splitlines())) + self.assertEqual(out, '\n'.join(( + './.yamllint:1:1: ' + docstart, + './link.yaml:1:1: ' + docstart, + ))) + + os.chdir(backup_wd) + shutil.rmtree(wd) diff --git a/yamllint/cli.py b/yamllint/cli.py index 8d13000a..6fe43507 100644 --- a/yamllint/cli.py +++ b/yamllint/cli.py @@ -30,7 +30,8 @@ def find_files_recursively(items, conf): for root, _dirnames, filenames in os.walk(item): for f in filenames: filepath = os.path.join(root, f) - if conf.is_yaml_file(filepath): + if (conf.is_yaml_file(filepath) and + not conf.is_file_ignored(filepath)): yield filepath else: yield item @@ -209,8 +210,7 @@ def run(argv=None): if args.list_files: for file in find_files_recursively(args.files, conf): - if not conf.is_file_ignored(file): - print(file) + print(file) sys.exit(0) max_level = 0 diff --git a/yamllint/linter.py b/yamllint/linter.py index a2faa061..5d283f99 100644 --- a/yamllint/linter.py +++ b/yamllint/linter.py @@ -222,9 +222,6 @@ def run(input, conf, filepath=None): :param input: buffer, string or stream to read from :param conf: yamllint configuration object """ - if filepath is not None and conf.is_file_ignored(filepath): - return () - if isinstance(input, (bytes, str)): return _run(input, conf, filepath) elif isinstance(input, io.IOBase):