diff --git a/ansiblelater/candidate.py b/ansiblelater/candidate.py index 6aed0bb..3545d89 100644 --- a/ansiblelater/candidate.py +++ b/ansiblelater/candidate.py @@ -23,7 +23,8 @@ class Candidate: self.path = filename self.binary = False self.vault = False - self.filetype = type(self).__name__.lower() + self.filemeta = type(self).__name__.lower() + self.kind = type(self).__name__.lower() self.faulty = False self.config = settings.config self.settings = settings @@ -54,7 +55,7 @@ class Candidate: self.rules = SingleRules(self.config["rules"]["dir"]).rules for rule in self._filter_rules(): - if type(self).__name__.lower() not in rule.types: + if self.kind not in rule.types: continue result = rule.check(self, self.config) @@ -145,7 +146,7 @@ class Candidate: return rule_id def __repr__(self): - return f"{type(self).__name__.lower()} ({self.path})" + return f"{self.kind} ({self.path})" def __getitem__(self, item): return self.__dict__.get(item) @@ -177,7 +178,7 @@ class Task(RoleFile): def __init__(self, filename, settings={}, rules=[]): # noqa super().__init__(filename, settings, rules) - self.filetype = "tasks" + self.filemeta = "tasks" class Handler(RoleFile): @@ -185,7 +186,7 @@ class Handler(RoleFile): def __init__(self, filename, settings={}, rules=[]): # noqa super().__init__(filename, settings, rules) - self.filetype = "handlers" + self.filemeta = "handlers" class Vars(Candidate): diff --git a/ansiblelater/rules/CheckKeyOrder.py b/ansiblelater/rules/CheckKeyOrder.py new file mode 100644 index 0000000..581b6bb --- /dev/null +++ b/ansiblelater/rules/CheckKeyOrder.py @@ -0,0 +1,89 @@ +# Original code written by the authors of ansible-lint + +import functools + +from ansiblelater.rule import RuleBase + +SORTER_TASKS = ( + "name", + # "__module__", + # "action", + # "args", + None, # <-- None include all modules that not using action and * + # "when", + # "notify", + # "tags", + "block", + "rescue", + "always", +) + + +class CheckKeyOrder(RuleBase): + rid = "ANS129" + description = "Check optimized key order" + helptext = "{type} key order can be improved to `{sorted_keys}`" + types = ["playbook", "task", "handler"] + + def check(self, candidate, settings): + errors = [] + tasks, err = self.get_normalized_tasks(candidate, settings) + + if err: + return self.Result(candidate.path, err) + + for task in tasks: + is_sorted, keys = self._sort_keys(task.get("__raw_task__")) + if not is_sorted: + errors.append( + self.Error( + task["__line__"], + self.helptext.format(type="task", sorted_keys=", ".join(keys)), + ) + ) + + if candidate.kind == "playbook": + tasks, err = self.get_tasks(candidate, settings) + + if err: + return self.Result(candidate.path, err) + + for task in tasks: + is_sorted, keys = self._sort_keys(task) + if not is_sorted: + errors.append( + self.Error( + task["__line__"], + self.helptext.format(type="play", sorted_keys=", ".join(keys)), + ) + ) + + return self.Result(candidate.path, errors) + + @staticmethod + def _sort_keys(task): + if not task: + return True, [] + + keys = [str(key) for key in task if not key.startswith("_")] + sorted_keys = sorted(keys, key=functools.cmp_to_key(_task_property_sorter)) + + return (keys == sorted_keys), sorted_keys + + +def _task_property_sorter(property1, property2): + """Sort task properties based on SORTER.""" + v_1 = _get_property_sort_index(property1) + v_2 = _get_property_sort_index(property2) + return (v_1 > v_2) - (v_1 < v_2) + + +def _get_property_sort_index(name): + """Return the index of the property in the sorter.""" + a_index = -1 + for i, v in enumerate(SORTER_TASKS): + if v == name: + return i + if v is None: + a_index = i + return a_index diff --git a/ansiblelater/rules/CheckWhenFormat.py b/ansiblelater/rules/CheckWhenFormat.py index 2d1f2fc..11c10a1 100644 --- a/ansiblelater/rules/CheckWhenFormat.py +++ b/ansiblelater/rules/CheckWhenFormat.py @@ -5,8 +5,9 @@ class CheckWhenFormat(RuleBase): rid = "ANS122" description = "Don't use Jinja2 in when" helptext = ( - "`when` is a raw Jinja2 expression, redundant {{ }} " "should be removed from variable(s)" + "`when` is a raw Jinja2 expression, redundant `{{ }}` should be removed from variable(s)" ) + types = ["playbook", "task", "handler"] def check(self, candidate, settings): diff --git a/ansiblelater/utils/yamlhelper.py b/ansiblelater/utils/yamlhelper.py index 10a4b47..b1026bc 100644 --- a/ansiblelater/utils/yamlhelper.py +++ b/ansiblelater/utils/yamlhelper.py @@ -21,6 +21,7 @@ # THE SOFTWARE. import codecs +import copy import os from contextlib import suppress @@ -436,10 +437,10 @@ def normalize_task(task, filename, custom_modules=None): return normalized -def action_tasks(yaml, file): +def action_tasks(yaml, candidate): tasks = [] - if file["filetype"] in ["tasks", "handlers"]: - tasks = add_action_type(yaml, file["filetype"]) + if candidate.filemeta in ["tasks", "handlers"]: + tasks = add_action_type(yaml, candidate.filemeta) else: tasks.extend(extract_from_list(yaml, ["tasks", "handlers", "pre_tasks", "post_tasks"])) @@ -480,7 +481,12 @@ def extract_from_list(blocks, candidates): meta_data = dict(block) for key in delete_meta_keys: meta_data.pop(key, None) - results.extend(add_action_type(block[candidate], candidate, meta_data)) + + actions = add_action_type(block[candidate], candidate, meta_data) + for action in actions: + action["__raw_task__"] = copy.copy(block) + + results.extend(actions) elif block[candidate] is not None: raise RuntimeError( f"Key '{candidate}' defined, but bad value: '{block[candidate]!s}'" diff --git a/docs/content/included_rules/_index.md b/docs/content/included_rules/_index.md index 7dfe3d0..4b8a590 100644 --- a/docs/content/included_rules/_index.md +++ b/docs/content/included_rules/_index.md @@ -44,4 +44,5 @@ Reviews are useless without some rules to check against. `ansible-later` comes w | CheckChangedInWhen | ANS126 | Use handlers instead of `when: changed`. | | | CheckChangedInWhen | ANS127 | Deprecated bare variables in loops must not be used. | | | CheckFQCNBuiltin | ANS128 | Module actions should use full qualified collection names. | | +| CheckFQCNBuiltin | ANS129 | Check optimized playbook/tasks key order. | | | CheckDeprecated | ANS999 | Deprecated features of `ansible-later` should not be used. | |