Repository: MarketSquare/robotframework-robocop (python)

Issue:
Don't sort `ProjectChecker` messages by line number
`ProjectChecker` messages are sorted [here](https://github.com/MarketSquare/robotframework-robocop/blob/master/robocop/run.py#L156) and because of the [comparison function](https://github.com/MarketSquare/robotframework-robocop/blob/master/robocop/rules.py#L497-L502) for messages they get sorted by line number, column and then rule id.

I don't think this makes sense specifically for rules that operate on the whole project. I would prefer the checker output didn't get sorted at all in `run_project_checks()` and so the order would be up to the checker. But sorting by source path first would also be an improvement.

Pull request:
Re-add ProjectChecker
ProjectChecker was temporarily removed during merge of the tools. It should now work again. Due to change when the diagnostics are processed (at the end of whole run), we also fixed the bug where project-level issues were sorted by line for all files.

Fixes #1121 

Patch:
diff --git a/docs/releasenotes/6.0.0a3.rst b/docs/releasenotes/6.0.0a3.rst
index 26506a8..d22bee7 100644
--- a/docs/releasenotes/6.0.0a3.rst
+++ b/docs/releasenotes/6.0.0a3.rst
@@ -947,3 +947,11 @@ Following code will now not raise an issue::
 
 Since we don't track variable values in linting, we do not check if ``${name}`` use upper case or not - such cases
 are ignored.
+
+ProjectChecker rules sorted by line number (#1121)
+--------------------------------------------------
+
+Project-level rules were previous reported after other runs. Sorting them by line numbers caused issues from several
+files to mix up.
+
+Now project-level rules are grouped and sorted together with other rules which fixes this issue.
diff --git a/src/robocop/config.py b/src/robocop/config.py
index 9e67fe3..74bd937 100644
--- a/src/robocop/config.py
+++ b/src/robocop/config.py
@@ -21,7 +21,7 @@ from robocop.formatter import formatters
 from robocop.formatter.skip import SkipConfig
 from robocop.formatter.utils import misc  # TODO merge with linter misc
 from robocop.linter import exceptions, rules
-from robocop.linter.rules import BaseChecker, RuleSeverity
+from robocop.linter.rules import BaseChecker, ProjectChecker, RuleSeverity
 from robocop.linter.utils.misc import compile_rule_pattern
 from robocop.linter.utils.version_matching import Version
 
@@ -192,6 +192,7 @@ class LinterConfig:
     return_result: bool = False
     config_source: str = field(default="cli", compare=False)
     _checkers: list[BaseChecker] | None = field(default=None, compare=False)
+    _project_checkers: list[BaseChecker] | None = field(default=None, compare=False)
     _rules: dict[str, Rule] | None = field(default=None, compare=False)
 
     def __post_init__(self):
@@ -199,11 +200,19 @@ class LinterConfig:
             self.target_version = ROBOT_VERSION
 
     @property
-    def checkers(self):
+    def checkers(self) -> list[BaseChecker]:
         if self._checkers is None:
             self.load_configuration()
         return self._checkers
 
+    @property
+    def project_checkers(self) -> list[ProjectChecker]:
+        if self._project_checkers is None:
+            self._project_checkers = [
+                checker for checker in self.checkers if not checker.disabled and isinstance(checker, ProjectChecker)
+            ]
+        return self._project_checkers
+
     @property
     def rules(self):
         if self._rules is None:
diff --git a/src/robocop/linter/rules/__init__.py b/src/robocop/linter/rules/__init__.py
index eb2d326..84e5646 100644
--- a/src/robocop/linter/rules/__init__.py
+++ b/src/robocop/linter/rules/__init__.py
@@ -520,6 +520,7 @@ class BaseChecker:
         extended_disablers: tuple[int, int] | None = None,
         sev_threshold_value: int | None = None,
         source: str | None = None,
+        ast_model: File | None = None,
         **kwargs,
     ) -> None:
         if not rule.enabled:
@@ -534,7 +535,7 @@ class BaseChecker:
         diagnostic = Diagnostic(
             rule=rule,
             node=node,
-            model=self.ast_model,
+            model=ast_model or self.ast_model,
             lineno=lineno,
             col=col,
             end_lineno=end_lineno,
diff --git a/src/robocop/linter/rules/usage.py b/src/robocop/linter/rules/usage.py
index 9b91a8b..7639539 100644
--- a/src/robocop/linter/rules/usage.py
+++ b/src/robocop/linter/rules/usage.py
@@ -87,6 +87,7 @@ class KeywordDefinition:
 @dataclass
 class RobotFile:
     path: str
+    ast_model: File
     is_suite: bool = False
     normal_keywords: dict[str, KeywordDefinition] = field(default_factory=dict)
     embedded_keywords: dict[str, KeywordDefinition] = field(default_factory=dict)
@@ -150,6 +151,7 @@ class UnusedKeywords(ProjectChecker):
                 self.report(
                     self.unused_keyword,
                     source=robot_file.path,
+                    ast_model=robot_file.ast_model,
                     node=keyword.keyword_node,
                     keyword_name=name,
                     end_col=len(name) + 1,
@@ -157,7 +159,7 @@ class UnusedKeywords(ProjectChecker):
         return self.issues
 
     def visit_File(self, node: File) -> None:  # noqa: N802
-        self.current_file = RobotFile(node.source)  # TODO: handle "-"
+        self.current_file = RobotFile(str(node.source), node)  # TODO: handle "-"
         self.generic_visit(node)
         self.files[self.current_file.path] = self.current_file
 
diff --git a/src/robocop/linter/runner.py b/src/robocop/linter/runner.py
index e2cecc9..93ea052 100644
--- a/src/robocop/linter/runner.py
+++ b/src/robocop/linter/runner.py
@@ -56,6 +56,9 @@ class RobocopLinter:
             diagnostics = self.run_check(model, source, config)
             issues_no += len(diagnostics)
             self.diagnostics.extend(diagnostics)
+        # several configs, each can have or have not project checkers..
+        # so we would need to first gather info, and then "mamy
+        self.diagnostics.extend(self.run_project_checks())
         if not files:
             print("No Robot files were found with the existing configuration.")
         if "file_stats" in self.reports:
@@ -93,6 +96,18 @@ class RobocopLinter:
             ]
         return found_diagnostics
 
+    def run_project_checks(self) -> list[Diagnostic]:
+        found_diagnostics = []
+        for checker in self.config_manager.default_config.linter.project_checkers:
+            found_diagnostics.extend(
+                [
+                    diagnostic
+                    for diagnostic in checker.scan_project()
+                    if not diagnostic.severity < self.config_manager.default_config.linter.threshold
+                ]
+            )
+        return found_diagnostics
+
     def return_with_exit_code(self, issues_count: int) -> NoReturn:
         """
         Exit the Robocop with exit code.


Test patch:
diff --git a/tests/linter/rules/keywords/unused_keyword/expected_extended.txt b/tests/linter/rules/keywords/unused_keyword/expected_extended.txt
index e69de29..c7249ab 100644
--- a/tests/linter/rules/keywords/unused_keyword/expected_extended.txt
+++ b/tests/linter/rules/keywords/unused_keyword/expected_extended.txt
@@ -0,0 +1,29 @@
+resource.robot:10:1 KW04 Keyword 'Private Not Used Keyword' is not used
+    |
+ 10 | Private Not Used Keyword
+    | ^^^^^^^^^^^^^^^^^^^^^^^^ KW04
+ 11 |     [Tags]    robot:private
+ 12 |     No Operation
+    |
+
+suite.robot:19:1 KW04 Keyword 'Not Used Keyword' is not used
+    |
+ 19 | Not Used Keyword
+    | ^^^^^^^^^^^^^^^^ KW04
+ 20 |     Nested Not Used Keyword
+    |
+
+suite.robot:34:1 KW04 Keyword 'Embedded ${not} Used' is not used
+    |
+ 34 | Embedded ${not} Used
+    | ^^^^^^^^^^^^^^^^^^^^ KW04
+ 35 |     No Operation
+    |
+
+tasks.robot:16:1 KW04 Keyword 'Not Used Keyword' is not used
+    |
+ 16 | Not Used Keyword
+    | ^^^^^^^^^^^^^^^^ KW04
+ 17 |     Nested Not Used Keyword
+    |
+
diff --git a/tests/linter/rules/keywords/unused_keyword/test_rule.py b/tests/linter/rules/keywords/unused_keyword/test_rule.py
index 315c685..f283e09 100644
--- a/tests/linter/rules/keywords/unused_keyword/test_rule.py
+++ b/tests/linter/rules/keywords/unused_keyword/test_rule.py
@@ -1,10 +1,7 @@
-import pytest
-
 from tests.linter.utils import RuleAcceptance
 
 
 class TestRuleAcceptance(RuleAcceptance):
-    @pytest.mark.xfail(reason="Project checker needs to be reimplemented")  # FIXME:
     def test_rule(self):
         self.check_rule(src_files=["."], expected_file="expected_output.txt", issue_format="end_col")