From 861610f1457cae0a049ab3ef3cda18660e0229f3 Mon Sep 17 00:00:00 2001 From: Kate Case Date: Fri, 7 Jun 2024 09:33:07 -0400 Subject: [PATCH 1/4] Add test for problematic case --- examples/playbooks/transform-no-free-form.transformed.yml | 4 ++++ examples/playbooks/transform-no-free-form.yml | 3 +++ test/test_transformer.py | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/examples/playbooks/transform-no-free-form.transformed.yml b/examples/playbooks/transform-no-free-form.transformed.yml index d9e238e249..d0fc8d1eb5 100644 --- a/examples/playbooks/transform-no-free-form.transformed.yml +++ b/examples/playbooks/transform-no-free-form.transformed.yml @@ -17,3 +17,7 @@ - name: Example task with usage for '=' as module params ansible.builtin.debug: msg: "'Hello there world'" + + - name: Task that has a non-debug string with spaces + ansible.builtin.set_fact: + foo: '"String with spaces"' diff --git a/examples/playbooks/transform-no-free-form.yml b/examples/playbooks/transform-no-free-form.yml index 54cb266b12..8ae63c6c6c 100644 --- a/examples/playbooks/transform-no-free-form.yml +++ b/examples/playbooks/transform-no-free-form.yml @@ -12,3 +12,6 @@ - name: Example task with usage for '=' as module params ansible.builtin.debug: msg='Hello there world' + + - name: Task that has a non-debug string with spaces + ansible.builtin.set_fact: foo="String with spaces" diff --git a/test/test_transformer.py b/test/test_transformer.py index 0767f4b7e1..6d11d0a7f1 100644 --- a/test/test_transformer.py +++ b/test/test_transformer.py @@ -143,7 +143,7 @@ def fixture_runner_result( ), pytest.param( "examples/playbooks/transform-no-free-form.yml", - 3, + 4, True, True, id="no_free_form_transform", From b2029db6131ac446576b66a81bdf41129178c4e0 Mon Sep 17 00:00:00 2001 From: Kate Case Date: Fri, 7 Jun 2024 13:27:01 -0400 Subject: [PATCH 2/4] Rework how inline is handled to be more general --- src/ansiblelint/rules/no_free_form.py | 47 +++++++++++++++++---------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/src/ansiblelint/rules/no_free_form.py b/src/ansiblelint/rules/no_free_form.py index 8b3a3884af..a0322d9bba 100644 --- a/src/ansiblelint/rules/no_free_form.py +++ b/src/ansiblelint/rules/no_free_form.py @@ -107,14 +107,36 @@ def filter_values( val: str, filter_key: str, filter_dict: dict[str, Any], - ) -> bool: - """Return True if module option is not present in the string.""" + ) -> str: + """Pull out key=value pairs from a string and set them in filter_dict. + + Returns unmatched strings. + """ if filter_key not in val: - return True + return val + + extra = "" + [k, v] = val.split(filter_key, 1) + if " " in k: + extra, k = k.rsplit(" ", 1) + + if v[0] in "\"'": + # Keep quoted strings together + quote = v[0] + _, v, remainder = v.split(quote, 2) + v = f"{quote}{v}{quote}" + else: + try: + v, remainder = v.split(" ", 1) + except ValueError: + remainder = "" - [k, v] = val.split(filter_key) filter_dict[k] = v - return False + + extra = " ".join( + (extra, filter_values(remainder, filter_key, filter_dict)), + ) + return extra.strip() if match.tag == "no-free-form": module_opts: dict[str, Any] = {} @@ -122,18 +144,9 @@ def filter_values( k, v = task.popitem(False) # identify module as key and process its value if len(k.split(".")) == 3 and isinstance(v, str): - # if it is a message - if "msg" in v: - filter_values(v, "=", module_opts) - else: - # Filter the module options and command - module_opts["cmd"] = " ".join( - [ - item - for item in v.split(" ") - if filter_values(item, "=", module_opts) - ], - ) + cmd = filter_values(v, "=", module_opts) + if cmd: + module_opts["cmd"] = cmd sorted_module_opts = {} for key in sorted( From c2f05102e7a506f232db3c407da0a57489d76f7a Mon Sep 17 00:00:00 2001 From: Kate Case Date: Fri, 7 Jun 2024 14:02:38 -0400 Subject: [PATCH 3/4] Transform incorrectly moves the empty line above the arguments The changed_when avoids this. This appears to be an unrelated issue, but can be worked around this way. --- examples/playbooks/transform-no-free-form.transformed.yml | 1 + examples/playbooks/transform-no-free-form.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/examples/playbooks/transform-no-free-form.transformed.yml b/examples/playbooks/transform-no-free-form.transformed.yml index d0fc8d1eb5..7cc5f0ad77 100644 --- a/examples/playbooks/transform-no-free-form.transformed.yml +++ b/examples/playbooks/transform-no-free-form.transformed.yml @@ -17,6 +17,7 @@ - name: Example task with usage for '=' as module params ansible.builtin.debug: msg: "'Hello there world'" + changed_when: false - name: Task that has a non-debug string with spaces ansible.builtin.set_fact: diff --git a/examples/playbooks/transform-no-free-form.yml b/examples/playbooks/transform-no-free-form.yml index 8ae63c6c6c..b651609dd2 100644 --- a/examples/playbooks/transform-no-free-form.yml +++ b/examples/playbooks/transform-no-free-form.yml @@ -12,6 +12,7 @@ - name: Example task with usage for '=' as module params ansible.builtin.debug: msg='Hello there world' + changed_when: false - name: Task that has a non-debug string with spaces ansible.builtin.set_fact: foo="String with spaces" From 190942f531395b5fdc106e4636bb372686c2306d Mon Sep 17 00:00:00 2001 From: Kate Case Date: Mon, 10 Jun 2024 09:36:58 -0400 Subject: [PATCH 4/4] args in free-form syntax can come before or after implicit cmd --- examples/playbooks/transform-no-free-form.transformed.yml | 6 ++++++ examples/playbooks/transform-no-free-form.yml | 4 ++++ src/ansiblelint/rules/no_free_form.py | 2 +- test/test_transformer.py | 2 +- 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/examples/playbooks/transform-no-free-form.transformed.yml b/examples/playbooks/transform-no-free-form.transformed.yml index 7cc5f0ad77..e947c345af 100644 --- a/examples/playbooks/transform-no-free-form.transformed.yml +++ b/examples/playbooks/transform-no-free-form.transformed.yml @@ -8,6 +8,12 @@ cmd: touch foo changed_when: false + - name: Create a placefolder file + ansible.builtin.command: # <-- command can also go first + chdir: /tmp + cmd: touch bar + changed_when: false + - name: Use raw to echo ansible.builtin.raw: echo foo # <-- don't use executable= args: diff --git a/examples/playbooks/transform-no-free-form.yml b/examples/playbooks/transform-no-free-form.yml index b651609dd2..c57da0ce3e 100644 --- a/examples/playbooks/transform-no-free-form.yml +++ b/examples/playbooks/transform-no-free-form.yml @@ -6,6 +6,10 @@ ansible.builtin.command: chdir=/tmp touch foo # <-- don't use shorthand changed_when: false + - name: Create a placefolder file + ansible.builtin.command: touch bar chdir=/tmp # <-- command can also go first + changed_when: false + - name: Use raw to echo ansible.builtin.raw: executable=/bin/bash echo foo # <-- don't use executable= changed_when: false diff --git a/src/ansiblelint/rules/no_free_form.py b/src/ansiblelint/rules/no_free_form.py index a0322d9bba..13489efa3d 100644 --- a/src/ansiblelint/rules/no_free_form.py +++ b/src/ansiblelint/rules/no_free_form.py @@ -80,7 +80,7 @@ def matchtask( "win_command", "win_shell", ): - if self.cmd_shell_re.match(action_value): + if self.cmd_shell_re.search(action_value): fail = True else: fail = True diff --git a/test/test_transformer.py b/test/test_transformer.py index 6d11d0a7f1..51e97d5f4a 100644 --- a/test/test_transformer.py +++ b/test/test_transformer.py @@ -143,7 +143,7 @@ def fixture_runner_result( ), pytest.param( "examples/playbooks/transform-no-free-form.yml", - 4, + 5, True, True, id="no_free_form_transform",