Open BCGST opened 2 years ago
Already addressed by #430
Yeah, private keywords brought by #430 in RF 5.1 aren't exactly the same as private imports. They are highly related, though, and #430 is a great step forward. This issue is more closely related to #2581.
The functionality that whatever resource files import is visible to whomever imports that resource file is by design and often useful. It's also so widely used that due to backwards compatibility reasons we couldn't really change it even if we wanted to. I do agree this behavior isn't always desired, though, and would be open for ideas how to explicitly disable it when needed. Possibilities include:
Private Resource
and Private Library
that can be used instead of the existing Resource
and Library
when needed.Resource
and Library
so that imports are private. This would avoid the need for a new setting, but with libraries something like private=True
could clash with existing library config parameters. __all__
in Python.robot:private
so this should probably be robot:public
. We'd then also need some way to indicate that anything that's not explicitly public shouldn't be exposed.I think the first two methods are compelling because they apply to Library
and Resource
where the last too seem like they would work strictly with Resource
(or user generated libraries).
I'll add this tentatively to RF 6.1 scope. I cannot make promises it will be included nor when we actually start 6.1 development.
I doubt this will be too complicated to implement. The hardest part is likely coming up with good syntax.
My current thinking is that it would be best to handle this with a new setting that specifies what a resource file exposes. It would be similar to __all__
in Python that affects what the importer gets with from x import *
. Some reasons why I feel that way:
from x import *
in Python and using a similar solution as they use for liming what's exposed feels pretty logical.Library
and Resource
, which makes it easier to enhance them otherwise in the future. I'd like our imports to support Python style from x import y, z
and import x
imports, and imports both having configuration for what to import and what to expose can make them pretty complicated.Library
and Resource
is that configuring what's exposed is relevant only in resource files. Making a separate setting only available in resource files is easier than having a setting that can be used differently depending on the context.Library
and Resource
, that would avoid the two problems listed above, would be adding new settings Private Library
and Private Resource
. I find having two settings for importing libraries and another two for importing resources a bit confusing. Just a single setting that controls the "external interface" of the resource files feels simpler.robot:private
tag for that purpose, but if you have only few public keywords and many private ones it would be easier to list the former than tag the latter.Private Variables
, which also wouldn't solve how to limit what variables in the Variables
section are exposed.Some examples how this could work below:
# Expose some libraries and resources
Expose library=Browser library=SSHLibrary resource=example.resource
# Expose no library
Expose library=NONE
# Expose keywords and variables (no prefix needed)
Expose My Keyword Browser.Close Browser ${VARIABLE}
I have one question regarding this:
Why not follow the Python route here and instead of only providing a Library Import *
/ Resource Import *
, provide a From Library.. Import ...
/ From Resource... Import ...
? In general with an Import *
and even with the proposed Expose
, I think it's pretty hard to control what really gets into your namespace.
Also, maybe don't exactly follow 100% the Python route and make those lazy by default -- Python wants to do that now, but it's probably too late for Python, so, workarounds abound :wink:
A related problem is that Robot doesn't have proper namespaces for resource files. When a resource file imports a library, another resource file, or a variable file, all imported keywords and variables are placed into the suite namespace. That causes issues like #2581. Fixing that problem basically requires re-implementing namespaces so that the suite namespace only contains keywords and variables in the suite file as well as references to libraries, resource files and variable files imported in that suite. Imported resource files would then have their own keywords and variables as well as references to libraries/resources/variables they import. Keyword and variable lookups would then go through these nested resource imports chains.
After the above change, it would be relatively easy to limit the visibility of the imported resource files. I guess it could somehow be done also without that change, but I wouldn't like to enhance the current namespace too much because there are pretty big reasons why it should be rewritten altogether. These two topics should be thought together and implemented in a same release.
I get that the internals of RF may need to be improved due to the current implementation not taking that into account, but personally, I'd much rather have a proper implementation even if it takes longer than having those kind of fast but subpar workarounds...
In my opinion, for better organization and ease of understanding, only the keywords implemented in a Resource should be available in the file that will use the Resource.
Today the behavior is like this:
file1.robot
*** Settings ***
Library SeleniumLibrary
*** Keywords ***
keywork file 1
Press Keys Here I can use keywords from the SeleniumLibrary
file2.robot
Resource file1.robot
*** Keywords ***
keywork file 2
keywork file 1
Press Keys But I can also use the SeleniumLibrary keywords even if I don't add it as a Resource on file2.robot
My suggestion is to add a private MODIFIER, as exists in Java. I have not analyzed the Robot implementation to see if this modification is simple, but the idea would be to allow the Settings(Resource/Library/Variables) to allow the use of access/scope modifier.
If in the Resource/Library/Variables you don't inform the modifier, then it uses the default, as if it were a public one, remaining the current behavior.
Example:
file1.robot
*** Settings ***
private Library SeleniumLibrary
*** Keywords ***
keywork file 1
Press Keys Here I can use keywords from the SeleniumLibrary
file2.robot
Resource file1.robot
*** Keywords ***
keywork file 2
keywork file 1
# I cannot use the SeleniumLibrary keywords because the private modifier in the Resource in file1.robot was used
Too big for RF 7.
Options?
*** Settings ***
Library Browser show_keyword_call_banner=True PRIVATE AS CoolLibrary
Library Browser show_keyword_call_banner=True AS CoolLibrary PRIVATE
Library Browser show_keyword_call_banner=True AS PRIVATE CoolLibrary
Library Browser show_keyword_call_banner=True AS CoolLibrary #PRIVATE
PrivateLibrary Browser show_keyword_call_banner=True AS CoolLibrary
I see the situation very realistic that alias (AS) and PRIVAT is used at the same time like
*** Settings ***
Library Browser AS private.Browser PRIVATE
*** Test Cases ***
Test
private.Browser.click asdasdasd
I proposed earlier that instead of modifying imports, we'd add a separate setting for controlling what a resource file exposes. That would have a benefit that in addition to avoiding imported keywords to be exposed, it would allow also limiting what keywords from the resource file itself are exposed. The latter can nowadays be handled fairly well with the robot:private
tag, though, and a separate setting for controlling what's exposed would probably be more complicated to use than just modifying imports.
With imports I believe a separate control option is better than having new settings like Private Library
. Both of these alternatives look pretty good to me:
Library Example private=True
Library Example PRIVATE
We've used control options like private=True
with various control structures (e.g. WHILE limit=10
) and they have worked well. With libraries there's a problem that libraries them can accept named arguments so, unless you are familiar with this syntax, it's easy to consider private=True
to be an argument to the library and not a special option controlling the import. There could also be a conflict with a library accepting an argument private
, but that ought to be pretty rare and could be mitigated by using something like this:
Library Example private=this goes to library private=False
The PRIVATE
(case-sensitive) modifier at the end wouldn't have the problems private=True
has and it would also be consistent with the AS alias
syntax we have. The main problem with it is that we may want to add more options for controlling imports later and some of them could require values. The name=value
syntax is typically better with name-value pairs, but the aforementioned AS alias
demonstrates that also other options work. For example, if we wanted to allow importing only some keywords from a library, something like below would look good to me:
Library Example EXPOSE Keyword 1 Keyword 42
Library Example EXPOSE Keyword 1 Keyword 42 PRIVATE
Based on the above I believe that the PRIVATE
modifier at the end is the best solution. It doesn't require much new syntax, looks good, and it ought to work well with IDEs as well. An IDE could, for example, propose both Library
and Library (private)
in code completion and add the PRIVATE
modifier at the end if the latter is selected.
I can make a prototype implementation at some point to see how easy or hard this would be to implement. If it isn't too hard, we can do that already in RF 7.2. If it requires more work, then it's better to wait for RF 8.0 where we are planning to make other import and namespace related changes. In that case we could still consider making the parser changes to detect PRIVATE
and to tokenize it properly so that IDEs could start using it. The parser supporting PRIVATE
but keywords anyway being exposed would be somewhat problematic though, because when PRIVATE
would actually work later, it could cause backwards compatibility problems. Alternattively IDEs could start supporting # PRIVATE
comment for this purpose. That would have a benefit that it would work also with earlier Robot versions and could be a good idea anyway.
It could be beneficial if users could import resource files and libraries in a "private" way. Currently, resource files are imported in a sort of recursive way where you have access to all keywords that resource file imports as well, as described in the user guide here. This can be unhelpful when you are abstracting a library or resource such that other contributors should not make direct calls to the underlying libraries.
This was especially important to me on a recent project where I was refactoring a test suite to be able choose whether to run with Browser library or SeleniumLibrary to compare their pros and cons. You could pick which browser control to use but writing a test that made direct keyword calls to the opposite library would cause that control to crash out. Using an abstracted resource file allows you to pick which library to use on the fly, however with recursive importation, a contributor could still mistakenly call a non-abstracted keyword from one or the other library and cause the opposing library crash the test when they reach the unexpected keyword.
In more biased terms, private importation of libraries and resources feels like a much more familiar mode for project structure for me given that is how python seems to handle resource/library importation.