Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure line and column numbers start with 1 #1556

Merged
merged 1 commit into from May 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/ansiblelint/errors.py
Expand Up @@ -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,
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/ansiblelint/rules/AnsibleSyntaxCheckRule.py
Expand Up @@ -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

Expand Down Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/rules/__init__.py
Expand Up @@ -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 = "",
Expand Down
20 changes: 10 additions & 10 deletions test/TestExamples.py
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion test/TestUtils.py
Expand Up @@ -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
Expand Down