Add format checking for when lines

[greg-hellings]

Jinja2 should not be included in "when" lines. This produces a
warning in Ansible 2.3+

[willthames]

This seems great, can you fix failing tests?

[greg-hellings]

Ok, that should fix the failing test.

However, there was one limitation of the code that is exposed that I couldn't figure out how to address. A "when" clause can also be applied to a block, and it doesn't seem that blocks are properly exposed to the rules. Any chance you could expose blocks to a rule so that I could easily add testing for a block's "when" clause?

[greg-hellings]

I realized my id still referred to my local project where I developed this code. I've updated it to be ANSIBLE0019 instead of CINCH0001. The issue of not detecting issues on "blocks" still exists.

[willthames]

In terms of exposing blocks, you might have to do it a bit more manually through matchplay.

We do have other rules that use matchplay to see how that might work (and there are yet more in ansible-review - https://github.com/willthames/ansible-review/blob/master/lib/ansiblereview/examples/lint-rules)

[greg-hellings]

I'm not finding anything that's dealing directly with blocks except for where your parsing code is expanding them. If I have the following play in a playbook, do I just need to walk recursively down through the blocks until all child blocks are found, from within matchplay?

- hosts: all
  tasks:
    - name: test when with jinja2
      debug: msg=text
      when: "{{ false }}"
    - block:
        - name: no
          command: echo fail
      when: "{{ false }}"

[greg-hellings]

OK, so I have added some basic recursive work into the PR. Now I'm running into something that I think might be an issue with how the rest of the library parses blocks. I've seen in some other places that blocks don't seem to be parsed recursively, and my own example seems to verify that in this case, at least.

The following playbook example should generate 5 errors from my rule, however, the innermost task of the second-level block is not getting parsed into the matchtask list. Namely, the line when: "{{ another_failure }}" is never picked up, because that task never gets passed into matchtask. Is that something I'm doing wrong?

- hosts: all
  tasks:
    - name: test when with jinja2
      debug: msg=text
      when: "{{ false }}"
    - block:
        - name: no
          command: echo fail
        - block:
            - name: another no
              command: echo another fail
              when: "{{ another_failure }}"
          when: "{{ no_templating_should_be_here }}"
      when: "{{ false }}"

- hosts: all
  roles:
    - role: test
      when: "{{ '1' = '1' }}"

[greg-hellings]

@willthames Is there anything that needs doing here? Anything holding this up from being merged, or should I just close it out?

[willthames]

@greg-hellings sorry, I took my eye off the ball here - this looks good to go to me

[ssbarnea]

Thanks! I am sure this is going to explode number of linting errors on the next release. Still, I do find it useful as linter should be able to avoid most ansible warnings and thus is a very common one.