Skip to content

Commit

Permalink
Changed rules to use docstring as shortdesc
Browse files Browse the repository at this point in the history
Simplifies rule by ensuring that we do use the docstring as their
short description. This also enables us to address most pylint
no-class-docstring excludes.
  • Loading branch information
ssbarnea committed Mar 14, 2022
1 parent c281c3a commit 0999256
Show file tree
Hide file tree
Showing 44 changed files with 93 additions and 69 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 0999256

Please sign in to comment.