diff --git a/.github/workflows/pull-request-management.yml b/.github/workflows/pull-request-management.yml index 8cfebb1..027c221 100644 --- a/.github/workflows/pull-request-management.yml +++ b/.github/workflows/pull-request-management.yml @@ -35,7 +35,7 @@ jobs: runs-on: ubuntu-20.04 strategy: matrix: - python: ["3.8", "3.9", "3.10", "3.11", "3.12"] + python: ["3.9", "3.10", "3.11", "3.12"] steps: - uses: actions/checkout@v4 - name: Setup Python diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6868553..c2f0559 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -2,7 +2,7 @@ # See https://pre-commit.com/hooks.html for more hooks ci: autoupdate_commit_msg: "CI: pre-commit autoupdate" - + files: ^(j2lint|tests)/ repos: @@ -42,24 +42,26 @@ repos: - '{#||#}' - --no-extra-eol - - repo: https://github.com/pycqa/flake8 - rev: 7.1.1 + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.8.4 hooks: - - id: flake8 - name: Check for PEP8 error on Python files - args: - - --config=/dev/null - - --max-line-length=160 - types: [python] + - id: ruff + name: Run Ruff linter + args: [ --fix ] + - id: ruff-format + name: Run Ruff formatter - repo: https://github.com/pycqa/pylint rev: v3.3.3 hooks: - - id: pylint # Use pylintrc file in repository + - id: pylint name: Check for Linting error on Python files description: This hook runs pylint. types: [python] args: + - -rn # Only display messages + - -sn # Don't display the score + - --rcfile=pyproject.toml # Link to config file # Suppress duplicate code for modules header - -d duplicate-code additional_dependencies: @@ -67,24 +69,20 @@ repos: - rich exclude: ^tests/ - - repo: https://github.com/pycqa/isort - rev: 5.13.2 - hooks: - - id: isort - name: Check for changes when running isort on all python files - - - repo: https://github.com/psf/black - rev: 24.10.0 - hooks: - - id: black - name: Check for changes when running Black on all python files - - repo: https://github.com/pre-commit/mirrors-mypy rev: v1.14.0 hooks: - id: mypy args: - --config-file=pyproject.toml - # additional_dependencies: # Do not run on test files: ^(j2lint)/ + + - repo: https://github.com/codespell-project/codespell + rev: v2.3.0 + hooks: + - id: codespell + name: Checks for common misspellings in text files. + entry: codespell + language: python + types: [text] diff --git a/README.md b/README.md index e119c50..202b740 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ Build a Jinja2 linter that will provide the following capabilities: ### Requirements -Python version 3.8+ +Minimym Python version: 3.9 ### Install with pip diff --git a/j2lint/__init__.py b/j2lint/__init__.py index 4fae462..ee383e6 100644 --- a/j2lint/__init__.py +++ b/j2lint/__init__.py @@ -1,8 +1,10 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""__init__.py - A command-line utility that checks for best practices in Jinja2. -""" +"""__init__.py - A command-line utility that checks for best practices in Jinja2.""" + +from rich.console import Console + NAME = "j2lint" VERSION = "v1.1.0" DESCRIPTION = __doc__ @@ -10,3 +12,5 @@ __author__ = "Arista Networks" __license__ = "MIT" __version__ = VERSION + +CONSOLE = Console() diff --git a/j2lint/__main__.py b/j2lint/__main__.py index 8300fca..c66a03c 100644 --- a/j2lint/__main__.py +++ b/j2lint/__main__.py @@ -1,17 +1,16 @@ -#!/usr/bin/python # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""__main__.py - A command-line utility that checks for best practices in Jinja2. -""" +"""__main__.py - A command-line utility that checks for best practices in Jinja2.""" + import sys -import traceback +from j2lint import CONSOLE from j2lint.cli import run if __name__ == "__main__": try: sys.exit(run()) - except Exception: - print(traceback.format_exc()) + except Exception: # noqa: BLE001 + CONSOLE.print_exception(show_locals=True) raise SystemExit from BaseException diff --git a/j2lint/cli.py b/j2lint/cli.py index 91988d5..a771433 100644 --- a/j2lint/cli.py +++ b/j2lint/cli.py @@ -1,26 +1,28 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""cli.py - Command line argument parser. -""" +"""cli.py - Command line argument parser.""" + from __future__ import annotations import argparse import json import logging -import os import sys import tempfile +from pathlib import Path +from typing import TYPE_CHECKING -from rich.console import Console from rich.tree import Tree -from . import DESCRIPTION, NAME, VERSION -from .linter.collection import DEFAULT_RULE_DIR, RulesCollection -from .linter.error import LinterError -from .linter.runner import Runner -from .logger import add_handler, logger -from .utils import get_files +from j2lint import CONSOLE, DESCRIPTION, NAME, VERSION +from j2lint.linter.collection import DEFAULT_RULE_DIR, RulesCollection +from j2lint.linter.runner import Runner +from j2lint.logger import add_handler, logger +from j2lint.utils import get_files + +if TYPE_CHECKING: + from .linter.error import LinterError IGNORE_RULES = WARN_RULES = [ "jinja-syntax-error", @@ -45,14 +47,14 @@ "V2", ] -CONSOLE = Console() - def create_parser() -> argparse.ArgumentParser: - """Initializes a new argument parser object + """Initialize a new argument parser object. - Returns: - ArgumentParser: Argument parser object + Returns + ------- + ArgumentParser + Argument parser object """ parser = argparse.ArgumentParser(prog=NAME, description=DESCRIPTION) @@ -63,9 +65,7 @@ def create_parser() -> argparse.ArgumentParser: default=[], help="files or directories to lint", ) - parser.add_argument( - "-l", "--list", default=False, action="store_true", help="list of lint rules" - ) + parser.add_argument("-l", "--list", default=False, action="store_true", help="list of lint rules") parser.add_argument( "-r", "--rules_dir", @@ -88,12 +88,8 @@ def create_parser() -> argparse.ArgumentParser: help="comma delimited list of file extensions, default is 'j2,jinja,jinja2'", type=lambda s: [f".{item}" for item in s.split(",")], ) - parser.add_argument( - "-d", "--debug", default=False, action="store_true", help="enable debug logs" - ) - parser.add_argument( - "-j", "--json", default=False, action="store_true", help="enable JSON output" - ) + parser.add_argument("-d", "--debug", default=False, action="store_true", help="enable debug logs") + parser.add_argument("-j", "--json", default=False, action="store_true", help="enable JSON output") parser.add_argument( "-s", "--stdin", @@ -101,15 +97,9 @@ def create_parser() -> argparse.ArgumentParser: action="store_true", help="accept template from STDIN", ) - parser.add_argument( - "--log", default=False, action="store_true", help="enable logging" - ) - parser.add_argument( - "--version", default=False, action="store_true", help="Version of j2lint" - ) - parser.add_argument( - "-o", "--stdout", default=False, action="store_true", help="stdout logging" - ) + parser.add_argument("--log", default=False, action="store_true", help="enable logging") + parser.add_argument("--version", default=False, action="store_true", help="Version of j2lint") + parser.add_argument("-o", "--stdout", default=False, action="store_true", help="stdout logging") parser.add_argument( "-i", "--ignore", @@ -131,26 +121,43 @@ def create_parser() -> argparse.ArgumentParser: def sort_issues(issues: list[LinterError]) -> list[LinterError]: - """Sorted list of issues + """Sorted list of issues. - Args: - issues (list): list of issue dictionaries + Parameters + ---------- + issues + List of issue dictionaries - Returns: - list: list of sorted issue dictionaries + Returns + ------- + list + List of sorted issue dictionaries """ - issues.sort( - key=lambda issue: (issue.filename, issue.line_number, issue.rule.rule_id) - ) + issues.sort(key=lambda issue: (issue.filename, issue.line_number, issue.rule.rule_id)) return issues def get_linting_issues( - files: list[str], collection: RulesCollection, checked_files: list[str] -) -> tuple[dict[str, list[LinterError]], dict[str, list[LinterError]]]: - """checking errors and warnings""" - lint_errors: dict[str, list[LinterError]] = {} - lint_warnings: dict[str, list[LinterError]] = {} + files: list[Path], collection: RulesCollection, checked_files: list[Path] +) -> tuple[dict[Path, list[LinterError]], dict[Path, list[LinterError]]]: + """Check errors and warnings. + + Parameters + ---------- + files + List of files. + collection + The RulesCollection to use on the file. + checked_files + List of files already checked. + + Returns + ------- + tuple[dict[Path, list[LinterError]], dict[Path, list[LinterError]]] + A two tuple containing two dictionaries. The first dictionary contains the errors and the second dictionary the warnings. + """ + lint_errors: dict[Path, list[LinterError]] = {} + lint_warnings: dict[Path, list[LinterError]] = {} # Get linting issues for file_name in files: @@ -166,15 +173,28 @@ def get_linting_issues( def print_json_output( - lint_errors: dict[str, list[LinterError]], - lint_warnings: dict[str, list[LinterError]], + lint_errors: dict[Path, list[LinterError]], + lint_warnings: dict[Path, list[LinterError]], ) -> tuple[int, int]: - """printing json output""" + """Print json output. + + Parameters + ---------- + lint_errors + a dictionary containing pairs of type {filename: list of errors} + lint_warnings + a dictionary containing pairs of type {filename: list of warnings} + + Returns + ------- + tuple[int, int] + A two tuple containing the total number of errors and the total number of warnings. + """ json_output: dict[str, list[str]] = {"ERRORS": [], "WARNINGS": []} - for _, errors in lint_errors.items(): + for errors in lint_errors.values(): for error in errors: json_output["ERRORS"].append(json.loads(str(error.to_json()))) - for _, warnings in lint_warnings.items(): + for warnings in lint_warnings.values(): for warning in warnings: json_output["WARNINGS"].append(json.loads(str(warning.to_json()))) CONSOLE.print_json(f"\n{json.dumps(json_output)}") @@ -183,15 +203,29 @@ def print_json_output( def print_string_output( - lint_errors: dict[str, list[LinterError]], - lint_warnings: dict[str, list[LinterError]], + lint_errors: dict[Path, list[LinterError]], + lint_warnings: dict[Path, list[LinterError]], + *, verbose: bool, ) -> tuple[int, int]: - """print non-json output""" + """Print string output. + + Parameters + ---------- + lint_errors + a dictionary containing pairs of type {filename: list of errors} + lint_warnings + a dictionary containing pairs of type {filename: list of warnings} + verbose + When True, output a string if neither an error nor a warning was passed. + + Returns + ------- + tuple[int, int] + A tuple containing the total number of errors and the total number of warnings. + """ - def print_issues( - lint_issues: dict[str, list[LinterError]], issue_type: str - ) -> None: + def print_issues(lint_issues: dict[Path, list[LinterError]], issue_type: str) -> None: CONSOLE.rule(f"[bold red]JINJA2 LINT {issue_type}") for key, issues in lint_issues.items(): if not issues: @@ -199,11 +233,11 @@ def print_issues( tree = Tree(f"{key}") for j2_issue in issues: - tree.add(j2_issue.to_rich(verbose)) + tree.add(j2_issue.to_rich(verbose=verbose)) CONSOLE.print(tree) - total_lint_errors = sum(len(issues) for _, issues in lint_errors.items()) - total_lint_warnings = sum(len(issues) for _, issues in lint_warnings.items()) + total_lint_errors = sum(len(issues) for issues in lint_errors.values()) + total_lint_warnings = sum(len(issues) for issues in lint_warnings.values()) if total_lint_errors: print_issues(lint_errors, "ERRORS") @@ -214,41 +248,60 @@ def print_issues( if verbose: CONSOLE.print("Linting complete. No problems found!", style="green") else: - CONSOLE.print( - f"\nJinja2 linting finished with " - f"{total_lint_errors} error(s) and {total_lint_warnings} warning(s)" - ) + CONSOLE.print(f"\nJinja2 linting finished with " f"{total_lint_errors} error(s) and {total_lint_warnings} warning(s)") return total_lint_errors, total_lint_warnings -def remove_temporary_file(stdin_filename: str) -> None: - """Remove temporary file""" +def remove_temporary_file(stdin_filename: Path) -> None: + """Remove temporary file. + + Parameters + ---------- + stdin_filename + The name of the temporary file to be removed. + """ if stdin_filename: - os.unlink(stdin_filename) + stdin_filename.unlink() def print_string_rules(collection: RulesCollection) -> None: - """Print active rules as string""" + """Print active rules as string. + + Parameters + ---------- + collection + The RulesCollection to print. + """ CONSOLE.rule("[bold red]Rules in the Collection") CONSOLE.print(collection.to_rich()) def print_json_rules(collection: RulesCollection) -> None: - """Print active rules as json""" + """Print active rules as json. + + Parameters + ---------- + collection + The RulesCollection to print as JSON. + """ CONSOLE.print_json(collection.to_json()) def run(args: list[str] | None = None) -> int: - """Runs jinja2 linter + """Run jinja2 linter. - Args: - args ([string], optional): Command line arguments. Defaults to None. + Parameters + ---------- + args + Command line arguments. Defaults to None. - Returns: - int: 0 on success + Returns + ------- + int + 0 on success """ - # pylint: disable=too-many-branches + # ruff: noqa: PLR0912,C901 # given the number of input parameters, it is acceptable to keep these many branches. parser = create_parser() @@ -262,32 +315,26 @@ def run(args: list[str] | None = None) -> int: else: log_level = logging.DEBUG if options.debug else logging.INFO if options.log: - add_handler(logger, False, log_level) + add_handler(logger, log_level, stream_handler=False) if options.stdout: - add_handler(logger, True, log_level) + add_handler(logger, log_level, stream_handler=True) logger.debug("Lint options selected %s", options) stdin_filename = None - file_or_dir_names: list[str] = list(set(options.files)) - checked_files: list[str] = [] + file_or_dir_names: list[Path] = [Path(pathname) for pathname in set(options.files)] + checked_files: list[Path] = [] if options.stdin and not sys.stdin.isatty(): - with tempfile.NamedTemporaryFile( - "w", suffix=".j2", delete=False - ) as stdin_tmpfile: + with tempfile.NamedTemporaryFile("w", suffix=".j2", delete=False) as stdin_tmpfile: stdin_tmpfile.write(sys.stdin.read()) - stdin_filename = stdin_tmpfile.name + stdin_filename = Path(stdin_tmpfile.name) file_or_dir_names.append(stdin_filename) # Collect the rules from the configuration - collection = RulesCollection(options.verbose) + collection = RulesCollection(verbose=options.verbose) for rules_dir in options.rules_dir: - collection.extend( - RulesCollection.create_from_directory( - rules_dir, options.ignore, options.warn - ).rules - ) + collection.extend(RulesCollection.create_from_directory(rules_dir, options.ignore, options.warn).rules) # List lint rules if options.list: @@ -315,9 +362,7 @@ def run(args: list[str] | None = None) -> int: logger.debug("JSON output enabled") total_lint_errors, _ = print_json_output(lint_errors, lint_warnings) else: - total_lint_errors, _ = print_string_output( - lint_errors, lint_warnings, options.verbose - ) + total_lint_errors, _ = print_string_output(lint_errors, lint_warnings, verbose=options.verbose) # Remove temporary file if stdin_filename is not None: diff --git a/j2lint/linter/__init__.py b/j2lint/linter/__init__.py index e602bd6..682efb0 100644 --- a/j2lint/linter/__init__.py +++ b/j2lint/linter/__init__.py @@ -1,3 +1,4 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. +"""j2lint.linter packages.""" diff --git a/j2lint/linter/collection.py b/j2lint/linter/collection.py index 287fae6..5f3ab62 100644 --- a/j2lint/linter/collection.py +++ b/j2lint/linter/collection.py @@ -1,68 +1,89 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""collection.py - Class to create a collection of linting rules. -""" +"""collection.py - Class to create a collection of linting rules.""" + from __future__ import annotations import json -import os -import pathlib -from collections.abc import Iterable +from pathlib import Path +from typing import TYPE_CHECKING from rich.console import Group from rich.tree import Tree +from j2lint.linter.error import JinjaLinterError from j2lint.logger import logger from j2lint.utils import is_rule_disabled, load_plugins -from .error import LinterError -from .rule import Rule +if TYPE_CHECKING: + from collections.abc import Iterator + + from .error import LinterError + from .rule import Rule -DEFAULT_RULE_DIR = pathlib.Path(__file__).parent.parent / "rules" +DEFAULT_RULE_DIR = Path(__file__).parent.parent / "rules" class RulesCollection: """RulesCollection class which checks the linting rules against a file.""" - def __init__(self, verbose: bool = False) -> None: + def __init__(self, *, verbose: bool = False) -> None: self.rules: list[Rule] = [] self.verbose = verbose - def __iter__(self) -> Iterable[Rule]: + def __iter__(self) -> Iterator[Rule]: + """Return iterable of Rules in the collection. + + Returns + ------- + Iterable[Rule] + """ return iter(self.rules) def __len__(self) -> int: + """Return the number of rules in the collection. + + Returns + ------- + int + The number of rules in the collection. + """ return len(self.rules) def extend(self, more: list[Rule]) -> None: - """Extends list of rules + """Extend list of rules. - Args: - more (list): list of rules classes + TODO: This does not protect against duplicate rules - Note: This does not protect against duplicate rules + Parameters + ---------- + more + List of rules classes to append to the collection. """ self.rules.extend(more) - def run(self, file_path: str) -> tuple[list[LinterError], list[LinterError]]: - """Runs the linting rules for given file + def run(self, file_path: Path) -> tuple[list[LinterError], list[LinterError]]: + """Run the linting rules for given file. - Args: - file_dict (dict): file path and file type + Parameters + ---------- + file_path + Path - Returns: - tuple(list, list): a tuple containing the list of linting errors - and the list of linting warnings found + Returns + ------- + tuple[list, list] + A tuple containing the list of linting errors and the list of linting warnings found. """ text = "" errors: list[LinterError] = [] warnings: list[LinterError] = [] try: - with open(file_path, mode="r", encoding="utf-8") as file: + with file_path.open(encoding="utf-8") as file: text = file.read() - except IOError as err: + except OSError as err: logger.warning("Could not open %s - %s", file_path, err.strerror) return errors, warnings @@ -81,10 +102,10 @@ def run(self, file_path: str) -> tuple[list[LinterError], list[LinterError]]: logger.debug("Running linting rule %s on file %s", rule, file_path) if rule in rule.warn: - warnings.extend(rule.checkrule(file_path, text)) + warnings.extend(rule.checkrule(str(file_path), text)) else: - errors.extend(rule.checkrule(file_path, text)) + errors.extend(rule.checkrule(str(file_path), text)) for error in errors: logger.error(error.to_rich()) @@ -95,6 +116,12 @@ def run(self, file_path: str) -> tuple[list[LinterError], list[LinterError]]: return errors, warnings def __repr__(self) -> str: + """Return a representation of a RulesCollection object. + + Returns + ------- + str + """ res = [] current_origin = None for rule in sorted(self.rules, key=lambda x: (x.origin, x.rule_id)): @@ -106,16 +133,18 @@ def __repr__(self) -> str: return "\n".join(res) def to_rich(self) -> Group: - """ - Return a rich Group containing a rich Tree for each different origin - for the rules + """Return a rich Group containing a rich Tree for each different origin for the rules. Each Tree contain the rule.to_rich() output + Examples + -------- + ``` Origin: BUILT-IN ├── S0 Jinja syntax should be correct (jinja-syntax-error) ├── S1 (single-space-decorator) └── V2 (jinja-variable-format) + ``` """ res = [] current_origin = None @@ -125,36 +154,41 @@ def to_rich(self) -> Group: current_origin = rule.origin tree = Tree(f"Origin: {rule.origin}") res.append(tree) - assert tree + if not tree: + msg = "Something went wrong while converting to rich output. Please raise an issue on Github." + raise JinjaLinterError(msg) tree.add(rule.to_rich()) return Group(*res) def to_json(self) -> str: - """Return a json representation of the collection as a list of the rules""" - return json.dumps( - [ - json.loads(rule.to_json()) - for rule in sorted(self.rules, key=lambda x: (x.origin, x.rule_id)) - ] - ) + """Return a json representation of the collection as a list of the rules. + + Returns + ------- + str + """ + return json.dumps([json.loads(rule.to_json()) for rule in sorted(self.rules, key=lambda x: (x.origin, x.rule_id))]) @classmethod - def create_from_directory( - cls, rules_dir: str, ignore_rules: list[str], warn_rules: list[str] - ) -> RulesCollection: - """Creates a collection from all rule modules - - Args: - rules_dir (string): rules directory - ignore_rules (list): list of rule short_descriptions or ids to ignore - warn_rules (list): list of rule short_descriptions or ids to consider as - warnings rather than errors - - Returns: - list: a collection of rule objects + def create_from_directory(cls, rules_dir: Path, ignore_rules: list[str], warn_rules: list[str]) -> RulesCollection: + """Create a collection from all rule modules. + + Parameters + ---------- + rules_dir + The directory in which to look for the rules. + ignore_rules + List of rule short_descriptions or ids to ignore. + warn_rules + List of rule short_descriptions or ids to consider as warnings rather than errors. + + Returns + ------- + RulesCollection + A RulesColleciton object containing the rules from rules_dir except for the ignored ones. """ result = cls() - result.rules = load_plugins(os.path.expanduser(rules_dir)) + result.rules = load_plugins(rules_dir.expanduser()) for rule in result.rules: if rule.short_description in ignore_rules or rule.rule_id in ignore_rules: rule.ignore = True @@ -162,6 +196,6 @@ def create_from_directory( rule.warn.append(rule) if rules_dir != DEFAULT_RULE_DIR: for rule in result.rules: - rule.origin = rules_dir + rule.origin = str(rules_dir) logger.info("Created collection from rules directory %s", rules_dir) return result diff --git a/j2lint/linter/error.py b/j2lint/linter/error.py index 7e6529f..ad45e3c 100644 --- a/j2lint/linter/error.py +++ b/j2lint/linter/error.py @@ -1,8 +1,8 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""error.py - Error classes to format the lint errors. -""" +"""error.py - Error classes to format the lint errors.""" + from __future__ import annotations import json @@ -34,8 +34,8 @@ def __init__( self.rule = rule self.message = message or rule.description - def to_rich(self, verbose: bool = False) -> Text: - """setting string output format""" + def to_rich(self, *, verbose: bool = False) -> Text: + """Generate string output format.""" text = Text() if not verbose: text.append(self.filename, "green") @@ -59,7 +59,7 @@ def to_rich(self, verbose: bool = False) -> Text: return text def to_json(self) -> str: - """setting json output format""" + """Generate json output format.""" return json.dumps( { "id": self.rule.rule_id, @@ -73,4 +73,4 @@ def to_json(self) -> str: class JinjaLinterError(Exception): - """Jinja Linter Error""" + """Jinja Linter Error.""" diff --git a/j2lint/linter/indenter/__init__.py b/j2lint/linter/indenter/__init__.py index e602bd6..a591a4d 100644 --- a/j2lint/linter/indenter/__init__.py +++ b/j2lint/linter/indenter/__init__.py @@ -1,3 +1,4 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. +"""j2lint.linter.indenter submodule.""" diff --git a/j2lint/linter/indenter/node.py b/j2lint/linter/indenter/node.py index d8741f6..240da07 100644 --- a/j2lint/linter/indenter/node.py +++ b/j2lint/linter/indenter/node.py @@ -1,12 +1,11 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""node.py - Class node for creating a parse tree for jinja statements and - checking jinja statement indentation. -""" +"""node.py - Class node for creating a parse tree for jinja statements and checking jinja statement indentation.""" + from __future__ import annotations -from typing import NoReturn, Tuple +from typing import NoReturn from j2lint.linter.error import JinjaLinterError from j2lint.linter.indenter.statement import JINJA_STATEMENT_TAG_NAMES, JinjaStatement @@ -24,12 +23,11 @@ jinja_node_stack: list[Node] = [] jinja_delimiter_stack: list[str] = [] -# Using Tuple from typing for 3.8 support -NodeIndentationError = Tuple[int, str, str] +NodeIndentationError = tuple[int, str, str] class Node: - """Node class which represents a jinja file as a tree""" + """Node class which represents a jinja file as a tree.""" # pylint: disable=too-many-instance-attributes # Eight arguments is reasonable in this case @@ -43,18 +41,23 @@ def __init__(self) -> None: self.block_start_indent: int = 0 self.expected_indent: int = 0 - # pylint: disable=fixme - # TODO - This should be called create_child_node + # TODO: This should be called create_child_node def create_node(self, line: Statement, line_no: int, indent_level: int = 0) -> Node: - """Initializes a Node class object - - Args: - line (Statement): Parsed line of the template using get_jinja_statement - line_no (int): line number - indent_level (int, optional): expected indentation level. Defaults to 0. - - Returns: - Node: new Node class object + """Initialize a Node class object. + + Parameters + ---------- + line + Parsed line of the template using get_jinja_statement. + line_no + Line number. + indent_level + Expected indentation level. Defaults to 0. + + Returns + ------- + Node + A new Node class object. """ node = Node() statement = JinjaStatement(line) @@ -67,17 +70,20 @@ def create_node(self, line: Statement, line_no: int, indent_level: int = 0) -> N return node @staticmethod - def create_indentation_error( - node: Node, message: str - ) -> NodeIndentationError | None: - """Creates indentation error tuple - - Args: - node (Node): Node class object to create error for - message (string): error message for the line - - Returns: - tuple: tuple representing the indentation error + def create_indentation_error(node: Node, message: str) -> NodeIndentationError | None: + """Create indentation error tuple. + + Parameters + ---------- + node + Node object to create error for. + message + Error message for the line. + + Returns + ------- + NodeIndentationError | None + A tuple representing the indentation error. """ if node.statement is None: return None @@ -92,27 +98,21 @@ def create_indentation_error( message, ) - def check_indent_level( - self, result: list[NodeIndentationError], node: Node - ) -> None: - """check if the actual and expected indent level for a line match + def check_indent_level(self, result: list[NodeIndentationError], node: Node) -> None: + """Check if the actual and expected indent level for a line match. - Args: - result (list): list of tuples of indentation errors - node (Node): Node object for which to check the level is correct + Parameters + ---------- + result + List of tuples of indentation errors + node + Node object for which to check the level is correct """ if node.statement is None: return actual = node.statement.begin - if ( - jinja_node_stack - and jinja_node_stack[0].statement is not None - and jinja_node_stack[0].statement.start_delimiter in JINJA_START_DELIMITERS - ): - self.block_start_indent = 1 - elif ( - node.expected_indent == 0 - and node.statement.start_delimiter in JINJA_START_DELIMITERS + if (jinja_node_stack and jinja_node_stack[0].statement is not None and jinja_node_stack[0].statement.start_delimiter in JINJA_START_DELIMITERS) or ( + node.expected_indent == 0 and node.statement.start_delimiter in JINJA_START_DELIMITERS ): self.block_start_indent = 1 else: @@ -121,9 +121,7 @@ def check_indent_level( if node.statement.start_delimiter in JINJA_START_DELIMITERS: expected = node.expected_indent + self.block_start_indent else: - expected = ( - node.expected_indent + DEFAULT_WHITESPACES + self.block_start_indent - ) + expected = node.expected_indent + DEFAULT_WHITESPACES + self.block_start_indent if actual != expected: message = f"Bad Indentation, expected {expected}, got {actual}" if (error := self.create_indentation_error(node, message)) is not None: @@ -131,20 +129,34 @@ def check_indent_level( logger.debug(error) def _assert_not_none(self, current_line_no: int, new_line_no: int | None) -> int: - """ - Helper function to verify that the new_line_no is not None + """Verify that the new_line_no is not None. + + Parameters + ---------- + current_line_no + The current line number with the opening Jinja tag. + new_line_no + The new line number + + Returns + ------- + int + The new line number if not None. + + Raises + ------ + JinjaLinterError + If new line number is None. + TODO: Probably should never return None and instead raise in check_indentation """ if new_line_no is None: - raise JinjaLinterError( - "Recursive check_indentation returned None for an opening tag " - f"line {current_line_no} - missing closing tag", - ) + msg = f"Recursive check_indentation returned None for an opening tag line {current_line_no} - missing closing tag" + raise JinjaLinterError(msg) return new_line_no # pylint: disable=inconsistent-return-statements,fixme - # TODO - newer version of pylint (2.17.0) catches some error here - # address in refactoring + # TODO: newer version of pylint (2.17.0) catches some error here address in refactoring def check_indentation( self, result: list[NodeIndentationError], @@ -152,30 +164,35 @@ def check_indentation( line_no: int = 0, indent_level: int = 0, ) -> int | None: - """Checks indentation for a list of lines - Updates the 'result' list argument with indentation errors - - Args: - result (list): list of indentation error tuples - lines (list): lines which are to be checked for indentation - line_no (int, optional): the current lines number being evaluated. - Defaults to 0. - indent_level (int, optional): the expected indent level for the - current line. Defaults to 0. - - Raises: - JinjaLinterError: Raises error if the text file has jinja tags - which are not supported by this indenter - ValueError: Raised when no begin_tag_tuple can be found in a node in the stack - - Returns: - line_no (int) or None + """Check indentation for a list of lines and update the 'result' list argument with indentation errors. + + Parameters + ---------- + result + List of indentation error tuples. + lines + Lines which are to be checked for indentation. + line_no + The current lines number being evaluated. Defaults to 0. + indent_level + The expected indent level for the current line. Defaults to 0. + + Raises + ------ + JinjaLinterError + Raised when the text file has jinja tags which are not supported by this indenter. + ValueError + Raised when no begin_tag_tuple can be found in a node in the stack. + + Returns + ------- + int | None + line_no or None """ + # ruff: noqa: PLR0915,C901 def _append_error_to_result_and_raise(message: str) -> NoReturn: - """ - Helper function to append error to result and raise a JinjaLinterError - """ + """Append error to result and raise a JinjaLinterError.""" if (error := self.create_indentation_error(node, message)) is not None: result.append(error) raise JinjaLinterError(message) @@ -185,9 +202,7 @@ def _handle_begin_tag(node: Node, line_no: int) -> int: self.children.append(node) line_no = self._assert_not_none( line_no, - node.check_indentation( - result, lines, line_no + 1, indent_level + INDENT_SHIFT - ), + node.check_indentation(result, lines, line_no + 1, indent_level + INDENT_SHIFT), ) self.check_indent_level(result, node) @@ -203,9 +218,7 @@ def _handle_middle_tag(node: Node, line_no: int) -> int: line_no = self._assert_not_none( line_no, - node.check_indentation( - result, lines, line_no + 1, indent_level + INDENT_SHIFT - ), + node.check_indentation(result, lines, line_no + 1, indent_level + INDENT_SHIFT), ) self.check_indent_level(result, node) @@ -219,7 +232,7 @@ def _handle_end_tag(node: Node, line_no: int) -> int: matchnode.node_end = line_no node.node_end = line_no node.expected_indent = matchnode.expected_indent - self.parent.children.append(node) # type: ignore + self.parent.children.append(node) if matchnode == self: line_no += 1 self.check_indent_level(result, node) @@ -240,15 +253,11 @@ def _handle_end_tag(node: Node, line_no: int) -> int: return _handle_end_tag(node, line_no) if node.tag in MIDDLE_TAGS: - begin_tag_tuple = get_tuple( - JINJA_STATEMENT_TAG_NAMES, jinja_node_stack[-1].tag - ) + begin_tag_tuple = get_tuple(JINJA_STATEMENT_TAG_NAMES, jinja_node_stack[-1].tag) if begin_tag_tuple is None: - _append_error_to_result_and_raise( - f"Node {jinja_node_stack[-1]} should have been a begin_tag" - ) + _append_error_to_result_and_raise(f"Node {jinja_node_stack[-1]} should have been a begin_tag") - if node.tag in begin_tag_tuple: # type: ignore + if node.tag in begin_tag_tuple: if jinja_node_stack[-1] != self: del node return line_no diff --git a/j2lint/linter/indenter/statement.py b/j2lint/linter/indenter/statement.py index fa843fa..8507058 100644 --- a/j2lint/linter/indenter/statement.py +++ b/j2lint/linter/indenter/statement.py @@ -1,14 +1,16 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""statement.py - Class and variables for jinja statements. -""" +"""statement.py - Class and variables for jinja statements.""" + from __future__ import annotations # pylint: disable=too-few-public-methods import re +from typing import TYPE_CHECKING -from j2lint.utils import Statement +if TYPE_CHECKING: + from j2lint.utils import Statement JINJA_STATEMENT_TAG_NAMES = [ ("for", "else", "endfor"), @@ -20,9 +22,7 @@ class JinjaStatement: """Class for representing a jinja statement.""" - # pylint: disable = fixme - # FIXME - this could probably be a method in Node rather than a class - # with no method - maybe a dataclass + # TODO: this could probably be a method in Node rather than a class with no method - maybe a dataclass def __init__(self, line: Statement) -> None: whitespaces = re.findall(r"\s*", line[0]) diff --git a/j2lint/linter/rule.py b/j2lint/linter/rule.py index c14f42e..e3b6a2f 100644 --- a/j2lint/linter/rule.py +++ b/j2lint/linter/rule.py @@ -1,9 +1,8 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""rule.py - Base class for all the lint rules with functions for mathching - line and text based rule. -""" +"""rule.py - Base class for all the lint rules with functions for matching line and text based rule.""" + from __future__ import annotations import json @@ -16,9 +15,7 @@ class Rule(ABC): - """Abstract rule class which acts as a base class for rules with regex match - functions. - """ + """Abstract rule class which acts as a base class for rules with regex match functions.""" rule_id: ClassVar[str] short_description: ClassVar[str] @@ -27,15 +24,17 @@ class Rule(ABC): def __init__( self, + *, ignore: bool = False, warn: list[Any] | None = None, origin: str = "BUILT-IN", - ): + ) -> None: self.ignore = ignore self.warn = warn if warn is not None else [] self.origin = origin - def __init_subclass__(cls, *args: Any, **kwargs: Any) -> None: + def __init_subclass__(cls, *args: Any, **kwargs: Any) -> None: # noqa: ANN401 + """Override the way a subclass of Rule is instantiated.""" super().__init_subclass__(**kwargs) # Mandatory class attributes mandatory_attributes = [ @@ -46,22 +45,25 @@ def __init_subclass__(cls, *args: Any, **kwargs: Any) -> None: ] for attr in mandatory_attributes: if not hasattr(cls, attr): - raise NotImplementedError( - f"Class {cls} is missing required class attribute {attr}" - ) + msg = f"Class {cls} is missing required class attribute {attr}" + raise NotImplementedError(msg) if cls.severity not in [None, "LOW", "MEDIUM", "HIGH"]: - raise JinjaLinterError( - f"Rule {cls.rule_id}: severity must be in [None, 'LOW', 'MEDIUM', 'HIGH'], {cls.severity} was provided" - ) + msg = f"Rule {cls.rule_id}: severity must be in [None, 'LOW', 'MEDIUM', 'HIGH'], {cls.severity} was provided" + raise JinjaLinterError(msg) def __repr__(self) -> str: + """Return a representation of a Rule object.""" return f"{self.rule_id}: {self.description}" def to_rich(self) -> Text: - """ - Return a rich reprsentation of the rule, e.g.: - S0 Jinja syntax should be correct (jinja-syntax-error) + """Return a rich representation of the rule. + + Examples + -------- + ``` + S0 Jinja syntax should be correct (jinja-syntax-error) + ``` Where `S0` is in red and `(jinja-syntax-error)` in blue """ @@ -72,7 +74,7 @@ def to_rich(self) -> Text: return res def to_json(self) -> str: - """Return a json representation of the rule""" + """Return a json representation of the rule.""" return json.dumps( { "rule_id": self.rule_id, @@ -85,23 +87,58 @@ def to_json(self) -> str: @abstractmethod def checktext(self, filename: str, text: str) -> list[LinterError]: - """This method is expected to be overriden by child classes""" + """Check the rule against a full file content. - @abstractmethod - def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """This method is expected to be overriden by child classes""" + This method is expected to be overrididen by child classes. - def checkrule(self, filename: str, text: str) -> list[LinterError]: + Parameters + ---------- + filename + The filename to which the line belongs. + text + Entire text content of the file + + Returns + ------- + list[LinterError] + The list of LinterError generated by this rule. """ - Checks the string text against the current rule by calling - either the checkline or checktext method depending on which one is implemented - Args: - filename (string): file path of the file to be checked - text (string): file text of the same file + @abstractmethod + def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: + """Check the rule against a full file content. + + This method is expected to be overrididen by child classes. + + Parameters + ---------- + filename + The filename to which the line belongs. + line + The content of the line to check. + line_no: + The line number. + + Returns + ------- + list[LinterError] + The list of LinterError generated by this rule. + """ - Returns: - list: list of LinterError from issues in the given file + def checkrule(self, filename: str, text: str) -> list[LinterError]: + """Check the string text against the current rule by calling either the checkline or checktext method depending on which one is implemented. + + Parameters + ---------- + filename + The filename to which the line belongs. + text + Entire text content of the file + + Returns + ------- + list[LinterError] + The list of LinterError generated by this rule. """ errors: list[LinterError] = [] @@ -113,14 +150,10 @@ def checkrule(self, filename: str, text: str) -> list[LinterError]: except NotImplementedError: # checkline it is for index, line in enumerate(text.split("\n")): - # pylint: disable = fixme - # FIXME - parsing jinja2 templates .. lines starting with `# - # should probably still be parsed somewhow as these - # are not comments. + # TODO: parsing jinja2 templates .. lines starting with `#` should probably still be parsed somewhow as these are not comments. if line.lstrip().startswith("#"): continue results = self.checkline(filename, line, line_no=index + 1) errors.extend(results) - # errors.append(LinterError(index + 1, line, file["path"], self)) return errors diff --git a/j2lint/linter/runner.py b/j2lint/linter/runner.py index 87b5f09..6d5b593 100644 --- a/j2lint/linter/runner.py +++ b/j2lint/linter/runner.py @@ -1,58 +1,62 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""runner.py - Class to run the rules collection for all the files. -""" +"""runner.py - Class to run the rules collection for all the files.""" + from __future__ import annotations +from typing import TYPE_CHECKING + from j2lint.logger import logger -from .collection import RulesCollection -from .error import LinterError +if TYPE_CHECKING: + from pathlib import Path + + from .collection import RulesCollection + from .error import LinterError class Runner: - """Class to run the rules collection for all the files + """Class to run the rules collection for all the files. TODO: refactor - with this code it seems that files will always be a set of 1 file - indeed, a different Runner is created for each file in cli.py """ - def __init__( - self, collection: RulesCollection, file_name: str, checked_files: list[str] - ) -> None: + def __init__(self, collection: RulesCollection, file_name: Path, checked_files: list[Path]) -> None: self.collection = collection - self.files: set[str] = {file_name} + self.files: set[Path] = {file_name} self.checked_files = checked_files - def is_already_checked(self, file_path: str) -> bool: - """Returns true if the file is already checked once + def is_already_checked(self, file_path: Path) -> bool: + """Return True if the file is already checked once. - Args: - file_path (string): file path + Parameters + ---------- + file_path + File path - Returns: - bool: True if file is already checked once + Returns + ------- + bool + True if file is already checked once """ return file_path in self.checked_files def run(self) -> tuple[list[LinterError], list[LinterError]]: - """Runs the lint rules collection on all the files - - Returns: - tuple(list, list): a tuple containing the list of linting errors - and the list of linting warnings found - TODO - refactor this - it is quite weird to do the conversion - from tuple to dict here - maybe simply init with the dict + """Run the lint rules collection on all the files. + + Returns + ------- + tuple[list, list] + A tuple containing the list of linting errors and the list of linting warnings found. + + TODO: refactor this - it is quite weird to do the conversion from tuple to dict here maybe simply init with the dict """ - files: list[str] = [] + files: list[Path] = [] for index, file in enumerate(self.files): - # pylint: disable = fixme - # FIXME - as of now it seems that both next tests - # will never occurs as self.files is always - # a single file. + # TODO: as of now it seems that both next tests will never occurs as self.files is always a single file. # Skip already checked files if self.is_already_checked(file): continue @@ -63,9 +67,7 @@ def run(self) -> tuple[list[LinterError], list[LinterError]]: errors: list[LinterError] = [] warnings: list[LinterError] = [] - # pylint: disable = fixme - # FIXME - if there are multiple files, errors and warnings are overwritten.. - # fortunately there is only one file currently + # TODO: if there are multiple files, errors and warnings are overwritten.. fortunately there is only one file currently for file in files: logger.debug("Running linting rules for %s", file) errors, warnings = self.collection.run(file) diff --git a/j2lint/logger.py b/j2lint/logger.py index db54a6b..7604e97 100644 --- a/j2lint/logger.py +++ b/j2lint/logger.py @@ -1,8 +1,8 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""logger.py - Creates logger object. -""" +"""logger.py - Creates logger object.""" + import logging from logging import handlers @@ -13,16 +13,22 @@ logger = logging.getLogger("") -def add_handler(log: logging.Logger, stream_handler: bool, log_level: int) -> None: - """defined logging handlers""" - log_format = logging.Formatter( - "%(asctime)s - %(name)s - %(levelname)s - %(message)s" - ) +def add_handler(log: logging.Logger, log_level: int, *, stream_handler: bool) -> None: + """Define logging handlers. + + Parameters + ---------- + log: + The logger to manipulate. + log_level: + The level to set on `log` as an integer. + stream_handler: + When True add a RotatingFileHandler to the logger, otherwise add a RichHandler. + """ + log_format = logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s") log.setLevel(log_level) if not stream_handler: - file_handler = handlers.RotatingFileHandler( - JINJA2_LOG_FILE, maxBytes=(1048576 * 5), backupCount=4 - ) + file_handler = handlers.RotatingFileHandler(JINJA2_LOG_FILE, maxBytes=(1048576 * 5), backupCount=4) file_handler.setFormatter(log_format) log.addHandler(file_handler) else: diff --git a/j2lint/rules/__init__.py b/j2lint/rules/__init__.py index e602bd6..5f535ce 100644 --- a/j2lint/rules/__init__.py +++ b/j2lint/rules/__init__.py @@ -1,3 +1,4 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. +"""j2lint built-in rules.""" diff --git a/j2lint/rules/jinja_operator_has_spaces_rule.py b/j2lint/rules/jinja_operator_has_spaces_rule.py index b273045..b573e9b 100644 --- a/j2lint/rules/jinja_operator_has_spaces_rule.py +++ b/j2lint/rules/jinja_operator_has_spaces_rule.py @@ -1,13 +1,12 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""jinja_operator_has_spaces_rule.py - Rule class to check if operator has - surrounding spaces. -""" +"""jinja_operator_has_spaces_rule.py - Rule class to check if operator has surrounding spaces.""" + from __future__ import annotations import re -from typing import Any +from typing import Any, ClassVar from j2lint.linter.error import LinterError from j2lint.linter.rule import Rule @@ -17,54 +16,55 @@ class JinjaOperatorHasSpacesRule(Rule): """Rule class to check if jinja filter has surrounding spaces.""" rule_id = "S2" - description = ( - "When variables are used in combination with an operator, " - "the operator should be enclosed by space: '{{ my_value | to_json }}'" - ) + description = "When variables are used in combination with an operator, the operator should be enclosed by space: '{{ my_value | to_json }}'" short_description = "operator-enclosed-by-spaces" severity = "LOW" # pylint: disable=fixme - # TODO make the regex detect the operator position - operators = ["|", "+", "=="] - regexes = [] + # TODO: make the regex detect the operator position + operators: ClassVar = ["|", "+", "=="] + regexes: ClassVar = [] for operator in operators: - operator = "\\" + operator + escaped_operator = "\\" + operator regex = ( r"({[{|%](.*?)([^ |^}]" - + operator + + escaped_operator + ")(.*?)[}|%]})|({[{|%](.*?)(" - + operator + + escaped_operator + r"[^ |^{])(.*?)[}|%]})|({[{|%](.*?)([^ |^}] \s+" - + operator + + escaped_operator + ")(.*?)[}|%]})|({[{|%](.*?)(" - + operator + + escaped_operator + r" \s+[^ |^{])(.*?)[}|%]})" ) regexes.append(re.compile(regex)) def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: - super().__init__() + super().__init__(ignore=ignore, warn=warn) def checktext(self, filename: str, text: str) -> list[LinterError]: + """Not Implemented for this rule.""" raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """Checks if the given line matches the error regex + """Check if the given line matches the error regex. - Args: - line (string): a single line from the file + Parameters + ---------- + filename + The filename to which the line belongs. + line + The content of the line to check. + line_no: + The line number. - Returns: - list[LinterError]: the list of LinterError generated by this rule + Returns + ------- + list[LinterError] + The list of LinterError generated by this rule. """ - errors: list[LinterError] = [] - # pylint: disable = fixme - # TODO - refactor - # This code removes any single quoted string - # and any double quoted string to avoid - # false positive on operators + # TODO: refactor this code removes any single quoted string and any double quoted string to avoid false positive on operators if "'" in line: regx = re.findall("'([^']*)'", line) for match in regx: @@ -75,19 +75,14 @@ def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError] for match in regx: line = line.replace(('"' + match + '"'), '""') - issues = [ - operator - for regex, operator in zip(self.regexes, self.operators) - if regex.search(line) - ] + issues = [operator for regex, operator in zip(self.regexes, self.operators) if regex.search(line)] errors.extend( LinterError( line_no, line, filename, self, - f"The operator {issue} needs to be enclosed" - " by a single space on each side", + f"The operator {issue} needs to be enclosed" " by a single space on each side", ) for issue in issues ) diff --git a/j2lint/rules/jinja_statement_delimiter_rule.py b/j2lint/rules/jinja_statement_delimiter_rule.py index b12f8ae..33352f6 100644 --- a/j2lint/rules/jinja_statement_delimiter_rule.py +++ b/j2lint/rules/jinja_statement_delimiter_rule.py @@ -1,9 +1,7 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""jinja_statement_delimiter_rule.py - Rule class to check if jinja delimiters - are wrong. -""" +"""jinja_statement_delimiter_rule.py - Rule class to check if jinja delimiters are wrong.""" from __future__ import annotations @@ -23,25 +21,29 @@ class JinjaStatementDelimiterRule(Rule): severity = "LOW" def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: - super().__init__() + super().__init__(ignore=ignore, warn=warn) def checktext(self, filename: str, text: str) -> list[LinterError]: + """Not Implemented for this rule.""" raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """Checks if the given line matches the wrong delimiters - - Args: - line (string): a single line from the file - - Returns: - list[LinterError]: the list of LinterError generated by this rule + """Check if the given line matches the wrong delimiters. + + Parameters + ---------- + filename + The filename to which the line belongs. + line + The content of the line to check. + line_no: + The line number. + + Returns + ------- + list[LinterError] + The list of LinterError generated by this rule. """ - # pylint: disable=fixme - # TODO think about a better error message that can identify characters + # TODO: think about a better error message that can identify characters statements = get_jinja_statements(line) - return [ - LinterError(line_no, line, filename, self) - for statement in statements - if statement[3] in ["{%-", "{%+"] or statement[4] == "-%}" - ] + return [LinterError(line_no, line, filename, self) for statement in statements if statement[3] in ["{%-", "{%+"] or statement[4] == "-%}"] diff --git a/j2lint/rules/jinja_statement_has_spaces_rule.py b/j2lint/rules/jinja_statement_has_spaces_rule.py index 965dc4f..ec634ce 100644 --- a/j2lint/rules/jinja_statement_has_spaces_rule.py +++ b/j2lint/rules/jinja_statement_has_spaces_rule.py @@ -1,10 +1,8 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""jinja_statement_has_spaces_rule.py - Rule class to check if jinja statement has - at least a single space surrounding the - delimiter. -""" +"""jinja_statement_has_spaces_rule.py - Rule class to check if jinja statement has at least a single space surrounding the delimiter.""" + from __future__ import annotations import re @@ -15,9 +13,7 @@ class JinjaStatementHasSpacesRule(Rule): - """Rule class to check if jinja statement has at least a single space - surrounding the delimiter. - """ + """Rule class to check if jinja statement has at least a single space surrounding the delimiter.""" rule_id = "S4" description = "Jinja statement should have at least a single space after '{%' and a single space before '%}'" @@ -27,19 +23,28 @@ class JinjaStatementHasSpacesRule(Rule): regex = re.compile(r"{%[^ \-\+]|{%[\-\+][^ ]|[^ \-\+]%}|[^ ][\-\+]%}") def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: - super().__init__() + super().__init__(ignore=ignore, warn=warn) def checktext(self, filename: str, text: str) -> list[LinterError]: + """Not Implemented for this rule.""" raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """Checks if the given line matches the error regex - - Args: - line (string): a single line from the file - - Returns: - list[LinterError]: the list of LinterError generated by this rule + """Check if the given line matches the error regex. + + Parameters + ---------- + filename + The filename to which the line belongs. + line + The content of the line to check. + line_no: + The line number. + + Returns + ------- + list[LinterError] + The list of LinterError generated by this rule. """ matches = self.regex.search(line) return [LinterError(line_no, line, filename, self)] if matches else [] diff --git a/j2lint/rules/jinja_template_indentation_rule.py b/j2lint/rules/jinja_template_indentation_rule.py index 20be596..e2c485b 100644 --- a/j2lint/rules/jinja_template_indentation_rule.py +++ b/j2lint/rules/jinja_template_indentation_rule.py @@ -1,9 +1,8 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""jinja_template_indentation_rule.py - Rule class to check the jinja statement - indentation is correct. -""" +"""jinja_template_indentation_rule.py - Rule class to check the jinja statement indentation is correct.""" + from __future__ import annotations from typing import Any @@ -20,27 +19,28 @@ class JinjaTemplateIndentationRule(Rule): short_description = "jinja-statements-indentation" rule_id = "S3" - description = ( - "All J2 statements must be indented by 4 more spaces within jinja delimiter. " - "To close a control, end tag must have same indentation level." - ) + description = "All J2 statements must be indented by 4 more spaces within jinja delimiter. To close a control, end tag must have same indentation level." severity = "HIGH" def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: - super().__init__() + super().__init__(ignore=ignore, warn=warn) def checktext(self, filename: str, text: str) -> list[LinterError]: - """Checks if the given text has the error + """Check if the given text has the error. - Args: - file (string): file path - text (string): entire text content of the file + Parameters + ---------- + filename + The filename to which the line belongs. + text + Entire text content of the file - Returns: - list: Returns list of error objects + Returns + ------- + list[LinterError] + The list of LinterError generated by this rule. """ - # Collect only Jinja Statements within delimiters {% and %} - # and ignore the other statements + # Collect only Jinja Statements within delimiters {% and %} and ignore the other statements lines = get_jinja_statements(text, indentation=True) # Build a tree out of Jinja Statements to get the expected @@ -50,8 +50,7 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: try: root.check_indentation(node_errors, lines, 0) - # pylint: disable=fixme - # TODO need to fix this index error in Node + # TODO: need to fix this index error in Node except (JinjaLinterError, IndexError) as exc: logger.error( "Indentation check failed for file %s: Error: %s", @@ -59,10 +58,8 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: str(exc), ) - return [ - LinterError(line_no, section, filename, self, message) - for line_no, section, message in node_errors - ] + return [LinterError(line_no, section, filename, self, message) for line_no, section, message in node_errors] def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: + """Not Implemented for this rule.""" raise NotImplementedError diff --git a/j2lint/rules/jinja_template_no_tabs_rule.py b/j2lint/rules/jinja_template_no_tabs_rule.py index f2cea32..b772bc3 100644 --- a/j2lint/rules/jinja_template_no_tabs_rule.py +++ b/j2lint/rules/jinja_template_no_tabs_rule.py @@ -1,9 +1,8 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""jinja_template_no_tabs_rule.py - Rule class to check the file does not use tabs - for indentation. -""" +"""jinja_template_no_tabs_rule.py - Rule class to check the file does not use tabs for indentation.""" + from __future__ import annotations import re @@ -24,19 +23,28 @@ class JinjaTemplateNoTabsRule(Rule): regex = re.compile(r"\t+") def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: - super().__init__() + super().__init__(ignore=ignore, warn=warn) def checktext(self, filename: str, text: str) -> list[LinterError]: + """Not Implemented for this rule.""" raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """Checks if the given line matches the error regex - - Args: - line (string): a single line from the file - - Returns: - list[LinterError]: the list of LinterError generated by this rule + """Check if the given line matches the error regex. + + Parameters + ---------- + filename + The filename to which the line belongs. + line + The content of the line to check. + line_no: + The line number. + + Returns + ------- + list[LinterError] + The list of LinterError generated by this rule. """ matches = self.regex.search(line) return [LinterError(line_no, line, filename, self)] if matches else [] diff --git a/j2lint/rules/jinja_template_single_statement_rule.py b/j2lint/rules/jinja_template_single_statement_rule.py index c651a6d..b04ef8d 100644 --- a/j2lint/rules/jinja_template_single_statement_rule.py +++ b/j2lint/rules/jinja_template_single_statement_rule.py @@ -1,10 +1,8 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""jinja_template_single_statement_rule.py - Rule class to check if only a single - jinja statement is present on each - line. -""" +"""jinja_template_single_statement_rule.py - Rule class to check if only a single jinja statement is present on each line.""" + from __future__ import annotations from typing import Any @@ -15,9 +13,7 @@ class JinjaTemplateSingleStatementRule(Rule): - """Rule class to check if only a single jinja statement is present on each - line. - """ + """Rule class to check if only a single jinja statement is present on each line.""" rule_id = "S7" description = "Jinja statements should be on separate lines" @@ -25,22 +21,27 @@ class JinjaTemplateSingleStatementRule(Rule): severity = "MEDIUM" def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: - super().__init__() + super().__init__(ignore=ignore, warn=warn) def checktext(self, filename: str, text: str) -> list[LinterError]: + """Not Implemented for this rule.""" raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """Checks if the given line matches the error regex - - Args: - line (string): a single line from the file - - Returns: - list[LinterError]: the list of LinterError generated by this rule + """Check if the given line matches the error regex. + + Parameters + ---------- + filename + The filename to which the line belongs. + line + The content of the line to check. + line_no: + The line number. + + Returns + ------- + list[LinterError] + The list of LinterError generated by this rule. """ - return ( - [LinterError(line_no, line, filename, self)] - if len(get_jinja_statements(line)) > 1 - else [] - ) + return [LinterError(line_no, line, filename, self)] if len(get_jinja_statements(line)) > 1 else [] diff --git a/j2lint/rules/jinja_template_syntax_error_rule.py b/j2lint/rules/jinja_template_syntax_error_rule.py index ef48696..c336c6f 100644 --- a/j2lint/rules/jinja_template_syntax_error_rule.py +++ b/j2lint/rules/jinja_template_syntax_error_rule.py @@ -1,9 +1,8 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""jinja_template_syntax_error_rule.py - Rule class to check that file does not - have jinja syntax errors. -""" +"""jinja_template_syntax_error_rule.py - Rule class to check that file does not have jinja syntax errors.""" + from __future__ import annotations from typing import Any @@ -23,23 +22,26 @@ class JinjaTemplateSyntaxErrorRule(Rule): severity = "HIGH" def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: - super().__init__() + super().__init__(ignore=ignore, warn=warn) def checktext(self, filename: str, text: str) -> list[LinterError]: - """Checks if the given text has jinja syntax error - - Args: - file (string): file path - text (string): entire text content of the file - - Returns: - list: Returns list of error objects + """Check if the given text has jinja syntax error. + + Parameters + ---------- + filename + The filename to which the line belongs. + text + Entire text content of the file + + Returns + ------- + list[LinterError] + The list of LinterError generated by this rule. """ result = [] - env = jinja2.Environment( - extensions=["jinja2.ext.do", "jinja2.ext.loopcontrols"] - ) + env = jinja2.Environment(extensions=["jinja2.ext.do", "jinja2.ext.loopcontrols"], autoescape=True) try: env.parse(text) except jinja2.TemplateSyntaxError as error: @@ -56,4 +58,5 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: return result def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: + """Not Implemented for this rule.""" raise NotImplementedError diff --git a/j2lint/rules/jinja_variable_has_space_rule.py b/j2lint/rules/jinja_variable_has_space_rule.py index fb70a16..1dffc8d 100644 --- a/j2lint/rules/jinja_variable_has_space_rule.py +++ b/j2lint/rules/jinja_variable_has_space_rule.py @@ -1,51 +1,51 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""jinja_variable_has_space_rule.py - Rule class to check if jinja variables have - single space between curly brackets and - variable name. -""" +"""jinja_variable_has_space_rule.py - Rule class to check if jinja variables have single space between curly brackets and variable name.""" + from __future__ import annotations import re -from typing import Any +from typing import Any, ClassVar from j2lint.linter.error import LinterError from j2lint.linter.rule import Rule class JinjaVariableHasSpaceRule(Rule): - """Rule class to check if jinja variables have single space between curly - brackets and variable name. - """ + """Rule class to check if jinja variables have single space between curly brackets and variable name.""" rule_id = "S1" - description = ( - "A single space should be added between Jinja2 curly brackets " - "and a variable name: {{ ethernet_interface }}" - ) + description: ClassVar = "A single space should be added between Jinja2 curly brackets " "and a variable name: {{ ethernet_interface }}" short_description = "single-space-decorator" severity = "LOW" - regex = re.compile( - r"{{[^ \-\+\d][^}]+}}|{{[-\+][^ ][^}]+}}|{{[^}]+[^ \-\+\d]}}|{{[^}]+[^ {][-\+\d]}}|{{ \s+[^ \-\+]}}|{{[^}]+[^ \-\+] \s+}}" - ) + regex = re.compile(r"{{[^ \-\+\d][^}]+}}|{{[-\+][^ ][^}]+}}|{{[^}]+[^ \-\+\d]}}|{{[^}]+[^ {][-\+\d]}}|{{ \s+[^ \-\+]}}|{{[^}]+[^ \-\+] \s+}}") def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: - super().__init__() + super().__init__(ignore=ignore, warn=warn) def checktext(self, filename: str, text: str) -> list[LinterError]: + """Not Implemented for this rule.""" raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """Checks if the given line matches the error regex - - Args: - line (string): a single line from the file - - Returns: - list[LinterError]: the list of LinterError generated by this rule + """Check if the given line matches the error regex. + + Parameters + ---------- + filename + The filename to which the line belongs. + line + The content of the line to check. + line_no: + The line number. + + Returns + ------- + list[LinterError] + The list of LinterError generated by this rule. """ matches = self.regex.search(line) return [LinterError(line_no, line, filename, self)] if matches else [] diff --git a/j2lint/rules/jinja_variable_name_case_rule.py b/j2lint/rules/jinja_variable_name_case_rule.py index 4c3430d..9b01f4a 100644 --- a/j2lint/rules/jinja_variable_name_case_rule.py +++ b/j2lint/rules/jinja_variable_name_case_rule.py @@ -1,9 +1,8 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""jinja_variable_name_case_rule.py - Rule class to check the variables use - lower case. -""" +"""jinja_variable_name_case_rule.py - Rule class to check the variables use lower case.""" + from __future__ import annotations import re @@ -27,35 +26,33 @@ class JinjaVariableNameCaseRule(Rule): regex = re.compile(r"([a-zA-Z0-9-_\"']*[A-Z][a-zA-Z0-9-_\"']*)") def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: - super().__init__() + super().__init__(ignore=ignore, warn=warn) def checktext(self, filename: str, text: str) -> list[LinterError]: + """Not Implemented for this rule.""" raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """ - Checks if the given line matches the error regex, which matches - variables with non lower case characters - - Args: - line (string): a single line from the file - - Returns: - list[LinterError]: the list of LinterError generated by this rule + """Check if the given line matches the error regex, which matches variables with non lower case characters. + + Parameters + ---------- + filename + The filename to which the line belongs. + line + The content of the line to check. + line_no: + The line number. + + Returns + ------- + list[LinterError] + The list of LinterError generated by this rule. """ variables = get_jinja_variables(line) matches = [] for var in variables: matches = re.findall(self.regex, var) - matches = [ - match - for match in matches - if (match not in ["False", "True"]) - and ("'" not in match) - and ('"' not in match) - ] - - return [ - LinterError(line_no, line, filename, self, f"{self.description}: {match}") - for match in matches - ] + matches = [match for match in matches if (match not in ["False", "True"]) and ("'" not in match) and ('"' not in match)] + + return [LinterError(line_no, line, filename, self, f"{self.description}: {match}") for match in matches] diff --git a/j2lint/rules/jinja_variable_name_format_rule.py b/j2lint/rules/jinja_variable_name_format_rule.py index 00817ec..40bafe3 100644 --- a/j2lint/rules/jinja_variable_name_format_rule.py +++ b/j2lint/rules/jinja_variable_name_format_rule.py @@ -1,60 +1,56 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -""" jinja_variable_name_format_rule.py - Rule class to check that variable names - only use underscores. -""" +"""jinja_variable_name_format_rule.py - Rule class to check that variable names only use underscores.""" + from __future__ import annotations import re -from typing import Any +from typing import Any, ClassVar from j2lint.linter.error import LinterError from j2lint.linter.rule import Rule from j2lint.utils import get_jinja_variables -# pylint: disable=duplicate-code - class JinjaVariableNameFormatRule(Rule): """Rule class to check that variable names only use underscores.""" rule_id = "V2" - description = ( - "If variable is multi-words, underscore `_` should be used " - "as a separator: '{{ my_variable_name }}'" - ) + description: ClassVar = "If variable is multi-words, underscore `_` should be used " "as a separator: '{{ my_variable_name }}'" short_description = "jinja-variable-format" severity = "LOW" regex = re.compile(r"[a-zA-Z0-9-_\"']+[-][a-zA-Z0-9-_\"']+") def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: - super().__init__() + super().__init__(ignore=ignore, warn=warn) def checktext(self, filename: str, text: str) -> list[LinterError]: + """Not Implemented for this rule.""" raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """ - Checks if the given line matches the error regex, which matches - variables using `-` in their name - - Args: - line (string): a single line from the file - - Returns: - list[LinterError]: the list of LinterError generated by this rule + """Check if the given line matches the error regex, which matches variables using `-` in their name. + + Parameters + ---------- + filename + The filename to which the line belongs. + line + The content of the line to check. + line_no: + The line number. + + Returns + ------- + list[LinterError] + The list of LinterError generated by this rule. """ variables = get_jinja_variables(line) matches = [] for var in variables: matches = re.findall(self.regex, var) - matches = [ - match for match in matches if ("'" not in match) and ('"' not in match) - ] - - return [ - LinterError(line_no, line, filename, self, f"{self.description}: {match}") - for match in matches - ] + matches = [match for match in matches if ("'" not in match) and ('"' not in match)] + + return [LinterError(line_no, line, filename, self, f"{self.description}: {match}") for match in matches] diff --git a/j2lint/utils.py b/j2lint/utils.py index 300b63d..8df37da 100644 --- a/j2lint/utils.py +++ b/j2lint/utils.py @@ -1,48 +1,48 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -"""utils.py - Utility functions for jinja2 linter. -""" +"""utils.py - Utility functions for jinja2 linter.""" + from __future__ import annotations -import glob import importlib.util import os import re from collections.abc import Generator, Iterable -from typing import TYPE_CHECKING, Any, Tuple +from pathlib import Path +from typing import TYPE_CHECKING, Any from j2lint.logger import logger if TYPE_CHECKING: from .linter.rule import Rule -# Using Tuple from typing for 3.8 support -# Statement type is a tuple -# (line_without_delimiter, start_line, end_line, start_delimiter, end_delimiter) -Statement = Tuple[str, int, int, str, str] +# Statement type is a tuple (line_without_delimiter, start_line, end_line, start_delimiter, end_delimiter) +Statement = tuple[str, int, int, str, str] -def load_plugins(directory: str) -> list[Rule]: - """Loads and executes all the Rule modules from the specified directory +def load_plugins(directory: Path) -> list[Rule]: + """Load and execute all the Rule modules from the specified directory. - Args: - directory (string): Loads the modules a directory + Parameters + ---------- + directory : Path + Loads the modules a directory - Returns: - list: List of rule classes + Returns + ------- + list + List of rule classes """ result = [] file_handle = None - for plugin_file in glob.glob(os.path.join(directory, "[A-Za-z_]*.py")): - plugin_name = os.path.basename(plugin_file.replace(".py", "")) + for plugin_file in directory.glob(pattern="[A-Za-z_]*.py"): + plugin_name = plugin_file.name.replace(".py", "") try: logger.debug("Loading plugin %s", plugin_name) spec = importlib.util.spec_from_file_location(plugin_name, plugin_file) if plugin_name != "__init__" and spec is not None: - class_name = "".join( - str(name).capitalize() for name in plugin_name.split("_") - ) + class_name = "".join(str(name).capitalize() for name in plugin_name.split("_")) module = importlib.util.module_from_spec(spec) if spec.loader is not None: spec.loader.exec_module(module) @@ -56,63 +56,77 @@ def load_plugins(directory: str) -> list[Rule]: return result -def is_valid_file_type(file_name: str, extensions: list[str]) -> bool: - """Checks if the file extension is in the list of accepted extensions +def is_valid_file_type(filename: Path, extensions: list[str]) -> bool: + """Check if the file extension is in the list of accepted extensions. - Args: - file_name (string): file path with extension - extensions (list): list of file extensions to look for + Parameters + ---------- + filename + File path with extension + extensions + List of file extensions to look for - Returns: - boolean: True if file type is correct + Returns + ------- + boolean + True if file type is correct """ - extension = os.path.splitext(file_name)[1].lower() + extension = filename.suffix.lower() return extension in extensions -def get_files(file_or_dir_names: list[str], extensions: list[str]) -> list[str]: - """Get files from a directory recursively +def get_files(file_or_dir_names: list[Path], extensions: list[str]) -> list[Path]: + """Get files from a directory recursively. - Args: - file_or_dir_names (list): list of directories and files - extensions (list): list of file extensions to look for + Parameters + ---------- + file_or_dir_names + List of directories and files + extensions + List of file extensions to look for - Returns: - list: list of file paths + Returns + ------- + list + List of file paths """ - file_paths: list[str] = [] + file_paths: list[Path] = [] if not isinstance(file_or_dir_names, (list, set)): - raise TypeError( - f"get_files expects a list or a set and got {file_or_dir_names}" - ) + msg = f"get_files expects a list or a set and got {file_or_dir_names}" + raise TypeError(msg) for file_or_dir in file_or_dir_names: - if os.path.isdir(file_or_dir): - for root, _, files in os.walk(file_or_dir): + file_or_dir_path = file_or_dir + if file_or_dir_path.is_dir(): + for root, _, files in os.walk(str(file_or_dir_path)): + root_path = Path(root) for file in files: - file_path = os.path.join(root, file) + file_path = root_path / file if is_valid_file_type(file_path, extensions): file_paths.append(file_path) - elif is_valid_file_type(file_or_dir, extensions): - file_paths.append(file_or_dir) + elif is_valid_file_type(file_or_dir_path, extensions): + file_paths.append(file_or_dir_path) logger.debug("Linting directory %s: files %s", file_or_dir_names, file_paths) return file_paths def flatten(nested_list: Iterable[Any]) -> Generator[Any, Any, Any]: - """Flattens an iterable + """Flatten an iterable. - Args: - nested_list (list): Nested list + Parameters + ---------- + nested_list + Nested list - Returns: + Returns + ------- + Generator[Any, Any, Any] a generator that yields the elements of each object in the nested_list """ if not isinstance(nested_list, (list, tuple)): - raise TypeError( - f"flatten is expecting a list or tuple and received {nested_list}" - ) + msg = f"flatten is expecting a list or tuple and received {nested_list}" + raise TypeError(msg) for element in nested_list: if isinstance(element, Iterable) and not isinstance(element, (str, bytes)): yield from flatten(element) @@ -120,60 +134,74 @@ def flatten(nested_list: Iterable[Any]) -> Generator[Any, Any, Any]: yield element -def get_tuple( - list_of_tuples: list[tuple[Any, ...]], item: Any -) -> tuple[Any, ...] | None: - """Checks if an item is present in any of the tuples +def get_tuple(list_of_tuples: list[tuple[Any, ...]], item: Any) -> tuple[Any, ...] | None: # noqa: ANN401 + """Check if an item is present in any of the tuples. - Args: - list_of_tuples (list): list of tuples - item (object): single object which can be in a tuple + Parameters + ---------- + list_of_tuples + list of tuples + item + single object which can be in a tuple - Returns: - [tuple]: tuple if the item exists in any of the tuples + Returns + ------- + tuple | None + tuple if the item exists in any of the tuples """ return next((entry for entry in list_of_tuples if item in entry), None) -def get_jinja_statements(text: str, indentation: bool = False) -> list[Statement]: - """Gets jinja statements with {%[-/+] [-]%} delimiters +def get_jinja_statements(text: str, *, indentation: bool = False) -> list[Statement]: + """Get jinja statements with {%[-/+] [-]%} delimiters. The regex `regex_pattern` will return multiple groups when it matches Note that this is a multiline regex - Args: - text (string): multiline text to search the jinja statements in - indentation (bool): Set to True if parsing for indentation, it will allow - to retrieve multiple lines - Example: - - For this given template: - - {# tcam-profile #} - {% if switch.platform_settings.tcam_profile is arista.avd.defined %} - tcam_profile: - system: {{ switch.platform_settings.tcam_profile }} - {% endif %} - - With indentation=True - - Found jinja statements [(' if switch.platform_settings.tcam_profile - is arista.avd.defined ', 2, 2, '{%', '%}'), (' endif ', 5, 5, '{%', '%}')] - - With indentation=False + # TODO - should probably return a JinjaStatement object.. - Found jinja statements [] - Found jinja statements [(' if switch.platform_settings.tcam_profile is - arista.avd.defined ', 1, 1, '{%', '%}')] - Found jinja statements [] - Found jinja statements [] - Found jinja statements [(' endif ', 1, 1, '{%', '%}')] - Found jinja statements [] + Parameters + ---------- + text + multiline text to search the jinja statements in. + indentation + Set to True if parsing for indentation, it will allow to retrieve multiple lines. - Returns: - [list]: list of jinja statements + Examples + -------- + For this given template: - # TODO - should probably return a JinjaStatement object.. + ``` + {# tcam-profile #} + {% if switch.platform_settings.tcam_profile is arista.avd.defined %} + tcam_profile: + system: {{ switch.platform_settings.tcam_profile }} + {% endif %} + ``` + + With `indentation=True`: + + ``` + Found jinja statements [(' if switch.platform_settings.tcam_profile + is arista.avd.defined ', 2, 2, '{%', '%}'), (' endif ', 5, 5, '{%', '%}')] + ``` + + With `indentation=False` + + ``` + Found jinja statements [] + Found jinja statements [(' if switch.platform_settings.tcam_profile is + arista.avd.defined ', 1, 1, '{%', '%}')] + Found jinja statements [] + Found jinja statements [] + Found jinja statements [(' endif ', 1, 1, '{%', '%}')] + Found jinja statements [] + ``` + + Returns + ------- + list + List of jinja statements """ statements: list[Statement] = [] count = 0 @@ -201,25 +229,37 @@ def get_jinja_statements(text: str, indentation: bool = False) -> list[Statement def delimit_jinja_statement(line: str, start: str = "{%", end: str = "%}") -> str: - """Adds end delimiters for a jinja statement - - Args: - line (string): text line - - Returns: - [string]: jinja statement with jinja start and end delimiters + """Add start and end delimiters for a jinja statement. + + Parameters + ---------- + line + Text line + start + Start delimiter + end + End delimiter + + Returns + ------- + str + Jinja statement with jinja start and end delimiters """ return start + line + end def get_jinja_comments(text: str) -> list[str]: - """Gets jinja comments + """Get jinja comments. - Args: - line (string): text to get jinja comments + Parameters + ---------- + text + Text to get jinja comments - Returns: - [list]: returns list of jinja comments + Returns + ------- + list + List of jinja comments """ regex_pattern = re.compile("(\\{#)((.|\n)*?)(\\#})", re.MULTILINE) @@ -227,27 +267,36 @@ def get_jinja_comments(text: str) -> list[str]: def get_jinja_variables(text: str) -> list[str]: - """Gets jinja variables + """Get jinja variables. - Args: - line (string): text to get jinja variables + Parameters + ---------- + text + Text to get jinja variables - Returns: - [list]: returns list of jinja variables + Returns + ------- + list + List of jinja variables """ - regex_pattern = regex_pattern = re.compile("(\\{{)((.|\n)*?)(\\}})", re.MULTILINE) + regex_pattern = re.compile("(\\{{)((.|\n)*?)(\\}})", re.MULTILINE) return [line.group(2) for line in regex_pattern.finditer(text)] def is_rule_disabled(text: str, rule: Rule) -> bool: - """Check if rule is disabled - - Args: - text (string): text to check - rule (Rule): Rule object - - Returns: - [boolean]: True if rule is disabled + """Check if rule is disabled. + + Parameters + ---------- + text + Text to check + rule + Rule object + + Returns + ------- + boolean + True if rule is disabled """ comments = get_jinja_comments(text) regex = re.compile(r"j2lint\s*:\s*disable\s*=\s*([\w-]+)") diff --git a/pyproject.toml b/pyproject.toml index 8b04fa5..6a73611 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,6 @@ classifiers = [ "Intended Audience :: Developers", "Programming Language :: Python", "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", @@ -31,7 +30,7 @@ dependencies = [ "jinja2>=3.0", "rich>=13.5.2,<13.10.0", ] -requires-python = ">=3.8" +requires-python = ">=3.9" [project.optional-dependencies] dev = [ @@ -44,10 +43,9 @@ test = [ "pytest-cov>=4.1.0", ] lint = [ - "black>=23.10.1", - "isort[colors]>=5.12.0", "pylint>=3.0.0", - "flake8==7.1.1", + "ruff>=0.8.0,<0.9.0", + "codespell>=2.2.6,<2.4.0", ] type = [ "mypy==1.14.1", @@ -75,7 +73,7 @@ push = false "tests/test_cli.py" = ["{version}"] [tool.pylint.'MESSAGES CONTROL'] -max-line-length = 160 +max-line-length = 165 [tool.tox] legacy_tox_ini = """ @@ -95,7 +93,6 @@ isolated_build = True [gh-actions] python = - 3.8: py38 3.9: py39 3.10: py310 3.11: py311, coverage, report @@ -114,12 +111,10 @@ extras = lint test commands = - flake8 --max-line-length=160 --config=/dev/null j2lint - flake8 --max-line-length=160 --config=/dev/null tests + ruff check . + ruff format . --check pylint j2lint pylint tests - black --check --diff --color . - isort --check --diff --color . [testenv:type] description = check the code type @@ -187,5 +182,141 @@ strict_optional = true disallow_untyped_defs = true mypy_path = "j2lint" -[tool.isort] -profile = "black" +################################ +# Ruff +################################ +[tool.ruff] + +# Exclude a variety of commonly ignored directories. +exclude = [ + ".bzr", + ".direnv", + ".eggs", + ".git", + ".git-rewrite", + ".hg", + ".ipynb_checkpoints", + ".mypy_cache", + ".nox", + ".pants.d", + ".pyenv", + ".pytest_cache", + ".pytype", + ".ruff_cache", + ".svn", + ".tox", + ".venv", + ".vscode", + "__pypackages__", + "_build", + "buck-out", + "build", + "dist", + "node_modules", + "site-packages", + "venv", + ".github", +] + +line-length = 165 + +# Assume Python 3.9 as this is the lowest supported version for j2lint +target-version = "py39" + +[tool.ruff.lint] +# select all cause we like being suffering +select = ["ALL", + # By enabling a convention for docstrings, ruff automatically ignore some rules that need to be + # added back if we want them. + # https://docs.astral.sh/ruff/faq/#does-ruff-support-numpy-or-google-style-docstrings + "D415", + "D417", + "D212", +] +ignore = [ + "COM812", # Ignoring conflicting rules that may cause conflicts when used with the formatter + "ISC001", # Ignoring conflicting rules that may cause conflicts when used with the formatter + "TD002", # We don't have require authors in TODO + "TD003", # We don't have an issue link for all TODOs today + "FIX002", # Line contains TODO - ignoring for ruff for now + "F821", # Disable undefined-name until resolution of #10451 +] + +# Allow autofix for all enabled rules (when `--fix`) is provided. +fixable = ["ALL"] +unfixable = [] + +# Allow unused variables when underscore-prefixed. +dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$" + +[tool.ruff.lint.pydocstyle] +convention = "numpy" + +[tool.ruff.lint.pylint] +# These settings are used to configure pylint rules run in ruff. In order to keep sane and while +# we have not removed pylint completely, these settings should be kept in sync with our pylintrc file. +# https://github.com/astral-sh/ruff/issues/970 +max-branches = 13 + +[tool.ruff.lint.mccabe] +# Unlike Flake8, default to a complexity level of 10. +max-complexity = 10 + +[tool.ruff.lint.pep8-naming] +"ignore-names" = [ +# "RICH_COLOR_PALETTE" +] + +[tool.ruff.lint.per-file-ignores] +"tests/*" = [ + "S101", # Complains about asserts in units and libs. + "SLF001", # Lots of private member accessed for test purposes + "PLR0913", # Too many arguments to function call + "PLR2004", # Magic value used in comparison, consider replacing {value} with a constant variable +# "ARG002", # Sometimes we need to declare unused arguments when a parameter is not used but declared in @pytest.mark.parametrize +# "FBT001", # Boolean-typed positional argument in function definition +# "S105", # Passwords are indeed hardcoded in tests +# "S106", # Passwords are indeed hardcoded in tests +# "S108", # Probable insecure usage of temporary file or directory + "TD005", # TODOs in tests + "ANN401", # Using Any in tests signature +] +"tests/conftest.py" = [ + "ARG002", # Sometimes we need to declare unused arguments when a parameter is not used when mocking +] +"j2lint/rules/*" = [ + "FBT001", # TODO: Address bool in inputs + "FBT002", # TODO: switch ignore and warn arguments +] + +################################ +# Pylint +################################ +[tool.pylint] +disable = [ # Any rule listed here can be disabled: https://github.com/astral-sh/ruff/issues/970 + "invalid-name", + "fixme", + "unused-import", + "unused-argument", + "keyword-arg-before-vararg", + "protected-access", + "too-many-arguments", + "too-many-branches", + "too-many-positional-arguments", + "wrong-import-position", + "pointless-statement", + "broad-exception-caught", + "line-too-long", + "unused-variable", + "redefined-builtin", + "global-statement", + "reimported", + "wrong-import-order", + "wrong-import-position", + "abstract-class-instantiated", # Overlap with https://mypy.readthedocs.io/en/stable/error_code_list.html#check-instantiation-of-abstract-classes-abstract + "unexpected-keyword-arg", # Overlap with https://mypy.readthedocs.io/en/stable/error_code_list.html#check-arguments-in-calls-call-arg and other rules + "no-value-for-parameter" # Overlap with https://mypy.readthedocs.io/en/stable/error_code_list.html#check-arguments-in-calls-call-arg +] +max-line-length=165 +# making similarity lines limit a bit higher than default 4 +min-similarity-lines=10 diff --git a/tests/__init__.py b/tests/__init__.py index e602bd6..ab820b4 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,3 +1,4 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. +"""Tests for j2lint.""" diff --git a/tests/conftest.py b/tests/conftest.py index 7b31223..1a3b630 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,13 +1,16 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -""" -content of conftest.py -""" +"""Pytest fixtures for j2lint testing.""" + +from __future__ import annotations + import logging import pathlib from argparse import Namespace -from unittest.mock import create_autospec +from pathlib import Path +from typing import Callable +from unittest.mock import MagicMock, create_autospec import pytest @@ -23,61 +26,61 @@ # cf https://docs.pytest.org/en/stable/reference/reference.html#pytest-fixture # pylint: disable=fixme, redefined-outer-name -# TODO - proper way to compare LinterError following: +# TODO: proper way to compare LinterError following: # https://docs.pytest.org/en/7.1.x/how-to/assert.html#defining-your-own-explanation-for-failed-assertions class TestRule(Rule): - """ - TestRule class for tests - """ + """TestRule class for tests.""" rule_id = "TT" description = "test" short_description = "test" severity = "LOW" - def checktext(self, filename, text): - pass + def checktext(self, filename: Path, text: str) -> list[LinterError]: + """Fake checktext implementation.""" + return [] - def checkline(self, filename, line, line_no): - pass + def checkline(self, filename: Path, line: int, line_no: int) -> list[LinterError]: + """Fake checkline implementation.""" + return [] @pytest.fixture -def collection(): - """ - Return the collection with the default rules - """ +def collection() -> RulesCollection: + """Return the collection with the default rules.""" return RulesCollection.create_from_directory(DEFAULT_RULE_DIR, [], []) @pytest.fixture -def make_rules(): - """ - Return a Rule factory that takes one argument - `count` and returns count rules following the pattern - where `i` is the index +def make_rules() -> Callable[[int], list[Rule]]: + """Return a Rule factory that takes one argument `count` and returns count rules. + + The rules arefollowing the pattern where `i` is the index - rule_id = Ti - description = test rule i - short_description = test-rule-i - severity in [LOW, MEDIUM, HIGH] based on i % 3 + ``` + rule_id = Ti + description = test rule i + short_description = test-rule-i + severity in [LOW, MEDIUM, HIGH] based on i % 3 + ``` + Examples + -------- The factory can then be invoked as follow: - def test_blah(makes_rules): - rules = make_rules(5) - # do stuff with rules + ``` + def test_blah(makes_rules): + rules = make_rules(5) + # do stuff with rules + ``` """ - def __make_n_rules(count): - def get_severity(integer: int): - return ( - "LOW" - if integer % 3 == 0 - else ("MEDIUM" if integer % 3 == 1 else "HIGH") - ) + def __make_n_rules(count: int) -> list[Rule]: + def get_severity(integer: int) -> str: + """Return a severity based on the integer modulo 3.""" + return "LOW" if integer % 3 == 0 else ("MEDIUM" if integer % 3 == 1 else "HIGH") rules = [] for i in range(count): @@ -93,38 +96,40 @@ def get_severity(integer: int): @pytest.fixture -def test_rule(make_rules): - """ - return a Rule object to use in test - from the make_rules - it will have +def test_rule(make_rules: Callable[[int], list[Rule]]) -> Rule: + """Return a Rule object to use in test from the make_rules fixture. + + The rule is - rule_id = T0 - description = test rule 0 - short_description = test-rule-0 - severity = LOW + ``` + rule_id = T0 + description = test rule 0 + short_description = test-rule-0 + severity = LOW + ``` """ - yield make_rules(1)[0] + return make_rules(1)[0] @pytest.fixture -def test_other_rule(make_rules): - """ - return the second Rule object to use in test - from the make_rules - it will have +def test_other_rule(make_rules: Callable[[int], list[Rule]]) -> Rule: + """Return the second Rule object to use in test from the make_rules fixture. + + The rule is: - rule_id = T1 - description = test rule 1 - short_description = test-rule-1 - severity = MEDIUM + ``` + rule_id = T1 + description = test rule 1 + short_description = test-rule-1 + severity = MEDIUM + ``` """ - yield make_rules(2)[1] + return make_rules(2)[1] @pytest.fixture -def make_issues(make_rules): - """ - Returns a factory that generates `count` issues and - return them as a list +def make_issues(make_rules: Callable[[int], list[Rule]]) -> Callable[[int], list[LinterError]]: + """Return a factory that generates `count` issues and return them as a list. The fixture invokes `make_rules` first with count and every LinterError generated is using the rule @@ -138,81 +143,67 @@ def make_issues(make_rules): The factory can then be invoked as follow: - def test_blah(makes_issues): - issues = make_issues(5) - # do stuff with issues + ``` + def test_blah(makes_issues): + issues = make_issues(5) + # do stuff with issues + ``` """ - def __make_n_issues(count): - issues = [] + def __make_n_issues(count: int) -> list[LinterError]: rules = make_rules(count) - for i in range(count): - issues.append(LinterError(i + 1, "dummy", "dummy.j2", rules[i])) - return issues + return [LinterError(i + 1, "dummy", "dummy.j2", rules[i]) for i in range(count)] return __make_n_issues @pytest.fixture -def make_issue_from_rule(): - """ - Returns a factory that generates an issue based on - a Rule object +def make_issue_from_rule() -> Callable[[Rule], LinterError]: + """Return a factory that generates an issue based on a Rule object. - it uses line 42, the line content is "dummy" and the - filename is "dummy.j2" + it uses line 42, the line content is "dummy" and the filename is "dummy.j2". """ - def __make_issue_from_rule(rule): + def __make_issue_from_rule(rule: Rule) -> LinterError: yield LinterError(42, "dummy", "dummy.j2", rule) return __make_issue_from_rule @pytest.fixture -def test_issue(make_issues): - """ - Get the first issue from the make_issues factory +def test_issue(make_issues: Callable[[int], list[Rule]]) -> LinterError: + """Get the first issue from the make_issues factory. - Note: it will use rule T0 as per design + It will use rule T0 as per design. """ - yield make_issues(1)[0] + return make_issues(1)[0] @pytest.fixture -def test_collection(test_rule): - """ - test_collection using one rule `test_rule` - """ +def test_collection(test_rule: Rule) -> RulesCollection: + """test_collection using one rule `test_rule`.""" collection = RulesCollection() collection.extend([test_rule]) - yield collection + return collection @pytest.fixture -def test_runner(test_collection): - """ - Fixture to get a test runner using the test_collection - """ - yield Runner(test_collection, "test.j2", checked_files=[]) +def test_runner(test_collection: RulesCollection) -> Runner: + """Fixture to get a test runner using the test_collection.""" + return Runner(test_collection, "test.j2", checked_files=[]) @pytest.fixture -def j2lint_usage_string(): - """ - Fixture to get the help generated by argparse - """ - yield create_parser().format_help() +def j2lint_usage_string() -> str: + """Fixture to get the help generated by argparse.""" + return create_parser().format_help() -@pytest.fixture() -def template_tmp_dir(tmp_path_factory): - """ - Create a tmp directory with multiple files and hidden files - """ +@pytest.fixture +def template_tmp_dir(tmp_path_factory: object) -> list[str]: + """Create a tmp directory with multiple files and hidden files.""" tmp_dir = pathlib.Path(__file__).parent / "tmp" - # Hacking it # https://stackoverflow.com/questions/40566968/how-to-dynamically-change-pytests-tmpdir-base-directory # + # https://docs.pytest.org/en/7.1.x/_modules/_pytest/tmpdir.html @@ -244,14 +235,12 @@ def template_tmp_dir(tmp_path_factory): rules_hidden_subdir_txt = rules_hidden_subdir / "rules.txt" rules_hidden_subdir_txt.write_text(CONTENT) - yield [str(rules)] + return [str(rules)] @pytest.fixture -def default_namespace(): - """ - Default ArgPase namespace for j2lint - """ +def default_namespace() -> Namespace: + """Return the default ArgParse namespace for j2lint.""" return Namespace( files=[], ignore=[], @@ -269,8 +258,6 @@ def default_namespace(): @pytest.fixture -def logger(): - """ - Return a MagicMock object with the spec of logging.Logger - """ +def logger() -> MagicMock: + """Return a MagicMock object with the spec of logging.Logger.""" return create_autospec(logging.Logger) diff --git a/tests/test_cli.py b/tests/test_cli.py index f35657b..4b9154e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,20 +1,23 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -""" -Tests for j2lint.cli.py -""" +"""Tests for j2lint.cli.py.""" + +from __future__ import annotations + import logging -import os import re from argparse import Namespace +from pathlib import Path +from typing import TYPE_CHECKING, Any, Callable from unittest.mock import patch import pytest from rich.console import ConsoleDimensions +import j2lint +from j2lint import CONSOLE from j2lint.cli import ( - CONSOLE, create_parser, print_json_output, print_string_output, @@ -34,14 +37,17 @@ j2lint_default_rules_string, ) -# pylint: disable=fixme, too-many-arguments, too-many-positional-arguments +if TYPE_CHECKING: + from collections.abc import Generator + + from j2lint.linter.error import LinterError # Fixed size console for tests output CONSOLE.size = ConsoleDimensions(width=80, height=74) @pytest.mark.parametrize( - "argv, namespace_modifications", + ("argv", "namespace_modifications"), [ (pytest.param([], {"extensions": [".j2", ".jinja", ".jinja2"]}, id="default")), pytest.param( @@ -60,11 +66,10 @@ ), ], ) -def test_create_parser(default_namespace, argv, namespace_modifications): - """ - Test j2lint.cli.create_parser +def test_create_parser(default_namespace: Namespace, argv: list[str], namespace_modifications: dict[str, Any]) -> None: + """Test j2lint.cli.create_parser. - the namespace_modifications is a dictionnary where key + the namespace_modifications is a dictionary where key is one of the keys in the namespace and value is the value it should be overwritten to. @@ -80,7 +85,7 @@ def test_create_parser(default_namespace, argv, namespace_modifications): @pytest.mark.parametrize( - "number_issues, issues_modifications, expected_sorted_issues_ids", + ("number_issues", "issues_modifications", "expected_sorted_issues_ids"), [ (0, {}, []), (1, {}, [("dummy.j2", "T0", 1, "test-rule-0")]), @@ -111,10 +116,12 @@ def test_create_parser(default_namespace, argv, namespace_modifications): ], ) def test_sort_issues( - make_issues, number_issues, issues_modifications, expected_sorted_issues_ids -): - """ - Test j2lint.cli.sort_issues + make_issues: Callable[[int], list[LinterError]], + number_issues: int, + issues_modifications: dict[int, dict[str, Any]], + expected_sorted_issues_ids: list[tuple[Any]], +) -> None: + """Test j2lint.cli.sort_issues. the issues_modificartions is a dictionary that has the following structure: @@ -125,7 +132,6 @@ def test_sort_issues( appropriate issues, apply the sort_issues method and verifies the ordering is correct """ - issues = make_issues(number_issues) # In the next step we apply modifications on the generated LinterErrors @@ -152,7 +158,7 @@ def test_sort_issues( @pytest.mark.parametrize( - "options, number_errors, number_warnings, expected_output, expected_stdout", + ("options", "number_errors", "number_warnings", "expected_output", "expected_stdout"), [ pytest.param( Namespace(verbose=False), @@ -189,23 +195,18 @@ def test_sort_issues( ], ) def test_print_string_output( - capsys, - make_issues, - options, - number_errors, - number_warnings, - expected_output, - expected_stdout, -): - """ - Test j2lint.cli.print_string_output - """ - + capsys: pytest.LogCaptureFixture, + make_issues: Callable[[int], list[LinterError]], + options: Namespace, + number_errors: int, + number_warnings: int, + expected_output: tuple[int, int], + expected_stdout: str, +) -> None: + """Test j2lint.cli.print_string_output.""" errors = {"dummy.j2": make_issues(number_errors)} warnings = {"dummy.j2": make_issues(number_warnings)} - total_errors, total_warnings = print_string_output( - errors, warnings, options.verbose - ) + total_errors, total_warnings = print_string_output(errors, warnings, verbose=options.verbose) assert total_errors == expected_output[0] assert total_warnings == expected_output[1] @@ -215,43 +216,22 @@ def test_print_string_output( @pytest.mark.parametrize( - "number_errors, number_warnings, expected_output, expected_stdout", + ("number_errors", "number_warnings", "expected_output", "expected_stdout"), [ - pytest.param( - 0, - 0, - (0, 0), - NO_ERROR_NO_WARNING_JSON, - id="No issue - json", - ), - pytest.param( - 1, - 0, - (1, 0), - ONE_ERROR_JSON, - id="one error - json", - ), - pytest.param( - 1, - 1, - (1, 1), - ONE_ERROR_ONE_WARNING_JSON, - id="one error and one warning - json", - ), + pytest.param(0, 0, (0, 0), NO_ERROR_NO_WARNING_JSON, id="No issue - json"), + pytest.param(1, 0, (1, 0), ONE_ERROR_JSON, id="one error - json"), + pytest.param(1, 1, (1, 1), ONE_ERROR_ONE_WARNING_JSON, id="one error and one warning - json"), ], ) def test_print_json_output( - capsys, - make_issues, - number_errors, - number_warnings, - expected_output, - expected_stdout, -): - """ - Test j2lint.cli.print_json_output - """ - + capsys: pytest.Fixture, + make_issues: pytest.Fixture, + number_errors: int, + number_warnings: int, + expected_output: tuple[int, int], + expected_stdout: str, +) -> None: + """Test j2lint.cli.print_json_output.""" errors = {"ERRORS": make_issues(number_errors)} warnings = {"WARNINGS": make_issues(number_warnings)} total_errors, total_warnings = print_json_output(errors, warnings) @@ -260,12 +240,11 @@ def test_print_json_output( assert total_warnings == expected_output[1] captured = capsys.readouterr() - print(captured) assert captured.out == expected_stdout @pytest.mark.parametrize( - "argv, expected_stdout, expected_stderr, expected_exit_code, expected_raise, number_errors, number_warnings", + ("argv", "expected_stdout", "expected_stderr", "expected_exit_code", "expected_raise", "number_errors", "number_warnings"), [ pytest.param([], "", "HELP", 1, does_not_raise(), 0, 0, id="no input"), pytest.param(["-h"], "HELP", "", 0, pytest.raises(SystemExit), 0, 0, id="help"), @@ -342,22 +321,21 @@ def test_print_json_output( ], ) def test_run( - capsys, - caplog, - j2lint_usage_string, - make_issues, - argv, - expected_stdout, - expected_stderr, - expected_exit_code, - expected_raise, - number_errors, - number_warnings, -): - """ - Test the j2lint.cli.run method - - This test is a bit too complex and should probably be splitted out to test various + capsys: pytest.Fixture, + caplog: pytest.Fixture, + j2lint_usage_string: str, + make_issues: pytest.Fixture, + argv: list[str], + expected_stdout: str, + expected_stderr: str, + expected_exit_code: int, + expected_raise: Generator, + number_errors: int, + number_warnings: int, +) -> None: + """Test the j2lint.cli.run method. + + This test is a bit too complex and should probably be split out to test various functionalities the call is to test the various options of the main entry point, patching away inner @@ -367,53 +345,50 @@ def test_run( caplog.set_level(logging.INFO) if "-d" in argv or "--debug" in argv: caplog.set_level(logging.DEBUG) - # TODO this method needs to be split a bit as it has - # too many responsibility + # TODO: this method needs to be split a bit as it has too many responsibility if expected_stdout == "HELP": expected_stdout = j2lint_usage_string if expected_stdout == "DEFAULT_RULES": expected_stdout = j2lint_default_rules_string() if expected_stderr == "HELP": expected_stderr = j2lint_usage_string - with expected_raise: - with patch("j2lint.cli.Runner.run") as mocked_runner_run, patch( - "logging.disable" - ): - mocked_runner_run.return_value = ( - make_issues(number_errors), - make_issues(number_warnings), - ) - run_return_value = run(argv) - captured = capsys.readouterr() - if "-o" not in argv and "--stdout" not in argv: - assert str(captured.out) == expected_stdout - assert captured.out == expected_stdout - # Hmm - WHY - need to find why failing with stdout - assert captured.err == expected_stderr - else: - assert expected_stdout in captured.out - assert run_return_value == expected_exit_code - if ("-o" in argv or "--stdout" in argv) and ( - "-d" in argv or "--debug" in argv - ): - assert "DEBUG" in [record.levelname for record in caplog.records] + with expected_raise, patch("j2lint.cli.Runner.run") as mocked_runner_run, patch("logging.disable"): + mocked_runner_run.return_value = ( + make_issues(number_errors), + make_issues(number_warnings), + ) + run_return_value = run(argv) + captured = capsys.readouterr() + if "-o" not in argv and "--stdout" not in argv: + assert str(captured.out) == expected_stdout + assert captured.out == expected_stdout + # Hmm - WHY - need to find why failing with stdout + assert captured.err == expected_stderr + else: + assert expected_stdout in captured.out + assert run_return_value == expected_exit_code + if ("-o" in argv or "--stdout" in argv) and ("-d" in argv or "--debug" in argv): + assert "DEBUG" in [record.levelname for record in caplog.records] -def test_run_stdin(capsys): - """ - Test j2lint.cli.run when using stdin +def test_run_stdin(capsys: pytest.LogCaptureFixture) -> None: + """Test j2lint.cli.run when using stdin. + Note that the code is checking that this is not run from a tty A solution to run is something like: + ``` cat myfile.j2 | j2lint --stdin ``` In this test, the isatty answer is mocked. """ - with patch("sys.stdin") as patched_stdin, patch( - "os.unlink", side_effect=os.unlink - ) as mocked_os_unlink, patch("logging.disable"): + with ( + patch("sys.stdin") as patched_stdin, + patch.object(j2lint.cli.Path, "unlink", side_effect=j2lint.cli.Path.unlink, autospec=True) as mocked_os_unlink, + patch("logging.disable"), + ): patched_stdin.isatty.return_value = False patched_stdin.read.return_value = "{%set test=42 %}" run_return_value = run(["--log", "--stdin"]) @@ -426,6 +401,7 @@ def test_run_stdin(capsys): re.MULTILINE, ) assert matches is not None - mocked_os_unlink.assert_called_with(matches.groups()[0]) - assert os.path.exists(matches.groups()[0]) is False + path = Path(matches.groups()[0]) + mocked_os_unlink.assert_called_with(path) + assert path.exists() is False assert run_return_value == 2 diff --git a/tests/test_linter/__init__.py b/tests/test_linter/__init__.py new file mode 100644 index 0000000..bb30a41 --- /dev/null +++ b/tests/test_linter/__init__.py @@ -0,0 +1,4 @@ +# Copyright (c) 2021-2025 Arista Networks, Inc. +# Use of this source code is governed by the MIT license +# that can be found in the LICENSE file. +"""Test for j2lint.linter.""" diff --git a/tests/test_linter/test_collection.py b/tests/test_linter/test_collection.py index 04f631a..38ef9fe 100644 --- a/tests/test_linter/test_collection.py +++ b/tests/test_linter/test_collection.py @@ -1,11 +1,13 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -""" -Tests for j2lint.linter.collection.py -""" +"""Tests for j2lint.linter.collection.py.""" + +from __future__ import annotations + import logging -import pathlib +from pathlib import Path +from typing import TYPE_CHECKING, Callable from unittest import mock import pytest @@ -15,20 +17,22 @@ from j2lint.rules.jinja_statement_delimiter_rule import JinjaStatementDelimiterRule from j2lint.rules.jinja_template_syntax_error_rule import JinjaTemplateSyntaxErrorRule -TEST_DATA_DIR = pathlib.Path(__file__).parent / "data" +if TYPE_CHECKING: + from j2lint.linter.error import LinterError + from j2lint.linter.rule import Rule + +TEST_DATA_DIR = Path(__file__).parent / "data" class TestRulesCollection: - def test__len__(self, test_collection): - """ - Test the RuleCollection __len__ method - """ + """Test j2lint.linter.collection.RulesCollection.""" + + def test__len__(self, test_collection: RulesCollection) -> None: + """Test the RuleCollection __len__ method.""" assert len(test_collection) == 1 - def test__iter__(self): - """ - Test the RuleCollection __iter__ method - """ + def test__iter__(self) -> None: + """Test the RuleCollection __iter__ method.""" fake_rules = [1, 2, 3] collection = RulesCollection() assert len(collection) == 0 @@ -36,10 +40,8 @@ def test__iter__(self): for index, rule in enumerate(collection): assert rule == fake_rules[index] - def test_extend(self): - """ - Test the RuleCollection.extend method - """ + def test_extend(self) -> None: + """Test the RuleCollection.extend method.""" fake_rules = [1, 2, 3] collection = RulesCollection() assert len(collection) == 0 @@ -50,7 +52,7 @@ def test_extend(self): assert collection.rules == fake_rules + fake_rules @pytest.mark.parametrize( - "file_path, expected_results, verify_logs", + ("file_path", "expected_results", "verify_logs"), [ pytest.param( "dummy.j2", @@ -76,19 +78,21 @@ def test_extend(self): ) def test_run( self, - caplog, - test_collection, - make_rules, - make_issue_from_rule, - file_path, - expected_results, - verify_logs, - ): - """ - Generate a collection with 5 rules with: - * rule number 1 being a warning - * rule number 2 being ignored - * rule number 3 being disabled in the test file for test number 2 + caplog: pytest.LogCaptureFixture, + test_collection: RulesCollection, + make_rules: Callable[[int], list[Rule]], + make_issue_from_rule: Callable[[int], LinterError], + file_path: Path, + expected_results: tuple[list[tuple[str, str]], list[tuple[str, str]]], + *, + verify_logs: bool, + ) -> None: + """Test RulesCollection.run on a generated collection. + + The collection has 5 rules with. + * rule number 1 being a warning + * rule number 2 being ignored + * rule number 3 being disabled in the test file for test number 2 """ caplog.set_level(logging.DEBUG) @@ -98,7 +102,7 @@ def test_run( rules[2].ignore = True test_collection.rules = rules - def checks_side_effect(self, file_path, text): + def checks_side_effect(self, file_path: Path, text: str) -> None: # type: ignore[no-untyped-def] # noqa: ARG001, ANN001 return make_issue_from_rule(self) with mock.patch( @@ -106,14 +110,9 @@ def checks_side_effect(self, file_path, text): side_effect=checks_side_effect, autospec=True, ): - errors, warnings = test_collection.run(file_path) - error_tuples = [ - (error.rule.rule_id, error.rule.short_description) for error in errors - ] - warning_tuples = [ - (warning.rule.rule_id, warning.rule.short_description) - for warning in warnings - ] + errors, warnings = test_collection.run(Path(file_path)) + error_tuples = [(error.rule.rule_id, error.rule.short_description) for error in errors] + warning_tuples = [(warning.rule.rule_id, warning.rule.short_description) for warning in warnings] assert error_tuples == expected_results[0] assert warning_tuples == expected_results[1] @@ -121,39 +120,27 @@ def checks_side_effect(self, file_path, text): # True for every test that goes beyond the file do not exist assert any("Ignoring rule T2" in message for message in caplog.messages) # True for the test file - assert any( - "Skipping linting rule T3" in message for message in caplog.messages - ) + assert any("Skipping linting rule T3" in message for message in caplog.messages) - def test__repr__(self, test_collection, test_other_rule): - """ - Test the RuleCollection.extend method - """ + def test__repr__(self, test_collection: RulesCollection, test_other_rule: Rule) -> None: + """Test the RuleCollection.extend method.""" assert str(test_collection) == "Origin: BUILT-IN\nT0: test rule 0" test_other_rule.origin = "DUMMY" test_collection.extend([test_other_rule]) - assert ( - str(test_collection) - == "Origin: BUILT-IN\nT0: test rule 0\nOrigin: DUMMY\nT1: test rule 1" - ) + assert str(test_collection) == "Origin: BUILT-IN\nT0: test rule 0\nOrigin: DUMMY\nT1: test rule 1" @pytest.mark.parametrize( - "ignore_rules, warn_rules", + ("ignore_rules", "warn_rules"), [ - pytest.param( - ["S0", "operator-enclosed-by-spaces"], [], id="no_warn_no_ignore" - ), - pytest.param( - [], ["S0", "operator-enclosed-by-spaces"], id="warn_no_ignore" - ), + pytest.param(["S0", "operator-enclosed-by-spaces"], [], id="no_warn_no_ignore"), + pytest.param([], ["S0", "operator-enclosed-by-spaces"], id="warn_no_ignore"), pytest.param(["S0"], ["operator-enclosed-by-spaces"], id="ignore_no_warn"), pytest.param([], ["S42"], id="ignore_absent_rule"), pytest.param(["S42"], [], id="warn_absent_rule"), ], ) - def test_create_from_directory(self, ignore_rules, warn_rules): - """ - Test the RuleCollection.create_from_directory class method + def test_create_from_directory(self, ignore_rules: list[str], warn_rules: list[str]) -> None: + """Test the RuleCollection.create_from_directory class method. In this tests, the return of load_plugins are mocked. consequently, a dummy path can be used for rules_dir @@ -161,22 +148,20 @@ def test_create_from_directory(self, ignore_rules, warn_rules): with mock.patch("j2lint.linter.collection.load_plugins") as mocked_load_plugins: # importing 3 rules for the need of the tests # note that current pylint branch is returning classes - # not instances.. + # not instances. mocked_load_plugins.return_value = [ JinjaStatementDelimiterRule(), JinjaOperatorHasSpacesRule(), JinjaTemplateSyntaxErrorRule(), ] - collection = RulesCollection.create_from_directory( - "dummy", ignore_rules, warn_rules - ) - mocked_load_plugins.assert_called_once_with("dummy") + collection = RulesCollection.create_from_directory(Path("dummy"), ignore_rules, warn_rules) + mocked_load_plugins.assert_called_once_with(Path("dummy")) assert collection.rules == mocked_load_plugins.return_value + assert isinstance(collection, RulesCollection) + # Somehow pylint is convinced that rule are integers + # pylint: disable=no-member for rule in collection.rules: - if ( - rule.rule_id in ignore_rules - or rule.short_description in ignore_rules - ): + if rule.rule_id in ignore_rules or rule.short_description in ignore_rules: assert rule.ignore is True if rule.rule_id in warn_rules or rule.short_description in warn_rules: assert rule in rule.warn diff --git a/tests/test_linter/test_error.py b/tests/test_linter/test_error.py index 535e3e5..9e21fda 100644 --- a/tests/test_linter/test_error.py +++ b/tests/test_linter/test_error.py @@ -1,26 +1,23 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -""" -Tests for j2lint.linter.error.py -""" +"""Tests for j2lint.linter.error.py.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + import pytest -RULE_TEXT_OUTPUT = ( - "Linting rule: T0\n" - "Rule description: test rule 0\n" - "Error line: dummy.j2:1 dummy\n" - "Error message: test rule 0\n" -) -RULE_JSON_OUTPUT = ( - '{"id": "T0", "message": "test rule 0", ' - '"filename": "dummy.j2", "line_number": 1, ' - '"line": "dummy", "severity": "LOW"}' -) +if TYPE_CHECKING: + from j2lint.linter.error import LinterError + +RULE_TEXT_OUTPUT = "Linting rule: T0\nRule description: test rule 0\nError line: dummy.j2:1 dummy\nError message: test rule 0\n" +RULE_JSON_OUTPUT = '{"id": "T0", "message": "test rule 0", "filename": "dummy.j2", "line_number": 1, "line": "dummy", "severity": "LOW"}' @pytest.mark.parametrize( - "verbose, expected", + ("verbose", "expected"), [ (False, "dummy.j2:1 test rule 0 (test-rule-0)"), ( @@ -29,17 +26,11 @@ ), ], ) -def test_to_rich(test_issue, verbose, expected): - """ - Test the Rich string formats for LinterError - """ - - assert str(test_issue.to_rich(verbose)) == expected - +def test_to_rich(test_issue: LinterError, *, verbose: bool, expected: str) -> None: + """Test the Rich string formats for LinterError.""" + assert str(test_issue.to_rich(verbose=verbose)) == expected -def test_to_json(test_issue): - """ - Test the json format for LinterError - """ +def test_to_json(test_issue: LinterError) -> None: + """Test the json format for LinterError.""" assert test_issue.to_json() == RULE_JSON_OUTPUT diff --git a/tests/test_linter/test_indenter/__init__.py b/tests/test_linter/test_indenter/__init__.py new file mode 100644 index 0000000..efacd54 --- /dev/null +++ b/tests/test_linter/test_indenter/__init__.py @@ -0,0 +1,4 @@ +# Copyright (c) 2021-2025 Arista Networks, Inc. +# Use of this source code is governed by the MIT license +# that can be found in the LICENSE file. +"""Tests for j2lint.linter.indenter.""" diff --git a/tests/test_linter/test_indenter/test_node.py b/tests/test_linter/test_indenter/test_node.py index 26bf8eb..b6285d8 100644 --- a/tests/test_linter/test_indenter/test_node.py +++ b/tests/test_linter/test_indenter/test_node.py @@ -1,25 +1,25 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -""" -Tests for j2lint.linter.node.py -""" +"""Tests for j2lint.linter.node.py.""" + +from __future__ import annotations + import pytest from j2lint.linter.indenter.node import Node class TestNode: + """Test j2lint.linter.node.Node.""" + @pytest.mark.skip("No need to test this") - def test_create_node(self): - """ """ - # TODO - why is it not an __init__ method??? - pass - - def test_create_indentation_error(self): - """ - Test the Node.create_indentation_error method - """ + def test_create_node(self) -> None: + """N/A.""" + # TODO: why is it not an __init__ method??? + + def test_create_indentation_error(self) -> None: + """Test the Node.create_indentation_error method.""" line = ( " if switch.platform_settings.tcam_profile is arista.avd.defined ", 2, @@ -29,10 +29,8 @@ def test_create_indentation_error(self): ) root = Node() node = root.create_node(line, 2) - print(node.statement) indentation_error = node.create_indentation_error(node, "test") - print(type(indentation_error)) assert indentation_error == ( 2, "{% if switch.platform_settings.tcam_profile is arista.avd.defined %}", diff --git a/tests/test_linter/test_indenter/test_statement.py b/tests/test_linter/test_indenter/test_statement.py index c1ba641..6aff6a1 100644 --- a/tests/test_linter/test_indenter/test_statement.py +++ b/tests/test_linter/test_indenter/test_statement.py @@ -1,21 +1,29 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -""" -Tests for j2lint.linter.indenter.statement.py +"""Tests for j2lint.linter.indenter.statement.py. To understand the values given to this class it is paramount to read j2lint.utils.get_jinja_statement as this is the path in the code that will parse a file content and return a list of statements - abusively called lines """ + +# pylint: disable=R0903 + +from __future__ import annotations + +from typing import Any + import pytest from j2lint.linter.indenter.statement import JinjaStatement class TestJinjaStatement: + """Tests for JinjaStatement.""" + @pytest.mark.parametrize( - "line, expected_statement", + ("line", "expected_statement"), [ pytest.param( ( @@ -43,12 +51,12 @@ class TestJinjaStatement: # this next one is failing because we never expect to be called # with an empty list probably should be caught in __init__ # if we even keep the class.. - # pytest.param("[]", {}, id="empty_statement"), + # pytest.param("[]", {}, id="empty_statement"), # noqa: ERA001 ], ) - def test_jinja_statement(self, line, expected_statement): - """ """ - # TODO - why is it not an __init__ method??? + def test_jinja_statement(self, line: tuple[str, int, int, str, str], expected_statement: dict[str, Any]) -> None: + """Test function.""" + # TODO: why is it not an __init__ method??? statement = JinjaStatement(line) for key, value in expected_statement.items(): - assert statement.__getattribute__(key) == value + assert getattr(statement, key) == value diff --git a/tests/test_linter/test_rule.py b/tests/test_linter/test_rule.py index d529dbe..12816f8 100644 --- a/tests/test_linter/test_rule.py +++ b/tests/test_linter/test_rule.py @@ -1,30 +1,32 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -""" -Tests for j2lint.linter.rule.py -""" +"""Tests for j2lint.linter.rule.py.""" + +from __future__ import annotations + import logging -import pathlib +from pathlib import Path +from typing import TYPE_CHECKING, Any, Callable import pytest -TEST_DATA_DIR = pathlib.Path(__file__).parent / "data" +if TYPE_CHECKING: + from j2lint.linter.error import LinterError + from j2lint.linter.rule import Rule + +TEST_DATA_DIR = Path(__file__).parent / "data" class TestRule: - def test__repr__(self, test_rule): - """ - Test the Rule __repr__ format - """ - assert str(test_rule) == "T0: test rule 0" + """Test j2lint.linter.rule.Rule.""" - @pytest.mark.skip("This method will be removed and is tested through other methods") - def test_is_valid_language(self, test_rule, file): - """ """ + def test__repr__(self, test_rule: Rule) -> None: + """Test the Rule __repr__ format.""" + assert str(test_rule) == "T0: test rule 0" @pytest.mark.parametrize( - "checktext, checkline, file_path, expected_errors_ids, expected_logs", + ("checktext", "checkline", "file_path", "expected_errors_ids", "expected_logs"), [ pytest.param( None, @@ -54,56 +56,49 @@ def test_is_valid_language(self, test_rule, file): ) def test_checkrule( self, - caplog, - test_rule, - make_issue_from_rule, - checktext, - checkline, - file_path, - expected_errors_ids, - expected_logs, - ): - """ - Test the Rule.checkrule method + caplog: pytest.LogCaptureFixture, + test_rule: Rule, + make_issue_from_rule: Callable[[int], list[LinterError]], + checktext: None | int, + checkline: None | int, + file_path: dict[str, str], + expected_errors_ids: list[tuple[str, int]], + expected_logs: list[str], + ) -> None: + """Test the Rule.checkrule method. + + TODO: This text is too complex and should be rewritten + checktext and checkline values help selecting combination of possible rules. """ - def raise_NotImplementedError(*args, **kwargs): + def raise_notimplementederror(*args: Any, **kwargs: Any) -> None: raise NotImplementedError - def return_empty_list(*args, **kwargs): + def return_empty_list(*args: Any, **kwargs: Any) -> list[Any]: # noqa: ARG001 return [] caplog.set_level(logging.DEBUG) # Build checktext and checkline if checktext is None: - test_rule.checktext = raise_NotImplementedError + test_rule.checktext = raise_notimplementederror elif checktext == 0: test_rule.checktext = return_empty_list else: # checktext > 0 - test_rule.checktext = lambda x, y: [ - issue - for i in range(checktext) - for issue in make_issue_from_rule(test_rule) - ] + test_rule.checktext = lambda *_: [issue for i in range(checktext) for issue in make_issue_from_rule(test_rule)] if checkline is None: - test_rule.checkline = raise_NotImplementedError + test_rule.checkline = raise_notimplementederror elif checkline == 0: test_rule.checkline = return_empty_list else: # checkline > 0 - test_rule.checkline = lambda x, y, line_no=0: [ - issue - for i in range(checkline) - for issue in make_issue_from_rule(test_rule) - ] + test_rule.checkline = lambda *_, line_no=0: [issue for i in range(checkline) for issue in make_issue_from_rule(test_rule)] # noqa: ARG005 - with open(file_path["path"], "r", encoding="utf-8") as file_d: + with Path(file_path["path"]).open(encoding="utf-8") as file_d: errors = test_rule.checkrule(file_path, file_d.read()) - print(errors) errors_ids = [(error.rule.rule_id, error.line_number) for error in errors] assert errors_ids == expected_errors_ids assert caplog.record_tuples == expected_logs diff --git a/tests/test_linter/test_runner.py b/tests/test_linter/test_runner.py index 5d4d2e9..73773b5 100644 --- a/tests/test_linter/test_runner.py +++ b/tests/test_linter/test_runner.py @@ -1,34 +1,41 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -""" -Tests for j2lint.linter.runner.py -""" +"""Tests for j2lint.linter.runner.py.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING from unittest import mock import pytest +if TYPE_CHECKING: + from pathlib import Path + + from j2lint.linter.runner import Runner + class TestRunner: + """Test j2lint.linter.runner.Runner.""" + @pytest.mark.parametrize( - "file_path, checked_files, expected", + ("file_path", "checked_files", "expected"), [ ("test.j2", [], False), ("test.j2", ["other.j2"], False), ("test.j2", ["test.j2"], True), ], ) - def test_is_already_checked(self, test_runner, file_path, checked_files, expected): - """ - test Runner.is_already_checked method - """ + def test_is_already_checked(self, test_runner: Runner, file_path: Path, checked_files: list[Path], *, expected: bool) -> None: + """Test Runner.is_already_checked method.""" test_runner.checked_files = checked_files assert test_runner.is_already_checked(file_path) == expected - # FIXME: refactor the code and augment the tests.. - # today if we give multiple files to a runner - # only the errors, warnings of the last one will - # be kept.. + # TODO: refactor the code and augment the tests.. + # today if we give multiple files to a runner + # only the errors, warnings of the last one will + # be kept.. @pytest.mark.parametrize( "runner_files", [ @@ -38,9 +45,8 @@ def test_is_already_checked(self, test_runner, file_path, checked_files, expecte (["test.txt"]), ], ) - def test_run(self, test_runner, runner_files): - """ - test Runner.run method + def test_run(self, test_runner: Runner, runner_files: list[Path]) -> None: + """Test Runner.run method. For now only testing with one input file given that this is how the package is working. @@ -49,9 +55,7 @@ def test_run(self, test_runner, runner_files): """ test_runner.files = runner_files # Fake return - with mock.patch( - "j2lint.linter.collection.RulesCollection.run" - ) as patched_collection_run: + with mock.patch("j2lint.linter.collection.RulesCollection.run") as patched_collection_run: patched_collection_run.return_value = ([], []) result = test_runner.run() diff --git a/tests/test_logger.py b/tests/test_logger.py index abf32b6..ac7ef7b 100644 --- a/tests/test_logger.py +++ b/tests/test_logger.py @@ -1,38 +1,34 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -""" -Tests for j2lint.logger.py -""" +"""Tests for j2lint.logger.py.""" + +from __future__ import annotations + import logging -import sys from logging import handlers +from typing import TYPE_CHECKING import pytest from rich.logging import RichHandler from j2lint.logger import add_handler +if TYPE_CHECKING: + from unittest.mock import MagicMock -@pytest.mark.parametrize( - "stream_handler, log_level", [(False, logging.DEBUG), (True, logging.ERROR)] -) -def test_add_handler(logger, stream_handler, log_level): - """ - Test j2lint.logger.add_handler + +@pytest.mark.parametrize(("log_level", "stream_handler"), [(logging.DEBUG, False), (logging.ERROR, True)]) +def test_add_handler(logger: MagicMock, log_level: int, *, stream_handler: bool) -> None: + """Test j2lint.logger.add_handler. Verify that the correct type of handler is added """ - add_handler(logger, stream_handler, log_level) + add_handler(logger, log_level, stream_handler=stream_handler) logger.setLevel.assert_called_once_with(log_level) logger.addHandler.assert_called_once() - # call_args.args was introduced in Python 3.8 - handler_arg = None - if sys.version_info >= (3, 8): - handler_arg = logger.addHandler.call_args.args[0] - elif sys.version_info >= (3, 6): - handler_arg = logger.addHandler.call_args[0][0] + handler_arg = logger.addHandler.call_args.args[0] if not stream_handler: assert isinstance(handler_arg, handlers.RotatingFileHandler) diff --git a/tests/test_rules/__init__.py b/tests/test_rules/__init__.py new file mode 100644 index 0000000..9060631 --- /dev/null +++ b/tests/test_rules/__init__.py @@ -0,0 +1,4 @@ +# Copyright (c) 2021-2025 Arista Networks, Inc. +# Use of this source code is governed by the MIT license +# that can be found in the LICENSE file. +"""Tests for j2lint.rules.""" diff --git a/tests/test_rules/test_rules.py b/tests/test_rules/test_rules.py index b7653ff..e9bc3b3 100644 --- a/tests/test_rules/test_rules.py +++ b/tests/test_rules/test_rules.py @@ -1,12 +1,20 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. +"""Tests for rules.""" + +from __future__ import annotations + import logging -import pathlib +from pathlib import Path +from typing import TYPE_CHECKING import pytest -TEST_DATA_DIR = pathlib.Path(__file__).parent / "data" +if TYPE_CHECKING: + from j2lint.linter.collection import RulesCollection + +TEST_DATA_DIR = Path(__file__).parent / "data" PARAMS = [ pytest.param( @@ -89,13 +97,14 @@ [ # somehow this is not picked up when we should expect this log message (which can be seen in CLI) # probably some caplog issue here so commenting for now - # FIXME - # ( - # "root", - # logging.ERROR, - # f"Indentation check failed for file {TEST_DATA_DIR}/jinja_template_indentation_rule.IndexError.j2: " - # "Error: list index out of range", - # ) + # ruff: noqa: ERA001 + # TODO: + # ( + # "root", + # logging.ERROR, + # f"Indentation check failed for file {TEST_DATA_DIR}/jinja_template_indentation_rule.IndexError.j2: " + # "Error: list index out of range", + # ], ), pytest.param( @@ -138,31 +147,41 @@ @pytest.mark.parametrize( - "filename, j2_errors_ids, j2_warnings_ids, expected_log", + ("filename", "j2_errors_ids", "j2_warnings_ids", "expected_log"), PARAMS, ) def test_rules( - caplog, collection, filename, j2_errors_ids, j2_warnings_ids, expected_log -): - """ - caplog: fixture to capture logs - collection: a collection from the j2lint default rules - filename: the name of the file to parse - j2_errors_ids: the ids of the expected errors (, ) - j2_warnings_ids: the ids of the expected warnings (, ) - expected_log: a list of expected log tuples as defined per caplog.record_tuples - """ + caplog: pytest.LogCaptureFixture, + collection: RulesCollection, + filename: str, + j2_errors_ids: list[tuple[str, int]], + j2_warnings_ids: list[tuple[str, int]], + expected_log: list[str], +) -> None: + """Test all the rules. - with open(filename, "r") as f: - print(f.read()) + Parameters + ---------- + caplog + fixture to capture logs + collection + a collection from the j2lint default rules + filename + the name of the file to parse + j2_errors_ids + the ids of the expected errors (, ) + j2_warnings_ids + the ids of the expected warnings (, ) + expected_log + a list of expected log tuples as defined per caplog.record_tuples + """ caplog.set_level(logging.INFO) - errors, warnings = collection.run(filename) + errors, warnings = collection.run(Path(filename)) errors_ids = [(error.rule.rule_id, error.line_number) for error in errors] warnings_ids = [(warning.rule.rule_id, warning.line_number) for warning in warnings] - print(caplog.record_tuples) for record_tuple in expected_log: assert record_tuple in caplog.record_tuples diff --git a/tests/test_utils.py b/tests/test_utils.py index b95da2f..622081a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,10 +1,12 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -""" -Tests for j2lint.utils.py -""" -import pathlib +"""Tests for j2lint.utils.py.""" + +from __future__ import annotations + +from pathlib import Path +from typing import TYPE_CHECKING, Any import pytest @@ -19,21 +21,20 @@ from .utils import does_not_raise -# pylint: disable=fixme +if TYPE_CHECKING: + from collections.abc import Generator @pytest.mark.skip -def test_load_plugins(): - """ - Test the utils.load_plugins function +def test_load_plugins() -> None: + """Test the utils.load_plugins function. For now this is being tested via other calling methods """ - # TODO @pytest.mark.parametrize( - "file_name, extensions, expected", + ("file_name", "extensions", "expected"), [ ("test.j2", [".j2", ".jinja2", ".jinja"], True), ("test.jinja", [".j2", ".jinja2", ".jinja"], True), @@ -45,15 +46,13 @@ def test_load_plugins(): ("test.j2", [".toto"], False), ], ) -def test_is_valid_file_type(file_name, extensions, expected): - """ - Test the utils.is_valid_file_type function - """ - assert is_valid_file_type(file_name, extensions) == expected +def test_is_valid_file_type(file_name: str, extensions: list[str], *, expected: bool) -> None: + """Test the utils.is_valid_file_type function.""" + assert is_valid_file_type(Path(file_name), extensions) == expected @pytest.mark.parametrize( - "file_or_dir_names, extensions, expected_value, expectation", + ("file_or_dir_names", "extensions", "expected_value", "expectation"), [ (["test.j2"], [".j2", ".jinja2", ".jinja"], ["test.j2"], does_not_raise()), ( @@ -93,27 +92,23 @@ def test_is_valid_file_type(file_name, extensions, expected): does_not_raise(), ), (["test"], [".j2", ".jinja2", ".jinja"], [], does_not_raise()), - pytest.param( - "not_a_list", None, None, pytest.raises(TypeError), id="Invalid input type" - ), + pytest.param("not_a_list", None, None, pytest.raises(TypeError), id="Invalid input type"), ], ) -def test_get_files(file_or_dir_names, extensions, expected_value, expectation): - """ - Test the utils.get_files function - """ +def test_get_files(file_or_dir_names: list[str], extensions: list[str], expected_value: list[str], expectation: Generator) -> None: + """Test the utils.get_files function.""" with expectation: - assert get_files(file_or_dir_names, extensions) == expected_value + paths = [Path(file_or_dir_name) for file_or_dir_name in file_or_dir_names] + assert [str(filepath) for filepath in get_files(paths, extensions)] == expected_value -def test_get_files_dir(template_tmp_dir): - """ - Test the utils.get_files function +def test_get_files_dir(template_tmp_dir: str) -> None: + """Test the utils.get_files function. # sadly cannot use fixture in parametrize... # https://github.com/pytest-dev/pytest/issues/349 """ - tmp_dir = pathlib.Path(__file__).parent / "tmp" # as passed to pytest in pytest.ini + tmp_dir = Path(__file__).parent / "tmp" # as passed to pytest in pytest.ini expected = sorted( [ f"{tmp_dir}/rules/rules.j2", @@ -122,11 +117,12 @@ def test_get_files_dir(template_tmp_dir): ] ) with does_not_raise(): - assert sorted(get_files(template_tmp_dir, [".j2"])) == expected + paths = [Path(file) for file in template_tmp_dir] + assert sorted([str(filename) for filename in get_files(paths, [".j2"])]) == expected @pytest.mark.parametrize( - "input_list, expected, raising_context", + ("input_list", "expected", "raising_context"), [ ("not_a_list", [], pytest.raises(TypeError)), ([1, 2, 3], [1, 2, 3], does_not_raise()), @@ -134,16 +130,14 @@ def test_get_files_dir(template_tmp_dir): ([[1, 2], [3, 4]], [1, 2, 3, 4], does_not_raise()), ], ) -def test_flatten(input_list, expected, raising_context): - """ - Test the utils.flatten function - """ +def test_flatten(input_list: list[list[Any]], expected: list[Any], raising_context: Generator) -> None: + """Test the utils.flatten function.""" with raising_context: assert list(flatten(input_list)) == expected @pytest.mark.parametrize( - "tuple_list, lookup_object, expected_value", + ("tuple_list", "lookup_object", "expected_value"), [ ( [(1, 2, 3), (1, 2, 4)], @@ -155,58 +149,46 @@ def test_flatten(input_list, expected, raising_context): ([(1, 2, 3), (1, 2, 4)], 5, None), ], ) -def test_get_tuple(tuple_list, lookup_object, expected_value): - """ - Test the utils.get_tuple function - """ +def test_get_tuple(tuple_list: list[tuple[Any]], lookup_object: Any, expected_value: tuple[Any] | None) -> None: + """Test the utils.get_tuple function.""" assert get_tuple(tuple_list, lookup_object) == expected_value @pytest.mark.skip -def test_get_jinja_statements(): - """ - Test the utils.get_jinja_statements function - """ - # TODO +def test_get_jinja_statements() -> None: + """Test the utils.get_jinja_statements function.""" + # TODO: @pytest.mark.parametrize( - "line, kwargs, expected", + ("line", "kwargs", "expected"), [ ("foo", {}, "{%foo%}"), ("foo", {"start": "{#"}, "{#foo%}"), ("foo", {"start": "{#", "end": "#}"}, "{#foo#}"), ], ) -def test_delimit_jinja_statement(line, kwargs, expected): - """ - Test the utils.delimit_jinja_statement function - """ +def test_delimit_jinja_statement(line: str, kwargs: dict[str, str], expected: str) -> None: + """Test the utils.delimit_jinja_statement function.""" assert delimit_jinja_statement(line, **kwargs) == expected @pytest.mark.skip -def test_get_jinja_comments(): - """ - Test the utils.get_jinja_comments function - """ - # TODO +def test_get_jinja_comments() -> None: + """Test the utils.get_jinja_comments function.""" + # TODO: @pytest.mark.skip -def test_get_jinja_variables(): - """ - Test the utils.get_jinja_variables function - """ - # TODO +def test_get_jinja_variables() -> None: + """Test the utils.get_jinja_variables function.""" + # TODO: @pytest.mark.parametrize( - "comments, expected_value", + ("comments", "expected_value"), [ - pytest.param( - ["{# j2lint: disable=test-rule-0 #}"], True, id="found_short_description" - ), + pytest.param(["{# j2lint: disable=test-rule-0 #}"], True, id="found_short_description"), pytest.param(["{# j2lint: disable=T0 #}"], True, id="found_id"), pytest.param( ["{# j2lint: disable=test-rule-1 #}"], @@ -231,14 +213,11 @@ def test_get_jinja_variables(): ), ], ) -def test_is_rule_disabled(make_rules, comments, expected_value): - """ - Test the utils.is_rule_disabled function - """ +def test_is_rule_disabled(make_rules: pytest.Fixture, comments: list[str], *, expected_value: bool) -> None: + """Test the utils.is_rule_disabled function.""" # Generate one rule through fixture which is always # T0, test-rule-0 test_rule = make_rules(1)[0] comments_string = "\n".join(comments) - print(comments_string) assert is_rule_disabled(comments_string, test_rule) == expected_value diff --git a/tests/utils.py b/tests/utils.py index a9069bb..8e4ecba 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,25 +1,25 @@ # Copyright (c) 2021-2025 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. -""" -utils.py - functions to assist with tests -""" +"""utils.py - functions to assist with tests.""" + +from __future__ import annotations + from contextlib import contextmanager +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from collections.abc import Generator @contextmanager -def does_not_raise(): - """ - Provides a context manager that does not raise anything - for pytest tests - """ +def does_not_raise() -> Generator[Any, Any, Any]: + """Provide a context manager that does not raise anything for pytest tests.""" yield -def j2lint_default_rules_string(): - """ - The description of the default rules - """ +def j2lint_default_rules_string() -> str: + """Return the description of the default rules.""" return ( "─────────────────────────── Rules in the Collection ────────────────────────────\n" "Origin: BUILT-IN\n" @@ -98,7 +98,7 @@ def j2lint_default_rules_string(): "Jinja2 linting finished with 1 error(s) and 0 warning(s)\n" ) -# Cannot use """ because of the trailing whitspaces generated in rich Tree +# Cannot use """ because of the trailing whitespaces generated in rich Tree ONE_WARNING_VERBOSE = ( "───────────────────────────── JINJA2 LINT WARNINGS ─────────────────────────────\n" "dummy.j2\n" @@ -111,7 +111,7 @@ def j2lint_default_rules_string(): "Jinja2 linting finished with 0 error(s) and 1 warning(s)\n" ) -# Cannot use """ because of the trailing whitspaces generated in rich Tree +# Cannot use """ because of the trailing whitespaces generated in rich Tree ONE_ERROR_ONE_WARNING_VERBOSE = ( "────────────────────────────── JINJA2 LINT ERRORS ──────────────────────────────\n" "tests/data/test.j2\n"