Skip to content

Commit

Permalink
Allow noqa comments without colons (#2971
Browse files Browse the repository at this point in the history
Fixes: #2970
  • Loading branch information
ssbarnea authored Feb 2, 2023
1 parent 7ec6d6b commit 0206ddf
Show file tree
Hide file tree
Showing 14 changed files with 45 additions and 44 deletions.
16 changes: 8 additions & 8 deletions docs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,25 +160,25 @@ playbook. In cases like this, Ansible-lint can incorrectly trigger rule
violations.

To disable rule violations for specific tasks, and mute false positives, add
`# noqa [rule_id]` to the end of the line. It is best practice to add a comment
`# noqa: [rule_id]` to the end of the line. It is best practice to add a comment
that explains why rules are disabled.

You can add the `# noqa [rule_id]` comment to the end of any line in a task. You
can also skip multiple rules with a space-separated list.
You can add the `# noqa: [rule_id]` comment to the end of any line in a task.
You can also skip multiple rules with a space-separated list.

```yaml
- name: This task would typically fire git-latest and partial-become rules
become_user: alice # noqa git-latest partial-become
become_user: alice # noqa: git-latest partial-become
ansible.builtin.git: src=/path/to/git/repo dest=checkout
```
If the rule is line-based, `# noqa [rule_id]` must be at the end of the line.
If the rule is line-based, `# noqa: [rule_id]` must be at the end of the line.

```yaml
- name: This would typically fire LineTooLongRule 204 and jinja[spacing]
- name: This would typically fire jinja[spacing]
get_url:
url: http://example.com/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/file.conf # noqa 204
dest: "{{dest_proj_path}}/foo.conf" # noqa jinja[spacing]
url: http://example.com/file.conf
dest: "{{dest_proj_path}}/foo.conf" # noqa: jinja[spacing]
```

If you want Ansible-lint to skip a rule entirely, use the `-x` command line
Expand Down
2 changes: 1 addition & 1 deletion examples/playbooks/noqa-nested.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
block:
- name: 2nd level
block:
- ansible.builtin.debug: # noqa unnamed-task
- ansible.builtin.debug: # noqa: unnamed-task
msg: "test unnamed task in block"
2 changes: 1 addition & 1 deletion examples/playbooks/noqa.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
- hosts: localhost
tasks:
- name: This would typically fire latest[git] and partial-become
become_user: alice # noqa latest[git] partial-become
become_user: alice # noqa: latest[git] partial-become
git: src=/path/to/git/repo dest=checkout
2 changes: 1 addition & 1 deletion examples/playbooks/run-once-fail.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
strategy: free
gather_facts: false
tasks:
# - name: Task with run_once # noqa run-once[task] (Corrected code example)
# - name: Task with run_once # noqa: run-once[task] (Corrected code example)
- name: Task with run_once
ansible.builtin.debug:
msg: "Test"
Expand Down
10 changes: 5 additions & 5 deletions examples/playbooks/test_skip_inside_yaml.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
- skip_ansible_lint # should disable error at playbook level
tasks:
- name: Test # <-- 0 latest[hg]
action: ansible.builtin.hg # noqa fqcn[canonical]
- name: Test latest[hg] (skipped) # noqa latest[hg] fqcn[canonical]
action: ansible.builtin.hg # noqa: fqcn[canonical]
- name: Test latest[hg] (skipped) # noqa: latest[hg] fqcn[canonical]
action: ansible.builtin.hg

- name: Test latest[git] and partial-become
action: ansible.builtin.git
become_user: alice
- name: Test latest[git] and partial-become (skipped) # noqa latest[git] partial-become
- name: Test latest[git] and partial-become (skipped) # noqa: latest[git] partial-become
action: ansible.builtin.git
become_user: alice

Expand All @@ -24,8 +24,8 @@
- name: Test YAML and jinja[spacing] (skipped)
ansible.builtin.get_url:
# noqa: risky-file-permissions
url: http://example.com/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/file.conf # noqa yaml[line-length]
dest: "{{dest_proj_path}}/foo.conf" # noqa jinja[spacing]
url: http://example.com/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/file.conf # noqa: yaml[line-length]
dest: "{{dest_proj_path}}/foo.conf" # noqa: jinja[spacing]

- name: Test deprecated-command-syntax # <-- 3 deprecated-command-syntax
ansible.builtin.command: creates=B chmod 644 A # noqa: no-free-form
Expand Down
4 changes: 2 additions & 2 deletions examples/roles/meta_noqa/meta/main.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
galaxy_info: # noqa meta-incorrect
galaxy_info: # noqa: meta-incorrect
standalone: true
author: your-name # noqa meta-no-info
author: your-name # noqa: meta-no-info
description: missing min_ansible_version and platforms. author default not changed
license: MIT
min_ansible_version: "2.10"
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/rules/key_order.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def matchtask(
msg: "Debug without a name"
- name: Flush handlers
meta: flush_handlers
- no_log: true # noqa key-order
- no_log: true # noqa: key-order
shell: echo hello
name: Task with no_log on top
"""
Expand Down
5 changes: 3 additions & 2 deletions src/ansiblelint/skip_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import collections.abc
import logging
import re
from functools import lru_cache
from itertools import product
from typing import TYPE_CHECKING, Any, Generator, Sequence
Expand All @@ -47,6 +48,7 @@

_logger = logging.getLogger(__name__)
_found_deprecated_tags: set[str] = set()
_noqa_comment_re = re.compile(r"^# noqa(\s|:)")

# playbook: Sequence currently expects only instances of one of the two
# classes below but we should consider avoiding this chimera.
Expand Down Expand Up @@ -242,13 +244,12 @@ def traverse_yaml(obj: Any) -> None:
for v in entry:
if isinstance(v, CommentToken):
comment_str = v.value
if comment_str.startswith("# noqa:"):
if _noqa_comment_re.match(comment_str):
line = v.start_mark.line + 1 # ruamel line numbers start at 0
# column = v.start_mark.column + 1 # ruamel column numbers start at 0
lintable.line_skips[line].update(
get_rule_skips_from_line(comment_str.strip())
)

yaml_comment_obj_strings.append(str(obj.ca.items))
if isinstance(obj, dict):
for _, val in obj.items():
Expand Down
2 changes: 1 addition & 1 deletion test/rules/test_line_too_long.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
- name: Task example
debug:
msg: 'This is a very long text that is used in order to verify the rule that checks for very long lines. We do hope it was long enough to go over the line limit.'
""" # noqa 501
""" # noqa: E501


def test_long_line() -> None:
Expand Down
2 changes: 1 addition & 1 deletion test/test_import_include_role.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
ROLE_TASKS_MAIN = """\
---
- name: Shell instead of command
shell: echo hello world # noqa fqcn no-free-form
shell: echo hello world # noqa: fqcn no-free-form
changed_when: false
"""

Expand Down
2 changes: 1 addition & 1 deletion test/test_skip_import_playbook.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
hosts: all
tasks:
- name: Should be shell # noqa command-instead-of-shell no-changed-when no-free-form
- name: Should be shell # noqa: command-instead-of-shell no-changed-when no-free-form
ansible.builtin.shell: echo lol
- name: Should not be imported
Expand Down
12 changes: 6 additions & 6 deletions test/test_skip_inside_yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,29 @@
---
- ansible.builtin.debug:
msg: this should fail linting due lack of name
- ansible.builtin.debug: # noqa unnamed-task
- ansible.builtin.debug: # noqa: unnamed-task
msg: this should pass due to noqa comment
"""

ROLE_TASKS_WITH_BLOCK = """\
---
- name: Bad git 1 # noqa latest[git]
- name: Bad git 1 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 2
action: ansible.builtin.git a=b c=d
- name: Block with rescue and always section
block:
- name: Bad git 3 # noqa latest[git]
- name: Bad git 3 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 4
action: ansible.builtin.git a=b c=d
rescue:
- name: Bad git 5 # noqa latest[git]
- name: Bad git 5 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 6
action: ansible.builtin.git a=b c=d
always:
- name: Bad git 7 # noqa latest[git]
- name: Bad git 7 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 8
action: ansible.builtin.git a=b c=d
Expand All @@ -55,7 +55,7 @@ def test_role_tasks_with_block(default_text_runner: RunFromText) -> None:

@pytest.mark.parametrize(
("lintable", "expected"),
(pytest.param("examples/playbooks/test_skip_inside_yaml.yml", 11, id="yaml"),),
(pytest.param("examples/playbooks/test_skip_inside_yaml.yml", 10, id="yaml"),),
)
def test_inline_skips(
default_rules_collection: RulesCollection, lintable: str, expected: int
Expand Down
24 changes: 12 additions & 12 deletions test/test_skip_playbook_items.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
- name: Fixture
hosts: all
tasks:
- name: Bad git 1 # noqa latest[git]
- name: Bad git 1 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 2
action: ansible.builtin.git a=b c=d
pre_tasks:
- name: Bad git 3 # noqa latest[git]
- name: Bad git 3 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 4
action: ansible.builtin.git a=b c=d
Expand All @@ -24,12 +24,12 @@
- name: Fixture
hosts: all
tasks:
- name: Bad git 1 # noqa latest[git]
- name: Bad git 1 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 2
action: ansible.builtin.git a=b c=d
post_tasks:
- name: Bad git 3 # noqa latest[git]
- name: Bad git 3 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 4
action: ansible.builtin.git a=b c=d
Expand All @@ -40,12 +40,12 @@
- name: Fixture
hosts: all
tasks:
- name: Bad git 1 # noqa latest[git]
- name: Bad git 1 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 2
action: ansible.builtin.git a=b c=d
handlers:
- name: Bad git 3 # noqa latest[git]
- name: Bad git 3 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 4
action: ansible.builtin.git a=b c=d
Expand All @@ -56,15 +56,15 @@
- name: Fixture
hosts: all
tasks:
- name: Bad git 1 # noqa latest[git]
- name: Bad git 1 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 2
action: ansible.builtin.git a=b c=d
- name: Fixture 2
hosts: all
tasks:
- name: Bad git 3 # noqa latest[git]
- name: Bad git 3 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 4
action: ansible.builtin.git a=b c=d
Expand All @@ -75,23 +75,23 @@
- name: Fixture
hosts: all
tasks:
- name: Bad git 1 # noqa latest[git]
- name: Bad git 1 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 2
action: ansible.builtin.git a=b c=d
- name: Block with rescue and always section
block:
- name: Bad git 3 # noqa latest[git]
- name: Bad git 3 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 4
action: ansible.builtin.git a=b c=d
rescue:
- name: Bad git 5 # noqa latest[git]
- name: Bad git 5 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 6
action: ansible.builtin.git a=b c=d
always:
- name: Bad git 7 # noqa latest[git]
- name: Bad git 7 # noqa: latest[git]
action: ansible.builtin.git a=b c=d
- name: Bad git 8
action: ansible.builtin.git a=b c=d
Expand Down
4 changes: 2 additions & 2 deletions test/test_skiputils.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@
- name: Fixture
hosts: all
vars:
SOME_VAR_NOQA: "Foo" # noqa var-naming
SOME_VAR_NOQA: "Foo" # noqa: var-naming
SOME_VAR: "Bar"
tasks:
- name: "Set the SOME_OTHER_VAR"
ansible.builtin.set_fact:
SOME_OTHER_VAR_NOQA: "Baz" # noqa var-naming
SOME_OTHER_VAR_NOQA: "Baz" # noqa: var-naming
SOME_OTHER_VAR: "Bat"
"""

Expand Down

0 comments on commit 0206ddf

Please sign in to comment.