Ignore nested variables in rules 0309 and 0310

[mnojek]

Rules W0310 non-local-variables-should-be-uppercase and W0309 section-variable-not-uppercase
were previously reporting when the variable had another nested variable with lowercase name,
e.g. ${EXAMPLE_${lowercase}}.
Now, the nested variables are ignored and if the rest of the name is uppercase, the rules
will not report the issue anymore.

Closes #678

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (ec376dc) 96.88% compared to head (cbda1c0) 96.90%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #813      +/-   ##
==========================================
+ Coverage   96.88%   96.90%   +0.01%     
==========================================
  Files          25       25              
  Lines        3471     3484      +13     
==========================================
+ Hits         3363     3376      +13     
  Misses        108      108              

Impacted Files	Coverage Δ
robocop/config.py	94.69% <ø> (ø)
robocop/checkers/naming.py	98.83% <100.00%> (+0.02%)	⬆️
robocop/utils/misc.py	98.68% <100.00%> (+0.04%)	⬆️
robocop/version.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[bhirsz]

Create note in https://github.com/MarketSquare/robotframework-robocop/tree/master/docs/releasenotes to not lose the changes.

[mnojek]

So in RF v3.2.2, all these variables written with red text are not recognized as variables in Variables section. Is it expected?

[bhirsz]

So in RF v3.2.2, all these variables written with red text are not recognized as variables in Variables section. Is it expected?

Those are not proper variables - see if they have some errors in node? Variable needs to be left aligned and do not contain aby nested variables. Variable value can contain nested variables. So:

${VAR} ${nested${var}} is allowed

But

${nested${var}} value is not

[mnojek]

Those are not proper variables - see if they have some errors in node? Variable needs to be left aligned and do not contain aby nested variables. Variable value can contain nested variables. So:

${VAR} ${nested${var}} is allowed

But

${nested${var}} value is not

The nodes have the error Invalid variable name (...), but when I run this using robot, it works fine.

Can you point me to the place in User Guide where it says that such nested variables are not allowed? I couldn't find it...

[bhirsz]

Those are not proper variables - see if they have some errors in node? Variable needs to be left aligned and do not contain aby nested variables. Variable value can contain nested variables. So:
${VAR} ${nested${var}} is allowed
But
${nested${var}} value is not

The nodes have the error Invalid variable name (...), but when I run this using robot, it works fine.

Can you point me to the place in User Guide where it says that such nested variables are not allowed? I couldn't find it...

I had the same problem when working on RenameVariables in Robotidy and I have decided to just ignore nodes with errors - errors in variables are already handled by Robocop (just remember to use get_errors defined somewhere in Robocop repo, because RF3 have different logic for storing the errors).

The only mention in the user guide is:

(under Variable section)

Their main disadvantages are that values are always strings and they cannot be created dynamically

But it's about values, not variable names.. .

Edit. I have created test file:

*** Variables ***
${ITEM}    item2
${ITEM2}    value
${${ITEM2}}    value2

*** Test Cases ***
Test
    Log    ${ITEM}
    Log    ${ITEM2}
    Log    ${value}

If I run it, I will get:

[ ERROR ] Error in file 'nested_variable.robot' on line 4: Setting variable '${${ITEM2}}' failed: Invalid variable name '${${ITEM2}}'.

[mnojek]

@bhirsz Take a look at my example here:
robotframework/robotframework#4716

EDIT: If you run your code with robot --variable value:1 test.robot, it will work.

[bhirsz]

@bhirsz Take a look at my example here: robotframework/robotframework#4716

EDIT: If you run your code with robot --variable value:1 test.robot, it will work.

Hm, isn't it because "variable needs to exist to be used as part of the other variable name in *** Variables ***"? And --variable set variable globally so it does exist before parsing this file. Similar effect would be running the code with some set global variable / import variables. Tough subject but in our case I would say that everything in the variable name in *** Variables ***" needs to be upper case because we are 100% sure all variables needs to be global. So we can't ignore nested variables for such case:

*** Variables ***
${NAME}    ${VALUE}
${NAME_${SUBNAME}  value  # all those variables needs to be global, so they need to be upper case anyway

*** Keywords ***
    Keyword    ${UPPER_${lower}}  # here we cannot say, so it's safer to ignore nested

[mnojek]

@bhirsz Take a look at my example here: robotframework/robotframework#4716
EDIT: If you run your code with robot --variable value:1 test.robot, it will work.

Hm, isn't it because "variable needs to exist to be used as part of the other variable name in *** Variables ***"? And --variable set variable globally so it does exist before parsing this file. Similar effect would be running the code with some set global variable / import variables. Tough subject but in our case I would say that everything in the variable name in *** Variables ***" needs to be upper case because we are 100% sure all variables needs to be global. So we can't ignore nested variables for such case:

*** Variables ***
${NAME}    ${VALUE}
${NAME_${SUBNAME}  value  # all those variables needs to be global, so they need to be upper case anyway

*** Keywords ***
    Keyword    ${UPPER_${lower}}  # here we cannot say, so it's safer to ignore nested

Well, that makes sense! I will adjust the code.

I am still curious how RF should handle it. Let's see what Pekka responds.

[mnojek]

@bhirsz The PR is ready, please review when you have time.