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

Capture python warnings and report some of them as matches #3324

Merged
merged 1 commit into from
May 9, 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
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ jobs:
WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:GITHUB_STEP_SUMMARY
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 794
PYTEST_REQPASS: 795
steps:
- name: Activate WSL1
if: "contains(matrix.shell, 'wsl')"
Expand Down
8 changes: 8 additions & 0 deletions examples/playbooks/capture-warning.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
- name: Fixture to generate a warning
hosts: localhost
tasks:
- name: Generate a warning
ansible.builtin.debug:
msg: "This is a warning"
when: "{{ false }}" # noqa: 102 jinja[spacing]
14 changes: 14 additions & 0 deletions src/ansiblelint/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,20 @@
from ansiblelint.utils import Task


class LintWarning(Warning):
"""Used by linter."""


@dataclass
class WarnSource:
"""Container for warning information, so we can later create a MatchError from it."""

filename: Lintable
lineno: int
tag: str
message: str | None = None


class StrictModeError(RuntimeError):
"""Raise when we encounter a warning in strict mode."""

Expand Down
5 changes: 4 additions & 1 deletion src/ansiblelint/rules/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,10 @@ def matchlines(self, file: Lintable) -> list[MatchError]:
if line.lstrip().startswith("#"):
continue

rule_id_list = ansiblelint.skip_utils.get_rule_skips_from_line(line)
rule_id_list = ansiblelint.skip_utils.get_rule_skips_from_line(
line,
lintable=file,
)
if self.id in rule_id_list:
continue

Expand Down
5 changes: 4 additions & 1 deletion src/ansiblelint/rules/jinja.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,10 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
lines = file.content.splitlines()
for match in raw_results:
# lineno starts with 1, not zero
skip_list = get_rule_skips_from_line(lines[match.lineno - 1])
skip_list = get_rule_skips_from_line(
line=lines[match.lineno - 1],
lintable=file,
)
if match.rule.id not in skip_list and match.tag not in skip_list:
results.append(match)
else:
Expand Down
10 changes: 8 additions & 2 deletions src/ansiblelint/rules/var_naming.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
lines = file.content.splitlines()
for match in raw_results:
# lineno starts with 1, not zero
skip_list = get_rule_skips_from_line(lines[match.lineno - 1])
skip_list = get_rule_skips_from_line(
line=lines[match.lineno - 1],
lintable=file,
)
if match.rule.id not in skip_list and match.tag not in skip_list:
results.append(match)

Expand Down Expand Up @@ -175,7 +178,10 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
lines = file.content.splitlines()
for match in raw_results:
# lineno starts with 1, not zero
skip_list = get_rule_skips_from_line(lines[match.lineno - 1])
skip_list = get_rule_skips_from_line(
line=lines[match.lineno - 1],
lintable=file,
)
if match.rule.id not in skip_list and match.tag not in skip_list:
results.append(match)
else:
Expand Down
50 changes: 47 additions & 3 deletions src/ansiblelint/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@
import multiprocessing
import multiprocessing.pool
import os
import warnings
from dataclasses import dataclass
from fnmatch import fnmatch
from typing import TYPE_CHECKING, Any

import ansiblelint.skip_utils
import ansiblelint.utils
from ansiblelint._internal.rules import LoadingFailureRule
from ansiblelint._internal.rules import LoadingFailureRule, WarningRule
from ansiblelint.constants import States
from ansiblelint.errors import MatchError
from ansiblelint.errors import LintWarning, MatchError, WarnSource
from ansiblelint.file_utils import Lintable, expand_dirs_in_lintables
from ansiblelint.rules.syntax_check import AnsibleSyntaxCheckRule

Expand Down Expand Up @@ -112,8 +113,51 @@ def is_excluded(self, lintable: Lintable) -> bool:
for path in self.exclude_paths
)

def run(self) -> list[MatchError]: # noqa: C901
def run(self) -> list[MatchError]:
"""Execute the linting process."""
matches: list[MatchError] = []
with warnings.catch_warnings(record=True) as captured_warnings:
warnings.simplefilter("always")
matches = self._run()
for warn in captured_warnings:
# For the moment we are ignoring deprecation warnings as Ansible
# modules outside current content can generate them and user
# might not be able to do anything about them.
if warn.category is DeprecationWarning:
continue
if warn.category is LintWarning:
filename: None | Lintable = None
if isinstance(warn.source, WarnSource):
match = MatchError(
message=warn.source.message or warn.category.__name__,
rule=WarningRule(),
filename=warn.source.filename.filename,
tag=warn.source.tag,
lineno=warn.source.lineno,
)
else:
filename = warn.source
# breakpoint()
match = MatchError(
message=warn.message
if isinstance(warn.message, str)
else "?",
rule=WarningRule(),
filename=str(filename),
)
matches.append(match)
continue
_logger.warning(
"%s:%s %s %s",
warn.filename,
warn.lineno or 1,
warn.category.__name__,
warn.message,
)
return matches

