Add client-server mode with API

[bhirsz]

Client-server mode with API could be used by IDEs or separate programs for file scanning "on the fly" without need to wrap Robocop.

Example:

robocop = Robocop.server(*args)
robocop.scan_file(file)
robocop.scan_file(file)
...
robocop.scan_file(file)

Returned output would need to be different than std.out (for example json object) so it would be handled by other tools.

[bhirsz]

The example how one of the IDEs is handling integration with rflint:
https://github.com/nokia/RED/blob/master/src/RobotFrameworkCore/org.robotframework.ide.core-functions/src/main/python/scripts/rflint_integration.py

[bhirsz]

It would enable us to create restful API service like so (flask used in example):

@app.route('/scan/<str:path>', methods=['GET'])
def scan_file(path):
    # perform scan, save results to results json
   return jsonify({'results': results})

It could hosted as localhost (and used like in RED example) or in separate docker container

[mnojek]

That is really great idea. Cannot wait to introduce it to our tool 😎

[fabioz]

@bhirsz if it's possible to do, one nice thing may be passing the RobotFramework ast directly.

robocop = Robocop.server(*args)
robocop.scan_module(rf_ast)

This would allow existing language servers to call it without the overhead of actually parsing it again, which I think would be quite nice and would also solve being able to collect data from dirty buffers all at once ;)

Do you think that's possible?

[bhirsz]

Yes it's possible. Although there are two problems:

1. before running the tests we're looking in file for disablers like those:

Keyword With Some Lint Problem   # robocop: disable

It's readed directly from the file (the file is open from disk). But I can think of way of consuming ast in class that is looking for disablers

2. We have few checkers that are not using ast but file directly (mostly invalid spacing in the end of file, too long line etc). But for that I could also reuse ast to read lines from ast model

Regarding this issue currently it's possible to do this:

        import robocop
        from robocop.config import Config

        # create config instance and load config, for example from your plugin config
        config = Config()
        config.include = {'1003'}
        config.paths = ['tests\\atest\\rules\\section-out-of-order']

        robocop_runner = robocop.Robocop(config=config)
        issues = robocop_runner.run()
        issues = robocop_runner.run() # can be repeated

I could create separate method for testing passed ast, ie scan_ast but I need think about mentioned problems first. Do you need special output (predefined JSONs format) to make it easier to integrate?

[fabioz]

Maybe pass everything you need then?

i.e.:

robocop = robocop.Robocop(config)
robocop.scan_module(rf_ast, source, filename)

The detail is that although the filename usually maps to the filesystem, it's also possible to be for some file that wasn't even saved and exists only in-memory.

For the config, it'd be nice to load from the filesystem (so, it'd look for something as a .editorconfig file -- i.e.: https://editorconfig.org/)

config = Config.from_path('c:/temp/bar.robot')

For the in-memory case it'd probably just use the workspace root as the place to load info from.

As for the output, a list[dict] where the dict contents are something that can be later converted to a language server protocol Diagnostic (i.e.: search for diagnostic in: https://microsoft.github.io/language-server-protocol/specification) would be nice.

[bhirsz]

@fabioz

In proposed method:

robocop.scan_module(rf_ast, source, filename)

I understand that source is loaded file (or in-memory equivalent). In what format? ie it could be one big string, or list of lines, or some generator.. it's important to know so I can code around it. Would source be always present or only in case if it's only present in memory?

filename is then used just for supplying information to warning lines ie:

test.robot:3:0 [W] 0804 Multiple resource imports with path "resource.robot" in suite

And as for configuration, we already use config files so we can reuse that. For example we have concept of 'automatic config files' like .editorconfig. If you have .robocop file in project root robocop should detect it and load configutation from it. So if we can supply project root when creating Robocop instance it would be taken care of.

[fabioz]

I understand that source is loaded file (or in-memory equivalent). In what format? ie it could be one big string, or list of lines, or some generator.. it's important to know so I can code around it. Would source be always present or only in case if it's only present in memory?

It can be whatever works best for you... (I think one big string may be more common, but if you need it in a different format that'd be ok too).

I think it'd always be present.

[fabioz]

And as for configuration, we already use config files so we can reuse that. For example we have concept of 'automatic config files' like .editorconfig. If you have .robocop file in project root robocop should detect it and load configutation from it. So if we can supply project root when creating Robocop instance it would be taken care of.

Great ;)

[bhirsz]

PR #287 should cover what we discussed. I went with one string for source. Typically we should create instance of Robocop once (ie when opening the project) then supply files for checking:

Setup:

import robocop
from robocop.config import Config

config = Config(root='PUT PROJECT ROOT DIR HERE')

robocop_runner = robocop.Robocop(config=config)
robocop_runner.reload_config()  # required for registering rules and stuff after change in config

Later on:

from robocop.utils import issues_to_lsp_diagnostic

issues = robocop_runner.run_check(ast_model, filename, source)
diag_issues = issues_to_lsp_diagnostic(issues)
# additional parsing for your formats

If I need to add any other code or something is not working (I just created it, I will add tests before release) then let me know. I will also try to add documentation for this API.

[fabioz]

Seems great for me ;)

