Repository: MarketSquare/robotframework-robocop (python)

Issue:
[Bug] robocop-robotidy file level disabler strangeness
### What happened?

File-level disabler should be the very first line of the given robot file without empty line before *** Settings *** in this case it is working fine.
Working format:
```
# robocop: disable=0527,1004
*** Settings ***
Documentation    asdfasdf
```
But if I add an empy line between the disabler line and the section header, although the defined rules are disabled, but I get an 'ignored-data' (0704) finding:

```
# robocop: disable=0527,1004

*** Settings ***
Documentation    asdfasdf
```

If I run robotidy the file-level disabler will be added to a *** Comments *** section.
And after I rerun the robocop, it will not be recognized as a file-level disabler.

```
*** Comments ***
# robocop: disable=0527,1004

*** Settings ***
Documentation    asdfasdf
```

I tried only Robocop 5.0.0, the earlier version might affected as well.

### What command/code did you try to run?

Regular command execution.

### What is the full error message?

There is no error message, the regular finding messages will be printed.

### What did you expect to happen instead?

Either the robotidy should jump over the file-level robocop disablers (not put it under a *** Comments *** section),
or robocop should handle the disablers as file-level disablers when it can be found under the *** Comments *** section.

And should not be reported as ignore-data if there are empty lines between the file-level disabler and the first section.

### Operating System

Linux

### Robocop version

5.0.0

Pull request description:
Disable Robocop in the whole file from first comment section
Closes #1044

Patch:
diff --git a/docs/disablers.rst b/docs/disablers.rst
index 1d1b608..79ca11a 100644
--- a/docs/disablers.rst
+++ b/docs/disablers.rst
@@ -78,7 +78,8 @@ block (such as keyword, test case, "for" and "while" loops and "if" statement).
 Disabling files
 ---------------
 
-It is possible to ignore whole file if you start file with ``# robocop: off``.
+It is possible to ignore whole file if you put Robocop disabler in the first comment section, at the beginning of the
+file.
 
 ..  code-block:: robotframework
     :caption: example.robot
