Create a place where community rules can be stored

[mnojek]

As the idea sparked in the comments of #680, here is a follow-up issue to discuss the topic and how it can be achieved.

The idea

A centralized place for the rules that do not fit into the core of Robocop, from where the users can easily download them and use in their projects.

The reasons why a rule may not fit into the core of Robocop:

• it reports something that is possible/allowed in Robot Framework, but not very common
• it reports a very specific issue with the code, which is not a general problem
• it requires reading the context (more than one file) or importing the libraries/resources and so on
• ?

I imagine this in 3 ways (but there may be more, and even better than these):

1. A separate repository serving as a storage for custom rules, where users can suggest adding new ones by PR. Each rule may need to have basic tests to ensure that it's working properly for the scope it affects, and a meaningful documentation.
2. Custom rules located in a separate directory in this project and available for the users by running pip install robotframework-robocop[extra-rules] which will install them to their local Robocop instance (proposed by @bhirsz here).
3. Maybe just a documentation page where the working examples will be presented and available to download/copy, with examples and documentation?

Things for further investigation

1. Who would decide which rule should be added to the storage? Authors? Community (by voting)? Or anything that meets minimum requirements (tests, docs, etc.) would be accepted?
2. The custom rules should be specifically marked, to not misguide people this is a built-in rule (some extra field or maybe just the ID convention - like 5-digit 1XXYY or 99YY)?
3. Should all the custom rules be downloaded and turned on (off?) locally by default, or should they be picked individually by the users before downloading? I imagine an issue when a user downloads all custom rules and some may actually be failing in his project, or maybe report too many issues, or just didn't apply to their specific case.

Rule candidates

I'll just mention the issues where the users asked for a specific rule to be added, which we rejected, due to various reasons.

• [Enhancement] Create a new rule to issue an INFO message when Test Setup and [Setup] or Test Teardown and [Teardown] are used in the same test suite #680 - inform the user that Test Teardown/Setup and Teardown/Setup are used within the same test suite
• underscore-in-keyword-name to support underscore prefix #766 - detect unusual symbols in keyword name / test case name
• Feature request: add a check ensuring the minimum spacing between keyword & arguments #688 - detect case where built-in keyword was not properly separated with the arguments (not enough spaces)
• Sleep keyword detect #778 - detect Sleep keyword
• More not-capitalized-test-case-title (0308) options #666 - detect if each word is capitalized in the test case name
• [Rule] Calling keyword outlook should follow keyword definition outlook  #566 - detect when the keyword is called with an acceptable but a non-standard way of writing its name
• [Rule] Suggest using robot:continue-on-failure #850 - suggest using robot:continue-on-failure tag
• Check settings name #509 - check if setting has space between the brackets ([ Tags ]) or not ([Tags])
• Check variable names for RF's recommendation #502 - validate characters used in the variable name
• detect if JIRA key (or other project issue id) is present in the comment or test case name

Let's see where we can get with this idea.

[bhirsz]

1. Separate directory is more "welcoming" to the users (afraid they could break anything with their own custom rules :D), but it will be harder to integrate it with testing/generating the documentation. With the same repository it would be easier to, for example, generate sub page "community rules" in the docs. I need to think about this.

2. pip installation with pip install robotframework-robocop[extra-rules] can be achieved with both separate and them same project. If our separate projest is python module called robotframework-robocop-extra-rules then we just need to add it to our dependencies.

For me anything that meets basic criteria - and if we are flooded, we can start adding special rules (but I doubt it will be the issue).

I believe selecting issues could be easier if we add "disabled" feature to the rules - allowing rules to be disabled by default and only included if there are enabled (maybe similarly to Robotidy with -c rule-name:enabled=True, or --enable). Thanks for that we can avoid situation where users include two contradicting rules to the project. With this feature we could set all issues in this project as disabled.
The other way of dealing with it would keeping every rule in the separate file (or group of rules). Users would use robocop --ext-rules robocop_extra.filename to load the rules (names are temporary).

I agree on id convention. To make adding the rules easier we should have precommit that would warn if rule with such id or name already exists (and maybe even propose next free id).

