Skip to content

Commit

Permalink
Address ruff TRY (#4140)
Browse files Browse the repository at this point in the history
Co-authored-by: Kate Case <[email protected]>
  • Loading branch information
ssbarnea and Qalthos authored May 7, 2024
1 parent d910de0 commit 2f3cb30
Show file tree
Hide file tree
Showing 20 changed files with 92 additions and 93 deletions.
9 changes: 5 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ repos:
- repo: meta
hooks:
- id: check-useless-excludes
- repo: https://github.com/pappasam/toml-sort
rev: v0.23.1
hooks:
- id: toml-sort-fix
# https://github.com/pappasam/toml-sort/issues/69
# - repo: https://github.com/pappasam/toml-sort
# rev: v0.23.1
# hooks:
# - id: toml-sort-fix
- repo: https://github.com/pre-commit/mirrors-prettier
# keep it before yamllint
rev: v4.0.0-alpha.8
Expand Down
57 changes: 28 additions & 29 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[build-system]
requires = [
"setuptools >= 65.3.0", # required by pyproject+setuptools_scm integration and editable installs
"setuptools_scm[toml] >= 7.0.5" # required for "no-local-version" scheme
"setuptools_scm[toml] >= 7.0.5", # required for "no-local-version" scheme
]
build-backend = "setuptools.build_meta"

Expand All @@ -12,9 +12,9 @@ dynamic = ["version", "dependencies", "optional-dependencies"]
name = "ansible-lint"
description = "Checks playbooks for practices and behavior that could potentially be improved"
readme = "README.md"
authors = [{"name" = "Will Thames", "email" = "[email protected]"}]
maintainers = [{"name" = "Ansible by Red Hat", "email" = "[email protected]"}]
license = {text = "GPLv3+"}
authors = [{ "name" = "Will Thames", "email" = "[email protected]" }]
maintainers = [{ "name" = "Ansible by Red Hat", "email" = "[email protected]" }]
license = { text = "GPLv3+" }
classifiers = [
"Development Status :: 5 - Production/Stable",
"Environment :: Console",
Expand All @@ -34,7 +34,7 @@ classifiers = [
"Topic :: System :: Systems Administration",
"Topic :: Software Development :: Quality Assurance",
"Topic :: Software Development :: Testing",
"Topic :: Utilities"
"Topic :: Utilities",
]
keywords = ["ansible", "lint"]

Expand Down Expand Up @@ -115,7 +115,7 @@ module = [
"ansiblelint._version", # generated
"license_expression",
"ruamel.yaml",
"yamllint.*"
"yamllint.*",
]
ignore_missing_imports = true
ignore_errors = true
Expand All @@ -130,7 +130,7 @@ extension-pkg-allow-list = ["black.parsing"]
bad-names = [
# spell-checker:ignore linenumber
"linenumber", # use lineno instead
"line_number" # use lineno instead
"line_number", # use lineno instead
]
# pylint defaults + f,fh,v,id
good-names = ["i", "j", "k", "Run", "_", "f", "fh", "v", "id", "T"]
Expand All @@ -151,10 +151,10 @@ disable = [
# https://github.com/PyCQA/pylint/issues/850
"cyclic-import",
# https://github.com/PyCQA/pylint/issues/8453
"preferred-module"
"preferred-module",
]
enable = [
"useless-suppression" # Identify unneeded pylint disable statements
"useless-suppression", # Identify unneeded pylint disable statements
]

[tool.pylint.REPORTING]
Expand Down Expand Up @@ -193,7 +193,7 @@ filterwarnings = [
# https://github.com/ansible/ansible/issues/81906
"ignore:'importlib.abc.TraversableResources' is deprecated and slated for removal in Python 3.14:DeprecationWarning",
# https://github.com/ansible/ansible/pull/80968
"ignore:Attribute s is deprecated and will be removed in Python 3.14; use value instead:DeprecationWarning"
"ignore:Attribute s is deprecated and will be removed in Python 3.14; use value instead:DeprecationWarning",
]
junit_duration_report = "call"
# Our github annotation parser from .github/workflows/tox.yml requires xunit1 format. Ref:
Expand All @@ -214,23 +214,26 @@ norecursedirs = [
"collections",
"dist",
"docs",
"src/ansible_lint.egg-info"
"src/ansible_lint.egg-info",
]
python_files = [
"test_*.py",
# Ref: https://docs.pytest.org/en/latest/reference.html#confval-python_files
# Needed to discover legacy nose test modules:
"Test*.py",
# Needed to discover embedded Rule tests
"rules/*.py"
"rules/*.py",
]
# Using --pyargs instead of testpath as we embed some tests
# See: https://github.com/pytest-dev/pytest/issues/6451#issuecomment-687043537
# testpaths =
xfail_strict = true

[tool.ruff]
ignore = [
target-version = "py310"
# Same as Black.
line-length = 88
lint.ignore = [
"D203", # incompatible with D211
"D213", # incompatible with D212
"E501", # we use black
Expand All @@ -246,42 +249,38 @@ ignore = [
"FBT003",
"PLR",
"RUF012", # Mutable class attributes should be annotated with `typing.ClassVar`
"TRY",
"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
]
select = ["ALL"]
target-version = "py310"
# Same as Black.
line-length = 88
lint.select = ["ALL"]

[tool.ruff.flake8-builtins]
[tool.ruff.lint.flake8-builtins]
builtins-ignorelist = ["id"]

[tool.ruff.flake8-pytest-style]
[tool.ruff.lint.flake8-pytest-style]
parametrize-values-type = "tuple"

[tool.ruff.isort]
[tool.ruff.lint.isort]
known-first-party = ["ansiblelint"]

[tool.ruff.mccabe]
[tool.ruff.lint.mccabe]
# Implicit 10 is too low for our codebase, even black uses 18 as default.
max-complexity = 20

[tool.ruff.per-file-ignores]
[tool.ruff.lint.per-file-ignores]
"test/**/*.py" = ["S"]
"src/ansiblelint/rules/*.py" = ["S"]
"src/ansiblelint/testing/*.py" = ["S"]
# Temporary disabled until we fix them:
"src/ansiblelint/{utils,file_utils,runner,loaders,constants,config,cli,_mockings}.py" = [
"PTH"
"PTH",
]

[tool.setuptools.dynamic]
dependencies = {file = [".config/requirements.in"]}
optional-dependencies.docs = {file = [".config/requirements-docs.in"]}
optional-dependencies.test = {file = [".config/requirements-test.in"]}
dependencies = { file = [".config/requirements.in"] }
optional-dependencies.docs = { file = [".config/requirements-docs.in"] }
optional-dependencies.test = { file = [".config/requirements-test.in"] }

[tool.setuptools_scm]
local_scheme = "no-local-version"
Expand All @@ -295,5 +294,5 @@ git_describe_command = [
"--tags",
"--long",
"--match",
"v*.*"
"v*.*",
]
2 changes: 1 addition & 1 deletion src/ansiblelint/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def initialize_options(arguments: list[str] | None = None) -> None | FileLock:
try:
cache_dir_lock.acquire(timeout=180)
except Timeout: # pragma: no cover
_logger.error(
_logger.error( # noqa: TRY400
"Timeout waiting for another instance of ansible-lint to release the lock.",
)
sys.exit(RC.LOCK_TIMEOUT)
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/_internal/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def rule_config(self) -> dict[str, Any]:
rule_config = self.options.rules.get(self.id, {})
if not isinstance(rule_config, dict): # pragma: no branch
msg = f"Invalid rule config for {self.id}: {rule_config}"
raise RuntimeError(msg)
raise RuntimeError(msg) # noqa: TRY004
return rule_config

@property
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def load_config(config_file: str | None) -> tuple[dict[Any, Any], str | None]:
config = clean_json(config_lintable.data)
if not isinstance(config, dict):
msg = "Schema failed to properly validate the config file."
raise RuntimeError(msg)
raise TypeError(msg)
config["config_file"] = config_path
config_dir = os.path.dirname(config_path)
expand_to_normalized_paths(config, config_dir)
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ class Options: # pylint: disable=too-many-instance-attributes
cache_dir: Path | None = None
colored: bool = True
configured: bool = False
cwd: Path = Path(".")
cwd: Path = Path()
display_relative_path: bool = True
exclude_paths: list[str] = field(default_factory=list)
format: str = "brief"
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ def is_relative_to(path: Path, *other: Any) -> bool:
"""Return True if the path is relative to another path or False."""
try:
path.resolve().absolute().relative_to(*other)
return True
except ValueError:
return False
return True


def normpath_path(path: str | Path) -> Path:
Expand Down
4 changes: 2 additions & 2 deletions src/ansiblelint/formatters/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def format_result(self, matches: list[MatchError]) -> str:
"""Format a list of match errors as a JSON string."""
if not isinstance(matches, list):
msg = f"The {self.__class__} was expecting a list of MatchError."
raise RuntimeError(msg)
raise TypeError(msg)

result = []
for match in matches:
Expand Down Expand Up @@ -213,7 +213,7 @@ def format_result(self, matches: list[MatchError]) -> str:
"""Format a list of match errors as a JSON string."""
if not isinstance(matches, list):
msg = f"The {self.__class__} was expecting a list of MatchError."
raise RuntimeError(msg)
raise TypeError(msg)

root_path = Path(str(self.base_dir)).as_uri()
root_path = root_path + "/" if not root_path.endswith("/") else root_path
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/rules/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ def __getitem__(self, item: Any) -> BaseRule:
"""Return a rule from inside the collection based on its id."""
if not isinstance(item, str):
msg = f"Expected str but got {type(item)} when trying to access rule by it's id"
raise RuntimeError(msg)
raise TypeError(msg)
for rule in self.rules:
if rule.id == item:
return rule
Expand Down
11 changes: 5 additions & 6 deletions src/ansiblelint/rules/args.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,7 @@ def matchtask(
name=loaded_module.resolved_fqcn,
location=loaded_module.plugin_resolved_path,
)
if spec:
assert spec.loader is not None
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
else:
if not spec:
assert file is not None
_logger.warning(
"Unable to load module %s at %s:%s for options validation",
Expand All @@ -160,6 +156,9 @@ def matchtask(
task[LINE_NUMBER_KEY],
)
return []
assert spec.loader is not None
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)

try:
if not hasattr(module, "main"):
Expand Down Expand Up @@ -190,9 +189,9 @@ def matchtask(
)

sanitized_results = self._sanitize_results(results, module_name)
return sanitized_results
except ValidationPassedError:
return []
return sanitized_results

# pylint: disable=unused-argument
def _sanitize_results(
Expand Down
4 changes: 2 additions & 2 deletions src/ansiblelint/rules/complexity.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
tasks = data.get("tasks", [])
if not isinstance(self._collection, RulesCollection):
msg = "Rules cannot be run outside a rule collection."
raise RuntimeError(msg)
raise TypeError(msg)
if len(tasks) > self._collection.options.max_tasks:
results.append(
self.create_matcherror(
Expand All @@ -53,7 +53,7 @@ def matchtask(self, task: Task, file: Lintable | None = None) -> list[MatchError

if not isinstance(self._collection, RulesCollection):
msg = "Rules cannot be run outside a rule collection."
raise RuntimeError(msg)
raise TypeError(msg)

if task.action == "block/always/rescue":
block_depth = self.calculate_block_depth(task)
Expand Down
51 changes: 25 additions & 26 deletions src/ansiblelint/rules/fqcn.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def matchtask(
module = task["action"]["__ansible_module_original__"]
if not isinstance(module, str):
msg = "Invalid data for module."
raise RuntimeError(msg)
raise TypeError(msg)

if module not in self.module_aliases:
loaded_module = load_plugin(module)
Expand Down Expand Up @@ -159,31 +159,30 @@ def matchtask(
tag="fqcn[action-core]",
),
)
else:
if module.count(".") < 2:
result.append(
self.create_matcherror(
message=f"Use FQCN for module actions, such `{self.module_aliases[module]}`.",
details=f"Action `{module}` is not FQCN.",
filename=file,
lineno=task["__line__"],
tag="fqcn[action]",
),
)
# TODO(ssbarnea): Remove the c.g. and c.n. exceptions from here once # noqa: FIX002
# community team is flattening these.
# https://github.com/ansible-community/community-topics/issues/147
elif not module.startswith("community.general.") or module.startswith(
"community.network.",
):
result.append(
self.create_matcherror(
message=f"You should use canonical module name `{self.module_aliases[module]}` instead of `{module}`.",
filename=file,
lineno=task["__line__"],
tag="fqcn[canonical]",
),
)
elif module.count(".") < 2:
result.append(
self.create_matcherror(
message=f"Use FQCN for module actions, such `{self.module_aliases[module]}`.",
details=f"Action `{module}` is not FQCN.",
filename=file,
lineno=task["__line__"],
tag="fqcn[action]",
),
)
# TODO(ssbarnea): Remove the c.g. and c.n. exceptions from here once # noqa: FIX002
# community team is flattening these.
# https://github.com/ansible-community/community-topics/issues/147
elif not module.startswith("community.general.") or module.startswith(
"community.network.",
):

This comment has been minimized.

Copy link
@lucastheisen

lucastheisen May 17, 2024

@ssbarnea , noticed this while digging into this bug report. I may be missing something, but this appears to be buggy in that the python order of precedence would apply the not before the or, so at best, the or portion is superfluous as it implies the not portion would have already been True so the condition would short-circuit.

But python isn't my first language so i may be missing a crucial detail...

result.append(
self.create_matcherror(
message=f"You should use canonical module name `{self.module_aliases[module]}` instead of `{module}`.",
filename=file,
lineno=task["__line__"],
tag="fqcn[canonical]",
),
)
return result

def matchyaml(self, file: Lintable) -> list[MatchError]:
Expand Down
Loading

0 comments on commit 2f3cb30

Please sign in to comment.