Ensure line and column numbers start with 1 (#1556)

src/ansiblelint/errors.py

@@ -26,7 +26,9 @@ class MatchError(ValueError):
     def __init__(
         self,
         message: Optional[str] = None,
-        linenumber: int = 0,
+        # most linters report use (1,1) base, including yamllint and flake8
+        # we should never report line 0 or column 0 in output.
+        linenumber: int = 1,
         column: Optional[int] = None,
         details: str = "",
         filename: Optional[Union[str, Lintable]] = None,
@@ -43,6 +45,17 @@ def __init__(
             )
 
         self.message = message or getattr(rule, 'shortdesc', "")
+
+        # Safety measture to ensure we do not endup with incorrect indexes
+        if linenumber == 0:
+            raise RuntimeError(
+                "MatchError called incorrectly as line numbers start with 1"
+            )
+        if column == 0:
+            raise RuntimeError(
+                "MatchError called incorrectly as column numbers start with 1"
+            )
+
         self.linenumber = linenumber
         self.column = column
         self.details = details

src/ansiblelint/rules/AnsibleSyntaxCheckRule.py

@@ -63,7 +63,7 @@ def _get_ansible_syntax_check_matches(lintable: Lintable) -> List[MatchError]:
         if run.returncode != 0:
             message = None
             filename = str(lintable.path)
-            linenumber = 0
+            linenumber = 1
             column = None
             tag = None
 
@@ -133,7 +133,7 @@ def test_empty_playbook() -> None:
         """Validate detection of empty-playbook."""
         lintable = Lintable('examples/playbooks/empty_playbook.yml', kind='playbook')
         result = AnsibleSyntaxCheckRule._get_ansible_syntax_check_matches(lintable)
-        assert result[0].linenumber == 0
+        assert result[0].linenumber == 1
         # We internally convert absolute paths returned by ansible into paths
         # relative to current directory.
         assert result[0].filename.endswith("/empty_playbook.yml")

src/ansiblelint/rules/__init__.py

@@ -51,7 +51,7 @@ def unjinja(text: str) -> str:
     def create_matcherror(
         self,
         message: Optional[str] = None,
-        linenumber: int = 0,
+        linenumber: int = 1,
         details: str = "",
         filename: Optional[Union[str, Lintable]] = None,
         tag: str = "",

test/TestExamples.py

@@ -22,23 +22,23 @@ def test_example(default_rules_collection):
 
 
 @pytest.mark.parametrize(
-    "filename",
+    ("filename", "line", "column"),
     (
         pytest.param(
-            'examples/playbooks/syntax-error-string.yml', id='syntax-error-string'
+            'examples/playbooks/syntax-error-string.yml', 1, 1, id='syntax-error-string'
         ),
-        pytest.param('examples/playbooks/syntax-error.yml', id='syntax-error'),
+        pytest.param('examples/playbooks/syntax-error.yml', 2, 3, id='syntax-error'),
     ),
 )
-def test_example_syntax_error(default_rules_collection, filename):
+def test_example_syntax_error(default_rules_collection, filename, line, column):
     """Validates that loading valid YAML string produce error."""
     result = Runner(filename, rules=default_rules_collection).run()
-    assert len(result) >= 1
-    passed = False
-    for match in result:
-        if match.rule.id == "syntax-check":
-            passed = True
-    assert passed, result
+    assert len(result) == 1
+    assert result[0].rule.id == "syntax-check"
+    # This also ensures that line and column numbers start at 1, so they
+    # match what editors will show (or output from other linters)
+    assert result[0].linenumber == line
+    assert result[0].column == column
 
 
 def test_example_custom_module(default_rules_collection):

test/TestUtils.py

@@ -315,7 +315,7 @@ def test_cli_auto_detect(capfd):
     assert "Discovering files to lint: git ls-files -z" in err
     # An expected rule match from our examples
     assert (
-        "examples/playbooks/empty_playbook.yml:0: "
+        "examples/playbooks/empty_playbook.yml:1: "
         "syntax-check Empty playbook, nothing to do" in out
     )
     # assures that our .ansible-lint exclude was effective in excluding github files