From 0ff4ee763b86ffd06b322fb8abd8db9bd8a83e5a Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Fri, 22 Nov 2024 10:36:24 +0000 Subject: [PATCH] Activate and address some ruff violations (preview) (#4421) --- .gitignore | 23 +++---- .taplo.toml | 2 + pyproject.toml | 22 ++++++- src/ansiblelint/app.py | 7 +- src/ansiblelint/file_utils.py | 3 +- src/ansiblelint/loaders.py | 4 +- src/ansiblelint/rules/__init__.py | 3 +- src/ansiblelint/rules/inline_env_var.py | 2 +- src/ansiblelint/rules/no_same_owner.py | 6 +- src/ansiblelint/rules/yaml_rule.py | 3 +- src/ansiblelint/runner.py | 2 + src/ansiblelint/testing/__init__.py | 4 +- src/ansiblelint/utils.py | 2 +- src/ansiblelint/yaml_utils.py | 2 +- test/rules/fixtures/__init__.py | 2 - test/schemas/src/rebuild.py | 86 ++++++++++++------------- test/test_formatter_sarif.py | 4 +- test/test_main.py | 4 +- 18 files changed, 99 insertions(+), 82 deletions(-) create mode 100644 .taplo.toml diff --git a/.gitignore b/.gitignore index 4e8710757a..7068c220ed 100644 --- a/.gitignore +++ b/.gitignore @@ -62,21 +62,18 @@ examples/playbooks/vars/strings.transformed.yml examples/playbooks/vars/transform_nested_data.yml # other -.cache +*.tar.gz +*.tmp.* .DS_Store -.vscode +.cache +.envrc .idea -src/ansiblelint/_version.py -*.tar.gz .pytest_cache -test/eco/CODENOTIFY.html -test/eco -test/schemas/node_modules -test/local-content -.envrc -# collections -# !/collections -site +.vscode/launch.json _readthedocs -*.tmp.* coverage.lcov +site +src/ansiblelint/_version.py +test/eco +test/local-content +test/schemas/node_modules diff --git a/.taplo.toml b/.taplo.toml new file mode 100644 index 0000000000..4cfd7f7d68 --- /dev/null +++ b/.taplo.toml @@ -0,0 +1,2 @@ +[formatting] +array_trailing_comma = false diff --git a/pyproject.toml b/pyproject.toml index 16a66824bc..b93e2c8f75 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,5 @@ +# cspell: ignore FURB + [build-system] build-backend = "setuptools.build_meta" requires = [ @@ -222,11 +224,13 @@ cache-dir = "./.cache/.ruff" fix = true # Same as Black. line-length = 88 +preview = true target-version = "py310" [tool.ruff.lint] ignore = [ "COM812", # conflicts with ISC001 on format + "CPY001", # missing-copyright-notice "D203", # incompatible with D211 "D213", # incompatible with D212 "E501", # we use black @@ -245,7 +249,21 @@ ignore = [ "RUF012", # Mutable class attributes should be annotated with `typing.ClassVar` "PERF203", "PD011", # We are not using pandas, any .values attributes are unrelated - "PLW0603" # global lock file in cache dir + "PLW0603", # global lock file in cache dir + # part of preview rules: + "B909", # raise-missing-from + "DOC201", # docstring-missing-returns + "DOC402", # docstring-missing-summary + "DOC501", # docstring-missing-exception + "FURB101", + "FURB103", + "FURB110", + "FURB113", + "FURB118", + "PLC0415", + "PLC2701", + "PLW1641", + "S404" ] select = ["ALL"] @@ -269,7 +287,7 @@ max-complexity = 20 "src/ansiblelint/{utils,file_utils,runner,loaders,constants,config,cli,_mockings}.py" = [ "PTH" ] -"test/**/*.py" = ["S"] +"test/**/*.py" = ["DOC201", "DOC501", "PLC2701", "S"] [tool.ruff.lint.pydocstyle] convention = "google" diff --git a/src/ansiblelint/app.py b/src/ansiblelint/app.py index 9b8c7ecb50..f48932ddbf 100644 --- a/src/ansiblelint/app.py +++ b/src/ansiblelint/app.py @@ -217,9 +217,8 @@ def report_outcome( os.environ.get("ANSIBLE_LINT_IGNORE_FILE", IGNORE_FILE.default), ) console_stderr.print(f"Writing ignore file to {ignore_file_path}") - lines = set() - for rule in result.matches: - lines.add(f"{rule.filename} {rule.tag}\n") + lines: set[str] = set() + lines.update(f"{rule.filename} {rule.tag}\n" for rule in result.matches) with ignore_file_path.open("w", encoding="utf-8") as ignore_file: ignore_file.write( "# This file contains ignores rule violations for ansible-lint\n", @@ -330,7 +329,7 @@ def report_summary( # pylint: disable=too-many-locals # noqa: C901 for tag, stats in summary.tag_stats.items(): table.add_row( str(stats.count), - f"[link={RULE_DOC_URL}{ tag.split('[')[0] }]{escape(tag)}[/link]", + f"[link={RULE_DOC_URL}{tag.split('[')[0]}]{escape(tag)}[/link]", stats.profile, f"{', '.join(stats.associated_tags)}{' (warning)' if stats.warning else ''}", style="yellow" if stats.warning else "red", diff --git a/src/ansiblelint/file_utils.py b/src/ansiblelint/file_utils.py index 4c31fa7aab..ef4dfff152 100644 --- a/src/ansiblelint/file_utils.py +++ b/src/ansiblelint/file_utils.py @@ -242,6 +242,7 @@ def __init__( self.file = NamedTemporaryFile( # noqa: SIM115 mode="w+", suffix="playbook.yml", + encoding="utf-8", ) self.filename = self.file.name self._content = sys.stdin.read() @@ -479,7 +480,7 @@ def discover_lintables(options: Options) -> list[str]: def strip_dotslash_prefix(fname: str) -> str: """Remove ./ leading from filenames.""" - return fname[2:] if fname.startswith("./") else fname + return fname.removeprefix("./") def find_project_root( diff --git a/src/ansiblelint/loaders.py b/src/ansiblelint/loaders.py index c369c892aa..fde75d9b88 100644 --- a/src/ansiblelint/loaders.py +++ b/src/ansiblelint/loaders.py @@ -74,10 +74,10 @@ def load_ignore_txt(filepath: Path | None = None) -> dict[str, set[str]]: __all__ = [ + "IGNORE_FILE", + "YAMLError", "load_ignore_txt", "yaml_from_file", "yaml_load", "yaml_load_safe", - "YAMLError", - "IGNORE_FILE", ] diff --git a/src/ansiblelint/rules/__init__.py b/src/ansiblelint/rules/__init__.py index 22bb135783..a77899d7dd 100644 --- a/src/ansiblelint/rules/__init__.py +++ b/src/ansiblelint/rules/__init__.py @@ -533,8 +533,7 @@ def known_tags(self) -> list[str]: tags = set() for rule in self.rules: tags.add(rule.id) - for tag in rule.tags: - tags.add(tag) + tags.update(rule.tags) return sorted(tags) def list_tags(self) -> str: diff --git a/src/ansiblelint/rules/inline_env_var.py b/src/ansiblelint/rules/inline_env_var.py index 1f0747e702..de2a4c1a30 100644 --- a/src/ansiblelint/rules/inline_env_var.py +++ b/src/ansiblelint/rules/inline_env_var.py @@ -64,7 +64,7 @@ def matchtask( task: Task, file: Lintable | None = None, ) -> bool | str: - if task["action"]["__ansible_module__"] in ["command"]: + if task["action"]["__ansible_module__"] == "command": first_cmd_arg = get_first_cmd_arg(task) if not first_cmd_arg: return False diff --git a/src/ansiblelint/rules/no_same_owner.py b/src/ansiblelint/rules/no_same_owner.py index becd65fd27..9ec6840f96 100644 --- a/src/ansiblelint/rules/no_same_owner.py +++ b/src/ansiblelint/rules/no_same_owner.py @@ -62,10 +62,8 @@ def handle_synchronize(task: Any, action: dict[str, Any]) -> bool: def handle_unarchive(task: Any, action: dict[str, Any]) -> bool: """Process unarchive task.""" delegate_to = task.get("delegate_to") - if ( - delegate_to == "localhost" - or delegate_to != "localhost" - and not action.get("remote_src") + if delegate_to == "localhost" or ( + delegate_to != "localhost" and not action.get("remote_src") ): src = action.get("src") if not isinstance(src, str): diff --git a/src/ansiblelint/rules/yaml_rule.py b/src/ansiblelint/rules/yaml_rule.py index 3ec5b59957..5f141d961d 100644 --- a/src/ansiblelint/rules/yaml_rule.py +++ b/src/ansiblelint/rules/yaml_rule.py @@ -115,8 +115,7 @@ def _combine_skip_rules(data: Any) -> set[str]: result = set(data.get(SKIPPED_RULES_KEY, [])) tags = data.get("tags", []) if tags and ( - isinstance(tags, Iterable) - and "skip_ansible_lint" in tags + (isinstance(tags, Iterable) and "skip_ansible_lint" in tags) or tags == "skip_ansible_lint" ): result.add("skip_ansible_lint") diff --git a/src/ansiblelint/runner.py b/src/ansiblelint/runner.py index 1d378cd95b..75767883ca 100644 --- a/src/ansiblelint/runner.py +++ b/src/ansiblelint/runner.py @@ -333,6 +333,7 @@ def _get_ansible_syntax_check_matches( mode="w", suffix=".yml", prefix="play", + encoding="utf-8", ) fh.write(playbook_text) fh.flush() @@ -598,6 +599,7 @@ def plugin_children(self, lintable: Lintable) -> list[Lintable]: examples.file = NamedTemporaryFile( # noqa: SIM115 mode="w+", suffix=f"_{lintable.path.name}.yaml", + encoding="utf-8", ) examples.file.write(content) examples.file.flush() diff --git a/src/ansiblelint/testing/__init__.py b/src/ansiblelint/testing/__init__.py index 9c5463f8df..a2eafc7e58 100644 --- a/src/ansiblelint/testing/__init__.py +++ b/src/ansiblelint/testing/__init__.py @@ -52,7 +52,9 @@ def run_playbook( prefix: str = "playbook", ) -> list[MatchError]: """Lints received text as a playbook.""" - with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", prefix=prefix) as fh: + with tempfile.NamedTemporaryFile( + mode="w", suffix=".yml", prefix=prefix, encoding="utf-8" + ) as fh: fh.write(playbook_text) fh.flush() results = self._call_runner(Path(fh.name)) diff --git a/src/ansiblelint/utils.py b/src/ansiblelint/utils.py index ec42ec3dd4..534db9cda5 100644 --- a/src/ansiblelint/utils.py +++ b/src/ansiblelint/utils.py @@ -859,7 +859,7 @@ def is_handler(self) -> bool: if file_name: paths = file_name.split("/") is_handler_file = "handlers" in paths - return is_handler_file if is_handler_file else ".handlers[" in self.position + return is_handler_file or ".handlers[" in self.position def __repr__(self) -> str: """Return a string representation of the task.""" diff --git a/src/ansiblelint/yaml_utils.py b/src/ansiblelint/yaml_utils.py index 3765845e40..c326afa8f5 100644 --- a/src/ansiblelint/yaml_utils.py +++ b/src/ansiblelint/yaml_utils.py @@ -637,7 +637,7 @@ def choose_scalar_style(self) -> Any: """Select how to quote scalars if needed.""" style = super().choose_scalar_style() if ( - style == "" + style == "" # noqa: PLC1901 and self.event.value.startswith("0") and len(self.event.value) > 1 ): diff --git a/test/rules/fixtures/__init__.py b/test/rules/fixtures/__init__.py index d049bf0473..62bd20c1dc 100644 --- a/test/rules/fixtures/__init__.py +++ b/test/rules/fixtures/__init__.py @@ -1,3 +1 @@ """Test rules resources.""" - -__all__ = ["ematcher", "raw_task", "unset_variable_matcher"] diff --git a/test/schemas/src/rebuild.py b/test/schemas/src/rebuild.py index 5eb4807e01..380c6ca7a7 100644 --- a/test/schemas/src/rebuild.py +++ b/test/schemas/src/rebuild.py @@ -10,49 +10,49 @@ play_keywords = list( filter( None, - """\ -any_errors_fatal -become -become_exe -become_flags -become_method -become_user -check_mode -collections -connection -debugger -diff -environment -fact_path -force_handlers -gather_facts -gather_subset -gather_timeout -handlers -hosts -ignore_errors -ignore_unreachable -max_fail_percentage -module_defaults -name -no_log -order -port -post_tasks -pre_tasks -remote_user -roles -run_once -serial -strategy -tags -tasks -throttle -timeout -vars -vars_files -vars_prompt -""".split(), + [ + "any_errors_fatal", + "become", + "become_exe", + "become_flags", + "become_method", + "become_user", + "check_mode", + "collections", + "connection", + "debugger", + "diff", + "environment", + "fact_path", + "force_handlers", + "gather_facts", + "gather_subset", + "gather_timeout", + "handlers", + "hosts", + "ignore_errors", + "ignore_unreachable", + "max_fail_percentage", + "module_defaults", + "name", + "no_log", + "order", + "port", + "post_tasks", + "pre_tasks", + "remote_user", + "roles", + "run_once", + "serial", + "strategy", + "tags", + "tasks", + "throttle", + "timeout", + "vars", + "vars_files", + "vars_prompt", + ], ), ) diff --git a/test/test_formatter_sarif.py b/test/test_formatter_sarif.py index 982bb6ef8f..171c494cc2 100644 --- a/test/test_formatter_sarif.py +++ b/test/test_formatter_sarif.py @@ -189,7 +189,9 @@ def test_sarif_parsable_ignored() -> None: ) def test_sarif_file(file: str, return_code: int) -> None: """Test ability to dump sarif file (--sarif-file).""" - with NamedTemporaryFile(mode="w", suffix=".sarif", prefix="output") as output_file: + with NamedTemporaryFile( + mode="w", suffix=".sarif", prefix="output", encoding="utf-8" + ) as output_file: cmd = [ sys.executable, "-m", diff --git a/test/test_main.py b/test/test_main.py index d2e48dc060..57f4700d1f 100644 --- a/test/test_main.py +++ b/test/test_main.py @@ -91,7 +91,7 @@ def test_get_version_warning( def test_get_version_warning_no_pip(mocker: MockerFixture) -> None: """Test that we do not display any message if install method is not pip.""" mocker.patch("ansiblelint.config.guess_install_method", return_value="") - assert get_version_warning() == "" + assert get_version_warning() == "" # noqa: PLC1901 def test_get_version_warning_remote_disconnect(mocker: MockerFixture) -> None: @@ -109,7 +109,7 @@ def test_get_version_warning_offline(mocker: MockerFixture) -> None: # ensures a real cache_file is not loaded mocker.patch("ansiblelint.config.CACHE_DIR", Path(temporary_directory)) options.offline = True - assert get_version_warning() == "" + assert get_version_warning() == "" # noqa: PLC1901 @pytest.mark.parametrize(