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

Address ruff S #1786

Merged
merged 1 commit into from
Jun 3, 2024
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
11 changes: 3 additions & 8 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -311,15 +311,7 @@ ignore = [
'RET505', # Unnecessary `else` after `return` statement
'RUF005', # [*] Consider `[self._name, *shlex.split(self._interaction.action.match.groupdict()["params"] or "")]` instead of concatenation
'RUF012', # Mutable class attributes should be annotated with `typing.ClassVar`
'S101', # Use of `assert` detected
'S103', # `os.chmod` setting a permissive mask `0o777` on file or directory
'S108', # Probable insecure usage of temporary file or directory: "/tmp"
'S311', # Standard pseudo-random generators are not suitable for cryptographic purposes
'S602', # `subprocess` call with `shell=True` identified, security issue
'S603', # `subprocess` call: check for execution of untrusted input
'S605', # Starting a process with a shell, possible injection detected
'S607', # Starting a process with a partial executable path
'SLF001', # Private member accessed: `_ui`
'T201' # `print` found
]
select = ["ALL"]
Expand All @@ -333,6 +325,9 @@ known-first-party = ["ansible_navigator"]
lines-after-imports = 2 # Ensures consistency for cases when there's variable vs function/class definitions after imports
lines-between-types = 1 # Separate import/from with 1 line

[tool.ruff.lint.per-file-ignores]
"tests/**" = ["SLF001", "S101", "S602"]

[tool.ruff.lint.pydocstyle]
convention = "pep257"

Expand Down
2 changes: 1 addition & 1 deletion src/ansible_navigator/action_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def run(self, _screen: Window) -> None:
action=action,
menu=None,
content=None,
ui=self._ui._ui,
ui=self._ui._ui, # noqa: SLF001
)
self._run_app(interaction)

Expand Down
4 changes: 2 additions & 2 deletions src/ansible_navigator/actions/open_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,9 @@
if isinstance(command, str):
if editor_console:
with SuspendCurses():
os.system(command)
os.system(command) # noqa:S605

Check warning on line 146 in src/ansible_navigator/actions/open_file.py

View check run for this annotation

Codecov / codecov/patch

src/ansible_navigator/actions/open_file.py#L146

Added line #L146 was not covered by tests
else:
os.system(command)
os.system(command) # noqa:S605

Check warning on line 148 in src/ansible_navigator/actions/open_file.py

View check run for this annotation

Codecov / codecov/patch

src/ansible_navigator/actions/open_file.py#L148

Added line #L148 was not covered by tests

@staticmethod
def _persist_content(content: ContentType, content_format: ContentFormat) -> Path:
Expand Down
4 changes: 2 additions & 2 deletions src/ansible_navigator/actions/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -686,8 +686,8 @@ def _handle_message(self, message: dict[str, Any]) -> None:
# Only runner on_* events are relevant now
try:
prefix, runner_event = event.rsplit("_", 1)
assert prefix == "runner_on"
assert runner_event in ("ok", "skipped", "start", "unreachable", "failed")
assert prefix == "runner_on" # noqa:S101
assert runner_event in ("ok", "skipped", "start", "unreachable", "failed") # noqa:S101
except (AssertionError, ValueError):
return

