diff --git a/src/ansiblelint/errors.py b/src/ansiblelint/errors.py index da5908d359..b4f59aa4c0 100644 --- a/src/ansiblelint/errors.py +++ b/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 diff --git a/src/ansiblelint/rules/AnsibleSyntaxCheckRule.py b/src/ansiblelint/rules/AnsibleSyntaxCheckRule.py index 161b911fce..267c2b3b02 100644 --- a/src/ansiblelint/rules/AnsibleSyntaxCheckRule.py +++ b/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") diff --git a/src/ansiblelint/rules/__init__.py b/src/ansiblelint/rules/__init__.py index d5ce489087..287931d9d7 100644 --- a/src/ansiblelint/rules/__init__.py +++ b/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 = "", diff --git a/test/TestExamples.py b/test/TestExamples.py index f3ca348b2e..5f8e3374a5 100644 --- a/test/TestExamples.py +++ b/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): diff --git a/test/TestUtils.py b/test/TestUtils.py index 252bc37a43..088afb6a09 100644 --- a/test/TestUtils.py +++ b/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