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.

Sign up for GitHub

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

fix: ignore-from-file relative to config file #701

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: ignore-from-file relative to config file #701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions docs/configuration.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ or:
ignore-from-file: [.gitignore, .yamlignore]

.. note:: However, this is mutually exclusive with the ``ignore`` key.
Also, this filepath is considered relative to the config file.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would find it clearer in a new note below. Also, we could avoid any misunderstandings like this:

Suggested change
Also, this filepath is considered relative to the config file.
.. note::
This file (e.g. ``.gitignore``) is searched relatively to the config file
(``.yamllint``). The paths of files to ignore are interpreted relatively to
the ignore file (e.g. ``.gitignore``).


If you need to know the exact list of files that yamllint would process,
without really linting them, you can use ``--list-files``:
Expand Down
91 changes: 91 additions & 0 deletions tests/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,29 @@ def test_ignore_from_file_not_exist(self):
config.YamlLintConfig, 'extends: default\n'
'ignore-from-file: not_found_file\n')

def test_ignore_from_file_exist(self):
with open(os.path.join(self.wd, '.gitignore'), 'w') as f:
f.write('*.dont-lint-me.yaml\n'
'/bin/\n'
'!/bin/*.lint-me-anyway.yaml\n')
parsed = config.YamlLintConfig('extends: default\n'
'ignore-from-file: .gitignore\n')
assert parsed.is_file_ignored("/bin/ignored") is True
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single quotes please 😉


def test_ignore_from_file_from_subdir_fails_without_config_file(self):
with open(os.path.join(self.wd, '.gitignore'), 'w') as f:
f.write('*.dont-lint-me.yaml\n'
'/bin/\n'
'!/bin/*.lint-me-anyway.yaml\n')
try:
os.chdir(os.path.join(self.wd, 'bin'))
self.assertRaises(
FileNotFoundError,
config.YamlLintConfig, 'extends: default\n'
'ignore-from-file: .gitignore\n')
finally:
os.chdir(self.wd)

def test_ignore_from_file_incorrect_type(self):
self.assertRaises(
YamlLintConfigError,
Expand Down Expand Up @@ -721,6 +744,74 @@ def test_run_with_ignore_from_file(self):
'./s/s/ign-trail/s/s/file2.lint-me-anyway.yaml:5:5: ' + hyphen,
)))

def test_run_in_subdir_with_ignore_from_file(self):
with open(os.path.join(self.wd, '.yamllint'), 'w') as f:
f.write('extends: default\n'
'ignore-from-file: .gitignore\n'
'rules:\n'
' key-duplicates:\n'
' ignore-from-file: .ignore-key-duplicates\n')

with open(os.path.join(self.wd, '.gitignore'), 'w') as f:
f.write('*.dont-lint-me.yaml\n'
'/bin/\n'
'!/bin/*.lint-me-anyway.yaml\n')

with open(os.path.join(self.wd, '.ignore-key-duplicates'), 'w') as f:
f.write('/ign-dup\n')

sys.stdout = StringIO()
try:
os.chdir(os.path.join(self.wd, 'bin'))
with self.assertRaises(SystemExit):
cli.run(('-f', 'parsable', '.'))
finally:
os.chdir(self.wd)

out = sys.stdout.getvalue()
out = '\n'.join(sorted(out.splitlines()))

keydup = '[error] duplication of key "key" in mapping (key-duplicates)'
trailing = '[error] trailing spaces (trailing-spaces)'
hyphen = '[error] too many spaces after hyphen (hyphens)'

self.assertEqual(out, '\n'.join((
'./file.lint-me-anyway.yaml:3:3: ' + keydup,
'./file.lint-me-anyway.yaml:4:17: ' + trailing,
'./file.lint-me-anyway.yaml:5:5: ' + hyphen,
)))

def test_run_in_subdir_with_ignore_from_file_in_subdir(self):
if os.path.exists(os.path.join(self.wd, '.gitignore')):
os.remove(os.path.join(self.wd, '.gitignore'))
Comment on lines +785 to +786
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can avoid this by deterministically cleaning .gitignore and .ignore-key-duplicates at the end of each test.

