bhirsz / robotframework-cop-academy

Tool for static code analysis and formatting of Robot Framework language
Apache License 2.0
2 stars 0 forks source link

[Design] Fixable rules #5

Open bhirsz opened 3 weeks ago

bhirsz commented 3 weeks ago

Robocop rules should have option to fix them.

To enable automatic fixing we can use fix option:

robocop check --fix

By default error message should mention if rule is fixable (if fix option is not provided).

Some rules should be safe to fix and some may cause issues in the code (for example renaming arguments or ordering import). Such rules can be fixed only if additional --unsafe-fixes id provided.

From implementation point of view, fixable rules should have fix argument provided with either robotidy formatter (essentialy running part of the formatter to fix particular rule) or class that only fix particular issue.

Rules summary should contain count of the potentially fixable rules.

bhirsz commented 3 weeks ago

Calling the formatter should be done in isolation, without caling whole robotidy. Even if user have custom configuration for given formatter, we want to call it with out settings in accordance with the rule we want to fix.

For example rule that reports problems with continuation line indentation should configure given formatted to fix only this issue if possible.

bhirsz commented 3 weeks ago

Example how fixer could be defined for the rule:

Rule(
... 
fix=NormalizeTags
) 

Rule(
unsafe_fix=MyRuleFixer

) 

If fixer needs configuration it could be provided using 'fix_args'. For performance reason we should prefer lazy instantation - thats why we cant create fixer instance in the Rule but only after reading config and assuring there is fix flag. That's why we can only pass arguments indirectly:

Rule(
... 
fix=FixerWithArgs, 
fix_args={'param': 2}
) 

Fixer class should use ModelTransformator class as its type.

bhirsz commented 3 weeks ago

Testing should follow similiar format to robotidy testing, with files before / after formatting (fixing in our case). Additionaly, we should rerun linting on the output file to ensure that all occurences of the issue is fixed.

bhirsz commented 1 week ago

Fixers should be lazy and first only collect places and fixes. We can't fix them when we find them because it would be tricky for other checks/fixes.

After checks are done, we will evaluate list of potential fixes. If there are several fixes affecting the same nodes, they should be ordered (if the order of the fix matter) or excluded (if the fix is potentially breaking for other).

There could be additionaly 'only display' type of fixes - too dangerous too apply but safe enough to print how to fix to the user