[mnojek]

2. pip installation with pip install robotframework-robocop[extra-rules] can be achieved with both separate and them same project. If our separate projest is python module called robotframework-robocop-extra-rules then we just need to add it to our dependencies.

A man learns his entire life :)

Good idea with disabled by default.

We can also provide a default .robocop file (like a template) that will contain all the custom rules marked as disabled, with some comment explaining quickly what each rule does. That way, people would not need to go through documentation of all the rules, they will just quickly skim through the configuration file, and they should at least has some basic understanding if they need it.

1. Separate directory is more "welcoming" to the users (afraid they could break anything with their own custom rules :D), but it will be harder to integrate it with testing/generating the documentation. With the same repository it would be easier to, for example, generate sub page "community rules" in the docs.

I agree that having people frequently contributing to the core repo may increase the risk of some errors and bugs. On the other hand, it will only increase the popularity of the package. If there would be a separate repository, it would be hard to promote it in the community, I think. I'm not saying we should go this way, just pointing out a "marketing" side of it. 😃

I agree on id convention. To make adding the rules easier we should have precommit that would warn if rule with such id or name already exists (and maybe even propose next free id).

What would be better?

1. Reserving one checker type for all custom rules like 99XX? This will disallow people to sort the custom rules by checker type, which is not the best way I think. Also, someone can already use these IDs with their custom rule.
2. Reserving a range of checker types from the end of the ID convention, e.g. from 90XX to 99XX? This will allow people to sort the new rules by type, but it leaves them with only 10 categories. And again there is a risk of conflict with already existing custom rules on the local computers of the users.
3. Reserving a 5-digit IDs for all official custom rules, like 1XXYY? I think this is the best approach, because we no longer risk any local conflicts, and this will also be an explicit indicator, that these are the official custom Robocop rules.

I think I have answered myself, but let's hear from you.

[bhirsz]

This default configuration file is very good idea - maybe we can provide something like that for default rules as well? There was similar idea (to generate config file from current config) but maybe we can just provide "generate default as config file, together with short description as comment". It would make it easy to see what rules are active, and to enable/disable them if necessary.

Yeah, it's easier to markete something that we already have (just 'extension' or extra place for rules in our place) and have already some recognition.

I'm also in favour of 3 option - we avoid risk of confliccts and it will be easier to see that given rule is "custom offical" one.

[mnojek]

Summing up...

1. We are going to add community rules directly to the Robocop project (this repository).
2. We need to differentiate between these new rules, so I suggest naming them "Community rules", and the ones that we already have to start naming "Built-in rules".
3. The community rules will be stored in a separate directory to indicate they are special rules, not the built-in ones.
4. The community rules will have 5-digit IDs that follow the pattern 1XXYY, where 1 is static, XX is the checker type and YY is the rule ID.
5. The community rules will be turned off by default, because there is a risk that they can interfere with other rules or even be contradicting to them.
6. The users will be able to install community rules by installing extra dependencies for Robocop, e.g. pip install robotframework-robocop[extra-rules] or pip install robotframework-robocop[community-rules].
7. The documentation will have an extra menu entry that will list the community rules separately from the built-in rules.
8. We need to define the minimum requirements for the community rules to be accepted and merged into the core (more info above), e.g. basic tests, some documentation, etc.
9. We need to advertise this change, when it's ready to be utilized by the community.
10. We will add a default configuration file that will list all the community rules with a short comment describing their purpose, so that it's easy for the users to select which rules they need and turn them on by editing the configuration file.

On the 6th point, I am not entirely sure. Maybe we should install the community rules by default as well, but just keep them disabled until the users explicitly allow them in their configuration files?

Let me know if all of the above is true and agreed or we need some more discussion.
If anything is missing, feel free to extend the list.

[bhirsz]

Very good summary.

11. We need to see how our test framework can be easily utilised for new rules, and our documentation updated so it is clear how the users can (easily) create their own tests.

We can install them by default. If they are disabled by default and will go by review process it should be fine.

[mnojek]

