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

Enforce use of lineno variable name inside the library #3326

Merged
merged 2 commits into from
Apr 24, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading