Skip to content

Commit

Permalink
Refactor internal skipped_rules to use private prefix (#2449)
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea committed Sep 19, 2022
1 parent f132d11 commit 08501d9
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 21 deletions.
2 changes: 2 additions & 0 deletions src/ansiblelint/constants.py
Expand Up @@ -119,3 +119,5 @@ def main():
"always",
"rescue",
]

SKIPPED_RULES_KEY = "__skipped_rules__"
4 changes: 2 additions & 2 deletions src/ansiblelint/rules/__init__.py
Expand Up @@ -39,7 +39,7 @@
)
from ansiblelint.config import PROFILES, get_rule_config
from ansiblelint.config import options as default_options
from ansiblelint.constants import RULE_DOC_URL
from ansiblelint.constants import RULE_DOC_URL, SKIPPED_RULES_KEY
from ansiblelint.errors import MatchError
from ansiblelint.file_utils import Lintable, expand_paths_vars

Expand Down Expand Up @@ -243,7 +243,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
if play is None:
continue

if self.id in play.get("skipped_rules", ()):
if self.id in play.get(SKIPPED_RULES_KEY, ()):
continue

if "skip_ansible_lint" in play.get("tags", []):
Expand Down
4 changes: 2 additions & 2 deletions src/ansiblelint/rules/meta_incorrect.py
Expand Up @@ -4,13 +4,13 @@

from typing import TYPE_CHECKING

from ansiblelint.constants import SKIPPED_RULES_KEY
from ansiblelint.rules import AnsibleLintRule
from ansiblelint.utils import LINE_NUMBER_KEY

if TYPE_CHECKING:
from typing import Any

from ansiblelint.constants import odict
from ansiblelint.errors import MatchError
from ansiblelint.file_utils import Lintable