diff --git a/docs/releasenotes/unreleased/other.1.rst b/docs/releasenotes/unreleased/other.1.rst
new file mode 100644
index 0000000..1931c90
--- /dev/null
+++ b/docs/releasenotes/unreleased/other.1.rst
@@ -0,0 +1,21 @@
+Robocop file disablers can be defined anywhere in the first comment section (#1044)
+-----------------------------------------------------------------------------------
+
+It was possible to disable Robocop for selected or all rules in the given file. However such disablers had to be defined
+in the the first line of the file::
+
+    # robocop: off
+
+    *** Test Cases ***
+    Test
+        Step
+
+It is now also possible to define file-level disablers anywhere in the first comment section::
+
+    # robocop: off=onerule
+    # explanation why the rule is disabled
+    # robocop: off=onerule2
+
+    *** Test Cases ***
+    Test
+        Step
diff --git a/robocop/utils/disablers.py b/robocop/utils/disablers.py
index c59760e..b09ce5d 100644
--- a/robocop/utils/disablers.py
+++ b/robocop/utils/disablers.py
@@ -4,8 +4,10 @@ Collection of classes for detecting checker disablers (like # robocop: disable)
 import re
 from collections import defaultdict
 from copy import deepcopy
+from typing import List, Optional
 
 from robot.api import Token
+from robot.parsing.model.blocks import CommentSection
 
 try:
     from robot.api.parsing import ModelVisitor
@@ -16,53 +18,48 @@ except ImportError:
 class DisablersInFile:  # pylint: disable=too-few-public-methods
     """Container for file disablers"""
 
-    def __init__(self):
+    def __init__(self, blocks: Optional[List] = None):
         self.lastblock = -1
         self.lines = set()
-        self.blocks = []
+        self.blocks = [] if not blocks else blocks
 
     def copy(self):
         """Used by defaultdict to create new instance for every new key in disablers container"""
         return deepcopy(self)
 
 
-class DisablersFinder(ModelVisitor):
-    """Visit and find robocop disablers in Robot Framework file."""
-
+class DisablersVisitor(ModelVisitor):
     ENABLERS = {"enable", "on"}
     DISABLERS = {"disable", "off"}
 
     def __init__(self, model):
         self.file_disabled = False
+        self.file_end = 1
+        self.is_first_comment_section = True
         self.keyword_or_test_section = False
         self.last_name_header_line = 0
-        self.rules_disabled_in_file = set()
         self.disablers_in_scope = []
         self.disabler_pattern = re.compile(r"robocop: ?(?P<disabler>disable|off|enable|on)=?(?P<rules>[\w\-,]*)")
         self.rules = defaultdict(DisablersInFile().copy)
         self.visit(model)
 
-    @property
-    def any_disabler(self):
-        return len(self.rules) != 0
-
     def visit_File(self, node):  # noqa
+        self.file_end = node.end_lineno
         self.generic_visit(node)
-        for rule in self.rules_disabled_in_file:
-            self.rules[rule].blocks.append((1, node.end_lineno))
-        self.file_disabled = self._is_file_disabled(node.end_lineno)
 
     def parse_disablers_in_node(self, node, last_line=None):
         self.disablers_in_scope.append(defaultdict(DisablersInFile().copy))
         self.generic_visit(node)
         for rule_name, rule_disabler in self.disablers_in_scope[-1].items():
-            if rule_disabler.lastblock == 1:  # disabler in first line, never enabled again -> file disabler
-                self.rules_disabled_in_file.add(rule_name)
+            if self.is_first_comment_section:
+                if rule_name == "all":
+                    self.file_disabled = True
+                self.rules[rule_name] = DisablersInFile(blocks=[(1, self.file_end)])
             else:
                 last_line = node.end_lineno if last_line is None else last_line
                 self._end_block(self.disablers_in_scope[-1], rule_name, last_line)
-            self.rules[rule_name].blocks.extend(rule_disabler.blocks)
-            self.rules[rule_name].lines.update(rule_disabler.lines)
+                self.rules[rule_name].blocks.extend(rule_disabler.blocks)
+                self.rules[rule_name].lines.update(rule_disabler.lines)
         self.disablers_in_scope.pop()
 
     def visit_KeywordSection(self, node):  # noqa
@@ -73,7 +70,9 @@ class DisablersFinder(ModelVisitor):
     visit_TestCaseSection = visit_KeywordSection
 
     def visit_Section(self, node):  # noqa
+        self.is_first_comment_section = self.is_first_comment_section and isinstance(node, CommentSection)
         self.parse_disablers_in_node(node)
+        self.is_first_comment_section = False
 
     visit_TestCase = visit_Keyword = visit_Try = visit_For = visit_ForLoop = visit_While = visit_Section
 
@@ -98,27 +97,6 @@ class DisablersFinder(ModelVisitor):
             is_inline = comment.lineno == self.last_name_header_line
             self.parse_comment_token(comment, is_inline=is_inline)
 
-    def is_rule_disabled(self, rule_msg):
-        """
-        Check if given `rule_msg` is disabled. All takes precedence, then line disablers, then block disablers.
-        We're checking for both message id and name.
-        """
-        if not self.any_disabler:
-            return False
-        return any(
-            self.is_line_disabled(line, rule)
-            for line in (rule_msg.line, *rule_msg.extended_disablers)
-            for rule in ("all", rule_msg.rule_id, rule_msg.name)
-        )
-
-    def is_line_disabled(self, line, rule):
-        """Helper method for is_rule_disabled that check if given line is in range of any disabled block"""
-        if rule not in self.rules:
-            return False
-        if line in self.rules[rule].lines:
-            return True
-        return any(block[0] <= line <= block[1] for block in self.rules[rule].blocks)
-
     def parse_comment_token(self, token, is_inline):
         if "#" not in token.value:
             return
@@ -148,20 +126,6 @@ class DisablersFinder(ModelVisitor):
             return self.disablers_in_scope[0]
         return self.disablers_in_scope[-1]
 
-    def _is_file_disabled(self, last_line):
-        """
-        The file is disabled if all rules are disabled in every line - we need to iterate every block to see
-        if they are linked from first to last line without breaks.
-        """
-        if "all" not in self.rules:
-            return False
-        prev_end = 1
-        for block in self.rules["all"].blocks:
-            if prev_end != block[0]:
-                return False
-            prev_end = block[1]
-        return prev_end == last_line
-
     def _add_inline_disabler(self, rule, lineno):
         self.rules[rule].lines.add(lineno)
 
@@ -184,3 +148,42 @@ class DisablersFinder(ModelVisitor):
             if rule == "all":
                 continue  # to avoid recursion
             self._end_block(scope, rule, lineno)
+
+
+class DisablersFinder(ModelVisitor):
+    """Visit and find robocop disablers in Robot Framework file."""
+
+    ENABLERS = {"enable", "on"}
+    DISABLERS = {"disable", "off"}
+
+    def __init__(self, model):
+        self.disabled = DisablersVisitor(model)
+
+    @property
+    def any_disabler(self):
+        return len(self.disabled.rules) != 0
+
+    @property
+    def file_disabled(self):
+        return self.disabled.file_disabled
+
+    def is_rule_disabled(self, rule_msg):
+        """
+        Check if given `rule_msg` is disabled. All takes precedence, then line disablers, then block disablers.
+        We're checking for both message id and name.
+        """
+        if not self.any_disabler:
+            return False
+        return any(
+            self.is_line_disabled(line, rule)
+            for line in (rule_msg.line, *rule_msg.extended_disablers)
+            for rule in ("all", rule_msg.rule_id, rule_msg.name)
+        )
+
+    def is_line_disabled(self, line, rule):
+        """Helper method for is_rule_disabled that check if given line is in range of any disabled block"""
+        if rule not in self.disabled.rules:
+            return False
+        if line in self.disabled.rules[rule].lines:
+            return True
+        return any(block[0] <= line <= block[1] for block in self.disabled.rules[rule].blocks)


Test patch:
diff --git a/tests/utest/test_disablers.py b/tests/utest/test_disablers.py
index 3486e07..94185d4 100644
--- a/tests/utest/test_disablers.py
+++ b/tests/utest/test_disablers.py
@@ -113,5 +113,5 @@ class TestDisablers:
             "rule3": [(22, 29), (55, 62)],
             "rule4": [(24, 25), (57, 58)],
         }
-        disabled_rules = {rule_name: sorted(rule.blocks) for rule_name, rule in disabler.rules.items()}
+        disabled_rules = {rule_name: sorted(rule.blocks) for rule_name, rule in disabler.disabled.rules.items()}
         assert disabled_rules == exp_disabled_rules