Skip to content

Commit

Permalink
Capture python warnings and report them as matches
Browse files Browse the repository at this point in the history
This change introduces a new way to generate warnings in linter, one
that uses the warnings support in Python. The benefit is that this
allows use to generate warnings from anywhere inside the code without
having to pass them from function to function.

For start we will be using this for internal rules, like ability to
report outdated noqa comments.
  • Loading branch information
ssbarnea committed Apr 24, 2023
1 parent 2cb73c6 commit f65e10e
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 15 deletions.
2 changes: 1 addition & 1 deletion playbook.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
- name: Example
hosts: localhost
tasks:
- name: include extra tasks
- name: include extra tasks # noqa: 102
ansible.builtin.include_tasks:
file: /dev/null
4 changes: 4 additions & 0 deletions src/ansiblelint/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
from ansiblelint.file_utils import Lintable, normpath


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


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 @@ -121,7 +121,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 @@ -200,7 +200,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 @@ -91,7 +91,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 @@ -171,7 +174,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
45 changes: 43 additions & 2 deletions src/ansiblelint/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,18 @@
import multiprocessing
import multiprocessing.pool
import os
import warnings
from collections.abc import Generator
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.config import Options
from ansiblelint.constants import States
from ansiblelint.errors import MatchError
from ansiblelint.errors import LintWarning, MatchError
from ansiblelint.file_utils import Lintable, expand_dirs_in_lintables
from ansiblelint.rules.syntax_check import AnsibleSyntaxCheckRule

Expand All @@ -33,6 +34,14 @@ class LintResult:
files: set[Lintable]


@dataclass
class Occurrence:
"""Class that tracks result of linting."""

file: Lintable
lineno: MatchError


class Runner:
"""Runner class performs the linting process."""

Expand Down Expand Up @@ -112,6 +121,38 @@ def is_excluded(self, lintable: Lintable) -> bool:

def run(self) -> list[MatchError]: # noqa: C901
"""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, Lintable):
filename = warn.source
match = MatchError(
message=warn.message if isinstance(warn.message, str) else "?",
rule=WarningRule(),
filename=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
"""Internal implementation of run."""
files: list[Lintable] = []
matches: list[MatchError] = []

Expand Down
30 changes: 23 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 collections.abc import Generator, Sequence
from functools import cache
from itertools import product
Expand All @@ -42,6 +43,7 @@
RENAMED_TAGS,
SKIPPED_RULES_KEY,
)
from ansiblelint.errors import LintWarning
from ansiblelint.file_utils import Lintable

if TYPE_CHECKING:
Expand All @@ -57,7 +59,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 @@ -66,10 +72,16 @@ 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 regressions"
warnings.warn(
message=msg,
category=LintWarning,
source={
"filename": lintable,
"lineno": lineno,
"tag": "warning[outdated-tag]",
},
stacklevel=0,
)
_found_deprecated_tags.add(v)
v = tag
Expand Down Expand Up @@ -253,7 +265,11 @@ def traverse_yaml(obj: Any) -> None:
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 @@ -273,7 +289,7 @@ def traverse_yaml(obj: Any) -> None:
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
2 changes: 1 addition & 1 deletion test/test_skiputils.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,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

0 comments on commit f65e10e

Please sign in to comment.