(see the global code snippet for tests in my main comment)

with open(os.path.join(self.wd, '.yamllint'), 'w') as f:
f.write('extends: default\n'
'ignore-from-file: .gitignore\n'
'rules:\n'
' key-duplicates:\n'
' ignore-from-file: .ignore-key-duplicates\n')

with open(os.path.join(self.wd, 'bin', '.gitignore'), 'w') as f:
f.write('*.dont-lint-me.yaml\n'
'/bin/\n'
'!/bin/*.lint-me-anyway.yaml\n')

with open(os.path.join(self.wd, '.ignore-key-duplicates'), 'w') as f:
f.write('/ign-dup\n')

sys.stdout = StringIO()
sys.stderr = StringIO()
try:
os.chdir(os.path.join(self.wd, 'bin'))
with self.assertRaises(FileNotFoundError):
cli.run(('-f', 'parsable', '.'))
finally:
os.chdir(self.wd)

out = sys.stdout.getvalue()

self.assertEqual(out, '')

def test_run_with_ignored_from_file(self):
with open(os.path.join(self.wd, '.yamllint'), 'w') as f:
f.write('ignore-from-file: [.gitignore, .yamlignore]\n'
Expand Down
20 changes: 18 additions & 2 deletions yamllint/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,24 @@ def __init__(self, content=None, file=None):
assert (content is None) ^ (file is None)

self.ignore = None
self.config_dir = None

self.yaml_files = pathspec.PathSpec.from_lines(
'gitwildmatch', ['*.yaml', '*.yml', '.yamllint'])

self.locale = None

if file is not None:
self.config_dir = os.path.dirname(file)
with open(file) as f:
content = f.read()

self.parse(content)
self.validate()

def is_file_ignored(self, filepath):
if self.config_dir:
filepath = os.path.relpath(filepath, start=self.config_dir)
return self.ignore and self.ignore.match_file(filepath)

def is_yaml_file(self, filepath):
Expand Down Expand Up @@ -109,6 +113,11 @@ def parse(self, raw_content):
raise YamlLintConfigError(
'invalid config: ignore-from-file should contain '
'filename(s), either as a list or string')
if self.config_dir is not None:
conf['ignore-from-file'] = [
f if os.path.basename(f) != f
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I don't understand this line: why should f be treated differently if it contains slashes?

  • To avoid bugs is it possible not to modify the conf object (nor conf['ignore-from-file']) if it's not strictly needed? E.g. create a local variable filepaths?

  • Maybe it's worth using os.path.relpath(filepath, start=self.config_dir) here? I'm not sure, I haven't tested. The advantage is that we wouldn't need the if self.config_dir is not None: above, because os.path.relpath() can be called with start=None or start=''.

else os.path.join(self.config_dir, f)
for f in conf['ignore-from-file']]
with fileinput.input(conf['ignore-from-file']) as f:
self.ignore = pathspec.PathSpec.from_lines('gitwildmatch', f)
elif 'ignore' in conf:
Expand Down Expand Up @@ -145,10 +154,12 @@ def validate(self):
except Exception as e:
raise YamlLintConfigError(f'invalid config: {e}') from e

self.rules[id] = validate_rule_conf(rule, self.rules[id])
self.rules[id] = validate_rule_conf(rule,
self.rules[id],
self.config_dir)


def validate_rule_conf(rule, conf):
def validate_rule_conf(rule, conf, config_dir=None):
if conf is False: # disable
return False

Expand All @@ -163,6 +174,11 @@ def validate_rule_conf(rule, conf):
raise YamlLintConfigError(
'invalid config: ignore-from-file should contain '
'valid filename(s), either as a list or string')
if config_dir is not None:
conf['ignore-from-file'] = [
f if os.path.basename(f) != f
else os.path.join(config_dir, f)
for f in conf['ignore-from-file']]
with fileinput.input(conf['ignore-from-file']) as f:
conf['ignore'] = pathspec.PathSpec.from_lines(
'gitwildmatch', f)
Expand Down
Loading