Expand Down Expand Up @@ -46,7 +46,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
for field, default in self.field_defaults:
value = galaxy_info.get(field, None)
if value and value == default:
if "meta-incorrect" in file.data.get("skipped_rules", []):
if "meta-incorrect" in file.data.get(SKIPPED_RULES_KEY, []):
continue
results.append(
self.create_matcherror(
Expand Down
9 changes: 5 additions & 4 deletions src/ansiblelint/rules/yaml_rule.py
Expand Up @@ -7,6 +7,7 @@

from yamllint.linter import run as run_yamllint

from ansiblelint.constants import SKIPPED_RULES_KEY
from ansiblelint.file_utils import Lintable
from ansiblelint.rules import AnsibleLintRule
from ansiblelint.utils import LINE_NUMBER_KEY
Expand Down Expand Up @@ -78,7 +79,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:

def _combine_skip_rules(data: Any) -> set[str]:
"""Return a consolidated list of skipped rules."""
result = set(data.get("skipped_rules", []))
result = set(data.get(SKIPPED_RULES_KEY, []))
tags = data.get("tags", [])
if tags and (
isinstance(tags, Iterable)
Expand All @@ -105,10 +106,10 @@ def _fetch_skips(data: Any, collector: dict[int, set[str]]) -> dict[int, set[str
entry
and hasattr(data, "get")
and LINE_NUMBER_KEY in entry
and "skipped_rules" in entry
and entry["skipped_rules"]
and SKIPPED_RULES_KEY in entry
and entry[SKIPPED_RULES_KEY]
):
collector[entry[LINE_NUMBER_KEY]].update(entry["skipped_rules"])
collector[entry[LINE_NUMBER_KEY]].update(entry[SKIPPED_RULES_KEY])
_fetch_skips(entry, collector)
return collector

Expand Down
15 changes: 11 additions & 4 deletions src/ansiblelint/skip_utils.py
Expand Up @@ -33,7 +33,12 @@
from ruamel.yaml.tokens import CommentToken

from ansiblelint.config import used_old_tags
from ansiblelint.constants import NESTED_TASK_KEYS, PLAYBOOK_TASK_KEYWORDS, RENAMED_TAGS
from ansiblelint.constants import (
NESTED_TASK_KEYS,
PLAYBOOK_TASK_KEYWORDS,
RENAMED_TAGS,
SKIPPED_RULES_KEY,
)
from ansiblelint.file_utils import Lintable

if TYPE_CHECKING:
Expand Down Expand Up @@ -133,14 +138,14 @@ def _append_skipped_rules( # noqa: max-complexity: 12
]:
# AnsibleMapping, dict
if hasattr(pyyaml_data, "get"):
pyyaml_data["skipped_rules"] = skipped_rules
pyyaml_data[SKIPPED_RULES_KEY] = skipped_rules
# AnsibleSequence, list
elif (
not isinstance(pyyaml_data, str)
and isinstance(pyyaml_data, collections.abc.Sequence)
and skipped_rules
):
pyyaml_data[0]["skipped_rules"] = skipped_rules
pyyaml_data[0][SKIPPED_RULES_KEY] = skipped_rules

return pyyaml_data

Expand Down Expand Up @@ -175,7 +180,9 @@ def _append_skipped_rules( # noqa: max-complexity: 12

if pyyaml_task.get("name") != ruamel_task.get("name"):
raise RuntimeError("Error in matching skip comment to a task")
pyyaml_task["skipped_rules"] = _get_rule_skips_from_yaml(ruamel_task, lintable)
pyyaml_task[SKIPPED_RULES_KEY] = _get_rule_skips_from_yaml(
ruamel_task, lintable
)

return pyyaml_data

Expand Down
9 changes: 7 additions & 2 deletions src/ansiblelint/utils.py
Expand Up @@ -55,7 +55,12 @@
)
from ansiblelint.app import get_app
from ansiblelint.config import options
from ansiblelint.constants import NESTED_TASK_KEYS, PLAYBOOK_TASK_KEYWORDS, FileType
from ansiblelint.constants import (
NESTED_TASK_KEYS,
PLAYBOOK_TASK_KEYWORDS,
SKIPPED_RULES_KEY,
FileType,
)
from ansiblelint.errors import MatchError
from ansiblelint.file_utils import Lintable, discover_lintables
from ansiblelint.skip_utils import is_nested_task
Expand Down Expand Up @@ -533,7 +538,7 @@ def _sanitize_task(task: dict[str, Any]) -> dict[str, Any]:
result = task.copy()
# task is an AnsibleMapping which inherits from OrderedDict, so we need
# to use `del` to remove unwanted keys.
for k in ["skipped_rules", FILENAME_KEY, LINE_NUMBER_KEY]:
for k in [SKIPPED_RULES_KEY, FILENAME_KEY, LINE_NUMBER_KEY]:
if k in result:
del result[k]
return result
Expand Down
10 changes: 7 additions & 3 deletions src/ansiblelint/yaml_utils.py
Expand Up @@ -33,7 +33,11 @@
from ruamel.yaml.tokens import CommentToken
from yamllint.config import YamlLintConfig

from ansiblelint.constants import NESTED_TASK_KEYS, PLAYBOOK_TASK_KEYWORDS
from ansiblelint.constants import (
NESTED_TASK_KEYS,
PLAYBOOK_TASK_KEYWORDS,
SKIPPED_RULES_KEY,
)
from ansiblelint.errors import MatchError
from ansiblelint.file_utils import Lintable
from ansiblelint.utils import get_action_tasks, normalize_task
Expand Down Expand Up @@ -127,7 +131,7 @@ def iter_tasks_in_file(
for raw_task in raw_tasks:
err: MatchError | None = None

skip_tags: list[str] = raw_task.get("skipped_rules", [])
skip_tags: list[str] = raw_task.get(SKIPPED_RULES_KEY, [])

try:
normalized_task = normalize_task(raw_task, str(lintable.path))
Expand Down Expand Up @@ -237,7 +241,7 @@ def _nested_items_path(
f"of type '{type(data_collection)}'"
)
for key, value in convert_data_collection_to_tuples():
if key in ("skipped_rules", "__file__", "__line__"):
if key in (SKIPPED_RULES_KEY, "__file__", "__line__"):
continue
yield key, value, parent_path
if isinstance(value, (dict, list)):
Expand Down
9 changes: 5 additions & 4 deletions test/test_skiputils.py
Expand Up @@ -6,6 +6,7 @@

import pytest

from ansiblelint.constants import SKIPPED_RULES_KEY
from ansiblelint.file_utils import Lintable
from ansiblelint.skip_utils import (
append_skipped_rules,
Expand Down Expand Up @@ -83,7 +84,7 @@ def test_playbook_noqa(default_text_runner: RunFromText) -> None:
"git": "src=/path/to/git/repo dest=checkout",
"__line__": 4,
"__file__": Path("examples/playbooks/noqa.yml"),
"skipped_rules": ["latest[git]", "partial-become"],
SKIPPED_RULES_KEY: ["latest[git]", "partial-become"],
}
],
"__line__": 2,
Expand Down Expand Up @@ -153,19 +154,19 @@ def test_playbook_noqa(default_text_runner: RunFromText) -> None:
"__file__": Path(
"examples/playbooks/noqa-nested.yml"
),
"skipped_rules": ["name[missing]"],
SKIPPED_RULES_KEY: ["name[missing]"],
}
],
"__line__": 6,
"__file__": Path(
"examples/playbooks/noqa-nested.yml"
),
"skipped_rules": ["name[missing]"],
SKIPPED_RULES_KEY: ["name[missing]"],
}
],
"__line__": 4,
"__file__": Path("examples/playbooks/noqa-nested.yml"),
"skipped_rules": ["name[missing]"],
SKIPPED_RULES_KEY: ["name[missing]"],
}
],
"__line__": 2,
Expand Down

0 comments on commit 08501d9

Please sign in to comment.