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

Make all rule identifiers text based #1306

Merged
merged 1 commit into from Feb 6, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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