def _run(self) -> list[MatchError]: # noqa: C901
"""Run the linting (inner loop)."""
files: list[Lintable] = []
matches: list[MatchError] = []

Expand Down
31 changes: 24 additions & 7 deletions src/ansiblelint/skip_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import collections.abc
import logging
import re
import warnings
from functools import cache
from itertools import product
from typing import TYPE_CHECKING, Any
Expand All @@ -41,6 +42,7 @@
RENAMED_TAGS,
SKIPPED_RULES_KEY,
)
from ansiblelint.errors import LintWarning, WarnSource

if TYPE_CHECKING:
from collections.abc import Generator, Sequence
Expand All @@ -60,7 +62,11 @@
# ansible.parsing.yaml.objects.AnsibleSequence


def get_rule_skips_from_line(line: str) -> list[str]:
def get_rule_skips_from_line(
line: str,
lintable: Lintable,
lineno: int = 1,
) -> list[str]:
"""Return list of rule ids skipped via comment on the line of yaml."""
_before_noqa, _noqa_marker, noqa_text = line.partition("# noqa")

Expand All @@ -69,10 +75,17 @@ def get_rule_skips_from_line(line: str) -> list[str]:
if v in RENAMED_TAGS:
tag = RENAMED_TAGS[v]
if v not in _found_deprecated_tags:
_logger.warning(
"Replaced outdated tag '%s' with '%s', replace it to avoid future regressions",
v,
tag,
msg = f"Replaced outdated tag '{v}' with '{tag}', replace it to avoid future errors"
warnings.warn(
message=msg,
category=LintWarning,
source=WarnSource(
filename=lintable,
lineno=lineno,
tag="warning[outdated-tag]",
message=msg,
),
stacklevel=0,
)
_found_deprecated_tags.add(v)
v = tag
Expand Down Expand Up @@ -257,7 +270,11 @@ def traverse_yaml(obj: Any) -> None: # noqa: C901
if _noqa_comment_re.match(comment_str):
line = v.start_mark.line + 1 # ruamel line numbers start at 0
lintable.line_skips[line].update(
get_rule_skips_from_line(comment_str.strip()),
get_rule_skips_from_line(
comment_str.strip(),
lintable=lintable,
lineno=line,
),
)
yaml_comment_obj_strings.append(str(obj.ca.items))
if isinstance(obj, dict):
Expand All @@ -277,7 +294,7 @@ def traverse_yaml(obj: Any) -> None: # noqa: C901
rule_id_list = []
for comment_obj_str in yaml_comment_obj_strings:
for line in comment_obj_str.split(r"\n"):
rule_id_list.extend(get_rule_skips_from_line(line))
rule_id_list.extend(get_rule_skips_from_line(line, lintable=lintable))

return [normalize_tag(tag) for tag in rule_id_list]

Expand Down
20 changes: 19 additions & 1 deletion test/test_skiputils.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from ansiblelint.constants import SKIPPED_RULES_KEY
from ansiblelint.file_utils import Lintable
from ansiblelint.runner import Runner
from ansiblelint.skip_utils import (
append_skipped_rules,
get_rule_skips_from_line,
Expand All @@ -17,6 +18,7 @@
if TYPE_CHECKING:
from ansible.parsing.yaml.objects import AnsibleBaseYAMLObject

from ansiblelint.rules import RulesCollection
from ansiblelint.testing import RunFromText

PLAYBOOK_WITH_NOQA = """\
Expand All @@ -43,7 +45,7 @@
)
def test_get_rule_skips_from_line(line: str, expected: str) -> None:
"""Validate get_rule_skips_from_line."""
v = get_rule_skips_from_line(line)
v = get_rule_skips_from_line(line, lintable=Lintable(""))
assert v == [expected]


Expand Down Expand Up @@ -232,3 +234,19 @@ def test_append_skipped_rules(
def test_is_nested_task(task: dict[str, Any], expected: bool) -> None:
"""Test is_nested_task() returns expected bool."""
assert is_nested_task(task) == expected


def test_capture_warning_outdated_tag(
default_rules_collection: RulesCollection,
) -> None:
"""Test that exclude paths do work."""
runner = Runner(
"examples/playbooks/capture-warning.yml",
rules=default_rules_collection,
)

matches = runner.run()
assert len(matches) == 1
assert matches[0].rule.id == "warning"
assert matches[0].tag == "warning[outdated-tag]"
assert matches[0].lineno == 8