feat: Fix rule where it is being report (#1630)

Add another method for registering the fix for the rule - when reporting the issue itself. Is is better to use fix() method in rule (because it's lazily loaded) but in some cases we may need access to more data not available in diagnostic itself. For those sitations we can now pass Fix() instance when reporting the issue.

Author: bhirsz

docs/linter/custom_rules.md

@@ -316,6 +316,7 @@ When you create a rule that can apply an automatic fix, you need to:
 - inherit from ``FixableRule``
 - specify fix_availability (which can be ``FixAvailability.ALWAYS`` or ``FixAvailability.SOMETIMES``)
 - implement ``fix()`` method and return ``Fix`` instance or ``None`` when no fix can be generated
+- or instead of implementing ``fix()`` method create ``Fix`` instance and pass it when reporting the issue
 
 A ``Fix`` contains a list of ``TextEdit`` entries that describe which lines in the original file should be replaced.
 The source file is processed as a list of lines, preserving newline characters (``\n``). If you replace one or more lines
@@ -342,10 +343,62 @@ def fix(self, diag: Diagnostic, source_lines: list[str]) -> Fix | None:
     )
 ```
 
-The ``fix()`` method gets the diagnostic object. You can also read the contents of  the source file from the
+The ``fix()`` method gets the diagnostic object. You can also read the contents of the source file from the
 ``source_lines`` list.
 
-If, for a particular case, it is not possible to create a valid fix, the ``fix()`` method should return None.
+If, for a particular case, it is not possible to create a valid fix, the  ``fix()`` method should return None.
+
+It is also possible to create ``Fix`` when reporting the issue. The main benefit is that ``fix()`` method only
+receives diagnostic range and raw source lines, and you may have access to more data on how to fix the issue in place
+where you found the issue.
+
+The following custom rule checks if the file contains the ``PLACEHOLD`` string at the end of file.
+The issue is reported at the the end of the first section, so ``fix()`` method can't fix the issue. That's why
+we can create ``Fix`` and pass it to ``report()`` method instead:
+
+```python
+from robot.parsing.model.statements import Error
+
+from robocop.linter.fix import Fix, FixAvailability, FixApplicability, TextEdit
+from robocop.linter.rules import FixableRule, RuleSeverity, VisitorChecker
+
+
+class CustomWithFix(FixableRule):
+    """
+    Custom rule that does have a fix.
+
+    The fix is available only when reporting the issue.
+    """
+    name = "fixable-rule"
+    rule_id = "FIX01"
+    message = "Custom rule message"
+    severity = RuleSeverity.INFO
+    added_in_version = "8.0.0"
+    fix_availability = FixAvailability.ALWAYS
+
+
+class CustomChecker(VisitorChecker):
+    fixable_rule: CustomWithFix
+
+    def visit_File(self, node):
+        if isinstance(node.sections[0].body[-1], Error):  # placeholder is not recognized as valid statement
+            return
+        fix = Fix(
+            edits=[
+                TextEdit(rule_id=self.fixable_rule.rule_id,
+                         rule_name=self.fixable_rule.name,
+                         start_line=node.end_lineno,
+                         end_line=node.end_lineno,
+                         start_col=node.end_col_offset + 1,
+                         end_col=node.end_col_offset + 1,
+                         replacement="PLACEHOLDER"
+                         )
+            ],
+            message="Replace last character of file with 'PLACEHOLDER'",
+            applicability=FixApplicability.SAFE
+        )
+        self.report(self.fixable_rule, lineno=node.lineno, col=node.col_offset + 1, fix=fix)
+```
 
 ## Project checks
 
@@ -361,7 +414,7 @@ be used to run any code, for example, analysis of the project dependencies and a
 Example project checker:
 
 ```python title="project_checker.py"
-from robocop.config import ConfigManager
+from robocop.config_manager import ConfigManager
 from robocop.linter.rules import Rule, ProjectChecker, RuleSeverity
 
 

src/robocop/linter/diagnostics.py

@@ -73,6 +73,7 @@ def __init__(
         node=None,
         extended_disablers: tuple[int, int] | None = None,
         sev_threshold_value: int | None = None,
+        fix: Fix | None = None,
         **kwargs,
     ) -> None:
         self.rule = rule
@@ -82,15 +83,13 @@ def __init__(
         self.extended_disablers = extended_disablers if extended_disablers else []
         self.reported_arguments = kwargs
         self.severity = rule.get_severity_with_threshold(sev_threshold_value)
+        self.fix = fix
         self._message = None
 
     @property
     def message(self) -> str:
         return self.rule.message.format(**self.reported_arguments)
 
-    def fix(self, source_lines: list[str]) -> Fix | None:
-        return self.rule.fix(self, source_lines)
-
     @staticmethod
     def get_range(
         lineno: int, col: int, end_lineno: int | None, end_col: int | None, node: type[Statement | Block] | None

src/robocop/linter/fix.py

@@ -160,8 +160,11 @@ def apply_fixes(self, source_file: SourceFile, fixes: list[Fix]) -> bool:
         applicable_fixes = [
             fix
             for fix in fixes
-            if fix.applicability == FixApplicability.SAFE
-            or (fix.applicability == FixApplicability.UNSAFE and allow_unsafe)
+            if fix
+            and (
+                fix.applicability == FixApplicability.SAFE
+                or (fix.applicability == FixApplicability.UNSAFE and allow_unsafe)
+            )
         ]
 
         # Collect all edits from safe fixes

src/robocop/linter/rules/__init__.py

@@ -28,7 +28,6 @@
 import inspect
 import pkgutil
 import sys
-from abc import ABC, abstractmethod
 from collections import defaultdict
 from enum import Enum
 from functools import total_ordering
@@ -40,7 +39,7 @@
 
 from robocop import __version__, exceptions
 from robocop.linter.diagnostics import Diagnostic
-from robocop.linter.fix import FixAvailability
+from robocop.linter.fix import Fix, FixAvailability
 from robocop.version_handling import Version, VersionSpecifier
 
 try:
@@ -482,7 +481,7 @@ def fix(self, diag: Diagnostic, source_lines: list[str]) -> Fix | None:  # noqa:
         return None
 
 
-class FixableRule(Rule, ABC):
+class FixableRule(Rule):
     """
     Abstract base class for rules that can automatically fix issues.
 
@@ -493,7 +492,6 @@ class FixableRule(Rule, ABC):
     fix_availability: FixAvailability
     fixable: bool = True
 
-    @abstractmethod
     def fix(self, diag: Diagnostic, source_lines: list[str]) -> Fix | None:
         """
         Generate TextEdit to fix the issue or return None if no fix available.
@@ -540,6 +538,7 @@ def report(
         extended_disablers: tuple[int, int] | None = None,
         sev_threshold_value: int | None = None,
         source: SourceFile | None = None,
+        fix: Fix | None = None,
         **kwargs,
     ) -> None:
         if not rule.enabled:
@@ -561,6 +560,7 @@ def report(
             source=source or self.source_file,
             extended_disablers=extended_disablers,
             sev_threshold_value=sev_threshold_value,
+            fix=fix,
             **kwargs,
         )
         self.issues.append(diagnostic)

src/robocop/linter/runner.py

@@ -184,7 +184,7 @@ def run_check(self, source_file: SourceFile, fix_applier: FixApplier = None) ->
             fix_applier.fix_stats.total_fixes += max(prev_fixable - len(fixable_diagnostics), 0)
             prev_fixable = len(fixable_diagnostics)
             # Collect fixes from diagnostics
-            fixes = [diag.fix(source_file) for diag in fixable_diagnostics]
+            fixes = [diag.fix or diag.rule.fix(diag, source_file) for diag in fixable_diagnostics]
             if not fix_applier.apply_fixes(source_file, fixes):
                 break
         if source_file.config.linter.fix and not source_file.config.linter.diff:

src/robocop/source_file.py

@@ -69,6 +69,7 @@ def source_lines(self) -> list[str]:
             list[str]: A list of source code lines.
 
         """
+        # TODO: potential issue: robotcode send model with the updated code, but the file is not saved to disk yet
         if self._source_lines is None:
             try:
                 self._source_lines = self._read_lines()

tests/linter/rules/custom_tests/rules_with_fix/custom_rules.py

@@ -0,0 +1,43 @@
+from robot.parsing.model.statements import Error
+
+from robocop.linter.fix import Fix, FixApplicability, FixAvailability, TextEdit
+from robocop.linter.rules import FixableRule, RuleSeverity, VisitorChecker
+
+
+class CustomWithFix(FixableRule):
+    """
+    Custom rule that does have a fix.
+
+    The fix is available only when reporting the issue.
+    """
+
+    name = "fixable-rule"
+    rule_id = "FIX01"
+    message = "Custom rule message"
+    severity = RuleSeverity.INFO
+    added_in_version = "8.0.0"
+    fix_availability = FixAvailability.ALWAYS
+
+
+class CustomChecker(VisitorChecker):
+    fixable_rule: CustomWithFix
+
+    def visit_File(self, node):  # noqa: N802
+        if isinstance(node.sections[0].body[-1], Error):  # placeholder added in first run
+            return
+        fix = Fix(
+            edits=[
+                TextEdit(
+                    rule_id=self.fixable_rule.rule_id,
+                    rule_name=self.fixable_rule.name,
+                    start_line=node.end_lineno,
+                    end_line=node.end_lineno,
+                    start_col=node.end_col_offset + 1,
+                    end_col=node.end_col_offset + 1,
+                    replacement="PLACEHOLDER",
+                )
+            ],
+            message="Replace last character of file with 'PLACEHOLDER'",
+            applicability=FixApplicability.SAFE,
+        )
+        self.report(self.fixable_rule, lineno=node.lineno, col=node.col_offset + 1, fix=fix)

tests/linter/rules/custom_tests/rules_with_fix/expected_fixed/expected_output.txt

@@ -0,0 +1,5 @@
+Fixed 1 issue:
+- actual_fixed${/}test.robot:
+    1 x FIX01 (fixable-rule)
+
+Found 1 issue (1 fixed, 0 remaining).

tests/linter/rules/custom_tests/rules_with_fix/expected_fixed/test.robot

@@ -0,0 +1,3 @@
+*** Settings ***
+Documentation    Last line should have PLACEHOLDER added.
+PLACEHOLDER

tests/linter/rules/custom_tests/rules_with_fix/test.robot

@@ -0,0 +1,2 @@
+*** Settings ***
+Documentation    Last line should have PLACEHOLDER added.