MarketSquare / MarketSquare/robotframework-robocop

Fix rules

Open
#1,631 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
269
Forks
58
Avg merge
4h 55m
Merged PRs (30d)
45

Description

Meta ticket for rules that can have fixes.

The tables below contain a brief overview of every rule: whether an automatic fix is possible, how difficult and how
safe it would be, whether an existing formatter already covers the rewrite, and the current implementation status.

**Legend**

| Column | Values |
| --- | --- |
| Fix | βœ… possible / ❌ not possible (needs manual intervention, usually a bigger refactor) |
| Difficulty | easy / mid / hard |
| Safety | safe / unsafe (fix may change the behaviour, would need `UNSAFE` applicability) |
| Status | βœ… implemented / πŸ”„ in progress / ⬜ to do / βž– not planned |

**Scope of `check --fix` vs `robocop format`**

`robocop format` focuses on whitespace, alignment and optional one-time housekeeping. It should *not* replace
deprecated syntax or solve single issues that are already fully covered by a rule - such formatters are being
deprecated (`NormalizeComments` in #1845, `RemoveEmptySettings` in #1849, `ReplaceEmptyValues` in #1853,
`ReplaceRunKeywordIf` in #1899, `NormalizeAssignments` in #1904, `DiscardEmptySections` in #1905) in favour
of a rule fix.
Whitespace/alignment formatters (`NormalizeSeparators`, `NormalizeNewLines`, `Align*`, `SplitTooLongLine`) stay,
and corresponding rules should document when `robocop format` fixes them. A rule fix is still appropriate when malformed
syntax prevents Robot Framework from producing an AST that formatters can process.
Rules with an existing formatter counterpart are marked in the *Notes* column - implementing the fix may allow us to
deprecate the formatter (as it happened with `RemoveEmptySettings`).

### ANN

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| ANN01 | missing-section-variable-type | ❌ | - | unsafe | βž– | could be derived from the value, but unsafe |
| ANN02 | missing-argument-type | ❌ | - | - | βž– | |
| ANN03 | missing-for-loop-variable-type | ❌ | - | - | βž– | |

### ARG

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| ARG01 | unused-argument | βœ… | easy | unsafe | βž– | extremely unsafe |
| ARG02 | argument-overwritten-before-usage | ❌ | - | unsafe | βž– | unclear what the fix should be |
| ARG03 | undefined-argument-default | βœ… | easy | safe | βœ… | #1852, adds `${EMPTY}` |
| ARG04 | undefined-argument-value | βœ… | easy | unsafe | ⬜ | |
| ARG05 | invalid-argument | ❌ | - | - | βž– | |
| ARG06 | duplicated-argument-name | ❌ | - | - | βž– | |
| ARG07 | arguments-per-line | βœ… | mid | safe | ⬜ | split into multiple lines |
| ARG09 | missing-argument-name | βœ… | - | safe | βœ… | #1837 |

### COM

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| COM01 | todo-in-comment | ❌ | - | - | βž– | |
| COM02 | missing-space-after-comment | βœ… | easy | safe | βœ… | #1844, `SOMETIMES` (`##comment` not fixed); replaces `NormalizeComments` (removed in #1845) |
| COM03 | invalid-comment | ❌ | - | - | βž– | |
| COM04 | ignored-data | βœ… | easy | safe | βœ… | #1844, `SOMETIMES` (data with the `language:` header not fixed) |
| COM05 | bom-encoding-in-file | ❌ | - | - | βž– | not worth the effort |
| COM06 | commented-out-code | ❌ | - | - | βž– | |

### DEPR

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| DEPR01 | if-can-be-used | βœ… | mid | safe | ⬜ | superseded by `deprecated-run-keyword-if` (DEPR08); `ReplaceRunKeywordIf` formatter removed in #1899 |
| DEPR02 | deprecated-statement | βœ… | mid | safe | ⬜ | rule needs to be split first |
| DEPR03 | deprecated-with-name | βœ… | easy | safe | βœ… | #1843 |
| DEPR04 | deprecated-singular-header | βœ… | easy | safe | βœ… | #1843 |
| DEPR05 | replace-set-variable-with-var | βœ… | mid | safe | ⬜ | `ReplaceWithVAR` |
| DEPR06 | replace-create-with-var | βœ… | mid | safe | ⬜ | `ReplaceWithVAR` |
| DEPR07 | deprecated-force-tags | βœ… | mid | safe | βœ… | |
| DEPR08 | deprecated-run-keyword-if | βœ… | mid | safe | βœ… | #1898, `SOMETIMES` (only body keyword calls are fixed); replaces `ReplaceRunKeywordIf` (removed in #1899) |
| DEPR10 | deprecated-return-keyword | βœ… | mid | safe | βœ… | |
| DEPR11 | deprecated-return-setting | βœ… | mid | safe | βœ… | |

### DOC

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| DOC01 | missing-doc-keyword | ❌ | - | - | βž– | |
| DOC02 | missing-doc-test-case | ❌ | - | - | βž– | |
| DOC03 | missing-doc-suite | ❌ | - | - | βž– | |
| DOC04 | missing-doc-resource-file | ❌ | - | - | βž– | |
| DOC05 | variable-in-documentation | ❌ | - | unsafe | βž– | escaping changes intentional interpolation; intentionally non-fixable (#1866) |

### DUP

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| DUP01 | duplicated-test-case | ❌ | - | - | βž– | |
| DUP02 | duplicated-keyword | ❌ | - | - | βž– | |
| DUP03 | duplicated-variable | βœ… | easy | safe | βœ… | #1857, removes the ignored (second) value |
| DUP04 | duplicated-resource | βœ… | easy | safe | βœ… | #1848 |
| DUP05 | duplicated-library | βœ… | easy | safe | βœ… | #1848 |
| DUP06 | duplicated-metadata | βœ… | mid | unsafe | ⬜ | hard to decide which one to keep |
| DUP07 | duplicated-variables-import | βœ… | easy | safe | βœ… | #1848 |
| DUP08 | section-already-defined | βœ… | hard | safe | ⬜ | merging whole sections, `MergeAndOrderSections` |
| DUP09 | both-tests-and-tasks | ❌ | - | - | βž– | |
| DUP10 | duplicated-setting | βœ… | easy | unsafe | ⬜ | |

### ERR

All `ERR*` rules report parsing/syntax errors and cannot be fixed automatically: ERR01, ERR03, ERR04, ERR05, ERR08,
ERR09, ERR12, ERR13, ERR14, ERR15, ERR16, ERR17.

### IMP

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| IMP01 | wrong-import-order | βœ… | easy | safe | βœ… | #1856 |
| IMP02 | builtin-imports-not-sorted | βœ… | easy | safe | βœ… | #1856 |
| IMP03 | non-builtin-imports-not-sorted | βœ… | mid | unsafe | ⬜ | import order may matter |
| IMP04 | resources-imports-not-sorted | βœ… | mid | unsafe | ⬜ | import order may matter |

### KW

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| KW01 | sleep-keyword-used | ❌ | - | - | βž– | |
| KW02 | not-allowed-keyword | ❌ | - | - | βž– | |
| KW03 | no-embedded-keyword-arguments | ❌ | - | - | βž– | |
| KW07 | missing-keyword-prefix | βœ… | - | safe | βœ… | #1838 |

### LEN

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| LEN01 | too-long-keyword | ❌ | - | - | βž– | |
| LEN02 | too-few-calls-in-keyword | ❌ | - | - | βž– | |
| LEN03 | too-many-calls-in-keyword | ❌ | - | - | βž– | |
| LEN04 | too-long-test-case | ❌ | - | - | βž– | |
| LEN05 | too-few-calls-in-test-case | ❌ | - | - | βž– | |
| LEN06 | too-many-calls-in-test-case | ❌ | - | - | βž– | |
| LEN07 | too-many-arguments | ❌ | - | - | βž– | |
| LEN08 | line-too-long | βž– | - | - | βž– | handled by `robocop format` (`SplitTooLongLine`) |
| LEN09 | empty-section | βœ… | easy | safe | βœ… | #1841, `SOMETIMES` (sections with comments are not fixed); replaces `DiscardEmptySections`, removed in #1905 (its `allow_only_comments=False` option is dropped) |
| LEN10 | number-of-returned-values | ❌ | - | - | βž– | |
| LEN11-LEN26 | empty-* settings | βœ… | easy | safe | βœ… | #1840, `RemoveEmptySettings` removed in #1849 |
| LEN27 | too-many-test-cases | ❌ | - | - | βž– | |
| LEN28 | file-too-long | ❌ | - | - | βž– | |
| LEN29 | empty-test-template | βœ… | easy | safe | βœ… | #1840 |
| LEN30 | empty-template | βœ… | easy | safe | βœ… | #1840 |
| LEN31 | empty-keyword-tags | βœ… | easy | safe | βœ… | #1840 |
| LEN32 | too-long-variable-name | ❌ | - | - | βž– | |

Note on the empty settings: the empty test case settings that can overwrite a suite setting (`[Setup]`, `[Teardown]`,
`[Timeout]`, `[Template]`) are not removed but replaced with an explicit `NONE`, since the suite setting can also
come from the parent `__init__.robot`.

### MISC

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| MISC01 | keyword-after-return | ❌ | hard | unsafe | βž– | reordering statements may change the behaviour |
| MISC02 | empty-return | βœ… | easy | safe | βœ… | #1846 |
| MISC03 | nested-for-loop | ❌ | hard | unsafe | βž– | |
| MISC04 | inconsistent-assignment | βœ… | mid | safe | βœ… | #1901, `ALWAYS`; extended to the `VAR` syntax in #1902; `NormalizeAssignments` removed in #1904 |
| MISC05 | inconsistent-assignment-in-variables | βœ… | easy | safe | βœ… | #1859; together with MISC04 (#1901, #1902) it replaces `NormalizeAssignments`, removed in #1904 |
| MISC06 | can-be-resource-file | ❌ | mid | unsafe | βž– | requires renaming the file |
| MISC07 | if-can-be-merged | βœ… | mid | safe | ⬜ | |
| MISC08 | statement-outside-loop | ❌ | hard | unsafe | βž– | |
| MISC09 | inline-if-can-be-used | βœ… | easy | safe | βœ… | #1900, `SOMETIMES` (inline IF over the line limit is not fixed); `InlineIf` stays (does more: bidirectional, ELSE/ELSE IF and assignments) |
| MISC10 | unreachable-code | βœ… | mid | safe | ⬜ | mechanical removal |
| MISC11 | multiline-inline-if | βž– | - | - | βž– | handled by `robocop format` (`InlineIf`), documented in #1859 |
| MISC13 | expression-can-be-simplified | βœ… | mid | safe | ⬜ | |
| MISC14 | misplaced-negative-condition | βœ… | easy | safe | βœ… | #1859, `SOMETIMES` (double negation is not fixed) |
| MISC15 | unused-disabler | βœ… | easy | safe | βœ… | #1855, `SOMETIMES` (only standalone directives are removed) |

### NAME

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| NAME01 | not-allowed-char-in-name | ❌ | hard | unsafe | βž– | renaming needs human judgement |
| NAME02 | wrong-case-in-keyword-name | βœ… | easy | safe | βœ… | #1860, `SOMETIMES` (configured `pattern` is not fixed) |
| NAME03 | keyword-name-is-reserved-word | ❌ | hard | unsafe | βž– | |
| NAME04 | underscore-in-keyword-name | βœ… | easy | safe | ⬜ | `RenameKeywords` |
| NAME05 | setting-name-not-in-title-case | βœ… | easy | safe | βœ… | #1858, `NormalizeSettingName` stays (does more than the rule) |
| NAME06 | section-name-invalid | βœ… | easy | safe | βœ… | #1858, `NormalizeSectionHeaderName` stays (does more than the rule) |
| NAME07 | not-capitalized-test-case-title | ❌ | mid | unsafe | βž– | `RenameTestCases`, test names are identifiers |
| NAME08 | section-variable-not-uppercase | βœ… | easy | safe | ⬜ | `RenameVariables` |
| NAME09 | else-not-upper-case | βœ… | easy | safe | βœ… | #1854 |
| NAME10 | keyword-name-is-empty | ❌ | - | - | βž– | |
| NAME11 | test-case-name-is-empty | ❌ | - | - | βž– | |
| NAME12 | empty-library-alias | βœ… | easy | safe | βœ… | #1847, `SOMETIMES` (multiline imports are not fixed) |
| NAME13 | duplicated-library-alias | βœ… | easy | safe | βœ… | #1847, `SOMETIMES` (multiline imports are not fixed) |
| NAME14 | bdd-without-keyword-call | ❌ | mid | unsafe | βž– | |
| NAME15 | not-allowed-char-in-filename | ❌ | hard | unsafe | βž– | requires renaming the file |
| NAME16 | invalid-section | ❌ | mid | unsafe | βž– | |
| NAME17 | mixed-task-test-settings | ❌ | mid | unsafe | βž– | |
| NAME18 | wrong-case-in-keyword-call | βœ… | easy | safe | βœ… | #1860, `SOMETIMES` (configured `pattern` is not fixed) |

### ORD

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| ORD01 | test-case-section-out-of-order | βœ… | easy | safe | ⬜ | `OrderSettings` |
| ORD02 | keyword-section-out-of-order | βœ… | easy | safe | ⬜ | `OrderSettings` |
| ORD03 | section-out-of-order | βœ… | mid | safe | ⬜ | `MergeAndOrderSections` |

### SPC

Whitespace and alignment stay the responsibility of `robocop format` when Robot Framework can parse the statement.
Rules for malformed merged cells remain fixable because AST formatters cannot see those statements.

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| SPC01 | trailing-whitespace | βœ… | easy | safe | βœ… | already implemented |
| SPC02 | missing-trailing-blank-line | βž– | - | - | βž– | handled by `robocop format` (`NormalizeNewLines`) |
| SPC03 | empty-lines-between-sections | βž– | - | - | βž– | handled by `robocop format` (`NormalizeNewLines`) |
| SPC04 | empty-lines-between-test-cases | βž– | - | - | βž– | handled by `robocop format` (`NormalizeNewLines`) |
| SPC05 | empty-lines-between-keywords | βž– | - | - | βž– | handled by `robocop format` (`NormalizeNewLines`) |
| SPC06 | mixed-tabs-and-spaces | βž– | - | - | βž– | handled by `robocop format` (`NormalizeSeparators`) |
| SPC08 | bad-indent | βž– | - | - | βž– | handled by `robocop format` (`NormalizeSeparators`) |
| SPC09 | empty-line-after-section | βž– | - | - | βž– | handled by `robocop format` (`NormalizeNewLines`) |
| SPC10 | too-many-trailing-blank-lines | βž– | - | - | βž– | handled by `robocop format` (`NormalizeNewLines`) |
| SPC11 | misaligned-continuation | βž– | - | - | βž– | handled by `robocop format` (`NormalizeSeparators`) |
| SPC12 | consecutive-empty-lines | βž– | - | - | βž– | handled by `robocop format` (`NormalizeNewLines`) |
| SPC13 | empty-lines-in-statement | βž– | - | - | βž– | handled by `robocop format` (`NormalizeNewLines`) |
| SPC14 | variable-not-left-aligned | βž– | - | - | βž– | handled by `robocop format` (`AlignVariablesSection`) |
| SPC15 | misaligned-continuation-row | βž– | - | - | βž– | handled by `robocop format` (`NormalizeSeparators`) |
| SPC16 | suite-setting-not-left-aligned | βž– | - | - | βž– | no formatter fix; malformed input has no usable AST |
| SPC17 | bad-block-indent | βž– | - | - | βž– | handled by `robocop format` (`NormalizeSeparators`) |
| SPC18 | first-argument-in-new-line | βž– | - | - | βž– | no formatter fix; malformed input has no usable AST |
| SPC19 | not-enough-whitespace-after-setting | βœ… | easy | safe | βœ… | #1862; formatter cannot parse merged cells |
| SPC20 | not-enough-whitespace-after-newline-marker | βœ… | easy | safe | βœ… | #1862; formatter cannot parse merged cells |
| SPC21 | not-enough-whitespace-after-variable | βœ… | easy | safe | βœ… | #1862; formatter cannot parse merged cells |
| SPC22 | not-enough-whitespace-after-suite-setting | βœ… | easy | safe | βœ… | #1862; formatter cannot parse merged cells |
| SPC23 | empty-line-in-test-template | βœ… | easy | safe | βœ… | #1865; removes ignored empty template data rows |

### TAG

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| TAG01 | tag-with-space | βœ… | mid | unsafe | ⬜ | changing the tag text can break tag selection |
| TAG02 | tag-with-or-and | βœ… | easy | safe | ⬜ | `NormalizeTags` (case normalization) |
| TAG03 | tag-with-reserved-word | ❌ | hard | unsafe | βž– | |
| TAG05 | could-be-test-tags | ❌ | hard | unsafe | βž– | multi-location refactor |
| TAG06 | tag-already-set-in-test-tags | βœ… | easy | safe | βœ… | #1851 |
| TAG07 | unnecessary-default-tags | βœ… | easy | safe | βœ… | #1850 |
| TAG08 | empty-tags | βœ… | easy | safe | βœ… | #1842 |
| TAG09 | duplicated-tags | βœ… | easy | safe | βœ… | #1851, `NormalizeTags` stays (case normalization) |
| TAG10 | could-be-keyword-tags | ❌ | hard | unsafe | βž– | multi-location refactor |
| TAG11 | tag-already-set-in-keyword-tags | βœ… | easy | safe | βœ… | #1851 |
| TAG12 | redundant-continue-on-failure | βœ… | easy | safe | βœ… | #1868; unwraps redundant BuiltIn wrapper when effective tags preserve behavior |

### VAR

| Rule | Name | Fix | Difficulty | Safety | Status | Notes |
| --- | --- | --- | --- | --- | --- | --- |
| VAR01 | empty-variable | βœ… | easy | safe | βœ… | #1852, `ReplaceEmptyValues` removed in #1853 |
| VAR02 | unused-variable | ❌ | hard | unsafe | βž– | |
| VAR03 | variable-overwritten-before-usage | βœ… | mid | safe | ⬜ | replace the unused assignment with `${_}` |
| VAR04 | no-global-variable | ❌ | hard | unsafe | βž– | `ReplaceWithVAR` partially covers it |
| VAR05 | no-suite-variable | ❌ | hard | unsafe | βž– | `ReplaceWithVAR` partially covers it |
| VAR06 | no-test-variable | ❌ | hard | unsafe | βž– | `ReplaceWithVAR` partially covers it |
| VAR07 | non-local-variables-should-be-uppercase | βœ… | easy | safe | ⬜ | `RenameVariables` |
| VAR08 | possible-variable-overwriting | βœ… | mid | unsafe | ⬜ | `RenameVariables` |
| VAR09 | hyphen-in-variable-name | βœ… | easy | unsafe | ⬜ | renaming changes the variable name |
| VAR10 | inconsistent-variable-name | βœ… | mid | unsafe | ⬜ | `RenameVariables` |
| VAR11 | overwriting-reserved-variable | ❌ | hard | unsafe | βž– | |
| VAR12 | duplicated-assigned-var-name | βœ… | easy | safe | ⬜ | |
| VAR13 | automatic-variable-not-available | ❌ | - | - | βž– | requires choosing a replacement or moving code based on intent; no safe automatic fix (#1867) |

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up β€” it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the rule tables and the `check --fix` versus `robocop format` scope, then select one rule marked ⬜ whose fix is sufficiently bounded. Done means the selected rule's automatic fix is implemented consistently with its listed safety and difficulty, and its status is updated in this meta ticket.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.