There's one more thing that needs to be decided. How we are going to handle these community rules from the CLI?

1. How a community rule can be enabled? Only through a configuration file (I don't like this option, because configuration files just support all CLI options)? Or through an extra CLI option (supported in configuration files as well)?
2. Are we going to add a new option that will allow enabling all community rules at once? For example, do we want to have something like robocop --community-rules <src> which will enable all the rules at once? Is it safe to allow for something like this, since some may be contradicting?
3. Or maybe we want the users to enable them one-by-one? For example, the user would configure them like robocop -c 10101:disabled:false -c 10102:disabled:false (alternatively by enable RuleParam)? Or (based on the previous example) to list them like this robocop --community-rules 10101,10102,10103 <src>?
4. Do the CLI options like --list, --list-configurable should support displaying the community rules by default, or only when the option to enable them explicitly is called within the same command?
5. Should CLI options --include and --exclude support the community rules by default, or only when they are explicitly enabled in the same command?

[bhirsz]

My idea of community rules are optional rules, that can be easily enabled by the users if they want to. They should work the same as built in rules but only be disabled by default.

With that, I think:

1. They could be enabled using enabled= True parameter (its the same convention as in robotidy for non default transformers, so it will be easier for some users). Both from cli or config file.
2. I would not add option to enable them all at once - I would prefer users to consciously select their rules. Hovewer we should option to enable multiple rules at once - maybe --enable? I'm not using --community-rule name because we could allow users to use "disabled by default" rules in their own custom rules too and they could use the same options to enable them.
3. --list should display them only when they are enabled or if user wishes so (ie like we can do --list ENABLED we could do --rules COMMUNITY or smth)
4. Since they should work the same as other rules, include and exclude should work on them - the main difference is that --enable would add community rules to default ones, and include would only run included rules, no matter if default or custom

[mnojek]

I agree, all that makes sense.

Although, I am not able to start working on that for the next month, as I have some other stuff to do and I will be unavailable for some time also. Just letting you know :)
It's good that we have established the requirements and the concept is now clear.

[bhirsz]

It's very good to have everything planned to avoid unnecessary work / initial issues. I still have some work in the Robotidy so I will focus on that, then take some old issues from the Robocop first.

[bhirsz]

We should start implementing it from creating the place for the rules. If we're going with installed by default rules, where they should be stored? I was thinking about creating separate directory inside robocop/checkers directory, ie robocop/checkers/community_rules.

Then:

• we can use our existing test framework the same way we are doing for default rules
• we can add test that would ensure that all rules inside community_rules are in fact not enabled by default

I could add some simple example rule inside.

Next step would be docs creation - we should have separate section/page listing community rules. Maybe since we have:

It could be "Community Rules" page, with exact same design as Rules.

And then the next step would be creating a way of generating the configuration with all rules commented our and simple description comment. We discussed it in the thread but then forgot about it. I have noticed some of our users create such file when trying to integrate our tools in their projects (and then they uncomment/remove lines from configuration as they go and fix the issues).

[mnojek]

We should start implementing it from creating the place for the rules. If we're going with installed by default rules, where they should be stored? I was thinking about creating separate directory inside robocop/checkers directory, ie robocop/checkers/community_rules.

Then:

• we can use our existing test framework the same way we are doing for default rules
• we can add test that would ensure that all rules inside community_rules are in fact not enabled by default

I could add some simple example rule inside.

Next step would be docs creation - we should have separate section/page listing community rules. Maybe since we have: It could be "Community Rules" page, with exact same design as Rules.

And then the next step would be creating a way of generating the configuration with all rules commented our and simple description comment. We discussed it in the thread but then forgot about it. I have noticed some of our users create such file when trying to integrate our tools in their projects (and then they uncomment/remove lines from configuration as they go and fix the issues).

Yes to all :)

Do you think we should create more issues to make the work on it more granular and easier to be split? This issue started being a bit too big and I am afraid that we would miss something important.

[bhirsz]

Yes, we should create more issues, I already forgot about some tasks here and I had to reread it all :)

[mnojek]

Ok, I will create them by the end of tomorrow 👍🏻