Keywords with a dot character (punctuation mark) are being incorrectly highlighted in test cases

[pedrothaines]

Describe the bug
Keywords that have a dot character (punctuation mark) in its name are being incorrectly highlighted in test cases.

Please, take a look at line 13 in the screenshot below.

This highlighting problem is probably related to the fact that one can use a keyword using its full reference to the library that implements it, eg.:
SeleniumLibrary.Open Browser

Please, take a look at line 7 in the screenshot below.

In this case (full keyword library reference) the keyword highlighting is adequate.

To Reproduce
Steps to reproduce the behavior:

1. Create a keyword with the dot character in its name
2. Use the previously created keyword in a test case
3. Keyword in test case will be incorrectly highlighted

Expected behavior
I would expect that a keyword with a dot character in its name to be highlighted as any other keyword in a test case.

Screenshots
Already provided it in the bug description (better for contextualization).

Versions:

• OS: Ubuntu 20.04.4 LTS
• Robot Framework Version: 4.1.3
• Robot Framework Language Server Version: 0.41.0
• Client Version: VSCode 1.64.2

Logs
I don't think it is really needed for this specific case.

[fabioz]

Agreed, I guess that the analysis really needs to be semantic (analyzing imports) to do the proper coloring (right now it just checks for a . in the keyword name).

Marking as high priority.

[weltings]

My 2 cents.

Three options:

1. Current situation - Any Keyword that contains a dot is broken
2. Maximize functionality using semantics. All keywords and library prefixes will be highligted perfectly. But this costly in terms of performance (time behaviour and resource utilization) and costly in terms of implementation ( I would like to do it but will probably take me 40 hours)
3. Use simple algorithm. Library prefixes will appear at start of keyword. Most of them contain no spaces etc. Can devise some regex that covers 80% of the use cases. This is a good balance between performance, functionality and cost to implement.

I might be willing to do option 3

[fabioz]

@weltings For this specific case it shouldn't be costly to verify available imports (since everything is local there's no need to do an actual find definition, just checking the import/resource names / WITH NAME declarations and build a cache with what's valid before the dots should be straightforward and can cover all the cases).

I think it's closer to 8 hours work (although reviews back and forth can potentiality raise it). If you'd like to tackle with that approach let me know (otherwise I'll check it as this is currently high priority).

[weltings]

@fabioz When you say all imports are local, do you mean that all imports are defined in the Settings of the .robot or .resource file that is currently being processed?

Because that is not always true. I have seen many teams in enterprises or large projects that combine " standard" or " default" imports into separate files so they are always available. Nested imports.

So File_1.robot imports File_2.resource, and File_2.resource imports 10+ other files

[weltings]

But, if the scope for implementing this is decided to be local, and you promise to cut down on the nit-pickiness about 20% so I have some artistic freedom, then I can look into this ;-)

[fabioz]

But, if the scope for implementing this is decided to be local, and you promise to cut down on the nit-pickiness about 20% so I have some artistic freedom, then I can look into this ;-)

Seems good to me. At some point in the future we'll need to have some better caches (to handle the TODO in semantic tokens regarding the 'catenate') and then we can tackle imports from other files too.

[weltings]

Any suggestions on where to get the imports?

At first I was hoping they could be built whilst iterating inside _tokenize_tokens in semantic_tokens. Since that is the only function I'm currently familiar with.

I'd suspect the imports might already be lying around somewhere, waiting to be retrieved. I see the following:

semantic_tokens.py
semantic_tokens_full_from_ast
    for _stack, node in ast_utils._iter_nodes(ast, recursive=True):
        tokens = getattr(node, "tokens", None)    

naming.py
def visit_LibraryImport(self, node)
    with_name = node.get_token(Token.WITH_NAME)

Any advice?

[fabioz]

You're right, it's already available through:

robotframework_ls.impl.completion_context.CompletionContext.get_imported_libraries
robotframework_ls.impl.completion_context.CompletionContext.get_resource_imports

(for the imported libraries you want to check the alias in the received node).

Note that robotframework_ls.impl.semantic_tokens.semantic_tokens_full already receives a CompletionContext.

There was a version that didn't receive the completion context and received just the AST, but as just the AST isn't enough in the current scenario, I've just removed that version (i.e.: 744afc4).

[fabioz]

This is now fixed.

As a note, it's available in a pre-release if you want to check it (see: https://code.visualstudio.com/updates/v1_63#_pre-release-extensions on how to use pre-releases).

[weltings]

Thanks you. Your last few messages ended up in my spam, though for some reason this one was ok.

[fabioz]

Thanks you. Your last few messages ended up in my spam, though for some reason this one was ok.

I was really wondering why you turned silent ;)

So, I ended up implementing the last things related to highlighting for the next release (including the caching of the imports considering dependencies / keyword calls with variables) and provided a new pre-release for it (which I hope to release Monday if no blocker is found until then).