Skip to content

Commit

Permalink
Changed rules to use docstring as shortdesc (#1994)
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea authored Mar 14, 2022
1 parent c281c3a commit 911b186
Show file tree
Hide file tree
Showing 43 changed files with 92 additions and 65 deletions.
5 changes: 3 additions & 2 deletions docs/custom-rules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ An example rule using ``match`` is:
from ansiblelint.rules import AnsibleLintRule
class DeprecatedVariableRule(AnsibleLintRule):
"""Deprecated variable declarations."""
id = 'EXAMPLE002'
shortdesc = 'Deprecated variable declarations'
description = 'Check for lines that have old style ${var} ' + \
'declarations'
tags = { 'deprecations' }
Expand All @@ -57,8 +57,9 @@ An example rule using ``matchtask`` is:
from ansiblelint.file_utils import Lintable
class TaskHasTag(AnsibleLintRule):
"""Tasks must have tag."""
id = 'EXAMPLE001'
shortdesc = 'Tasks must have tag'
description = 'Tasks must have tag'
tags = ['productivity']
Expand Down
1 change: 0 additions & 1 deletion examples/rules/task_has_tag.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ class TaskHasTag(AnsibleLintRule):
"""Tasks must have tag."""

id = "EXAMPLE001"
shortdesc = "Tasks must have tag"
description = "Tasks must have tag"
tags = ["productivity", "tags"]

Expand Down
15 changes: 8 additions & 7 deletions src/ansiblelint/_internal/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,18 @@ class BaseRule:

id: str = ""
tags: List[str] = []
shortdesc: str = ""
description: str = ""
version_added: str = ""
severity: str = ""
link: str = ""
has_dynamic_tags: bool = False
needs_raw_task: bool = False

@property
def shortdesc(self) -> str:
"""Return the short description of the rule, basically the docstring."""
return self.__doc__ or ""

def getmatches(self, file: "Lintable") -> List["MatchError"]:
"""Return all matches while ignoring exceptions."""
matches = []
Expand Down Expand Up @@ -94,10 +98,9 @@ def __lt__(self, other: "BaseRule") -> bool:


class RuntimeErrorRule(BaseRule):
"""Used to identify errors."""
"""Unexpected internal error."""

id = "internal-error"
shortdesc = "Unexpected internal error"
description = (
"This error can be caused by internal bugs but also by "
"custom rules. Instead of just stopping linter we generate the errors and "
Expand All @@ -110,21 +113,19 @@ class RuntimeErrorRule(BaseRule):


class AnsibleParserErrorRule(BaseRule):
"""Used to mark errors received from Ansible."""
"""AnsibleParserError."""

id = "parser-error"
shortdesc = "AnsibleParserError"
description = "Ansible parser fails; this usually indicates an invalid file."
severity = "VERY_HIGH"
tags = ["core"]
version_added = "v5.0.0"


class LoadingFailureRule(BaseRule):
"""File loading failure."""
"""Failed to load or parse file."""

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"
tags = ["core"]
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/command_instead_of_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@


class CommandsInsteadOfModulesRule(AnsibleLintRule):
"""Using command rather than module."""

id = "command-instead-of-module"
shortdesc = "Using command rather than module"
description = (
"Executing a command when there is an Ansible module is generally a bad idea"
)
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/command_instead_of_shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,9 @@


class UseCommandInsteadOfShellRule(AnsibleLintRule):
"""Use shell only when shell functionality is required."""

id = "command-instead-of-shell"
shortdesc = "Use shell only when shell functionality is required"
description = (
"Shell should only be used when piping, redirecting "
"or chaining commands (and Ansible would be preferred "
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/deprecated_bare_vars.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@


class UsingBareVariablesIsDeprecatedRule(AnsibleLintRule):
"""Using bare variables is deprecated."""

id = "deprecated-bare-vars"
shortdesc = "Using bare variables is deprecated"
description = (
"Using bare variables is deprecated. Update your "
"playbooks so that the environment value uses the full variable "
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/deprecated_command_syntax.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@


class CommandsInsteadOfArgumentsRule(AnsibleLintRule):
"""Using command rather than an argument to e.g. file."""

id = "deprecated-command-syntax"
shortdesc = "Using command rather than an argument to e.g. file"
description = (
"Executing a command when there are arguments to modules "
"is generally a bad idea"
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/deprecated_local_action.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@


class TaskNoLocalAction(AnsibleLintRule):
"""Do not use 'local_action', use 'delegate_to: localhost'."""

id = "deprecated-local-action"
shortdesc = "Do not use 'local_action', use 'delegate_to: localhost'"
description = "Do not use ``local_action``, use ``delegate_to: localhost``"
severity = "MEDIUM"
tags = ["deprecations"]
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/deprecated_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@


class DeprecatedModuleRule(AnsibleLintRule):
"""Deprecated module."""

id = "deprecated-module"
shortdesc = "Deprecated module"
description = (
"These are deprecated modules, some modules are kept "
"temporarily for backwards compatibility but usage is discouraged. "
Expand Down
5 changes: 3 additions & 2 deletions src/ansiblelint/rules/empty_string_compare.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@


class ComparisonToEmptyStringRule(AnsibleLintRule):
"""Don't compare to empty string."""

id = "empty-string-compare"
shortdesc = "Don't compare to empty string"
description = (
'Use ``when: var|length > 0`` rather than ``when: var != ""`` (or '
'conversely ``when: var|length == 0`` rather than ``when: var == ""``)'
Expand Down Expand Up @@ -83,7 +84,7 @@ def test_rule_empty_string_compare_fail(rule_runner: RunFromText) -> None:
results = rule_runner.run_playbook(FAIL_PLAY)
assert len(results) == 2
for result in results:
assert result.message == ComparisonToEmptyStringRule.shortdesc
assert result.message == ComparisonToEmptyStringRule().shortdesc

@pytest.mark.parametrize(
"rule_runner", (ComparisonToEmptyStringRule,), indirect=["rule_runner"]
Expand Down
5 changes: 3 additions & 2 deletions src/ansiblelint/rules/fqcn_builtins.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,10 @@


class FQCNBuiltinsRule(AnsibleLintRule):
"""Use FQCN for builtin actions."""

id = "fqcn-builtins"
severity = "MEDIUM"
shortdesc = "Use FQCN for builtin actions"
description = (
"Check whether the long version starting with ``ansible.builtin`` "
"is used in the playbook"
Expand Down Expand Up @@ -124,7 +125,7 @@ def test_fqcn_builtin_fail(rule_runner: RunFromText) -> None:
results = rule_runner.run_playbook(FAIL_PLAY)
assert len(results) == 1
for result in results:
assert result.message == FQCNBuiltinsRule.shortdesc
assert result.message == FQCNBuiltinsRule().shortdesc

@pytest.mark.parametrize(
"rule_runner", (FQCNBuiltinsRule,), indirect=["rule_runner"]
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/git_latest.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@


class GitHasVersionRule(AnsibleLintRule):
"""Git checkouts must contain explicit version."""

id = "git-latest"
shortdesc = "Git checkouts must contain explicit version"
description = (
"All version control checkouts must point to "
"an explicit commit or tag, not just ``latest``"
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/hg_latest.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@


class MercurialHasRevisionRule(AnsibleLintRule):
"""Mercurial checkouts must contain explicit revision."""

id = "hg-latest"
shortdesc = "Mercurial checkouts must contain explicit revision"
description = (
"All version control checkouts must point to "
"an explicit commit or tag, not just ``latest``"
Expand Down
5 changes: 1 addition & 4 deletions src/ansiblelint/rules/ignore_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,9 @@


class IgnoreErrorsRule(AnsibleLintRule):
"""Describe and test the IgnoreErrorsRule."""
"""Use failed_when and specify error conditions instead of using ignore_errors."""

id = "ignore-errors"
shortdesc = (
"Use failed_when and specify error conditions instead of using ignore_errors"
)
description = (
"Instead of ignoring all errors, ignore the errors only when using ``{{ ansible_check_mode }}``, "
"register the errors using ``register``, "
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/inline_env_var.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@


class EnvVarsInCommandRule(AnsibleLintRule):
"""Command module does not accept setting environment variables inline."""

id = "inline-env-var"
shortdesc = "Command module does not accept setting environment variables inline"
description = (
"Use ``environment:`` to set environment variables "
"or use ``shell`` module which accepts both"
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/literal_compare.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@


class ComparisonToLiteralBoolRule(AnsibleLintRule):
"""Don't compare to literal True/False."""

id = "literal-compare"
shortdesc = "Don't compare to literal True/False"
description = (
"Use ``when: var`` rather than ``when: var == True`` "
"(or conversely ``when: not var``)"
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/meta_incorrect.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@


class MetaChangeFromDefaultRule(AnsibleLintRule):
"""meta/main.yml default values should be changed."""

id = "meta-incorrect"
shortdesc = "meta/main.yml default values should be changed"
field_defaults = [
("author", "your name"),
("description", "your description"),
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/meta_no_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,9 @@ def _galaxy_info_errors_itr(


class MetaMainHasInfoRule(AnsibleLintRule):
"""meta/main.yml should contain relevant info."""

id = "meta-no-info"
shortdesc = "meta/main.yml should contain relevant info"
str_info = META_STR_INFO
info = META_INFO
description = "meta/main.yml should contain: ``{}``".format(", ".join(info))
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/meta_no_tags.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@


class MetaTagValidRule(AnsibleLintRule):
"""Tags must contain lowercase letters and digits only."""

id = "meta-no-tags"
shortdesc = "Tags must contain lowercase letters and digits only"
description = (
"Tags must contain lowercase letters and digits only, "
"and ``galaxy_tags`` is expected to be a list"
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/meta_video_links.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@


class MetaVideoLinksRule(AnsibleLintRule):
"""meta/main.yml video_links should be formatted correctly."""

id = "meta-video-links"
shortdesc = "meta/main.yml video_links should be formatted correctly"
description = (
"Items in ``video_links`` in meta/main.yml should be "
"dictionaries, and contain only keys ``url`` and ``title``, "
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/no_changed_when.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@


class CommandHasChangesCheckRule(AnsibleLintRule):
"""Commands should not change things if nothing needs doing."""

id = "no-changed-when"
shortdesc = "Commands should not change things if nothing needs doing"
description = """
Tasks should tell Ansible when to return ``changed``, unless the task only reads
information. To do this, set ``changed_when``, use the ``creates`` or
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/no_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ def _changed_in_when(item: str) -> bool:


class UseHandlerRatherThanWhenChangedRule(AnsibleLintRule):
"""Tasks that run when changed should likely be handlers."""

id = "no-handler"
shortdesc = "Tasks that run when changed should likely be handlers"
description = (
"If a task has a ``when: result.changed`` setting, it is effectively "
"acting as a handler. You could use notify and move that task to "
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/no_jinja_nesting.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@


class NestedJinjaRule(AnsibleLintRule):
"""Nested jinja pattern."""

id = "no-jinja-nesting"
shortdesc = "Nested jinja pattern"
description = (
"There should not be any nested jinja pattern. "
"Example (bad): ``{{ list_one + {{ list_two | max }} }}``, "
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/no_jinja_when.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@


class NoFormattingInWhenRule(AnsibleLintRule):
"""No Jinja2 in when."""

id = "no-jinja-when"
shortdesc = "No Jinja2 in when"
description = (
"``when`` is a raw Jinja2 expression, remove redundant {{ }} from variable(s)."
)
Expand Down
3 changes: 1 addition & 2 deletions src/ansiblelint/rules/no_log_password.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@


class NoLogPasswordsRule(AnsibleLintRule):
"""Describe and test the NoLogPasswordsRule."""
"""Password should not be logged."."""

id = "no-log-password"
shortdesc = "password should not be logged."
description = (
"When passing password argument you should have no_log configured "
"to a non False value to avoid accidental leaking of secrets."
Expand Down
2 changes: 0 additions & 2 deletions src/ansiblelint/rules/no_loop_var_prefix.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ class RoleLoopVarPrefix(AnsibleLintRule):
"""Role loop_var should use configured prefix."""

id = "no-loop-var-prefix"
shortdesc = __doc__
link = (
"https://docs.ansible.com/ansible/latest/user_guide/"
"playbooks_loops.html#defining-inner-and-outer-variable-names-with-loop-var"
Expand All @@ -38,7 +37,6 @@ def matchplay(self, file: Lintable, data: "odict[str, Any]") -> List[MatchError]
if not options.loop_var_prefix:
return results
self.prefix = options.loop_var_prefix.format(role=toidentifier(file.role))
self.shortdesc = f"{self.__class__.shortdesc}: {self.prefix}"

if file.kind not in ("tasks", "handlers"):
return results
Expand Down
3 changes: 2 additions & 1 deletion src/ansiblelint/rules/no_relative_paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@


class RoleRelativePath(AnsibleLintRule):
"""Doesn't need a relative path in role."""

id = "no-relative-paths"
shortdesc = "Doesn't need a relative path in role"
description = (
"``copy`` and ``template`` do not need to use relative path for ``src``"
)
Expand Down
4 changes: 2 additions & 2 deletions src/ansiblelint/rules/no_same_owner.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@


class NoSameOwnerRule(AnsibleLintRule):
"""Owner should not be kept between different hosts."""

id = "no-same-owner"
shortdesc = "Owner should not be kept between different hosts"
description = """
Optional rule that highlights dangers of assuming that user/group on the remote
machines may not exist on ansible controller or vice versa. Owner and group
Expand Down Expand Up @@ -133,4 +133,4 @@ def test_no_same_owner_rule(
results = Runner(test_file, rules=default_rules_collection).run()
assert len(results) == failures
for result in results:
assert result.message == NoSameOwnerRule.shortdesc
assert result.message == NoSameOwnerRule().shortdesc
Loading

0 comments on commit 911b186

Please sign in to comment.