Skip to content

Commit

Permalink
Replace numeric rule identifiers with text ones
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea committed Feb 6, 2021
1 parent ad22de1 commit 44c6222
Show file tree
Hide file tree
Showing 64 changed files with 237 additions and 208 deletions.
6 changes: 3 additions & 3 deletions .ansible-lint
Expand Up @@ -22,14 +22,14 @@ use_default_rules: true
# This makes linter to fully ignore rules/tags listed below
skip_list:
- skip_this_tag
- '401'
- git-latest

# Report only a subset of tags and fully ignore any others
# tags:
# - '206'
# - var-spacing

# This makes the linter display but not fail for rules/tags listed below:
warn_list:
- skip_this_tag
- '401'
- git-latest
- experimetal # experimental is included in the implicit list
50 changes: 14 additions & 36 deletions docs/rules.rst
Expand Up @@ -26,26 +26,11 @@ for each available rule, use the ``-T`` option.
The following shows the available tags in an example set of rules, and the
rules associated with each tag:

.. code-block:: console

$ ansible-lint -v -T
behaviour ['[503]']
bug ['[304]']
command-shell ['[305]', '[302]', '[304]', '[306]', '[301]', '[303]']
deprecations ['[105]', '[104]', '[103]', '[101]', '[102]']
experimental ['[208]']
formatting ['[104]', '[203]', '[201]', '[204]', '[206]', '[205]', '[202]']
idempotency ['[301]']
idiom ['[601]', '[602]']
metadata ['[701]', '[704]', '[703]', '[702]']
module ['[404]', '[401]', '[403]', '[402]']
oddity ['[501]']
readability ['[502]']
repeatability ['[401]', '[403]', '[402]']
resources ['[302]', '[303]']
safety ['[305]']
task ['[502]', '[503]', '[504]', '[501]']
.. command-output:: ansible-lint -v -T
:cwd: ..
:returncode: 0
:nostderr:

To run just the *idempotency* rules, for example, run the following:

