Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jinja-variable-lower-case wrongly detects printed strings as variables #85

Open
tacerus opened this issue Dec 31, 2023 · 2 comments
Open
Labels
enhancement New feature or request

Comments

@tacerus
Copy link

tacerus commented Dec 31, 2023

Hi,

if printing strings inside of a {{ statement, the tool complains about jinja-variable-lower-case:

For example:

{{ ' trustGuestRxFilters=\'yes\'' if iftype == 'direct' else '' }}

Results in:

All variables should use lower case: '{{ variable }}': trustGuestRxFilters (jinja-variable-lower-case)

Albeit trustGuestRxFilters not being a variable, but a printed string. The only variable in this line is iftype, which is lower case.

I think it is wrongly assuming {{ to only contain variable names?

@gmuloc
Copy link
Collaborator

gmuloc commented Jan 2, 2024

Hello - thanks for opening the issue - the rule is using the following regex and is indeed expecting that it contains only a variable name

regex = re.compile(r"([a-zA-Z0-9-_\"']*[A-Z][a-zA-Z0-9-_\"']*)")

https://github.com/aristanetworks/j2lint/blob/devel/j2lint/rules/jinja_variable_name_case_rule.py#L27

The rule does not support inline ifs (https://jinja.palletsprojects.com/en/3.1.x/templates/#if-expression) as it was never required until now (read: our main project using j2lint never uses them).
The support for this may take some time to implement and will require a smarter get_jinja_variables function: https://github.com/aristanetworks/j2lint/blob/devel/j2lint/utils.py#L229

In the meantime, you can always add an ignore statement for j2lint for this specific rule at the end of this line.

@tacerus
Copy link
Author

tacerus commented Jan 7, 2024

Hi,

thanks for the input. I understand it's not directly relevant to the intended implementation of j2lint.

Based on your pointers, I came up with this for get_jinja_variables:

tacerus@98da74a

Whilst it works for my example (extracts iftype), it likely needs more consideration for different kinds of if-expressions as they might not always be just comparisons, and could theoretically be split over multiple lines.
Seems my request is indeed a bit tricky! I will ignore the rule for now, but if this could stay a feature request it would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants