Modify VariableHasSpacesRule to check for spaces around filters

[nirmal-j-patel]

Modify VariableHasSpacesRule to check for spaces around the filter character |.

Addresses #2120

[ssbarnea]

I am personally inclined to prefer extending the already existing rule that checks for spaces around jinka2 vars to also cover filters instead of a new rule.

How hard it would be to adapt to existing rule?

[nirmal-j-patel]

@ssbarnea I agree with you that it makes sense to modify the existing rule. I am assuming you are talking about modifying this rule: https://github.com/ansible/ansible-lint/blob/main/src/ansiblelint/rules/var_spacing.py

If so, I believe using regex lookahead and lookbehind functionality can identify if there are spaces surrounding |. The following regex appears to work well: {{.*(?<=[^ \n\t])[|].*|.*[|](?=[^ \n\t]).*}}

I would like get your feedback on the following questions:

1. Is var_spacing.py the rule to be modified?
2. Do you think I should add experimental tag to this existing rule?
3. Should I use regex (because the current rule uses regex) or the current code to check if the rule is violated?

Thank you.

[ssbarnea]

Yes. As part of rule refactoring we will likely rename its IDs to jinja2-spacing or even rename the rule to jinja2 and add a spacing sub-rule id, so the report would look like jinja2[spacing].

No need to to worry about backwards compatibility as there is a place where we have aliases for old rule-id, that map to current ones, so we can avoid breaking config files.

@cognifloyd WDYT about his? I am asking you as it sounds as a good idea to keep the jinja2 stuff in a single place.

[ssbarnea]

@nirmal-j-patel Can you please update var-spacing rule and avoid creating a new rule?

[nirmal-j-patel]

@ssbarnea I have updated the pull request so that var-spacing rule is updated instead of creating a new rule. Do the proposed changes look acceptable? A couple actions are failing. I can try to fix the issues if the current approach seems good. Thank you.

[cognifloyd]

Combining them makes sense. Jinja formatting is analogous to the yaml rule, and the --write reformatting will do the same thing.

So, I like the idea of a jinja rule that searches for common jinja formatting issues like spacing.

[cognifloyd]

@nirmal-j-patel Yes, I think these changes are going in the right direction. Please continue :) and thank you!

[cidrblock]

agreeing with @cognifloyd here, IMO, makes for a much more readable task, TY