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

Embedded keywords with variable are treated as not capitalised #238

Closed
MaciejWiczk opened this issue Nov 17, 2020 · 11 comments
Closed

Embedded keywords with variable are treated as not capitalised #238

MaciejWiczk opened this issue Nov 17, 2020 · 11 comments
Assignees
Labels
bug Something isn't working context reading This feature needs context reading

Comments

@MaciejWiczk
Copy link

MaciejWiczk commented Nov 17, 2020

Describe the bug
Embedded keywords with variable are treated as not capitalised, while IMHO they are fine formatted.

To Reproduce
Run robocop against file containing following code

*** Variables ***
${variable}    does not matter

*** Test Cases ***
Test Capitalised
    Embedded Keyword With ${variable}

*** Keywords ***
Embedded Keyword With ${variable}
    Log    ${variable}

Results in [W] 0302 Keyword name should be capitalized message form Robocop

Expected behavior
Since common approach to variables is to have them lowercased or uppercase, for global variables/Constans, rule should ignore embedded variable casing.

@bhirsz
Copy link
Member

bhirsz commented Nov 17, 2020

What robocop version are you using? This should work fine with actual in pip and its definietly working with new version 1.2.0 (which is going to be released to pip very soon). See this acceptance test:

. The output of the test does not have anything for this line : https://github.com/MarketSquare/robotframework-robocop/blob/master/tests/atest/rules/not-capitalized-keyword-name/expected_output.txt

@MaciejWiczk
Copy link
Author

@bhirsz a bit older, 1.1.0 I believe. Thanks!:)

@MaciejWiczk
Copy link
Author

MaciejWiczk commented Nov 18, 2020

@bhirsz how about embedded keywords without variables? I have such calls in tests:
Library scope Should Be scope: GLOBAL
With keyword defined like this:

Library ${field} Should Be ${value}
    Wait Until Element Is Visible    ${detail_view_library_${field}}    0.5s
    Element Text Should Be    ${detail_view_library_${field}}    ${value}

And Keyword capitalization warning is triggered. Checked on 1.2.0

@bhirsz
Copy link
Member

bhirsz commented Nov 19, 2020

We're lacking this feature #195 . Since we don't have robot context we cannot resolve keyword names with their definitions (and discover what is embedded keyword and what not). This also affects other rules. I will set it as high priority because it will be also enabler for many new rules (such as variable naming depending of scope, keyword usage etc).

@bhirsz
Copy link
Member

bhirsz commented Nov 19, 2020

In meantime if it's not often occurence and you want to get rid of warnings you can use disabler:

Library scope Should Be scope: GLOBAL  # robocop: disable

or

Library scope Should Be scope: GLOBAL  # robocop: disable=0302

@bhirsz bhirsz self-assigned this Nov 19, 2020
@bhirsz bhirsz added the bug Something isn't working label Nov 19, 2020
@MaciejWiczk
Copy link
Author

@bhirsz, nice hints, thanks. Awesome idea with context :)

@mnojek
Copy link
Member

mnojek commented Feb 27, 2021

I created a very small change but it partly supports embedded keywords now.

If you run robocop on:
Library ${field} Should Be ${value}
it will not throw an error anymore.

But all embedded values without variable notation will not be recognized because we do not read the context.
Do you think that it enables the issue to be closed now?

@MaciejWiczk
Copy link
Author

@mnojek I guess I'm tricky user, but without robot context it seems to be hard/impossible to find embedded keywords usage:

$ robocop rfhub2/tests/acceptance/ | grep -i capital
C:\repo\rfhub2\tests\acceptance\e2e.robot:133:0 [W] 0302 Keyword name should be capitalized
C:\repo\rfhub2\tests\acceptance\e2e.robot:135:0 [W] 0302 Keyword name should be capitalized
C:\repo\rfhub2\tests\acceptance\e2e.robot:137:0 [W] 0302 Keyword name should be capitalized
C:\repo\rfhub2\tests\acceptance\resources\keywords.resource:55:0 [W] 0302 Keyword name should be capitalized

And those are the lines:
image

@mnojek
Copy link
Member

mnojek commented Feb 27, 2021

@MaciejWiczk I know, you're right. But this requires a whole another massive feature, which I do not plan to implement at this moment :) I'm focusing now on fixing bugs and adding some new rules.

@bhirsz
Copy link
Member

bhirsz commented Feb 27, 2021

Let's keep it open for now because reading the context is on our roadmap - although I agree it's a lot of work (but I hope to steal some knowledge from other open source tools that are dealing with keyword definition information ;))

@mnojek mnojek added the context reading This feature needs context reading label Mar 27, 2021
@mnojek
Copy link
Member

mnojek commented Jul 10, 2021

Closing it since we decided some time ago that we will not implement context reading.
Such feature can be requested from our friends from https://github.com/robocorp/robotframework-lsp 😉

@mnojek mnojek closed this as completed Jul 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working context reading This feature needs context reading
Projects
None yet
Development

No branches or pull requests

3 participants