I'm finishing something and I plan on integrating with that API tomorrow...

[fabioz]

I'm testing it here.

In general the API seems to work pretty nice, but it is failing on the use-case where I only have in-memory contents.

i.e.: even though the in-memory file contents were passed, some checkers still try to load from the disk doing:

with open(self.source) as file

-- RawFileChecker and IgnoredDataChecker were the ones I spotted.

The other minor is that the line numbers should be 0-based (so, issues_to_lsp_diagnostic should have -1 on the lines).

I did apply fixes to those to the copy I'm testing with and it seemed to work well (robocorp/robotframework-lsp@1a061fb) -- I'm not sure it's the best approach in the case for the in-memory contents, so, I'm leaving it up to you how you'd like to tackle those officially ;)

Also, do you have a timeline on when an official release with that API would be released?

[bhirsz]

Ah right, I've forgot to update that part of the code related with checkers and only in-memory content.

All our lines are starting from 1 so I can decrease it by one. What about column / char?

Your changes in branch looks quite decent, I can apply them (with some minor changes). I can make release even today - I will try to to apply those changes and create some tests & docs (testing part will probably take more time than developing it..).

Do you have any screen shot how it works in IDE? Out of curiosity - if you have any at this point.
Also we're parsing DataErrors as our rule - you can disable it though by adding it to exclude. I'm mentioning it because I think you also raise RF DataErrors.

[fabioz]

Do you have any screen shot how it works in IDE? Out of curiosity - if you have any at this point.

Sure (note that it has some issues from Robocop and some issues from the language server itself):

[mnojek]

The other minor is that the line numbers should be 0-based (so, issues_to_lsp_diagnostic should have -1 on the lines).

Is it some kind of a standard in linters/lsp or is it how you've written your tool? We decided to use1-based because that's how lines are marked in IDE. Do you need 0-based just for the purpose of the LSP? Then maybe we can just provide you the data how you want to have it but we would keep using 1-based lines. Is that ok?

BTW This (the screenshot) looks fantastic :)

[fabioz]

The language server protocol expects 0-based...

i.e.: In https://microsoft.github.io/language-server-protocol/specifications/specification-current/ it says: Position in a text document expressed as zero-based line and zero-based character offset.

Also, the request is just to change issues_to_lsp_diagnostic -- given that it says that it's lsp-related, it seems fair that it provides those 0-based no?

-- Apart from that particular API, you're surely free to have it 1-based and work anyway you'd like internally.

[bhirsz]

Yes, since issues_to_lsp_diagnostic is converting between our internal type to external type it's totally fine to have it 0 based here.

[bhirsz]

I've created release 1.6 together with above changes

[idxn]

Folks, Is there any doc guiding us to get this working on vscode?

[fabioz]

Folks, Is there any doc guiding us to get this working on vscode?

Wait a few more hours and then update/install the Robot Framework Language Server (https://marketplace.visualstudio.com/items?itemName=robocorp.robotframework-lsp) version 0.11.0 in VSCode (I'm in the process of doing a release on the Robot Framework Language Server which will have Robocop integrated).

It should work out of the box (there will be a setting to disable it if you'd rather not use it).