Expand Down
2 changes: 1 addition & 1 deletion src/ansible_navigator/command_runner/command_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def run_command(command: Command) -> None:
capture_output=True,
check=True,
text=True,
shell=True,
shell=True, # noqa:S602
)
command.return_code = proc_out.returncode
command.stdout = proc_out.stdout
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ def version_added_sanity_check(version: str) -> None:
:raises AssertionError: If the version string is invalid
"""
re_version = re.compile(r"^v\d+\.\d+$")
assert re_version.match(version) is not None, "Version must be in the form of v{major}.{minor}"
assert ( # noqa:S101
re_version.match(version) is not None
), "Version must be in the form of v{major}.{minor}"


class Constants(Enum):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,11 @@
)
else:
try:
subprocess.run("ansible-lint --version", shell=True, check=True)
subprocess.run(

Check warning on line 675 in src/ansible_navigator/configuration_subsystem/navigator_post_processor.py

View check run for this annotation

Codecov / codecov/patch

src/ansible_navigator/configuration_subsystem/navigator_post_processor.py#L675

Added line #L675 was not covered by tests
"ansible-lint --version", # noqa:S607
shell=True, # noqa:S602
check=True,
)
except subprocess.CalledProcessError:
exit_messages.append(
ExitMessage(
Expand Down
4 changes: 2 additions & 2 deletions src/ansible_navigator/data/catalog_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ def run_command(cmd: list[str]) -> dict[str, str]:
capture_output=True,
check=True,
text=True,
shell=True,
shell=True, # noqa:S602
)
except subprocess.CalledProcessError as exc:
return {"error": str(exc)}
Expand Down Expand Up @@ -631,7 +631,7 @@ def main() -> dict[Any, Any]:
"collections": collections,
"errors": errors,
"stats": stats,
"messages": cc_obj._messages,
"messages": cc_obj._messages, # noqa: SLF001
}


Expand Down
2 changes: 1 addition & 1 deletion src/ansible_navigator/data/image_introspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def run_command(command: Command) -> None:
capture_output=True,
check=True,
text=True,
shell=True,
shell=True, # noqa:S602
)
command.stdout = proc_out.stdout
except subprocess.CalledProcessError as exc:
Expand Down
4 changes: 2 additions & 2 deletions src/ansible_navigator/image_manager/puller.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def _check_for_image(self) -> None:
cmd_parts = [self._container_engine, "image", "inspect", self._image]
self._log_message(level=logging.DEBUG, message=f"Command: {shlex_join(cmd_parts)}")
subprocess.run(
cmd_parts,
cmd_parts, # noqa:S603
check=True,
capture_output=True,
)
Expand Down Expand Up @@ -219,7 +219,7 @@ def pull_stdout(self) -> None:
cmd_to_run,
check=True,
stderr=stderr_pipe,
shell=True,
shell=True, # noqa:S602
env=os.environ,
)
self._log_message(level=logging.INFO, message="Execution environment updated")
Expand Down
2 changes: 1 addition & 1 deletion src/ansible_navigator/tm_tokenize/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def _compile_root(self, grammar: Grammar) -> PatternRule:
return PatternRule((grammar.scope_name,), make_regset(*regs), rules)

def _compile_rule(self, grammar: Grammar, rule: _Rule) -> CompiledRule:
assert rule.include is None, rule
assert rule.include is None, rule # noqa:S101
if rule.match is not None:
captures_ref = self._captures_ref(grammar, rule.captures)
return MatchRule(rule.name, captures_ref)
Expand Down
4 changes: 2 additions & 2 deletions src/ansible_navigator/ui_framework/form_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
else:
form = Form(type_=FormType.FORM)

form._dict = form_data # pylint: disable=protected-access
form._dict = form_data # pylint: disable=protected-access # noqa: SLF001
form.title = form_data["title"]
form.title_color = form_data.get("title_color", 0)

Expand Down Expand Up @@ -104,7 +104,7 @@
:param key_on_name: Bool used to filter via name
:returns: form as type dict
"""
res = form._dict # pylint: disable=protected-access
res = form._dict # pylint: disable=protected-access # noqa: SLF001

Check warning on line 107 in src/ansible_navigator/ui_framework/form_utils.py

View check run for this annotation

Codecov / codecov/patch

src/ansible_navigator/ui_framework/form_utils.py#L107

Added line #L107 was not covered by tests
res["cancelled"] = form.cancelled
res["submitted"] = form.submitted
for field_idx, field in enumerate(form.fields):
Expand Down
2 changes: 1 addition & 1 deletion src/ansible_navigator/ui_framework/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"""
if hint:
return "Please provide a value (optional)"
value = "*" * randrange(15, 20) if text else ""
value = "*" * randrange(15, 20) if text else "" # noqa:S311

Check warning on line 59 in src/ansible_navigator/ui_framework/validators.py

View check run for this annotation

Codecov / codecov/patch

src/ansible_navigator/ui_framework/validators.py#L59

Added line #L59 was not covered by tests
return Validation(value=value, error_msg="")

@staticmethod
Expand Down
4 changes: 2 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def valid_ce() -> str:
# the habit of getting stuck.
try:
cmd = [engine, "info"]
subprocess.check_output(cmd, stderr=subprocess.STDOUT, timeout=6)
subprocess.check_output(cmd, stderr=subprocess.STDOUT, timeout=6) # noqa:S603
except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as exc:
msg = f"Container engine is broken, fail to run: {' '.join(cmd)}: {exc}"
continue
Expand Down Expand Up @@ -133,7 +133,7 @@ def pullable_image(valid_container_engine: str) -> Generator[str, None, None]:
"""
image = ImageEntry.PULLABLE_IMAGE.get(app_name=APP_NAME)
yield image
subprocess.run([valid_container_engine, "image", "rm", image], check=True)
subprocess.run([valid_container_engine, "image", "rm", image], check=True) # noqa:S603


@pytest.fixture()
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/diagnostics/test_from_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def test(
settings_path, settings_file = settings_env_var_to_full

proc_out = subprocess.run(
"ansible-navigator --diagnostics",
"ansible-navigator --diagnostics", # noqa:S607
check=False,
shell=True,
capture_output=True,
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_circular_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,4 @@ def test_no_warnings(import_path: str) -> None:
f"import {import_path!s}",
)

subprocess.check_call(imp_cmd) # Input is trusted, generated above, not external
subprocess.check_call(imp_cmd) # Input is trusted, generated above, not external # noqa:S603
Loading