Skip to content

Commit

Permalink
Enforce use of lineno variable name inside the library (#3326)
Browse files Browse the repository at this point in the history 
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
@ssbarnea @pre-commit-ci
ssbarnea and pre-commit-ci[bot] committed Apr 24, 2023
1 parent 93887cb commit 6d037c5
Show file tree
Hide file tree
Showing 31 changed files with 144 additions and 138 deletions.
6 changes: 6 additions & 0 deletions pyproject.toml
View file
Expand Up @@ -126,6 +126,12 @@ extension-pkg-allow-list = ["black.parsing"]
preferred-modules = ["py:pathlib", "unittest:pytest"]

[tool.pylint.MASTER]
bad-names = [
# spell-checker:ignore linenumber
"linenumber", # use lineno instead
"line_number", # use lineno instead

]
# pylint defaults + f,fh,v,id
good-names = ["i", "j", "k", "ex", "Run", "_", "f", "fh", "v", "id", "T"]
# Ignore as being generated:
Expand Down
14 changes: 7 additions & 7 deletions src/ansiblelint/errors.py
View file
Expand Up @@ -45,7 +45,7 @@ def __init__(
message: str | None = None,
# 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,
lineno: int = 1,
column: int | None = None,
details: str = "",
filename: Lintable | None = None,
Expand All @@ -65,7 +65,7 @@ def __init__(
self.message = str(message or getattr(rule, "shortdesc", ""))

# Safety measure to ensure we do not end-up with incorrect indexes
if linenumber == 0: # pragma: no cover
if lineno == 0: # pragma: no cover
raise RuntimeError(
"MatchError called incorrectly as line numbers start with 1",
)
Expand All @@ -74,7 +74,7 @@ def __init__(
"MatchError called incorrectly as column numbers start with 1",
)

self.linenumber = linenumber
self.lineno = lineno
self.column = column
self.details = details
self.filename = ""
Expand Down Expand Up @@ -120,23 +120,23 @@ def __repr__(self) -> str:
_id,
self.message,
self.filename,
self.linenumber,
self.lineno,
self.details,
)

@property
def position(self) -> str:
"""Return error positioning, with column number if available."""
if self.column:
return f"{self.linenumber}:{self.column}"
return str(self.linenumber)
return f"{self.lineno}:{self.column}"
return str(self.lineno)

@property
def _hash_key(self) -> Any:
# line attr is knowingly excluded, as dict is not hashable
return (
self.filename,
self.linenumber,
self.lineno,
str(getattr(self.rule, "id", 0)),
self.message,
self.details,
Expand Down
8 changes: 4 additions & 4 deletions src/ansiblelint/formatters/__init__.py
View file
Expand Up @@ -116,7 +116,7 @@ class AnnotationsFormatter(BaseFormatter): # type: ignore
def format(self, match: MatchError) -> str:
"""Prepare a match instance for reporting as a GitHub Actions annotation."""
file_path = self._format_path(match.filename or "")
line_num = match.linenumber
line_num = match.lineno
severity = match.rule.severity
violation_details = self.escape(match.message)
col = f",col={match.column}" if match.column else ""
Expand Down Expand Up @@ -160,11 +160,11 @@ def format_result(self, matches: list[MatchError]) -> str:
if match.column:
issue["location"]["positions"] = {}
issue["location"]["positions"]["begin"] = {}
issue["location"]["positions"]["begin"]["line"] = match.linenumber
issue["location"]["positions"]["begin"]["line"] = match.lineno
issue["location"]["positions"]["begin"]["column"] = match.column
else:
issue["location"]["lines"] = {}
issue["location"]["lines"]["begin"] = match.linenumber
issue["location"]["lines"]["begin"] = match.lineno
if match.details:
issue["content"] = {}
issue["content"]["body"] = match.details
Expand Down Expand Up @@ -287,7 +287,7 @@ def _to_sarif_result(self, match: MatchError) -> dict[str, Any]:
"uriBaseId": self.BASE_URI_ID,
},
"region": {
"startLine": match.linenumber,
"startLine": match.lineno,
},
},
},
Expand Down
12 changes: 6 additions & 6 deletions src/ansiblelint/rules/__init__.py
View file
Expand Up @@ -75,15 +75,15 @@ def unjinja(text: str) -> str:
def create_matcherror(
self,
message: str | None = None,
linenumber: int = 1,
lineno: int = 1,
details: str = "",
filename: Lintable | None = None,
tag: str = "",
) -> MatchError:
"""Instantiate a new MatchError."""
match = MatchError(
message=message,
linenumber=linenumber,
lineno=lineno,
details=details,
filename=filename,
rule=copy.copy(self),
Expand Down Expand Up @@ -111,8 +111,8 @@ def _enrich_matcherror_with_task_details(
match.task = task
if not match.details:
match.details = "Task/Handler: " + ansiblelint.utils.task_to_str(task)
if match.linenumber < task[LINE_NUMBER_KEY]:
match.linenumber = task[LINE_NUMBER_KEY]
if match.lineno < task[LINE_NUMBER_KEY]:
match.lineno = task[LINE_NUMBER_KEY]

def matchlines(self, file: Lintable) -> list[MatchError]:
matches: list[MatchError] = []
Expand All @@ -134,7 +134,7 @@ def matchlines(self, file: Lintable) -> list[MatchError]:
message = result
matcherror = self.create_matcherror(
message=message,
linenumber=prev_line_no + 1,
lineno=prev_line_no + 1,
details=line,
filename=file,
)
Expand Down Expand Up @@ -204,7 +204,7 @@ def matchtasks(self, file: Lintable) -> list[MatchError]: # noqa: C901
message = result
match = self.create_matcherror(
message=message,
linenumber=task.normalized_task[LINE_NUMBER_KEY],
lineno=task.normalized_task[LINE_NUMBER_KEY],
filename=file,
)

Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/rules/args.py
View file
Expand Up @@ -251,7 +251,7 @@ def _parse_failed_msg(
results.append(
self.create_matcherror(
message=error_message,
linenumber=task[LINE_NUMBER_KEY],
lineno=task[LINE_NUMBER_KEY],
tag="args[module]",
filename=file,
),
Expand Down
8 changes: 4 additions & 4 deletions src/ansiblelint/rules/fqcn.py
View file
Expand Up @@ -135,7 +135,7 @@ def matchtask(
message=f"Use FQCN for builtin module actions ({module}).",
details=f"Use `{module_alias}` or `{legacy_module}` instead.",
filename=file,
linenumber=task["__line__"],
lineno=task["__line__"],
tag="fqcn[action-core]",
),
)
Expand All @@ -146,7 +146,7 @@ def matchtask(
message=f"Use FQCN for module actions, such `{self.module_aliases[module]}`.",
details=f"Action `{module}` is not FQCN.",
filename=file,
linenumber=task["__line__"],
lineno=task["__line__"],
tag="fqcn[action]",
),
)
Expand All @@ -160,7 +160,7 @@ def matchtask(
self.create_matcherror(
message=f"You should use canonical module name `{self.module_aliases[module]}` instead of `{module}`.",
filename=file,
linenumber=task["__line__"],
lineno=task["__line__"],
tag="fqcn[canonical]",
),
)
Expand All @@ -173,7 +173,7 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
return [
self.create_matcherror(
message="Avoid `collections` keyword by using FQCN for all plugins, modules, roles and playbooks.",
linenumber=data[LINE_NUMBER_KEY],
lineno=data[LINE_NUMBER_KEY],
tag="fqcn[keyword]",
filename=file,
),
Expand Down
4 changes: 2 additions & 2 deletions src/ansiblelint/rules/galaxy.py
View file
Expand Up @@ -88,7 +88,7 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
results.append(
self.create_matcherror(
message="galaxy.yaml should have version tag.",
linenumber=data[LINE_NUMBER_KEY],
lineno=data[LINE_NUMBER_KEY],
tag="galaxy[version-missing]",
filename=file,
),
Expand All @@ -103,7 +103,7 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
self.create_matcherror(
message="collection version should be greater than or equal to 1.0.0",
# pylint: disable=protected-access
linenumber=version._line_number,
lineno=version._line_number,
tag="galaxy[version-incorrect]",
filename=file,
),
Expand Down
18 changes: 9 additions & 9 deletions src/ansiblelint/rules/jinja.py
View file
Expand Up @@ -136,7 +136,7 @@ def matchtask( # noqa: C901
result.append(
self.create_matcherror(
message=str(exc),
linenumber=_get_error_line(task, path),
lineno=_get_error_line(task, path),
filename=file,
tag=f"{self.id}[invalid]",
),
Expand All @@ -155,7 +155,7 @@ def matchtask( # noqa: C901
value=v,
reformatted=reformatted,
),
linenumber=_get_error_line(task, path),
lineno=_get_error_line(task, path),
details=details,
filename=file,
tag=f"{self.id}[{tag}]",
Expand Down Expand Up @@ -190,7 +190,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
value=v,
reformatted=reformatted,
),
linenumber=v.ansible_pos[1],
lineno=v.ansible_pos[1],
details=details,
filename=file,
tag=f"{self.id}[{tag}]",
Expand All @@ -199,8 +199,8 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
if raw_results:
lines = file.content.splitlines()
for match in raw_results:
# linenumber starts with 1, not zero
skip_list = get_rule_skips_from_line(lines[match.linenumber - 1])
# lineno starts with 1, not zero
skip_list = get_rule_skips_from_line(lines[match.lineno - 1])
if match.rule.id not in skip_list and match.tag not in skip_list:
results.append(match)
else:
Expand Down Expand Up @@ -388,7 +388,7 @@ def fixture_lint_error_lines() -> list[int]:
collection.register(JinjaRule())
lintable = Lintable("examples/playbooks/jinja-spacing.yml")
results = Runner(lintable, rules=collection).run()
return list(map(lambda item: item.linenumber, results))
return list(map(lambda item: item.lineno, results))

def test_jinja_spacing_playbook(
error_expected_lines: list[int],
Expand All @@ -411,7 +411,7 @@ def test_jinja_spacing_vars() -> None:
error_expected_lineno = [14, 15, 16, 17, 18, 19, 32]
assert len(results) == len(error_expected_lineno)
for idx, err in enumerate(results):
assert err.linenumber == error_expected_lineno[idx]
assert err.lineno == error_expected_lineno[idx]

@pytest.mark.parametrize(
("text", "expected", "tag"),
Expand Down Expand Up @@ -702,10 +702,10 @@ def test_jinja_invalid() -> None:
assert len(errs) == 2
assert errs[0].tag == "jinja[spacing]"
assert errs[0].rule.id == "jinja"
assert errs[0].linenumber == 9
assert errs[0].lineno == 9
assert errs[1].tag == "jinja[invalid]"
assert errs[1].rule.id == "jinja"
assert errs[1].linenumber == 9
assert errs[1].lineno == 9

def test_jinja_valid() -> None:
"""Tests our ability to parse jinja, even when variables may not be defined."""
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/rules/meta_incorrect.py
View file
Expand Up @@ -47,7 +47,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
results.append(
self.create_matcherror(
filename=file,
linenumber=file.data[LINE_NUMBER_KEY],
lineno=file.data[LINE_NUMBER_KEY],
message=f"Should change default metadata: {field}",
),
)
Expand Down
20 changes: 10 additions & 10 deletions src/ansiblelint/rules/name.py
View file
Expand Up @@ -38,7 +38,7 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
return [
self.create_matcherror(
message="All plays should be named.",
linenumber=data[LINE_NUMBER_KEY],
lineno=data[LINE_NUMBER_KEY],
tag="name[play]",
filename=file,
),
Expand All @@ -47,7 +47,7 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
self._check_name(
data["name"],
lintable=file,
linenumber=data[LINE_NUMBER_KEY],
lineno=data[LINE_NUMBER_KEY],
),
)
return results
Expand All @@ -63,7 +63,7 @@ def matchtask(
results.append(
self.create_matcherror(
message="All tasks should be named.",
linenumber=task[LINE_NUMBER_KEY],
lineno=task[LINE_NUMBER_KEY],
tag="name[missing]",
filename=file,
),
Expand All @@ -73,7 +73,7 @@ def matchtask(
self._prefix_check(
name,
lintable=file,
linenumber=task[LINE_NUMBER_KEY],
lineno=task[LINE_NUMBER_KEY],
),
)
return results
Expand All @@ -82,7 +82,7 @@ def _prefix_check(
self,
name: str,
lintable: Lintable | None,
linenumber: int,
lineno: int,
) -> list[MatchError]:
results: list[MatchError] = []
effective_name = name
Expand All @@ -94,7 +94,7 @@ def _prefix_check(
self._check_name(
effective_name,
lintable=lintable,
linenumber=linenumber,
lineno=lineno,
),
)
return results
Expand All @@ -103,7 +103,7 @@ def _check_name(
self,
name: str,
lintable: Lintable | None,
linenumber: int,
lineno: int,
) -> list[MatchError]:
# This rules applies only to languages that do have uppercase and
# lowercase letter, so we ignore anything else. On Unicode isupper()
Expand All @@ -124,7 +124,7 @@ def _check_name(
results.append(
self.create_matcherror(
message=f"Task name should start with '{prefix}'.",
linenumber=linenumber,
lineno=lineno,
tag="name[prefix]",
filename=lintable,
),
Expand All @@ -141,7 +141,7 @@ def _check_name(
results.append(
self.create_matcherror(
message="All names should start with an uppercase letter.",
linenumber=linenumber,
lineno=lineno,
tag="name[casing]",
filename=lintable,
),
Expand All @@ -150,7 +150,7 @@ def _check_name(
results.append(
self.create_matcherror(
message="Jinja templates should only be at the end of 'name'",
linenumber=linenumber,
lineno=lineno,
tag="name[template]",
filename=lintable,
),
Expand Down

0 comments on commit 6d037c5

Please sign in to comment.