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

Add name[casing] to identify wrongly capitalized task names #2274

Merged
merged 1 commit into from
Aug 9, 2022
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
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ jobs:
WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:TOX_PARALLEL_NO_SPINNER
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 658
PYTEST_REQPASS: 659

steps:
- name: Activate WSL1
Expand Down
6 changes: 6 additions & 0 deletions examples/playbooks/task-name-lowercase.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
- hosts: all
tasks:
- name: this task has a name is not correctly capitalized
ansible.builtin.command: echo "Hello World"
changed_when: false
6 changes: 3 additions & 3 deletions src/ansiblelint/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import yaml

from ansiblelint.config import DEFAULT_KINDS, PROFILES
from ansiblelint.config import DEFAULT_KINDS, DEFAULT_WARN_LIST, PROFILES
from ansiblelint.constants import (
CUSTOM_RULESDIR_ENVVAR,
DEFAULT_RULESDIR,
Expand Down Expand Up @@ -367,7 +367,7 @@ def get_cli_parser() -> argparse.ArgumentParser:
default=[],
action="append",
help="only warn about these rules, unless overridden in "
"config file defaults to 'experimental'",
f"config file. Current version default value is: {', '.join(DEFAULT_WARN_LIST)}",
)
parser.add_argument(
"--enable-list",
Expand Down Expand Up @@ -443,7 +443,7 @@ def merge_config(file_config: Dict[Any, Any], cli_config: Namespace) -> Namespac
"rulesdir": [],
"skip_list": [],
"tags": [],
"warn_list": ["experimental", "role-name"],
"warn_list": DEFAULT_WARN_LIST,
"mock_modules": [],
"mock_roles": [],
"enable_list": [],
Expand Down
2 changes: 2 additions & 0 deletions src/ansiblelint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

from ansiblelint.loaders import yaml_from_file

DEFAULT_WARN_LIST = ["experimental", "name[casing]", "role-name"]

DEFAULT_KINDS = [
# Do not sort this list, order matters.
{"jinja2": "**/*.j2"}, # jinja2 templates are not always parsable as something else
Expand Down
24 changes: 24 additions & 0 deletions src/ansiblelint/rules/name.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
## name

This rule identifies several problems related to task naming. This is important
because these names are the primary way to identify executed tasks on console,
logs or web interface. Their role is also to document what a task is supposed
to do.

- `name[missing]` - All tasks are required to have a name
- `name[casing]` - All task names should start with a capital letter

### Problematic code

```yaml
---
- ansible.builtin.command: touch /tmp/.placeholder
```

### Correct code

```yaml
---
- name: Create placeholder file
ansible.builtin.command: touch /tmp/.placeholder
```
28 changes: 25 additions & 3 deletions src/ansiblelint/rules/name.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@


class NameRule(AnsibleLintRule):
"""All tasks should be named."""
"""Rule for checking task names and their content."""

id = "name"
description = (
Expand All @@ -26,9 +26,20 @@ class NameRule(AnsibleLintRule):
def matchtask(
self, task: Dict[str, Any], file: "Optional[Lintable]" = None
) -> Union[bool, str, MatchError]:
if not task.get("name"):
name = task.get("name")
if not name:
return self.create_matcherror(
linenumber=task["__line__"], tag="name[missing]", filename=file
message="All tasks should be named.",
linenumber=task["__line__"],
tag="name[missing]",
filename=file,
)
if not name[0].isupper():
return self.create_matcherror(
message="All names should start with an uppercase letter.",
linenumber=task["__line__"],
tag="name[casing]",
filename=file,
)
return False

Expand All @@ -54,3 +65,14 @@ def test_file_negative() -> None:
bad_runner = Runner(failure, rules=collection)
errs = bad_runner.run()
assert len(errs) == 4

def test_rule_name_lowercase() -> None:
"""Negative test for a task that starts with lowercase."""
collection = RulesCollection()
collection.register(NameRule())
failure = "examples/playbooks/task-name-lowercase.yml"
bad_runner = Runner(failure, rules=collection)
errs = bad_runner.run()
assert len(errs) == 1
assert errs[0].tag == "name[casing]"
assert errs[0].rule.id == "name"