[enhancement] configure non-local-variables-should-be-uppercase to allow lower-case variables

[rikerfi]

It should be possible to configure non-local-variables-should-be-uppercase to allow lower-case variables in it.

*** Variables ***
@{PROTOCOLS}  TCP  UDP


*** Test Cases ***
Test
    [Documentation]  Doc.
    KW


*** Keywords ***
KW
    [Documentation]  non-local-variables-should-be-uppercase
    FOR  ${proto}  IN  @{PROTOCOLS}
        ${stuff}  Get Stuff  ${proto}
        # This is good, but confusing:
        Set Test Variable  ${EXAMPLE_${PROTO}}  ${stuff}
        # This could be allowed via configure:
        Set Test Variable  ${EXAMPLE_${proto}}  ${stuff}
    END

[bhirsz]

I agree. We need better mechanism for parsing the variables though - maybe steal something from the core Robot Framework. It's now always possible to recognize what is a variable, nested variable and what just text (Robot uses dynamic context to decide that) but it should be possible to cover basic cases (especially if we at least cache the local scope),

[mnojek]

We can check if the variable contains another variable in its name and allow it to be lower-case by default for this rule, because we can't really tell what's the value of the inserted variable, so we should not report an issue then.

For example, here:

Set Test Variable    ${EXAMPLE_${proto}}    ${stuff}

we don't know what's the value of ${proto}, so I think we should just check the rest of the name and ignore this from the check, by default. @bhirsz What do you think?

When it comes to the original request - I would allow for parametrizing this rule to allow lower-case characters, because it will not make much sense to allow lower-case characters in the rule named non-local-variables-should-be-uppercase.

From the other hand, we could create a new rule (like local-variables-case) and deprecate this one, but then it would go against the official RF User Guide, where it is several times emphasized that all Test, Suite and Global variables should be named with upper-case characters.

[bhirsz]

Well, we don't need to know value of ${proto} - we only need it know if ${proto} is local variable. Because we only care about what is actually written in the file (and how the dynamic variable name will look like is outside that).

In previous example we would want following code do not raise any issues:

 Set Test Variable  ${EXAMPLE_${proto}}  ${stuff}

Because stuff, proto are local variables and ${EXAMPLE_..} is not recognized so it should be global. It's actually quite challenging topic because normally first argument of any Set .. Variable need to be uppercase (since it's name of global variable) but the name itself is coming from local variable ${proto}.. .

[mnojek]

I don't know if I was properly understood, so I will try to be more specific.

I think that we can just detect if inside any non-local variable name, there is an element like ${...} (or starting with @ or &) and then exclude it from this variable's name, and only then check if it's uppercase or not. So we just ignore what's inside the nested variable, and of course we ignore what's the value of it (because we can't ensure we know it).

Example:

*** Variables ***
${PROTO}              0  # this is fine; checking whole name
${SUITE_VAR}          1  # this is fine; checking whole name
${SUITE_${var}}       2  # this is fine; checking only SUITE_
${SUITE_${var}_SOME}  3  # this is fine; checking only SUITE__SOME
${suite_${VAR}}       4  # this is NOT fine; checking only suite_

*** Test Cases ***
My Test Case
    Set Test Variable    ${EXAMPLE_${proto}}    1  # this is fine
    Set Test Variable    ${EXAMPLE_${PROTO}}    2  # this is fine
    Set Test Variable    ${example_${PROTO}}    3  # this is NOT fine

I think this would be the way we can improve this rule, and it would address the original issue.

[bhirsz]

Ah, ok. I agree. There is from robot.variables import VariableIterator that can be helpful in this case - for example given ${var_${variable}_var} it will yield pair of tuples with var_, ${variable} and _var so it's easy to distinguish variables from name.

[mnojek]

@rikerfi The change will be released as Robocop 3.1.0 soon.