Expand All @@ -56,20 +41,13 @@ To run just the *idempotency* rules, for example, run the following:
Excluding Rules
```````````````

To exclude rules from the available set of rules, use the ``-x SKIP_LIST``
To exclude rules using their identifiers or tags, use the ``-x SKIP_LIST``
option. For example, the following runs all of the rules except those with the
tags *readability* and *safety*:

.. code-block:: bash
$ ansible-lint -x readability,safety playbook.yml
It's also possible to skip specific rules by passing the rule ID. For example,
the following excludes rule *502*:
tags *formatting* and *metadata*:

.. code-block:: bash
$ ansible-lint -x 502 playbook.yml
$ ansible-lint -x formatting,metadata playbook.yml
Ignoring Rules
``````````````
Expand Down Expand Up @@ -101,19 +79,19 @@ a space-separated list.

.. code-block:: yaml
- name: this would typically fire GitHasVersionRule 401 and BecomeUserWithoutBecomeRule 501
become_user: alice # noqa 401 501
- name: this would typically fire git-latest and partial-become
become_user: alice # noqa git-latest partial-become
git: src=/path/to/git/repo dest=checkout
If the rule is line-based, ``# noqa [rule_id]`` must be at the end of the
particular line to be skipped

.. code-block:: yaml
- name: this would typically fire LineTooLongRule 204 and VariableHasSpacesRule 206
- name: this would typically fire LineTooLongRule 204 and var-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 206
dest: "{{dest_proj_path}}/foo.conf" # noqa var-spacing
It's also a good practice to comment the reasons why a task is being skipped.
Expand All @@ -128,15 +106,15 @@ works with the *command* or *shell* modules only. Examples:

.. code-block:: yaml
- name: this would typically fire CommandsInsteadOfArgumentRule 302
- name: this would typically fire deprecated-command-syntax
command: warn=no chmod 644 X
- name: this would typically fire CommandsInsteadOfModuleRule 303
- name: this would typically fire command-instead-of-module
command: git pull --rebase
args:
warn: False
- name: this would typically fire GitHasVersionRule 401
- name: this would typically fire git-latest
git: src=/path/to/git/repo dest=checkout
tags:
- skip_ansible_lint
33 changes: 14 additions & 19 deletions examples/playbooks/skiptasks.yml
Expand Up @@ -3,68 +3,63 @@

tasks:

- name: test 401
- name: test git-latest
action: git

- name: test 402
- name: test hg-latest
action: hg

- name: test 303
- name: test command-instead-of-module
command: git log
changed_when: False

- name: test 302
- name: test deprecated-command-syntax
command: creates=B chmod 644 A

- name: test invalid action (skip)
foo: bar
tags:
- skip_ansible_lint

- name: test 401 (skip)
- name: test git-latest (skip)
action: git
tags:
- skip_ansible_lint

- name: test 402 (skip)
- name: test hg-latest (skip)
action: hg
tags:
- skip_ansible_lint

- name: test 303 (skip)
- name: test command-instead-of-module (skip)
command: git log
tags:
- skip_ansible_lint

- name: test 302 (skip)
- name: test deprecated-command-syntax (skip)
command: chmod 644 A
tags:
- skip_ansible_lint

- name: test 401 (don't warn)
- name: test git-latest (don't warn)
command: git log
args:
warn: False
changed_when: False

- name: test 402 (don't warn)
- name: test hg-latest (don't warn)
command: chmod 644 A
args:
warn: False
creates: B

- name: test 402 (warn)
- name: test hg-latest (warn)
command: chmod 644 A
args:
warn: yes
creates: B

- name: test 401 (don't warn single line)
- name: test git-latest (don't warn single line)
command: warn=False chdir=/tmp/blah git log
changed_when: False

- name: test 402 (don't warn single line)
- name: test hg-latest (don't warn single line)
command: warn=no creates=B chmod 644 A

- name: test 402 (warn single line)
- name: test hg-latest (warn single line)
command: warn=yes creates=B chmod 644 A
2 changes: 1 addition & 1 deletion examples/playbooks/tasks/directory with spaces/main.yml
@@ -1,3 +1,3 @@
# this should generate: 502: All tasks should be named
# this should generate: unnamed-task: All tasks should be named
- assert:
fail_msg: tasks in directory with spaces included
2 changes: 1 addition & 1 deletion examples/playbooks/tasks/simpletask.yml
@@ -1,4 +1,4 @@
---
# 502: All tasks should be named
# unnamed-task: All tasks should be named
- assert:
fail_msg: foo
@@ -1,4 +1,4 @@
# Should raise 206 at lines 25, 28, 31
# Should raise var-spacing at lines 25, 28, 31
- hosts: all
tasks:
- name: good variable format
Expand Down Expand Up @@ -29,7 +29,7 @@
- name: bad variable format
debug:
msg: "{{bad_format }}"
- name: not a jinja variable # noqa 206
- name: not a jinja variable # noqa var-spacing
debug:
msg: "data = ${lookup{$local_part}lsearch{/etc/aliases}}"
- name: JSON inside jinja is valid
Expand Down
@@ -1,2 +1,2 @@
# this task is missing a name (502)
# this task is missing a name (unnamed-task)
- ping:
@@ -1,2 +1,2 @@
# this task is missing a name (502)
# this task is missing a name (unnamed-task)
- ping:
21 changes: 18 additions & 3 deletions src/ansiblelint/__main__.py
Expand Up @@ -35,8 +35,9 @@
from ansiblelint._prerun import check_ansible_presence, prepare_environment
from ansiblelint.app import App
from ansiblelint.color import console, console_options, console_stderr, reconfigure, render_yaml
from ansiblelint.config import options
from ansiblelint.config import options, used_old_tags
from ansiblelint.file_utils import cwd
from ansiblelint.skip_utils import normalize_tag
from ansiblelint.version import __version__

if TYPE_CHECKING:
Expand Down Expand Up @@ -84,6 +85,11 @@ def initialize_options(arguments: List[str]):
for k, v in new_options.__dict__.items():
setattr(options, k, v)

# rename deprecated ids/tags to newer names
options.tags = [normalize_tag(tag) for tag in options.tags]
options.skip_list = [normalize_tag(tag) for tag in options.skip_list]
options.warn_list = [normalize_tag(tag) for tag in options.warn_list]


def report_outcome(result: "LintResult", options, mark_as_success=False) -> int:
"""Display information about how to skip found rules.
Expand All @@ -106,13 +112,22 @@ def report_outcome(result: "LintResult", options, mark_as_success=False) -> int:
else:
warnings += 1

entries = []
for key in sorted(matched_rules.keys()):
if {key, *matched_rules[key].tags}.isdisjoint(options.warn_list):
msg += f" - '{key}' # {matched_rules[key].shortdesc}\n"
entries.append(f" - {key} # {matched_rules[key].shortdesc}\n")
for match in result.matches:
if "experimental" in match.rule.tags:
msg += " - experimental # all rules tagged as experimental\n"
entries.append(" - experimental # all rules tagged as experimental\n")
break
msg += "".join(sorted(entries))

for k, v in used_old_tags.items():
_logger.warning(
"Replaced deprecated tag '%s' with '%s' but it will become an "
"error in the future.",
k,
v)

if result.matches and not options.quiet:
console_stderr.print(
Expand Down
7 changes: 4 additions & 3 deletions src/ansiblelint/_internal/rules.py
Expand Up @@ -19,6 +19,7 @@ class BaseRule:
version_added: str = ""
severity: str = ""
link: str = ""
has_dynamic_tags: bool = False

def getmatches(self, file: "Lintable") -> List["MatchError"]:
"""Return all matches while ignoring exceptions."""
Expand Down Expand Up @@ -71,7 +72,7 @@ def __lt__(self, other: "BaseRule") -> bool:
class RuntimeErrorRule(BaseRule):
"""Used to identify errors."""

id = '999'
id = 'internal-error'
shortdesc = 'Unexpected internal error'
description = (
'This error can be caused by internal bugs but also by '
Expand All @@ -86,7 +87,7 @@ class RuntimeErrorRule(BaseRule):
class AnsibleParserErrorRule(BaseRule):
"""Used to mark errors received from Ansible."""

id = '998'
id = 'parser-error'
shortdesc = 'AnsibleParserError'
description = (
'Ansible parser fails, this usually indicate invalid file.')
Expand All @@ -98,7 +99,7 @@ class AnsibleParserErrorRule(BaseRule):
class LoadingFailureRule(BaseRule):
"""File loading failure."""

id = '901'
id = 'load-failure'
shortdesc = 'Failed to load or parse file'
description = 'Linter failed to process a YAML file, possible not an Ansible file.'
severity = 'VERY_HIGH'
Expand Down
4 changes: 4 additions & 0 deletions src/ansiblelint/config.py
@@ -1,5 +1,6 @@
"""Store configuration options as a singleton."""
from argparse import Namespace
from typing import Dict

DEFAULT_KINDS = [
# Do not sort this list, order matters.
Expand Down Expand Up @@ -41,3 +42,6 @@
mock_roles=[],
loop_var_prefix=None
)

# Used to store detected tag deprecations
used_old_tags: Dict[str, str] = {}
36 changes: 36 additions & 0 deletions src/ansiblelint/constants.py
Expand Up @@ -57,3 +57,39 @@ def main():
from collections import OrderedDict as odict # noqa: 401
except ImportError:
pass

# Deprecated tags/ids and their newer names
RENAMED_TAGS = {
'102': 'no-jinja-when',
'104': 'deprecated-bare-vars',
'105': 'deprecated-module',
'106': 'role-name',
'202': 'risky-octal',
'203': 'no-tabs',
'205': 'playbook-extension',
'206': 'var-spacing',
'207': 'no-jinja-nesting',
'208': 'risky-file-permissions',
'301': 'no-changed-when',
'302': 'deprecated-command-syntax',
'303': 'command-instead-of-module',
'304': 'inline-env-var',
'305': 'command-instead-of-shell',
'306': 'risky-shell-pipe',
'401': 'git-latest',
'402': 'hg-latest',
'403': 'package-latest',
'404': 'no-relative-paths',
'501': 'partial-become',
'502': 'unnamed-task',
'503': 'no-handler',
'504': 'deprecated-local-action',
'505': 'missing-import',
'601': 'literal-compare',
'602': 'empty-string-compare',
'701': 'meta-no-info',
'702': 'meta-no-tags',
'703': 'meta-incorrect',
'704': 'meta-video-links',
'911': 'syntax-check',
}
2 changes: 1 addition & 1 deletion src/ansiblelint/formatters/__init__.py
Expand Up @@ -118,7 +118,7 @@ def format(self, match: "MatchError") -> str:
col = ""
return (
f"::{level} file={file_path},line={line_num}{col},severity={severity}"
f"::E{rule_id} {violation_details}"
f"::{rule_id} {violation_details}"
)

@staticmethod
Expand Down

0 comments on commit 44c6222

Please sign in to comment.