From 128116bbb47b2d9f795636a7581ce37c111a72cb Mon Sep 17 00:00:00 2001 From: gmuloc Date: Mon, 23 Sep 2024 22:05:47 +0200 Subject: [PATCH 01/14] ci: First ruff fixes --- .pre-commit-config.yaml | 44 +++-- j2lint/__init__.py | 4 +- j2lint/__main__.py | 4 +- j2lint/cli.py | 74 +++----- j2lint/linter/collection.py | 25 ++- j2lint/linter/error.py | 8 +- j2lint/linter/indenter/node.py | 59 +++---- j2lint/linter/indenter/statement.py | 4 +- j2lint/linter/rule.py | 14 +- j2lint/linter/runner.py | 14 +- j2lint/logger.py | 14 +- .../rules/jinja_operator_has_spaces_rule.py | 21 +-- .../rules/jinja_statement_delimiter_rule.py | 11 +- .../rules/jinja_statement_has_spaces_rule.py | 8 +- .../rules/jinja_template_indentation_rule.py | 16 +- j2lint/rules/jinja_template_no_tabs_rule.py | 6 +- .../jinja_template_single_statement_rule.py | 14 +- .../rules/jinja_template_syntax_error_rule.py | 10 +- j2lint/rules/jinja_variable_has_space_rule.py | 17 +- j2lint/rules/jinja_variable_name_case_rule.py | 21 +-- .../rules/jinja_variable_name_format_rule.py | 24 +-- j2lint/utils.py | 50 +++--- pyproject.toml | 158 +++++++++++++++++- tests/conftest.py | 23 ++- tests/test_cli.py | 24 +-- tests/test_linter/test_collection.py | 36 +--- tests/test_linter/test_error.py | 16 +- tests/test_linter/test_indenter/test_node.py | 2 +- .../test_indenter/test_statement.py | 1 + tests/test_linter/test_rule.py | 15 +- tests/test_linter/test_runner.py | 5 +- tests/test_logger.py | 5 +- tests/test_rules/test_rules.py | 7 +- tests/test_utils.py | 9 +- tests/utils.py | 1 + 35 files changed, 387 insertions(+), 377 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3fbaf67..305d66d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -39,24 +39,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.6.7 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.2.7 + rev: "v3.3.0" 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: @@ -64,24 +66,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.8.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.11.2 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/j2lint/__init__.py b/j2lint/__init__.py index ab04c07..c63a3fa 100644 --- a/j2lint/__init__.py +++ b/j2lint/__init__.py @@ -1,8 +1,8 @@ # Copyright (c) 2021-2024 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.""" + NAME = "j2lint" VERSION = "v1.1.0" DESCRIPTION = __doc__ diff --git a/j2lint/__main__.py b/j2lint/__main__.py index 0e55f1b..07c6753 100644 --- a/j2lint/__main__.py +++ b/j2lint/__main__.py @@ -2,8 +2,8 @@ # Copyright (c) 2021-2024 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 diff --git a/j2lint/cli.py b/j2lint/cli.py index 8fa519d..ffaae9c 100644 --- a/j2lint/cli.py +++ b/j2lint/cli.py @@ -1,8 +1,8 @@ # Copyright (c) 2021-2024 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 @@ -51,7 +51,8 @@ def create_parser() -> argparse.ArgumentParser: """Initializes a new argument parser object - Returns: + Returns + ------- ArgumentParser: Argument parser object """ parser = argparse.ArgumentParser(prog=NAME, description=DESCRIPTION) @@ -63,9 +64,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 +87,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 +96,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", @@ -136,19 +125,16 @@ def sort_issues(issues: list[LinterError]) -> list[LinterError]: Args: issues (list): list of issue dictionaries - Returns: + 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""" +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]] = {} @@ -169,7 +155,7 @@ def print_json_output( lint_errors: dict[str, list[LinterError]], lint_warnings: dict[str, list[LinterError]], ) -> tuple[int, int]: - """printing json output""" + """Printing json output""" json_output: dict[str, list[str]] = {"ERRORS": [], "WARNINGS": []} for _, errors in lint_errors.items(): for error in errors: @@ -187,11 +173,9 @@ def print_string_output( lint_warnings: dict[str, list[LinterError]], verbose: bool, ) -> tuple[int, int]: - """print non-json output""" + """Print non-json output""" - def print_issues( - lint_issues: dict[str, list[LinterError]], issue_type: str - ) -> None: + def print_issues(lint_issues: dict[str, 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: @@ -214,10 +198,7 @@ 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 @@ -245,7 +226,8 @@ def run(args: list[str] | None = None) -> int: Args: args ([string], optional): Command line arguments. Defaults to None. - Returns: + Returns + ------- int: 0 on success """ # pylint: disable=too-many-branches @@ -273,9 +255,7 @@ def run(args: list[str] | None = None) -> int: checked_files: list[str] = [] 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 file_or_dir_names.append(stdin_filename) @@ -283,11 +263,7 @@ def run(args: list[str] | None = None) -> int: # Collect the rules from the configuration collection = RulesCollection(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 +291,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, options.verbose) # Remove temporary file if stdin_filename is not None: diff --git a/j2lint/linter/collection.py b/j2lint/linter/collection.py index e44354d..23102de 100644 --- a/j2lint/linter/collection.py +++ b/j2lint/linter/collection.py @@ -1,8 +1,8 @@ # Copyright (c) 2021-2024 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 @@ -51,7 +51,8 @@ def run(self, file_path: str) -> tuple[list[LinterError], list[LinterError]]: Args: file_dict (dict): file path and file type - Returns: + Returns + ------- tuple(list, list): a tuple containing the list of linting errors and the list of linting warnings found """ @@ -60,9 +61,9 @@ def run(self, file_path: str) -> tuple[list[LinterError], list[LinterError]]: warnings: list[LinterError] = [] try: - with open(file_path, mode="r", encoding="utf-8") as file: + with open(file_path, 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 @@ -131,17 +132,10 @@ def to_rich(self) -> Group: 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 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: + 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: @@ -150,7 +144,8 @@ def create_from_directory( warn_rules (list): list of rule short_descriptions or ids to consider as warnings rather than errors - Returns: + Returns + ------- list: a collection of rule objects """ result = cls() diff --git a/j2lint/linter/error.py b/j2lint/linter/error.py index 2a70f98..e790bc0 100644 --- a/j2lint/linter/error.py +++ b/j2lint/linter/error.py @@ -1,8 +1,8 @@ # Copyright (c) 2021-2024 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 @@ -35,7 +35,7 @@ def __init__( self.message = message or rule.description def to_rich(self, verbose: bool = False) -> Text: - """setting string output format""" + """Setting 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""" + """Setting json output format""" return json.dumps( { "id": self.rule.rule_id, diff --git a/j2lint/linter/indenter/node.py b/j2lint/linter/indenter/node.py index bf7ddaf..06bdfda 100644 --- a/j2lint/linter/indenter/node.py +++ b/j2lint/linter/indenter/node.py @@ -2,8 +2,9 @@ # 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. +checking jinja statement indentation. """ + from __future__ import annotations from typing import NoReturn, Tuple @@ -53,7 +54,8 @@ def create_node(self, line: Statement, line_no: int, indent_level: int = 0) -> N line_no (int): line number indent_level (int, optional): expected indentation level. Defaults to 0. - Returns: + Returns + ------- Node: new Node class object """ node = Node() @@ -67,16 +69,15 @@ 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: + 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: + Returns + ------- tuple: tuple representing the indentation error """ if node.statement is None: @@ -92,10 +93,8 @@ 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 @@ -104,15 +103,8 @@ def check_indent_level( 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 +113,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: @@ -137,8 +127,7 @@ def _assert_not_none(self, current_line_no: int, new_line_no: int | None) -> int """ 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", + "Recursive check_indentation returned None for an opening tag " f"line {current_line_no} - missing closing tag", ) return new_line_no @@ -163,12 +152,14 @@ def check_indentation( indent_level (int, optional): the expected indent level for the current line. Defaults to 0. - Raises: + 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: + Returns + ------- line_no (int) or None """ @@ -185,9 +176,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 +192,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) @@ -240,13 +227,9 @@ 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 jinja_node_stack[-1] != self: diff --git a/j2lint/linter/indenter/statement.py b/j2lint/linter/indenter/statement.py index def655e..14999eb 100644 --- a/j2lint/linter/indenter/statement.py +++ b/j2lint/linter/indenter/statement.py @@ -1,8 +1,8 @@ # Copyright (c) 2021-2024 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 diff --git a/j2lint/linter/rule.py b/j2lint/linter/rule.py index 1afe71c..abf6435 100644 --- a/j2lint/linter/rule.py +++ b/j2lint/linter/rule.py @@ -2,8 +2,9 @@ # 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. +line and text based rule. """ + from __future__ import annotations import json @@ -46,14 +47,10 @@ 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}" - ) + raise NotImplementedError(f"Class {cls} is missing required class attribute {attr}") 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" - ) + raise JinjaLinterError(f"Rule {cls.rule_id}: severity must be in [None, 'LOW', 'MEDIUM', 'HIGH'], {cls.severity} was provided") def __repr__(self) -> str: return f"{self.rule_id}: {self.description}" @@ -100,7 +97,8 @@ def checkrule(self, filename: str, text: str) -> list[LinterError]: filename (string): file path of the file to be checked text (string): file text of the same file - Returns: + Returns + ------- list: list of LinterError from issues in the given file """ errors: list[LinterError] = [] diff --git a/j2lint/linter/runner.py b/j2lint/linter/runner.py index 827bcad..a31e85b 100644 --- a/j2lint/linter/runner.py +++ b/j2lint/linter/runner.py @@ -1,8 +1,8 @@ # Copyright (c) 2021-2024 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 j2lint.logger import logger @@ -19,9 +19,7 @@ class Runner: 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: str, checked_files: list[str]) -> None: self.collection = collection self.files: set[str] = {file_name} self.checked_files = checked_files @@ -32,7 +30,8 @@ def is_already_checked(self, file_path: str) -> bool: Args: file_path (string): file path - Returns: + Returns + ------- bool: True if file is already checked once """ return file_path in self.checked_files @@ -40,7 +39,8 @@ def is_already_checked(self, file_path: str) -> bool: def run(self) -> tuple[list[LinterError], list[LinterError]]: """Runs the lint rules collection on all the files - Returns: + 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 diff --git a/j2lint/logger.py b/j2lint/logger.py index b870b8e..58bc420 100644 --- a/j2lint/logger.py +++ b/j2lint/logger.py @@ -1,8 +1,8 @@ # Copyright (c) 2021-2024 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 @@ -14,15 +14,11 @@ 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" - ) + """Defined logging handlers""" + 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/jinja_operator_has_spaces_rule.py b/j2lint/rules/jinja_operator_has_spaces_rule.py index 0d12cc4..f1f0375 100644 --- a/j2lint/rules/jinja_operator_has_spaces_rule.py +++ b/j2lint/rules/jinja_operator_has_spaces_rule.py @@ -2,8 +2,9 @@ # 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. +surrounding spaces. """ + from __future__ import annotations import re @@ -17,10 +18,7 @@ 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" @@ -55,10 +53,10 @@ def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError] Args: line (string): a single line from the file - Returns: + Returns + ------- list[LinterError]: the list of LinterError generated by this rule """ - errors: list[LinterError] = [] # pylint: disable = fixme # TODO - refactor @@ -75,19 +73,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 595dda7..6e10860 100644 --- a/j2lint/rules/jinja_statement_delimiter_rule.py +++ b/j2lint/rules/jinja_statement_delimiter_rule.py @@ -2,7 +2,7 @@ # 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. +are wrong. """ from __future__ import annotations @@ -34,14 +34,11 @@ def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError] Args: line (string): a single line from the file - Returns: + 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 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 f0174a8..acf9a66 100644 --- a/j2lint/rules/jinja_statement_has_spaces_rule.py +++ b/j2lint/rules/jinja_statement_has_spaces_rule.py @@ -2,9 +2,10 @@ # 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. +at least a single space surrounding the +delimiter. """ + from __future__ import annotations import re @@ -38,7 +39,8 @@ def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError] Args: line (string): a single line from the file - Returns: + Returns + ------- list[LinterError]: the list of LinterError generated by this rule """ matches = self.regex.search(line) diff --git a/j2lint/rules/jinja_template_indentation_rule.py b/j2lint/rules/jinja_template_indentation_rule.py index 15964db..00dad4a 100644 --- a/j2lint/rules/jinja_template_indentation_rule.py +++ b/j2lint/rules/jinja_template_indentation_rule.py @@ -2,8 +2,9 @@ # 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. +indentation is correct. """ + from __future__ import annotations from typing import Any @@ -20,10 +21,7 @@ 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: @@ -36,7 +34,8 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: file (string): file path text (string): entire text content of the file - Returns: + Returns + ------- list: Returns list of error objects """ # Collect only Jinja Statements within delimiters {% and %} @@ -59,10 +58,7 @@ 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]: raise NotImplementedError diff --git a/j2lint/rules/jinja_template_no_tabs_rule.py b/j2lint/rules/jinja_template_no_tabs_rule.py index fc25c5c..2dbdf24 100644 --- a/j2lint/rules/jinja_template_no_tabs_rule.py +++ b/j2lint/rules/jinja_template_no_tabs_rule.py @@ -2,8 +2,9 @@ # 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. +for indentation. """ + from __future__ import annotations import re @@ -35,7 +36,8 @@ def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError] Args: line (string): a single line from the file - Returns: + Returns + ------- list[LinterError]: the list of LinterError generated by this rule """ matches = self.regex.search(line) diff --git a/j2lint/rules/jinja_template_single_statement_rule.py b/j2lint/rules/jinja_template_single_statement_rule.py index 3acdcac..90f6817 100644 --- a/j2lint/rules/jinja_template_single_statement_rule.py +++ b/j2lint/rules/jinja_template_single_statement_rule.py @@ -2,9 +2,10 @@ # 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 statement is present on each +line. """ + from __future__ import annotations from typing import Any @@ -36,11 +37,8 @@ def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError] Args: line (string): a single line from the file - Returns: + 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 c577dbf..43eb4c7 100644 --- a/j2lint/rules/jinja_template_syntax_error_rule.py +++ b/j2lint/rules/jinja_template_syntax_error_rule.py @@ -2,8 +2,9 @@ # 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. +have jinja syntax errors. """ + from __future__ import annotations from typing import Any @@ -32,14 +33,13 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: file (string): file path text (string): entire text content of the file - Returns: + Returns + ------- list: Returns list of error objects """ result = [] - env = jinja2.Environment( - extensions=["jinja2.ext.do", "jinja2.ext.loopcontrols"] - ) + env = jinja2.Environment(extensions=["jinja2.ext.do", "jinja2.ext.loopcontrols"]) try: env.parse(text) except jinja2.TemplateSyntaxError as error: diff --git a/j2lint/rules/jinja_variable_has_space_rule.py b/j2lint/rules/jinja_variable_has_space_rule.py index ae25b09..68f188f 100644 --- a/j2lint/rules/jinja_variable_has_space_rule.py +++ b/j2lint/rules/jinja_variable_has_space_rule.py @@ -2,9 +2,10 @@ # 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. +single space between curly brackets and +variable name. """ + from __future__ import annotations import re @@ -20,17 +21,12 @@ class JinjaVariableHasSpaceRule(Rule): """ rule_id = "S1" - description = ( - "A single space should be added between Jinja2 curly brackets " - "and a variable name: {{ ethernet_interface }}" - ) + description = "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__() @@ -44,7 +40,8 @@ def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError] Args: line (string): a single line from the file - Returns: + Returns + ------- list[LinterError]: the list of LinterError generated by this rule """ matches = self.regex.search(line) diff --git a/j2lint/rules/jinja_variable_name_case_rule.py b/j2lint/rules/jinja_variable_name_case_rule.py index 17f144e..275991c 100644 --- a/j2lint/rules/jinja_variable_name_case_rule.py +++ b/j2lint/rules/jinja_variable_name_case_rule.py @@ -2,8 +2,9 @@ # 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. +lower case. """ + from __future__ import annotations import re @@ -40,22 +41,14 @@ def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError] Args: line (string): a single line from the file - Returns: + 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 7543d61..1358205 100644 --- a/j2lint/rules/jinja_variable_name_format_rule.py +++ b/j2lint/rules/jinja_variable_name_format_rule.py @@ -1,9 +1,10 @@ # Copyright (c) 2021-2024 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 @@ -20,10 +21,7 @@ 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 = "If variable is multi-words, underscore `_` should be used " "as a separator: '{{ my_variable_name }}'" short_description = "jinja-variable-format" severity = "LOW" @@ -43,18 +41,14 @@ def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError] Args: line (string): a single line from the file - Returns: + 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 48f70a1..480e9a4 100644 --- a/j2lint/utils.py +++ b/j2lint/utils.py @@ -1,8 +1,8 @@ # Copyright (c) 2021-2024 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 @@ -29,7 +29,8 @@ def load_plugins(directory: str) -> list[Rule]: Args: directory (string): Loads the modules a directory - Returns: + Returns + ------- list: List of rule classes """ result = [] @@ -40,9 +41,7 @@ def load_plugins(directory: str) -> list[Rule]: 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) @@ -63,7 +62,8 @@ def is_valid_file_type(file_name: str, extensions: list[str]) -> bool: file_name (string): file path with extension extensions (list): list of file extensions to look for - Returns: + Returns + ------- boolean: True if file type is correct """ extension = os.path.splitext(file_name)[1].lower() @@ -77,15 +77,14 @@ def get_files(file_or_dir_names: list[str], extensions: list[str]) -> list[str]: file_or_dir_names (list): list of directories and files extensions (list): list of file extensions to look for - Returns: + Returns + ------- list: list of file paths """ file_paths: list[str] = [] 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}" - ) + raise TypeError(f"get_files expects a list or a set and got {file_or_dir_names}") for file_or_dir in file_or_dir_names: if os.path.isdir(file_or_dir): @@ -106,13 +105,12 @@ def flatten(nested_list: Iterable[Any]) -> Generator[Any, Any, Any]: Args: nested_list (list): Nested list - Returns: + Returns + ------- 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}" - ) + raise TypeError(f"flatten is expecting a list or tuple and received {nested_list}") for element in nested_list: if isinstance(element, Iterable) and not isinstance(element, (str, bytes)): yield from flatten(element) @@ -120,16 +118,15 @@ 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: +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 Args: list_of_tuples (list): list of tuples item (object): single object which can be in a tuple - Returns: + Returns + ------- [tuple]: tuple if the item exists in any of the tuples """ return next((entry for entry in list_of_tuples if item in entry), None) @@ -170,7 +167,8 @@ def get_jinja_statements(text: str, indentation: bool = False) -> list[Statement Found jinja statements [(' endif ', 1, 1, '{%', '%}')] Found jinja statements [] - Returns: + Returns + ------- [list]: list of jinja statements # TODO - should probably return a JinjaStatement object.. @@ -206,7 +204,8 @@ def delimit_jinja_statement(line: str, start: str = "{%", end: str = "%}") -> st Args: line (string): text line - Returns: + Returns + ------- [string]: jinja statement with jinja start and end delimiters """ return start + line + end @@ -218,7 +217,8 @@ def get_jinja_comments(text: str) -> list[str]: Args: line (string): text to get jinja comments - Returns: + Returns + ------- [list]: returns list of jinja comments """ regex_pattern = re.compile("(\\{#)((.|\n)*?)(\\#})", re.MULTILINE) @@ -232,7 +232,8 @@ def get_jinja_variables(text: str) -> list[str]: Args: line (string): text to get jinja variables - Returns: + Returns + ------- [list]: returns list of jinja variables """ regex_pattern = regex_pattern = re.compile("(\\{{)((.|\n)*?)(\\}})", re.MULTILINE) @@ -246,7 +247,8 @@ def is_rule_disabled(text: str, rule: Rule) -> bool: text (string): text to check rule (Rule): Rule object - Returns: + Returns + ------- [boolean]: True if rule is disabled """ comments = get_jinja_comments(text) diff --git a/pyproject.toml b/pyproject.toml index 8746a0e..0a52651 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,10 +44,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.5.4,<0.7.0", + "codespell>=2.2.6,<2.4.0", ] type = [ "mypy==1.11.2", @@ -187,5 +186,154 @@ 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 ANTA +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 + # TODO: Augment the numpy convention rules to make sure we add all the params + # Uncomment below D417 + "D415", + "D417", +] +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.flake8-type-checking] +# These classes require that type annotations be available at runtime +runtime-evaluated-base-classes = ["pydantic.BaseModel", "anta.models.AntaTest.Input"] + + +[tool.ruff.lint.per-file-ignores] +"tests/*" = [ + "S101", # Complains about asserts in units and libs. + "SLF001", # Lots of private member accessed for test purposes +] +"tests/units/*" = [ + "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 + "PLR0913", # Too many arguments to function call + "PLR2004", # Magic value used in comparison, consider replacing {value} with a constant variable + "S105", # Passwords are indeed hardcoded in tests + "S106", # Passwords are indeed hardcoded in tests + "S108", # Probable insecure usage of temporary file or directory +] +"tests/units/anta_tests/test_interfaces.py" = [ + "S104", # False positive for 0.0.0.0 bindings in test inputs +] +"tests/units/anta_tests/*" = [ + "F401", # In this module, we import tests.units.anta_tests.test without using it to auto-generate tests +] +"anta/*" = [ + "TRY400", # Use `logging.exception` instead of `logging.error` - we know what we are doing +] +"anta/cli/exec/utils.py" = [ + "SLF001", # TODO: some private members, lets try to fix +] +"anta/cli/__init__.py" = [ + "T201", # Allow print statements +] +"anta/cli/*" = [ + "PLR0913", # Allow more than 5 input arguments in CLI functions + "ANN401", # TODO: Check if we can update the Any type hints in the CLI +] +"anta/tests/field_notices.py" = [ + "PLR2004", # Magic value used in comparison, consider replacing 2131 with a constant variable - Field notice IDs are magic values + "C901", # TODO: test function is too complex, needs a refactor + "PLR0911", # TODO: Too many return statements, same as above needs a refactor +] +"anta/tests/routing/isis.py" = [ + "C901", # TODO: test function is too complex, needs a refactor + "PLR0912" # Too many branches (15/12) (too-many-branches), needs a refactor +] +"anta/decorators.py" = [ + "ANN401", # Ok to use Any type hint in our decorators +] +"anta/tools.py" = [ + "ANN401", # Ok to use Any type hint in our custom get functions + "PLR0913", # Ok to have more than 5 arguments in our custom get functions +] +"anta/device.py" = [ + "PLR0913", # Ok to have more than 5 arguments in the AntaDevice classes +] +"anta/inventory/__init__.py" = [ + "PLR0913", # Ok to have more than 5 arguments in the AntaInventory class +] +"examples/anta_runner.py" = [ # This is an example script and linked in snippets + "S108", # Probable insecure usage of temporary file or directory + "S105", # Possible hardcoded password + "INP001", # Implicit packages +] diff --git a/tests/conftest.py b/tests/conftest.py index 6849817..aa3f16c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,6 +4,7 @@ """ content of conftest.py """ + import logging import pathlib from argparse import Namespace @@ -73,11 +74,7 @@ def test_blah(makes_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") - ) + return "LOW" if integer % 3 == 0 else ("MEDIUM" if integer % 3 == 1 else "HIGH") rules = [] for i in range(count): @@ -103,7 +100,7 @@ def test_rule(make_rules): short_description = test-rule-0 severity = LOW """ - yield make_rules(1)[0] + return make_rules(1)[0] @pytest.fixture @@ -117,7 +114,7 @@ def test_other_rule(make_rules): short_description = test-rule-1 severity = MEDIUM """ - yield make_rules(2)[1] + return make_rules(2)[1] @pytest.fixture @@ -176,7 +173,7 @@ def test_issue(make_issues): Note: it will use rule T0 as per design """ - yield make_issues(1)[0] + return make_issues(1)[0] @pytest.fixture @@ -186,7 +183,7 @@ def test_collection(test_rule): """ collection = RulesCollection() collection.extend([test_rule]) - yield collection + return collection @pytest.fixture @@ -194,7 +191,7 @@ def test_runner(test_collection): """ Fixture to get a test runner using the test_collection """ - yield Runner(test_collection, "test.j2", checked_files=[]) + return Runner(test_collection, "test.j2", checked_files=[]) @pytest.fixture @@ -202,10 +199,10 @@ def j2lint_usage_string(): """ Fixture to get the help generated by argparse """ - yield create_parser().format_help() + return create_parser().format_help() -@pytest.fixture() +@pytest.fixture def template_tmp_dir(tmp_path_factory): """ Create a tmp directory with multiple files and hidden files @@ -244,7 +241,7 @@ 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 diff --git a/tests/test_cli.py b/tests/test_cli.py index a0c9c4b..c826253 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -4,6 +4,7 @@ """ Tests for j2lint.cli.py """ + import logging import os import re @@ -110,9 +111,7 @@ def test_create_parser(default_namespace, argv, namespace_modifications): ), ], ) -def test_sort_issues( - make_issues, number_issues, issues_modifications, expected_sorted_issues_ids -): +def test_sort_issues(make_issues, number_issues, issues_modifications, expected_sorted_issues_ids): """ Test j2lint.cli.sort_issues @@ -125,7 +124,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 @@ -200,12 +198,9 @@ def test_print_string_output( """ 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, options.verbose) assert total_errors == expected_output[0] assert total_warnings == expected_output[1] @@ -251,7 +246,6 @@ def test_print_json_output( """ 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) @@ -376,9 +370,7 @@ def test_run( 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" - ): + 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), @@ -393,9 +385,7 @@ def test_run( 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 - ): + 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] @@ -411,9 +401,7 @@ def test_run_stdin(capsys): 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("os.unlink", side_effect=os.unlink) 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"]) diff --git a/tests/test_linter/test_collection.py b/tests/test_linter/test_collection.py index 7985d47..6b80ae5 100644 --- a/tests/test_linter/test_collection.py +++ b/tests/test_linter/test_collection.py @@ -4,6 +4,7 @@ """ Tests for j2lint.linter.collection.py """ + import logging import pathlib from unittest import mock @@ -107,13 +108,8 @@ def checks_side_effect(self, file_path, text): 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 - ] + 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,9 +117,7 @@ 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): """ @@ -132,20 +126,13 @@ def test__repr__(self, test_collection, test_other_rule): 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", [ - 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"), @@ -167,16 +154,11 @@ def test_create_from_directory(self, ignore_rules, warn_rules): JinjaOperatorHasSpacesRule(), JinjaTemplateSyntaxErrorRule(), ] - collection = RulesCollection.create_from_directory( - "dummy", ignore_rules, warn_rules - ) + collection = RulesCollection.create_from_directory("dummy", ignore_rules, warn_rules) mocked_load_plugins.assert_called_once_with("dummy") assert collection.rules == mocked_load_plugins.return_value 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 a247d69..a313ea8 100644 --- a/tests/test_linter/test_error.py +++ b/tests/test_linter/test_error.py @@ -4,19 +4,11 @@ """ Tests for j2lint.linter.error.py """ + 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"}' -) +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"}' @pytest.mark.parametrize( @@ -33,7 +25,6 @@ def test_to_rich(test_issue, verbose, expected): """ Test the Rich string formats for LinterError """ - assert str(test_issue.to_rich(verbose)) == expected @@ -41,5 +32,4 @@ def test_to_json(test_issue): """ Test the json format for LinterError """ - assert test_issue.to_json() == RULE_JSON_OUTPUT diff --git a/tests/test_linter/test_indenter/test_node.py b/tests/test_linter/test_indenter/test_node.py index 2b6d903..eeffa4b 100644 --- a/tests/test_linter/test_indenter/test_node.py +++ b/tests/test_linter/test_indenter/test_node.py @@ -4,6 +4,7 @@ """ Tests for j2lint.linter.node.py """ + import pytest from j2lint.linter.indenter.node import Node @@ -14,7 +15,6 @@ class TestNode: def test_create_node(self): """ """ # TODO - why is it not an __init__ method??? - pass def test_create_indentation_error(self): """ diff --git a/tests/test_linter/test_indenter/test_statement.py b/tests/test_linter/test_indenter/test_statement.py index f74cf4d..ac19d22 100644 --- a/tests/test_linter/test_indenter/test_statement.py +++ b/tests/test_linter/test_indenter/test_statement.py @@ -8,6 +8,7 @@ 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 """ + import pytest from j2lint.linter.indenter.statement import JinjaStatement diff --git a/tests/test_linter/test_rule.py b/tests/test_linter/test_rule.py index 295b847..d8f031e 100644 --- a/tests/test_linter/test_rule.py +++ b/tests/test_linter/test_rule.py @@ -4,6 +4,7 @@ """ Tests for j2lint.linter.rule.py """ + import logging import pathlib @@ -82,11 +83,7 @@ def return_empty_list(*args, **kwargs): 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 x, y: [issue for i in range(checktext) for issue in make_issue_from_rule(test_rule)] if checkline is None: test_rule.checkline = raise_NotImplementedError @@ -95,13 +92,9 @@ def return_empty_list(*args, **kwargs): 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 x, y, line_no=0: [issue for i in range(checkline) for issue in make_issue_from_rule(test_rule)] - with open(file_path["path"], "r", encoding="utf-8") as file_d: + with open(file_path["path"], 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] diff --git a/tests/test_linter/test_runner.py b/tests/test_linter/test_runner.py index ea41809..acb23ce 100644 --- a/tests/test_linter/test_runner.py +++ b/tests/test_linter/test_runner.py @@ -4,6 +4,7 @@ """ Tests for j2lint.linter.runner.py """ + from unittest import mock import pytest @@ -49,9 +50,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 3436a7e..a1981ad 100644 --- a/tests/test_logger.py +++ b/tests/test_logger.py @@ -4,6 +4,7 @@ """ Tests for j2lint.logger.py """ + import logging import sys from logging import handlers @@ -14,9 +15,7 @@ from j2lint.logger import add_handler -@pytest.mark.parametrize( - "stream_handler, log_level", [(False, logging.DEBUG), (True, logging.ERROR)] -) +@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 diff --git a/tests/test_rules/test_rules.py b/tests/test_rules/test_rules.py index 396a1f8..251b016 100644 --- a/tests/test_rules/test_rules.py +++ b/tests/test_rules/test_rules.py @@ -141,9 +141,7 @@ "filename, j2_errors_ids, j2_warnings_ids, expected_log", PARAMS, ) -def test_rules( - caplog, collection, filename, j2_errors_ids, j2_warnings_ids, expected_log -): +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 @@ -152,8 +150,7 @@ def test_rules( j2_warnings_ids: the ids of the expected warnings (, ) expected_log: a list of expected log tuples as defined per caplog.record_tuples """ - - with open(filename, "r") as f: + with open(filename) as f: print(f.read()) caplog.set_level(logging.INFO) errors, warnings = collection.run(filename) diff --git a/tests/test_utils.py b/tests/test_utils.py index 75def18..c60fc6f 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -4,6 +4,7 @@ """ Tests for j2lint.utils.py """ + import pathlib import pytest @@ -93,9 +94,7 @@ 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): @@ -204,9 +203,7 @@ def test_get_jinja_variables(): @pytest.mark.parametrize( "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 #}"], diff --git a/tests/utils.py b/tests/utils.py index e3cd15f..693a344 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -4,6 +4,7 @@ """ utils.py - functions to assist with tests """ + from contextlib import contextmanager From cf56f9dde8b91ad173527325a6708b913a6836d1 Mon Sep 17 00:00:00 2001 From: gmuloc Date: Mon, 23 Sep 2024 23:35:43 +0200 Subject: [PATCH 02/14] ci: Ruff fix number 2 --- j2lint/__main__.py | 3 +- j2lint/cli.py | 137 ++++++++--- j2lint/linter/__init__.py | 1 + j2lint/linter/collection.py | 104 +++++--- j2lint/linter/indenter/__init__.py | 1 + j2lint/linter/indenter/node.py | 125 ++++++---- j2lint/linter/indenter/statement.py | 8 +- j2lint/linter/rule.py | 88 +++++-- j2lint/linter/runner.py | 43 ++-- j2lint/logger.py | 15 +- .../rules/jinja_operator_has_spaces_rule.py | 47 ++-- .../rules/jinja_statement_delimiter_rule.py | 24 +- .../rules/jinja_statement_has_spaces_rule.py | 26 +- .../rules/jinja_template_indentation_rule.py | 28 ++- j2lint/rules/jinja_template_no_tabs_rule.py | 21 +- .../jinja_template_single_statement_rule.py | 26 +- .../rules/jinja_template_syntax_error_rule.py | 22 +- j2lint/rules/jinja_variable_has_space_rule.py | 28 ++- j2lint/rules/jinja_variable_name_case_rule.py | 21 +- .../rules/jinja_variable_name_format_rule.py | 25 +- j2lint/utils.py | 231 +++++++++++------- pyproject.toml | 2 +- tests/test_cli.py | 4 +- tests/utils.py | 4 +- 24 files changed, 673 insertions(+), 361 deletions(-) diff --git a/j2lint/__main__.py b/j2lint/__main__.py index 07c6753..379cddb 100644 --- a/j2lint/__main__.py +++ b/j2lint/__main__.py @@ -1,4 +1,3 @@ -#!/usr/bin/python # Copyright (c) 2021-2024 Arista Networks, Inc. # Use of this source code is governed by the MIT license # that can be found in the LICENSE file. @@ -12,6 +11,6 @@ if __name__ == "__main__": try: sys.exit(run()) - except Exception: + except Exception: # noqa: BLE001 print(traceback.format_exc()) raise SystemExit from BaseException diff --git a/j2lint/cli.py b/j2lint/cli.py index ffaae9c..880bb48 100644 --- a/j2lint/cli.py +++ b/j2lint/cli.py @@ -8,20 +8,23 @@ 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 +if TYPE_CHECKING: + from .linter.error import LinterError + IGNORE_RULES = WARN_RULES = [ "jinja-syntax-error", "single-space-decorator", @@ -49,11 +52,13 @@ def create_parser() -> argparse.ArgumentParser: - """Initializes a new argument parser object + """ + Initialize a new argument parser object. Returns ------- - ArgumentParser: Argument parser object + ArgumentParser + Argument parser object """ parser = argparse.ArgumentParser(prog=NAME, description=DESCRIPTION) @@ -120,21 +125,41 @@ 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 + list + List of sorted issue dictionaries """ 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""" + """ + 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[str, list[LinterError]], dict[str, list[LinterError]]] + A two tuple containing two dictionaries. The first dictionary contains the errors and the second dictionary the warnings. + """ lint_errors: dict[str, list[LinterError]] = {} lint_warnings: dict[str, list[LinterError]] = {} @@ -155,12 +180,26 @@ def print_json_output( lint_errors: dict[str, list[LinterError]], lint_warnings: dict[str, 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)}") @@ -171,9 +210,26 @@ def print_json_output( def print_string_output( lint_errors: dict[str, list[LinterError]], lint_warnings: dict[str, 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 when no error nor warning was passed. + + Returns + ------- + tuple[int, int] + A two 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: CONSOLE.rule(f"[bold red]JINJA2 LINT {issue_type}") @@ -203,34 +259,59 @@ def print_issues(lint_issues: dict[str, list[LinterError]], issue_type: str) -> 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 + 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() @@ -244,20 +325,20 @@ 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)) + file_or_dir_names: list[Path] = list(set(options.files)) checked_files: list[str] = [] if options.stdin and not sys.stdin.isatty(): 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 @@ -291,7 +372,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 8d81cb3..5934387 100644 --- a/j2lint/linter/__init__.py +++ b/j2lint/linter/__init__.py @@ -1,3 +1,4 @@ # Copyright (c) 2021-2024 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 23102de..bd1b3cc 100644 --- a/j2lint/linter/collection.py +++ b/j2lint/linter/collection.py @@ -6,9 +6,8 @@ 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 @@ -16,52 +15,76 @@ 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 Iterable -DEFAULT_RULE_DIR = pathlib.Path(__file__).parent.parent / "rules" + from .error import LinterError + from .rule import Rule + +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]: + """ + 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 + 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, encoding="utf-8") as file: + with file_path.open(encoding="utf-8") as file: text = file.read() except OSError as err: logger.warning("Could not open %s - %s", file_path, err.strerror) @@ -96,6 +119,13 @@ 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)): @@ -108,15 +138,18 @@ def __repr__(self) -> str: 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 @@ -131,25 +164,36 @@ def to_rich(self) -> Group: return Group(*res) def to_json(self) -> str: - """Return a json representation of the collection as a list of the rules""" + """ + 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 + def create_from_directory(cls, rules_dir: Path, ignore_rules: list[str], warn_rules: list[str]) -> RulesCollection: + """ + Create 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 + 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 ------- - list: a collection of rule objects + 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 diff --git a/j2lint/linter/indenter/__init__.py b/j2lint/linter/indenter/__init__.py index 8d81cb3..6f7a477 100644 --- a/j2lint/linter/indenter/__init__.py +++ b/j2lint/linter/indenter/__init__.py @@ -1,3 +1,4 @@ # Copyright (c) 2021-2024 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 06bdfda..c1c31c8 100644 --- a/j2lint/linter/indenter/node.py +++ b/j2lint/linter/indenter/node.py @@ -1,9 +1,7 @@ # Copyright (c) 2021-2024 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 @@ -30,7 +28,7 @@ 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 @@ -44,19 +42,24 @@ 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 + """ + Initialize 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. + 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: new Node class object + Node + A new Node class object. """ node = Node() statement = JinjaStatement(line) @@ -70,15 +73,20 @@ def create_node(self, line: Statement, line_no: int, indent_level: int = 0) -> N @staticmethod def create_indentation_error(node: Node, message: str) -> NodeIndentationError | None: - """Creates indentation error tuple + """ + Create indentation error tuple. - Args: - node (Node): Node class object to create error for - message (string): error message for the line + Parameters + ---------- + node + Node object to create error for. + message + Error message for the line. Returns ------- - tuple: tuple representing the indentation error + NodeIndentationError | None + A tuple representing the indentation error. """ if node.statement is None: return None @@ -94,11 +102,15 @@ def create_indentation_error(node: Node, message: str) -> NodeIndentationError | ) 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 + """ + Check if the actual and expected indent level for a line match. + + 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 @@ -122,18 +134,34 @@ def check_indent_level(self, result: list[NodeIndentationError], node: Node) -> 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 = ("Recursive check_indentation returned None for an opening tag " f"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], @@ -141,32 +169,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. + """ + 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: 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 + 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 ------- - line_no (int) or None + int | None + line_no or None """ 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) @@ -206,7 +237,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) @@ -231,7 +262,7 @@ def _handle_end_tag(node: Node, line_no: int) -> int: if begin_tag_tuple is None: _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 14999eb..f2ee438 100644 --- a/j2lint/linter/indenter/statement.py +++ b/j2lint/linter/indenter/statement.py @@ -7,8 +7,10 @@ # 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 abf6435..e52ecd0 100644 --- a/j2lint/linter/rule.py +++ b/j2lint/linter/rule.py @@ -1,9 +1,7 @@ # Copyright (c) 2021-2024 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 @@ -17,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] @@ -31,12 +27,13 @@ def __init__( 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: + """Override the way a subclass of Rule is instantiated.""" super().__init_subclass__(**kwargs) # Mandatory class attributes mandatory_attributes = [ @@ -47,18 +44,26 @@ 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 """ @@ -69,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, @@ -82,24 +87,61 @@ 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. + + This method is expected to be overrididen by child classes. + + 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. + """ @abstractmethod def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """This method is expected to be overriden by child classes""" + """ + 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. + """ def checkrule(self, filename: str, text: str) -> list[LinterError]: """ - Checks the string text against the current rule by calling - either the checkline or checktext method depending on which one is implemented + Check 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 + Parameters + ---------- + filename + The filename to which the line belongs. + text + Entire text content of the file Returns ------- - list: list of LinterError from issues in the given file + list[LinterError] + The list of LinterError generated by this rule. """ errors: list[LinterError] = [] @@ -111,14 +153,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 a31e85b..8fffcbc 100644 --- a/j2lint/linter/runner.py +++ b/j2lint/linter/runner.py @@ -5,14 +5,18 @@ 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 .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 @@ -25,34 +29,35 @@ def __init__(self, collection: RulesCollection, file_name: str, checked_files: l self.checked_files = checked_files def is_already_checked(self, file_path: str) -> bool: - """Returns true if the file is already checked once + """ + 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 + 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 + """ + 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 + 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] = [] 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 +68,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 58bc420..50b73d5 100644 --- a/j2lint/logger.py +++ b/j2lint/logger.py @@ -13,8 +13,19 @@ logger = logging.getLogger("") -def add_handler(log: logging.Logger, stream_handler: bool, log_level: int) -> None: - """Defined logging handlers""" +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: diff --git a/j2lint/rules/jinja_operator_has_spaces_rule.py b/j2lint/rules/jinja_operator_has_spaces_rule.py index f1f0375..cdb56e4 100644 --- a/j2lint/rules/jinja_operator_has_spaces_rule.py +++ b/j2lint/rules/jinja_operator_has_spaces_rule.py @@ -1,14 +1,12 @@ # Copyright (c) 2021-2024 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 @@ -18,25 +16,25 @@ 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)) @@ -45,24 +43,29 @@ def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: super().__init__() 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 + 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: diff --git a/j2lint/rules/jinja_statement_delimiter_rule.py b/j2lint/rules/jinja_statement_delimiter_rule.py index 6e10860..131fecd 100644 --- a/j2lint/rules/jinja_statement_delimiter_rule.py +++ b/j2lint/rules/jinja_statement_delimiter_rule.py @@ -1,9 +1,7 @@ # Copyright (c) 2021-2024 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 @@ -26,19 +24,27 @@ def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: super().__init__() 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 + """ + Check if the given line matches the wrong delimiters. - 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 + 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] == "-%}"] diff --git a/j2lint/rules/jinja_statement_has_spaces_rule.py b/j2lint/rules/jinja_statement_has_spaces_rule.py index acf9a66..5a0c90f 100644 --- a/j2lint/rules/jinja_statement_has_spaces_rule.py +++ b/j2lint/rules/jinja_statement_has_spaces_rule.py @@ -1,10 +1,7 @@ # Copyright (c) 2021-2024 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 @@ -16,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 '%}'" @@ -31,17 +26,26 @@ def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: super().__init__() 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 + 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 00dad4a..fa80d06 100644 --- a/j2lint/rules/jinja_template_indentation_rule.py +++ b/j2lint/rules/jinja_template_indentation_rule.py @@ -1,9 +1,7 @@ # Copyright (c) 2021-2024 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 @@ -21,25 +19,29 @@ 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__() 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 + 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 @@ -49,8 +51,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", @@ -61,4 +62,5 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: 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 2dbdf24..eed0a91 100644 --- a/j2lint/rules/jinja_template_no_tabs_rule.py +++ b/j2lint/rules/jinja_template_no_tabs_rule.py @@ -1,9 +1,7 @@ # Copyright (c) 2021-2024 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 @@ -28,17 +26,26 @@ def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: super().__init__() 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 + 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 90f6817..43a930d 100644 --- a/j2lint/rules/jinja_template_single_statement_rule.py +++ b/j2lint/rules/jinja_template_single_statement_rule.py @@ -1,10 +1,7 @@ # Copyright (c) 2021-2024 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 @@ -16,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" @@ -29,16 +24,25 @@ def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: super().__init__() 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 + 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 [] diff --git a/j2lint/rules/jinja_template_syntax_error_rule.py b/j2lint/rules/jinja_template_syntax_error_rule.py index 43eb4c7..c7c433b 100644 --- a/j2lint/rules/jinja_template_syntax_error_rule.py +++ b/j2lint/rules/jinja_template_syntax_error_rule.py @@ -1,9 +1,7 @@ # Copyright (c) 2021-2024 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 @@ -27,19 +25,24 @@ def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: super().__init__() def checktext(self, filename: str, text: str) -> list[LinterError]: - """Checks if the given text has jinja syntax error + """ + Check if the given text has jinja syntax 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 + 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 +59,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 68f188f..25c32a1 100644 --- a/j2lint/rules/jinja_variable_has_space_rule.py +++ b/j2lint/rules/jinja_variable_has_space_rule.py @@ -1,10 +1,7 @@ # Copyright (c) 2021-2024 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 @@ -16,12 +13,10 @@ 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" @@ -32,17 +27,26 @@ def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: super().__init__() 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 + 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 275991c..751ae3d 100644 --- a/j2lint/rules/jinja_variable_name_case_rule.py +++ b/j2lint/rules/jinja_variable_name_case_rule.py @@ -1,9 +1,7 @@ # Copyright (c) 2021-2024 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 @@ -31,19 +29,26 @@ def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: super().__init__() 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 + Check 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 + 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 + list[LinterError] + The list of LinterError generated by this rule. """ variables = get_jinja_variables(line) matches = [] diff --git a/j2lint/rules/jinja_variable_name_format_rule.py b/j2lint/rules/jinja_variable_name_format_rule.py index 1358205..5437093 100644 --- a/j2lint/rules/jinja_variable_name_format_rule.py +++ b/j2lint/rules/jinja_variable_name_format_rule.py @@ -1,14 +1,12 @@ # Copyright (c) 2021-2024 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 @@ -21,7 +19,7 @@ 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" @@ -31,19 +29,26 @@ def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: super().__init__() 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 + Check if the given line matches the error regex, which matches variables using `-` in their name. - 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 + list[LinterError] + The list of LinterError generated by this rule. """ variables = get_jinja_variables(line) matches = [] diff --git a/j2lint/utils.py b/j2lint/utils.py index 480e9a4..69497db 100644 --- a/j2lint/utils.py +++ b/j2lint/utils.py @@ -5,11 +5,11 @@ from __future__ import annotations -import glob import importlib.util import os import re from collections.abc import Generator, Iterable +from pathlib import Path from typing import TYPE_CHECKING, Any, Tuple from j2lint.logger import logger @@ -18,25 +18,29 @@ 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 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 + """ + Load and execute all the Rule modules from the specified directory. - Args: - directory (string): Loads the modules a directory + Parameters + ---------- + directory : string + Loads the modules a directory Returns ------- - list: List of rule classes + 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", "")) + directory_path = Path(directory) + for plugin_file in path.glob(directory_path / "[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) @@ -55,62 +59,80 @@ 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 + 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[str], 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 + 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 = Path(file_or_dir) + if file_or_dir_path.is_dir(): + for root, _, files in os.walk(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 ------- + 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) @@ -119,59 +141,75 @@ def flatten(nested_list: Iterable[Any]) -> Generator[Any, Any, Any]: 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 + """ + 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 + 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 + # TODO - should probably return a JinjaStatement object.. - Found jinja statements [(' if switch.platform_settings.tcam_profile - is arista.avd.defined ', 2, 2, '{%', '%}'), (' endif ', 5, 5, '{%', '%}')] + 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. - With indentation=False + Examples + -------- + For this given template: - 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 [] + ``` + {# 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 - - # TODO - should probably return a JinjaStatement object.. + list + List of jinja statements """ statements: list[Statement] = [] count = 0 @@ -199,27 +237,39 @@ 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 + """ + Add start and end delimiters for a jinja statement. - Args: - line (string): text line + Parameters + ---------- + line + Text line + start + Start delimiter + end + End delimiter Returns ------- - [string]: jinja statement with jinja start and end delimiters + 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 + list + List of jinja comments """ regex_pattern = re.compile("(\\{#)((.|\n)*?)(\\#})", re.MULTILINE) @@ -227,29 +277,38 @@ 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 + 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 + """ + Check if rule is disabled. - Args: - text (string): text to check - rule (Rule): Rule object + Parameters + ---------- + text + Text to check + rule + Rule object Returns ------- - [boolean]: True if rule is disabled + 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 0a52651..dd7c09d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -74,7 +74,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 = """ diff --git a/tests/test_cli.py b/tests/test_cli.py index c826253..39c7dae 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -65,7 +65,7 @@ def test_create_parser(default_namespace, argv, namespace_modifications): """ 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. @@ -351,7 +351,7 @@ def test_run( """ Test the j2lint.cli.run method - This test is a bit too complex and should probably be splitted out to test various + 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 diff --git a/tests/utils.py b/tests/utils.py index 693a344..0fb9eec 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -99,7 +99,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" @@ -112,7 +112,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" From 7de372a592261f21bbba947256d58860d0269400 Mon Sep 17 00:00:00 2001 From: gmuloc Date: Mon, 23 Sep 2024 23:38:10 +0200 Subject: [PATCH 03/14] ci: Start type checking --- j2lint/rules/jinja_variable_has_space_rule.py | 2 +- j2lint/utils.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/j2lint/rules/jinja_variable_has_space_rule.py b/j2lint/rules/jinja_variable_has_space_rule.py index 25c32a1..8ce1c71 100644 --- a/j2lint/rules/jinja_variable_has_space_rule.py +++ b/j2lint/rules/jinja_variable_has_space_rule.py @@ -6,7 +6,7 @@ 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 diff --git a/j2lint/utils.py b/j2lint/utils.py index 69497db..aa918c9 100644 --- a/j2lint/utils.py +++ b/j2lint/utils.py @@ -39,7 +39,7 @@ def load_plugins(directory: str) -> list[Rule]: result = [] file_handle = None directory_path = Path(directory) - for plugin_file in path.glob(directory_path / "[A-Za-z_]*.py"): + for plugin_file in directory_path.glob(pattern="[A-Za-z_]*.py"): plugin_name = plugin_file.name.replace(".py", "") try: logger.debug("Loading plugin %s", plugin_name) From 6328c26e443ba25dc0b24a83812e588cc9d51986 Mon Sep 17 00:00:00 2001 From: gmuloc Date: Mon, 23 Sep 2024 23:40:09 +0200 Subject: [PATCH 04/14] ci: Add pylint config to pyproject.toml --- pyproject.toml | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index dd7c09d..1a6b0fe 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -73,9 +73,6 @@ push = false "j2lint/__init__.py" = ["{version}"] "tests/test_cli.py" = ["{version}"] -[tool.pylint.'MESSAGES CONTROL'] -max-line-length = 165 - [tool.tox] legacy_tox_ini = """ [tox] @@ -337,3 +334,28 @@ runtime-evaluated-base-classes = ["pydantic.BaseModel", "anta.models.AntaTest.In "S105", # Possible hardcoded password "INP001", # Implicit packages ] + +################################ +# 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-positional-arguments", # New in pylint 3.3.0 + "wrong-import-position", + "pointless-statement", + "broad-exception-caught", + "line-too-long", + "unused-variable", + "redefined-builtin", + "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 From 0a31ada502c585e82a4531cb6bdde0f67cb706c7 Mon Sep 17 00:00:00 2001 From: gmuloc Date: Mon, 23 Sep 2024 23:44:23 +0200 Subject: [PATCH 05/14] WIP: tests --- tests/test_utils.py | 17 ++++++----------- tests/utils.py | 18 ++++++------------ 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index c60fc6f..c925d69 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,9 +1,7 @@ # Copyright (c) 2021-2024 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 -""" +"""Tests for j2lint.utils.py""" import pathlib @@ -24,17 +22,16 @@ @pytest.mark.skip -def test_load_plugins(): +def test_load_plugins() -> None: """ - Test the utils.load_plugins function + 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), @@ -46,10 +43,8 @@ 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 - """ +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(file_name, extensions) == expected diff --git a/tests/utils.py b/tests/utils.py index 0fb9eec..74c6672 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,26 +1,20 @@ # Copyright (c) 2021-2024 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 collections.abc import Generator from contextlib import contextmanager @contextmanager -def does_not_raise(): - """ - Provides a context manager that does not raise anything - for pytest tests - """ +def does_not_raise() -> Generator: + """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: + """The description of the default rules.""" return ( "─────────────────────────── Rules in the Collection ────────────────────────────\n" "Origin: BUILT-IN\n" From 87af8ed2162306d567f759373867919ab0052283 Mon Sep 17 00:00:00 2001 From: gmuloc Date: Mon, 30 Sep 2024 22:34:30 +0200 Subject: [PATCH 06/14] WIP: more ruff fixing --- .github/workflows/pull-request-management.yml | 2 +- j2lint/cli.py | 51 +++-- j2lint/linter/collection.py | 36 ++-- j2lint/linter/indenter/node.py | 15 +- j2lint/linter/rule.py | 15 +- j2lint/linter/runner.py | 19 +- j2lint/logger.py | 3 +- .../rules/jinja_operator_has_spaces_rule.py | 3 +- .../rules/jinja_statement_delimiter_rule.py | 3 +- .../rules/jinja_statement_has_spaces_rule.py | 3 +- .../rules/jinja_template_indentation_rule.py | 3 +- j2lint/rules/jinja_template_no_tabs_rule.py | 3 +- .../jinja_template_single_statement_rule.py | 3 +- .../rules/jinja_template_syntax_error_rule.py | 3 +- j2lint/rules/jinja_variable_has_space_rule.py | 3 +- j2lint/rules/jinja_variable_name_case_rule.py | 3 +- .../rules/jinja_variable_name_format_rule.py | 3 +- j2lint/utils.py | 30 +-- pyproject.toml | 121 ++++-------- tests/__init__.py | 1 + tests/conftest.py | 178 +++++++++--------- tests/test_cli.py | 167 +++++++--------- tests/test_linter/__init__.py | 3 + tests/test_linter/test_collection.py | 26 +-- tests/test_linter/test_error.py | 12 +- tests/test_linter/test_indenter/test_node.py | 8 +- .../test_indenter/test_statement.py | 3 +- tests/test_linter/test_rule.py | 68 +++---- tests/test_linter/test_runner.py | 35 ++-- tests/test_logger.py | 27 ++- tests/test_rules/__init__.py | 3 + tests/test_rules/test_rules.py | 66 ++++--- tests/test_utils.py | 83 ++++---- 33 files changed, 439 insertions(+), 563 deletions(-) create mode 100644 tests/test_linter/__init__.py create mode 100644 tests/test_rules/__init__.py 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/j2lint/cli.py b/j2lint/cli.py index 880bb48..15392b5 100644 --- a/j2lint/cli.py +++ b/j2lint/cli.py @@ -52,8 +52,7 @@ def create_parser() -> argparse.ArgumentParser: - """ - Initialize a new argument parser object. + """Initialize a new argument parser object. Returns ------- @@ -125,8 +124,7 @@ def create_parser() -> argparse.ArgumentParser: def sort_issues(issues: list[LinterError]) -> list[LinterError]: - """ - Sorted list of issues. + """Sorted list of issues. Parameters ---------- @@ -142,9 +140,10 @@ def sort_issues(issues: list[LinterError]) -> list[LinterError]: return issues -def get_linting_issues(files: list[str], collection: RulesCollection, checked_files: list[str]) -> tuple[dict[str, list[LinterError]], dict[str, list[LinterError]]]: - """ - Check errors and warnings. +def get_linting_issues( + files: list[Path], collection: RulesCollection, checked_files: list[Path] +) -> tuple[dict[Path, list[LinterError]], dict[Path, list[LinterError]]]: + """Check errors and warnings. Parameters ---------- @@ -157,11 +156,11 @@ def get_linting_issues(files: list[str], collection: RulesCollection, checked_fi Returns ------- - tuple[dict[str, list[LinterError]], dict[str, list[LinterError]]] + 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[str, list[LinterError]] = {} - lint_warnings: dict[str, list[LinterError]] = {} + lint_errors: dict[Path, list[LinterError]] = {} + lint_warnings: dict[Path, list[LinterError]] = {} # Get linting issues for file_name in files: @@ -177,11 +176,10 @@ def get_linting_issues(files: list[str], collection: RulesCollection, checked_fi 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]: - """ - Print json output. + """Print json output. Parameters ---------- @@ -208,13 +206,12 @@ 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 string output. + """Print string output. Parameters ---------- @@ -231,7 +228,7 @@ def print_string_output( A two 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: @@ -260,8 +257,7 @@ def print_issues(lint_issues: dict[str, list[LinterError]], issue_type: str) -> def remove_temporary_file(stdin_filename: Path) -> None: - """ - Remove temporary file. + """Remove temporary file. Parameters ---------- @@ -273,8 +269,7 @@ def remove_temporary_file(stdin_filename: Path) -> None: def print_string_rules(collection: RulesCollection) -> None: - """ - Print active rules as string. + """Print active rules as string. Parameters ---------- @@ -286,8 +281,7 @@ def print_string_rules(collection: RulesCollection) -> None: def print_json_rules(collection: RulesCollection) -> None: - """ - Print active rules as json. + """Print active rules as json. Parameters ---------- @@ -298,8 +292,7 @@ def print_json_rules(collection: RulesCollection) -> None: def run(args: list[str] | None = None) -> int: - """ - Run jinja2 linter. + """Run jinja2 linter. Parameters ---------- @@ -333,7 +326,7 @@ def run(args: list[str] | None = None) -> int: stdin_filename = None file_or_dir_names: list[Path] = list(set(options.files)) - checked_files: list[str] = [] + checked_files: list[Path] = [] if options.stdin and not sys.stdin.isatty(): with tempfile.NamedTemporaryFile("w", suffix=".j2", delete=False) as stdin_tmpfile: @@ -342,7 +335,7 @@ def run(args: list[str] | None = None) -> int: 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) diff --git a/j2lint/linter/collection.py b/j2lint/linter/collection.py index bd1b3cc..ca50b49 100644 --- a/j2lint/linter/collection.py +++ b/j2lint/linter/collection.py @@ -16,7 +16,7 @@ from j2lint.utils import is_rule_disabled, load_plugins if TYPE_CHECKING: - from collections.abc import Iterable + from collections.abc import Iterator from .error import LinterError from .rule import Rule @@ -31,9 +31,8 @@ def __init__(self, *, verbose: bool = False) -> None: self.rules: list[Rule] = [] self.verbose = verbose - def __iter__(self) -> Iterable[Rule]: - """ - Return iterable of Rules in the collection. + def __iter__(self) -> Iterator[Rule]: + """Return iterable of Rules in the collection. Returns ------- @@ -42,8 +41,7 @@ def __iter__(self) -> Iterable[Rule]: return iter(self.rules) def __len__(self) -> int: - """ - Return the number of rules in the collection. + """Return the number of rules in the collection. Returns ------- @@ -53,8 +51,7 @@ def __len__(self) -> int: return len(self.rules) def extend(self, more: list[Rule]) -> None: - """ - Extend list of rules. + """Extend list of rules. TODO: This does not protect against duplicate rules @@ -66,8 +63,7 @@ def extend(self, more: list[Rule]) -> None: self.rules.extend(more) def run(self, file_path: Path) -> tuple[list[LinterError], list[LinterError]]: - """ - Run the linting rules for given file. + """Run the linting rules for given file. Parameters ---------- @@ -105,10 +101,10 @@ def run(self, file_path: Path) -> 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()) @@ -119,8 +115,7 @@ def run(self, file_path: Path) -> tuple[list[LinterError], list[LinterError]]: return errors, warnings def __repr__(self) -> str: - """ - Return a representation of a RulesCollection object. + """Return a representation of a RulesCollection object. Returns ------- @@ -137,8 +132,7 @@ 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 @@ -164,8 +158,7 @@ def to_rich(self) -> Group: return Group(*res) def to_json(self) -> str: - """ - Return a json representation of the collection as a list of the rules. + """Return a json representation of the collection as a list of the rules. Returns ------- @@ -175,8 +168,7 @@ def to_json(self) -> str: @classmethod def create_from_directory(cls, rules_dir: Path, ignore_rules: list[str], warn_rules: list[str]) -> RulesCollection: - """ - Create a collection from all rule modules. + """Create a collection from all rule modules. Parameters ---------- @@ -193,7 +185,7 @@ def create_from_directory(cls, rules_dir: Path, ignore_rules: list[str], warn_ru A RulesColleciton object containing the rules from rules_dir except for the ignored ones. """ result = cls() - result.rules = load_plugins(rules_dir.expanduser()) + result.rules = load_plugins(str(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 @@ -201,6 +193,6 @@ def create_from_directory(cls, rules_dir: Path, ignore_rules: list[str], warn_ru 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/indenter/node.py b/j2lint/linter/indenter/node.py index c1c31c8..2c35f3f 100644 --- a/j2lint/linter/indenter/node.py +++ b/j2lint/linter/indenter/node.py @@ -44,8 +44,7 @@ def __init__(self) -> None: # TODO: This should be called create_child_node def create_node(self, line: Statement, line_no: int, indent_level: int = 0) -> Node: - """ - Initialize a Node class object. + """Initialize a Node class object. Parameters ---------- @@ -73,8 +72,7 @@ def create_node(self, line: Statement, line_no: int, indent_level: int = 0) -> N @staticmethod def create_indentation_error(node: Node, message: str) -> NodeIndentationError | None: - """ - Create indentation error tuple. + """Create indentation error tuple. Parameters ---------- @@ -102,8 +100,7 @@ def create_indentation_error(node: Node, message: str) -> NodeIndentationError | ) def check_indent_level(self, result: list[NodeIndentationError], node: Node) -> None: - """ - Check if the actual and expected indent level for a line match. + """Check if the actual and expected indent level for a line match. Parameters ---------- @@ -133,8 +130,7 @@ def check_indent_level(self, result: list[NodeIndentationError], node: Node) -> logger.debug(error) def _assert_not_none(self, current_line_no: int, new_line_no: int | None) -> int: - """ - Verify that the new_line_no is not None. + """Verify that the new_line_no is not None. Parameters ---------- @@ -169,8 +165,7 @@ def check_indentation( line_no: int = 0, indent_level: int = 0, ) -> int | None: - """ - Check indentation for a list of lines and update the 'result' list argument with indentation errors. + """Check indentation for a list of lines and update the 'result' list argument with indentation errors. Parameters ---------- diff --git a/j2lint/linter/rule.py b/j2lint/linter/rule.py index e52ecd0..c8e0bb2 100644 --- a/j2lint/linter/rule.py +++ b/j2lint/linter/rule.py @@ -24,9 +24,10 @@ class Rule(ABC): def __init__( self, - ignore: bool = False, warn: list[Any] | None = None, origin: str = "BUILT-IN", + *, + ignore: bool = False, ) -> None: self.ignore = ignore self.warn = warn if warn is not None else [] @@ -56,8 +57,7 @@ def __repr__(self) -> str: return f"{self.rule_id}: {self.description}" def to_rich(self) -> Text: - """ - Return a rich representation of the rule. + """Return a rich representation of the rule. Examples -------- @@ -87,8 +87,7 @@ def to_json(self) -> str: @abstractmethod def checktext(self, filename: str, text: str) -> list[LinterError]: - """ - Check the rule against a full file content. + """Check the rule against a full file content. This method is expected to be overrididen by child classes. @@ -107,8 +106,7 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: @abstractmethod def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """ - Check the rule against a full file content. + """Check the rule against a full file content. This method is expected to be overrididen by child classes. @@ -128,8 +126,7 @@ def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError] """ 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. + """Check the string text against the current rule by calling either the checkline or checktext method depending on which one is implemented. Parameters ---------- diff --git a/j2lint/linter/runner.py b/j2lint/linter/runner.py index 8fffcbc..1802889 100644 --- a/j2lint/linter/runner.py +++ b/j2lint/linter/runner.py @@ -10,27 +10,27 @@ from j2lint.logger import logger 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: - """ - Return 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. Parameters ---------- @@ -45,8 +45,7 @@ def is_already_checked(self, file_path: str) -> bool: return file_path in self.checked_files def run(self) -> tuple[list[LinterError], list[LinterError]]: - """ - Run the lint rules collection on all the files. + """Run the lint rules collection on all the files. Returns ------- @@ -55,7 +54,7 @@ def run(self) -> tuple[list[LinterError], list[LinterError]]: 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): # 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 diff --git a/j2lint/logger.py b/j2lint/logger.py index 50b73d5..9ffde3e 100644 --- a/j2lint/logger.py +++ b/j2lint/logger.py @@ -14,8 +14,7 @@ def add_handler(log: logging.Logger, log_level: int, *, stream_handler: bool) -> None: - """ - Define logging handlers. + """Define logging handlers. Parameters ---------- diff --git a/j2lint/rules/jinja_operator_has_spaces_rule.py b/j2lint/rules/jinja_operator_has_spaces_rule.py index cdb56e4..31ef8fa 100644 --- a/j2lint/rules/jinja_operator_has_spaces_rule.py +++ b/j2lint/rules/jinja_operator_has_spaces_rule.py @@ -47,8 +47,7 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """ - Check if the given line matches the error regex. + """Check if the given line matches the error regex. Parameters ---------- diff --git a/j2lint/rules/jinja_statement_delimiter_rule.py b/j2lint/rules/jinja_statement_delimiter_rule.py index 131fecd..b608217 100644 --- a/j2lint/rules/jinja_statement_delimiter_rule.py +++ b/j2lint/rules/jinja_statement_delimiter_rule.py @@ -28,8 +28,7 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """ - Check if the given line matches the wrong delimiters. + """Check if the given line matches the wrong delimiters. Parameters ---------- diff --git a/j2lint/rules/jinja_statement_has_spaces_rule.py b/j2lint/rules/jinja_statement_has_spaces_rule.py index 5a0c90f..c269503 100644 --- a/j2lint/rules/jinja_statement_has_spaces_rule.py +++ b/j2lint/rules/jinja_statement_has_spaces_rule.py @@ -30,8 +30,7 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """ - Check if the given line matches the error regex. + """Check if the given line matches the error regex. Parameters ---------- diff --git a/j2lint/rules/jinja_template_indentation_rule.py b/j2lint/rules/jinja_template_indentation_rule.py index fa80d06..df0fa15 100644 --- a/j2lint/rules/jinja_template_indentation_rule.py +++ b/j2lint/rules/jinja_template_indentation_rule.py @@ -26,8 +26,7 @@ def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: super().__init__() def checktext(self, filename: str, text: str) -> list[LinterError]: - """ - Check if the given text has the error. + """Check if the given text has the error. Parameters ---------- diff --git a/j2lint/rules/jinja_template_no_tabs_rule.py b/j2lint/rules/jinja_template_no_tabs_rule.py index eed0a91..5bca07d 100644 --- a/j2lint/rules/jinja_template_no_tabs_rule.py +++ b/j2lint/rules/jinja_template_no_tabs_rule.py @@ -30,8 +30,7 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """ - Check if the given line matches the error regex. + """Check if the given line matches the error regex. Parameters ---------- diff --git a/j2lint/rules/jinja_template_single_statement_rule.py b/j2lint/rules/jinja_template_single_statement_rule.py index 43a930d..ffea6cd 100644 --- a/j2lint/rules/jinja_template_single_statement_rule.py +++ b/j2lint/rules/jinja_template_single_statement_rule.py @@ -28,8 +28,7 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """ - Check if the given line matches the error regex. + """Check if the given line matches the error regex. Parameters ---------- diff --git a/j2lint/rules/jinja_template_syntax_error_rule.py b/j2lint/rules/jinja_template_syntax_error_rule.py index c7c433b..f5c1ca4 100644 --- a/j2lint/rules/jinja_template_syntax_error_rule.py +++ b/j2lint/rules/jinja_template_syntax_error_rule.py @@ -25,8 +25,7 @@ def __init__(self, ignore: bool = False, warn: list[Any] | None = None) -> None: super().__init__() def checktext(self, filename: str, text: str) -> list[LinterError]: - """ - Check if the given text has jinja syntax error. + """Check if the given text has jinja syntax error. Parameters ---------- diff --git a/j2lint/rules/jinja_variable_has_space_rule.py b/j2lint/rules/jinja_variable_has_space_rule.py index 8ce1c71..6eb1311 100644 --- a/j2lint/rules/jinja_variable_has_space_rule.py +++ b/j2lint/rules/jinja_variable_has_space_rule.py @@ -31,8 +31,7 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """ - Check if the given line matches the error regex. + """Check if the given line matches the error regex. Parameters ---------- diff --git a/j2lint/rules/jinja_variable_name_case_rule.py b/j2lint/rules/jinja_variable_name_case_rule.py index 751ae3d..698c7b8 100644 --- a/j2lint/rules/jinja_variable_name_case_rule.py +++ b/j2lint/rules/jinja_variable_name_case_rule.py @@ -33,8 +33,7 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """ - Check if the given line matches the error regex, which matches variables with non lower case characters. + """Check if the given line matches the error regex, which matches variables with non lower case characters. Parameters ---------- diff --git a/j2lint/rules/jinja_variable_name_format_rule.py b/j2lint/rules/jinja_variable_name_format_rule.py index 5437093..c588f5a 100644 --- a/j2lint/rules/jinja_variable_name_format_rule.py +++ b/j2lint/rules/jinja_variable_name_format_rule.py @@ -33,8 +33,7 @@ def checktext(self, filename: str, text: str) -> list[LinterError]: raise NotImplementedError def checkline(self, filename: str, line: str, line_no: int) -> list[LinterError]: - """ - Check if the given line matches the error regex, which matches variables using `-` in their name. + """Check if the given line matches the error regex, which matches variables using `-` in their name. Parameters ---------- diff --git a/j2lint/utils.py b/j2lint/utils.py index aa918c9..9bf51c3 100644 --- a/j2lint/utils.py +++ b/j2lint/utils.py @@ -23,8 +23,7 @@ def load_plugins(directory: str) -> list[Rule]: - """ - Load and execute all the Rule modules from the specified directory. + """Load and execute all the Rule modules from the specified directory. Parameters ---------- @@ -60,8 +59,7 @@ def load_plugins(directory: str) -> list[Rule]: def is_valid_file_type(filename: Path, extensions: list[str]) -> bool: - """ - Check if the file extension is in the list of accepted extensions. + """Check if the file extension is in the list of accepted extensions. Parameters ---------- @@ -80,8 +78,7 @@ def is_valid_file_type(filename: Path, extensions: list[str]) -> bool: def get_files(file_or_dir_names: list[str], extensions: list[str]) -> list[Path]: - """ - Get files from a directory recursively. + """Get files from a directory recursively. Parameters ---------- @@ -117,8 +114,7 @@ def get_files(file_or_dir_names: list[str], extensions: list[str]) -> list[Path] def flatten(nested_list: Iterable[Any]) -> Generator[Any, Any, Any]: - """ - Flatten an iterable. + """Flatten an iterable. Parameters ---------- @@ -141,8 +137,7 @@ def flatten(nested_list: Iterable[Any]) -> Generator[Any, Any, Any]: def get_tuple(list_of_tuples: list[tuple[Any, ...]], item: Any) -> tuple[Any, ...] | None: - """ - Check if an item is present in any of the tuples. + """Check if an item is present in any of the tuples. Parameters ---------- @@ -160,8 +155,7 @@ def get_tuple(list_of_tuples: list[tuple[Any, ...]], item: Any) -> tuple[Any, .. def get_jinja_statements(text: str, *, indentation: bool = False) -> list[Statement]: - """ - Get jinja statements with {%[-/+] [-]%} delimiters. + """Get jinja statements with {%[-/+] [-]%} delimiters. The regex `regex_pattern` will return multiple groups when it matches Note that this is a multiline regex @@ -237,8 +231,7 @@ def get_jinja_statements(text: str, *, indentation: bool = False) -> list[Statem def delimit_jinja_statement(line: str, start: str = "{%", end: str = "%}") -> str: - """ - Add start and end delimiters for a jinja statement. + """Add start and end delimiters for a jinja statement. Parameters ---------- @@ -258,8 +251,7 @@ def delimit_jinja_statement(line: str, start: str = "{%", end: str = "%}") -> st def get_jinja_comments(text: str) -> list[str]: - """ - Get jinja comments. + """Get jinja comments. Parameters ---------- @@ -277,8 +269,7 @@ def get_jinja_comments(text: str) -> list[str]: def get_jinja_variables(text: str) -> list[str]: - """ - Get jinja variables. + """Get jinja variables. Parameters ---------- @@ -295,8 +286,7 @@ def get_jinja_variables(text: str) -> list[str]: def is_rule_disabled(text: str, rule: Rule) -> bool: - """ - Check if rule is disabled. + """Check if rule is disabled. Parameters ---------- diff --git a/pyproject.toml b/pyproject.toml index 1a6b0fe..0ee082d 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.9.0", ] -requires-python = ">=3.8" +requires-python = ">=3.9" [project.optional-dependencies] dev = [ @@ -73,6 +72,9 @@ push = false "j2lint/__init__.py" = ["{version}"] "tests/test_cli.py" = ["{version}"] +[tool.pylint.'MESSAGES CONTROL'] +max-line-length = 165 + [tool.tox] legacy_tox_ini = """ [tox] @@ -91,7 +93,6 @@ isolated_build = True [gh-actions] python = - 3.8: py38 3.9: py39 3.10: py310 3.11: py311, coverage, report @@ -221,7 +222,7 @@ exclude = [ line-length = 165 -# Assume Python 3.9 as this is the lowest supported version for ANTA +# Assume Python 3.9 as this is the lowest supported version for j2lint target-version = "py39" [tool.ruff.lint] @@ -230,10 +231,9 @@ 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 - # TODO: Augment the numpy convention rules to make sure we add all the params - # Uncomment below D417 "D415", "D417", + "D212", ] ignore = [ "COM812", # Ignoring conflicting rules that may cause conflicts when used with the formatter @@ -266,73 +266,25 @@ max-complexity = 10 [tool.ruff.lint.pep8-naming] "ignore-names" = [ - "RICH_COLOR_PALETTE" +# "RICH_COLOR_PALETTE" ] -[tool.ruff.lint.flake8-type-checking] -# These classes require that type annotations be available at runtime -runtime-evaluated-base-classes = ["pydantic.BaseModel", "anta.models.AntaTest.Input"] - - [tool.ruff.lint.per-file-ignores] "tests/*" = [ "S101", # Complains about asserts in units and libs. "SLF001", # Lots of private member accessed for test purposes -] -"tests/units/*" = [ - "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 "PLR0913", # Too many arguments to function call "PLR2004", # Magic value used in comparison, consider replacing {value} with a constant variable - "S105", # Passwords are indeed hardcoded in tests - "S106", # Passwords are indeed hardcoded in tests - "S108", # Probable insecure usage of temporary file or directory -] -"tests/units/anta_tests/test_interfaces.py" = [ - "S104", # False positive for 0.0.0.0 bindings in test inputs -] -"tests/units/anta_tests/*" = [ - "F401", # In this module, we import tests.units.anta_tests.test without using it to auto-generate tests -] -"anta/*" = [ - "TRY400", # Use `logging.exception` instead of `logging.error` - we know what we are doing -] -"anta/cli/exec/utils.py" = [ - "SLF001", # TODO: some private members, lets try to fix -] -"anta/cli/__init__.py" = [ - "T201", # Allow print statements -] -"anta/cli/*" = [ - "PLR0913", # Allow more than 5 input arguments in CLI functions - "ANN401", # TODO: Check if we can update the Any type hints in the CLI -] -"anta/tests/field_notices.py" = [ - "PLR2004", # Magic value used in comparison, consider replacing 2131 with a constant variable - Field notice IDs are magic values - "C901", # TODO: test function is too complex, needs a refactor - "PLR0911", # TODO: Too many return statements, same as above needs a refactor -] -"anta/tests/routing/isis.py" = [ - "C901", # TODO: test function is too complex, needs a refactor - "PLR0912" # Too many branches (15/12) (too-many-branches), needs a refactor -] -"anta/decorators.py" = [ - "ANN401", # Ok to use Any type hint in our decorators -] -"anta/tools.py" = [ - "ANN401", # Ok to use Any type hint in our custom get functions - "PLR0913", # Ok to have more than 5 arguments in our custom get functions -] -"anta/device.py" = [ - "PLR0913", # Ok to have more than 5 arguments in the AntaDevice classes -] -"anta/inventory/__init__.py" = [ - "PLR0913", # Ok to have more than 5 arguments in the AntaInventory class +# "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 ] -"examples/anta_runner.py" = [ # This is an example script and linked in snippets - "S108", # Probable insecure usage of temporary file or directory - "S105", # Possible hardcoded password - "INP001", # Implicit packages +"tests/conftest.py" = [ + "ARG002", # Sometimes we need to declare unused arguments when a parameter is not used when mocking ] ################################ @@ -340,22 +292,29 @@ runtime-evaluated-base-classes = ["pydantic.BaseModel", "anta.models.AntaTest.In ################################ [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-positional-arguments", # New in pylint 3.3.0 - "wrong-import-position", - "pointless-statement", - "broad-exception-caught", - "line-too-long", - "unused-variable", - "redefined-builtin", - "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 + "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 8d81cb3..b64099f 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,3 +1,4 @@ # Copyright (c) 2021-2024 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 aa3f16c..18cbb1e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,14 +1,16 @@ # Copyright (c) 2021-2024 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 @@ -24,56 +26,60 @@ # 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): + 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 = [] @@ -90,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 + ``` """ 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 + ``` """ 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 @@ -135,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. """ 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]) return collection @pytest.fixture -def test_runner(test_collection): - """ - Fixture to get a test runner using the test_collection - """ +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 - """ +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 - """ +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 @@ -245,10 +239,8 @@ def template_tmp_dir(tmp_path_factory): @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=[], @@ -266,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 39c7dae..4f48f8b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,14 +1,15 @@ # Copyright (c) 2021-2024 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.""" import logging import os import re from argparse import Namespace +from collections.abc import Generator +from pathlib import Path +from typing import Any from unittest.mock import patch import pytest @@ -42,7 +43,7 @@ @pytest.mark.parametrize( - "argv, namespace_modifications", + ("argv", "namespace_modifications"), [ (pytest.param([], {"extensions": [".j2", ".jinja", ".jinja2"]}, id="default")), pytest.param( @@ -61,9 +62,8 @@ ), ], ) -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 dictionary where key is one of the keys in the namespace and value is the value @@ -81,7 +81,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,9 +111,10 @@ 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 +def test_sort_issues( + make_issues: pytest.Fixture, 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: @@ -150,7 +151,7 @@ def test_sort_issues(make_issues, number_issues, issues_modifications, expected_ @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), @@ -187,20 +188,18 @@ def test_sort_issues(make_issues, number_issues, issues_modifications, expected_ ], ) 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.Fixture, + make_issues: pytest.Fixture, + 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] @@ -210,42 +209,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) @@ -254,12 +233,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"), @@ -336,20 +314,19 @@ 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 + 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 @@ -361,37 +338,35 @@ 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.Fixture) -> 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: @@ -415,5 +390,5 @@ def test_run_stdin(capsys): ) assert matches is not None mocked_os_unlink.assert_called_with(matches.groups()[0]) - assert os.path.exists(matches.groups()[0]) is False + assert Path(matches.groups()[0]).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..8d81cb3 --- /dev/null +++ b/tests/test_linter/__init__.py @@ -0,0 +1,3 @@ +# Copyright (c) 2021-2024 Arista Networks, Inc. +# Use of this source code is governed by the MIT license +# that can be found in the LICENSE file. diff --git a/tests/test_linter/test_collection.py b/tests/test_linter/test_collection.py index 6b80ae5..dd5d9f4 100644 --- a/tests/test_linter/test_collection.py +++ b/tests/test_linter/test_collection.py @@ -1,9 +1,7 @@ # Copyright (c) 2021-2024 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""" import logging import pathlib @@ -21,15 +19,11 @@ class TestRulesCollection: def test__len__(self, test_collection): - """ - Test the RuleCollection __len__ method - """ + """Test the RuleCollection __len__ method""" assert len(test_collection) == 1 def test__iter__(self): - """ - Test the RuleCollection __iter__ method - """ + """Test the RuleCollection __iter__ method""" fake_rules = [1, 2, 3] collection = RulesCollection() assert len(collection) == 0 @@ -38,9 +32,7 @@ def test__iter__(self): assert rule == fake_rules[index] def test_extend(self): - """ - Test the RuleCollection.extend method - """ + """Test the RuleCollection.extend method""" fake_rules = [1, 2, 3] collection = RulesCollection() assert len(collection) == 0 @@ -85,8 +77,7 @@ def test_run( expected_results, verify_logs, ): - """ - Generate a collection with 5 rules with: + """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 @@ -120,9 +111,7 @@ def checks_side_effect(self, file_path, text): 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 - """ + """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]) @@ -139,8 +128,7 @@ def test__repr__(self, test_collection, test_other_rule): ], ) def test_create_from_directory(self, ignore_rules, warn_rules): - """ - Test the RuleCollection.create_from_directory class method + """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 diff --git a/tests/test_linter/test_error.py b/tests/test_linter/test_error.py index a313ea8..495e302 100644 --- a/tests/test_linter/test_error.py +++ b/tests/test_linter/test_error.py @@ -1,9 +1,7 @@ # Copyright (c) 2021-2024 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""" import pytest @@ -22,14 +20,10 @@ ], ) def test_to_rich(test_issue, verbose, expected): - """ - Test the Rich string formats for LinterError - """ + """Test the Rich string formats for LinterError""" assert str(test_issue.to_rich(verbose)) == expected def test_to_json(test_issue): - """ - Test the json format for LinterError - """ + """Test the json format for LinterError""" assert test_issue.to_json() == RULE_JSON_OUTPUT diff --git a/tests/test_linter/test_indenter/test_node.py b/tests/test_linter/test_indenter/test_node.py index eeffa4b..f5d87d0 100644 --- a/tests/test_linter/test_indenter/test_node.py +++ b/tests/test_linter/test_indenter/test_node.py @@ -1,9 +1,7 @@ # Copyright (c) 2021-2024 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""" import pytest @@ -17,9 +15,7 @@ def test_create_node(self): # TODO - why is it not an __init__ method??? def test_create_indentation_error(self): - """ - Test the Node.create_indentation_error method - """ + """Test the Node.create_indentation_error method""" line = ( " if switch.platform_settings.tcam_profile is arista.avd.defined ", 2, diff --git a/tests/test_linter/test_indenter/test_statement.py b/tests/test_linter/test_indenter/test_statement.py index ac19d22..ef45a3e 100644 --- a/tests/test_linter/test_indenter/test_statement.py +++ b/tests/test_linter/test_indenter/test_statement.py @@ -1,8 +1,7 @@ # Copyright (c) 2021-2024 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 diff --git a/tests/test_linter/test_rule.py b/tests/test_linter/test_rule.py index d8f031e..94afa90 100644 --- a/tests/test_linter/test_rule.py +++ b/tests/test_linter/test_rule.py @@ -1,31 +1,32 @@ # Copyright (c) 2021-2024 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, @@ -55,48 +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"], 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 acb23ce..8341274 100644 --- a/tests/test_linter/test_runner.py +++ b/tests/test_linter/test_runner.py @@ -1,35 +1,41 @@ # Copyright (c) 2021-2024 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", [ @@ -39,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. diff --git a/tests/test_logger.py b/tests/test_logger.py index a1981ad..4b0ac54 100644 --- a/tests/test_logger.py +++ b/tests/test_logger.py @@ -1,37 +1,34 @@ # Copyright (c) 2021-2024 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..8d81cb3 --- /dev/null +++ b/tests/test_rules/__init__.py @@ -0,0 +1,3 @@ +# Copyright (c) 2021-2024 Arista Networks, Inc. +# Use of this source code is governed by the MIT license +# that can be found in the LICENSE file. diff --git a/tests/test_rules/test_rules.py b/tests/test_rules/test_rules.py index 251b016..b607731 100644 --- a/tests/test_rules/test_rules.py +++ b/tests/test_rules/test_rules.py @@ -1,12 +1,20 @@ # Copyright (c) 2021-2024 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,20 +147,36 @@ @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 +def test_rules( + 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. + + 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 """ - with open(filename) as f: - print(f.read()) + # with Path(filename).open() as f: + # print(f.read()) caplog.set_level(logging.INFO) errors, warnings = collection.run(filename) @@ -159,7 +184,6 @@ def test_rules(caplog, collection, filename, j2_errors_ids, j2_warnings_ids, exp 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 c925d69..6a7f42a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,9 +1,12 @@ # Copyright (c) 2021-2024 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""" +"""Tests for j2lint.utils.py.""" + +from __future__ import annotations import pathlib +from typing import TYPE_CHECKING, Any import pytest @@ -18,13 +21,13 @@ from .utils import does_not_raise -# pylint: disable=fixme +if TYPE_CHECKING: + from collections.abc import Generator @pytest.mark.skip def test_load_plugins() -> None: - """ - Test the utils.load_plugins function. + """Test the utils.load_plugins function. For now this is being tested via other calling methods """ @@ -43,13 +46,13 @@ def test_load_plugins() -> None: ("test.j2", [".toto"], False), ], ) -def test_is_valid_file_type(file_name: str, extensions: list[str], expected: bool) -> None: +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(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()), ( @@ -92,17 +95,14 @@ def test_is_valid_file_type(file_name: str, extensions: list[str], expected: boo 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 -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 @@ -120,7 +120,7 @@ def test_get_files_dir(template_tmp_dir): @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()), @@ -128,16 +128,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)], @@ -149,54 +147,44 @@ 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=T0 #}"], True, id="found_id"), @@ -223,14 +211,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 From 3feb943043932d1d678daefa20f663b6af7471f9 Mon Sep 17 00:00:00 2001 From: gmuloc Date: Mon, 30 Sep 2024 23:37:46 +0200 Subject: [PATCH 07/14] WIP: All ruff --- j2lint/__init__.py | 4 ++ j2lint/__main__.py | 4 +- j2lint/cli.py | 15 ++-- j2lint/linter/collection.py | 5 +- j2lint/linter/error.py | 8 +-- j2lint/linter/indenter/node.py | 8 +-- j2lint/linter/rule.py | 6 +- j2lint/rules/__init__.py | 1 + .../rules/jinja_operator_has_spaces_rule.py | 2 +- .../rules/jinja_statement_delimiter_rule.py | 2 +- .../rules/jinja_statement_has_spaces_rule.py | 2 +- .../rules/jinja_template_indentation_rule.py | 2 +- j2lint/rules/jinja_template_no_tabs_rule.py | 2 +- .../jinja_template_single_statement_rule.py | 2 +- .../rules/jinja_template_syntax_error_rule.py | 2 +- j2lint/rules/jinja_variable_has_space_rule.py | 2 +- j2lint/rules/jinja_variable_name_case_rule.py | 2 +- .../rules/jinja_variable_name_format_rule.py | 4 +- j2lint/utils.py | 7 +- pyproject.toml | 4 ++ tests/test_linter/__init__.py | 1 + tests/test_linter/test_collection.py | 68 +++++++++++-------- tests/test_linter/test_error.py | 23 ++++--- tests/test_linter/test_indenter/__init__.py | 4 ++ tests/test_linter/test_indenter/test_node.py | 18 ++--- .../test_indenter/test_statement.py | 20 ++++-- tests/test_linter/test_rule.py | 2 +- tests/test_rules/__init__.py | 1 + tests/utils.py | 11 ++- 29 files changed, 138 insertions(+), 94 deletions(-) create mode 100644 tests/test_linter/test_indenter/__init__.py diff --git a/j2lint/__init__.py b/j2lint/__init__.py index c63a3fa..9f33b49 100644 --- a/j2lint/__init__.py +++ b/j2lint/__init__.py @@ -3,6 +3,8 @@ # that can be found in the LICENSE file. """__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 379cddb..89eec3d 100644 --- a/j2lint/__main__.py +++ b/j2lint/__main__.py @@ -4,13 +4,13 @@ """__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: # noqa: BLE001 - print(traceback.format_exc()) + CONSOLE.print_exception(show_locals=True) raise SystemExit from BaseException diff --git a/j2lint/cli.py b/j2lint/cli.py index 15392b5..eeabf64 100644 --- a/j2lint/cli.py +++ b/j2lint/cli.py @@ -13,14 +13,13 @@ 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.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 @@ -48,8 +47,6 @@ "V2", ] -CONSOLE = Console() - def create_parser() -> argparse.ArgumentParser: """Initialize a new argument parser object. @@ -236,7 +233,7 @@ def print_issues(lint_issues: dict[Path, list[LinterError]], issue_type: str) -> 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()) diff --git a/j2lint/linter/collection.py b/j2lint/linter/collection.py index ca50b49..16d49ad 100644 --- a/j2lint/linter/collection.py +++ b/j2lint/linter/collection.py @@ -12,6 +12,7 @@ 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 @@ -153,7 +154,9 @@ 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) diff --git a/j2lint/linter/error.py b/j2lint/linter/error.py index e790bc0..1a838dc 100644 --- a/j2lint/linter/error.py +++ b/j2lint/linter/error.py @@ -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/node.py b/j2lint/linter/indenter/node.py index 2c35f3f..ed079e8 100644 --- a/j2lint/linter/indenter/node.py +++ b/j2lint/linter/indenter/node.py @@ -5,7 +5,7 @@ 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 @@ -23,8 +23,7 @@ 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: @@ -152,7 +151,7 @@ def _assert_not_none(self, current_line_no: int, new_line_no: int | None) -> int TODO: Probably should never return None and instead raise in check_indentation """ if new_line_no is None: - msg = ("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 @@ -190,6 +189,7 @@ def check_indentation( int | None line_no or None """ + # ruff: noqa: PLR0915,C901 def _append_error_to_result_and_raise(message: str) -> NoReturn: """Append error to result and raise a JinjaLinterError.""" diff --git a/j2lint/linter/rule.py b/j2lint/linter/rule.py index c8e0bb2..c5c9d05 100644 --- a/j2lint/linter/rule.py +++ b/j2lint/linter/rule.py @@ -24,16 +24,16 @@ class Rule(ABC): def __init__( self, - warn: list[Any] | None = None, - origin: str = "BUILT-IN", *, 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 diff --git a/j2lint/rules/__init__.py b/j2lint/rules/__init__.py index 8d81cb3..57941e2 100644 --- a/j2lint/rules/__init__.py +++ b/j2lint/rules/__init__.py @@ -1,3 +1,4 @@ # Copyright (c) 2021-2024 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 31ef8fa..1595e75 100644 --- a/j2lint/rules/jinja_operator_has_spaces_rule.py +++ b/j2lint/rules/jinja_operator_has_spaces_rule.py @@ -40,7 +40,7 @@ class JinjaOperatorHasSpacesRule(Rule): 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.""" diff --git a/j2lint/rules/jinja_statement_delimiter_rule.py b/j2lint/rules/jinja_statement_delimiter_rule.py index b608217..cbe88a9 100644 --- a/j2lint/rules/jinja_statement_delimiter_rule.py +++ b/j2lint/rules/jinja_statement_delimiter_rule.py @@ -21,7 +21,7 @@ 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.""" diff --git a/j2lint/rules/jinja_statement_has_spaces_rule.py b/j2lint/rules/jinja_statement_has_spaces_rule.py index c269503..7bd37b2 100644 --- a/j2lint/rules/jinja_statement_has_spaces_rule.py +++ b/j2lint/rules/jinja_statement_has_spaces_rule.py @@ -23,7 +23,7 @@ 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.""" diff --git a/j2lint/rules/jinja_template_indentation_rule.py b/j2lint/rules/jinja_template_indentation_rule.py index df0fa15..5e33831 100644 --- a/j2lint/rules/jinja_template_indentation_rule.py +++ b/j2lint/rules/jinja_template_indentation_rule.py @@ -23,7 +23,7 @@ class JinjaTemplateIndentationRule(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]: """Check if the given text has the error. diff --git a/j2lint/rules/jinja_template_no_tabs_rule.py b/j2lint/rules/jinja_template_no_tabs_rule.py index 5bca07d..09963c2 100644 --- a/j2lint/rules/jinja_template_no_tabs_rule.py +++ b/j2lint/rules/jinja_template_no_tabs_rule.py @@ -23,7 +23,7 @@ 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.""" diff --git a/j2lint/rules/jinja_template_single_statement_rule.py b/j2lint/rules/jinja_template_single_statement_rule.py index ffea6cd..49d1adc 100644 --- a/j2lint/rules/jinja_template_single_statement_rule.py +++ b/j2lint/rules/jinja_template_single_statement_rule.py @@ -21,7 +21,7 @@ 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.""" diff --git a/j2lint/rules/jinja_template_syntax_error_rule.py b/j2lint/rules/jinja_template_syntax_error_rule.py index f5c1ca4..5aa3924 100644 --- a/j2lint/rules/jinja_template_syntax_error_rule.py +++ b/j2lint/rules/jinja_template_syntax_error_rule.py @@ -22,7 +22,7 @@ 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]: """Check if the given text has jinja syntax error. diff --git a/j2lint/rules/jinja_variable_has_space_rule.py b/j2lint/rules/jinja_variable_has_space_rule.py index 6eb1311..25a4cdc 100644 --- a/j2lint/rules/jinja_variable_has_space_rule.py +++ b/j2lint/rules/jinja_variable_has_space_rule.py @@ -24,7 +24,7 @@ class JinjaVariableHasSpaceRule(Rule): 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.""" diff --git a/j2lint/rules/jinja_variable_name_case_rule.py b/j2lint/rules/jinja_variable_name_case_rule.py index 698c7b8..7ba464d 100644 --- a/j2lint/rules/jinja_variable_name_case_rule.py +++ b/j2lint/rules/jinja_variable_name_case_rule.py @@ -26,7 +26,7 @@ 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.""" diff --git a/j2lint/rules/jinja_variable_name_format_rule.py b/j2lint/rules/jinja_variable_name_format_rule.py index c588f5a..0113062 100644 --- a/j2lint/rules/jinja_variable_name_format_rule.py +++ b/j2lint/rules/jinja_variable_name_format_rule.py @@ -12,8 +12,6 @@ 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.""" @@ -26,7 +24,7 @@ class JinjaVariableNameFormatRule(Rule): 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.""" diff --git a/j2lint/utils.py b/j2lint/utils.py index 9bf51c3..5b096a1 100644 --- a/j2lint/utils.py +++ b/j2lint/utils.py @@ -10,16 +10,15 @@ import re from collections.abc import Generator, Iterable from pathlib import Path -from typing import TYPE_CHECKING, Any, Tuple +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 = tuple[str, int, int, str, str] def load_plugins(directory: str) -> list[Rule]: @@ -136,7 +135,7 @@ 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: +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. Parameters diff --git a/pyproject.toml b/pyproject.toml index 0ee082d..ca3fdaf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -286,6 +286,10 @@ max-complexity = 10 "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 diff --git a/tests/test_linter/__init__.py b/tests/test_linter/__init__.py index 8d81cb3..cec695c 100644 --- a/tests/test_linter/__init__.py +++ b/tests/test_linter/__init__.py @@ -1,3 +1,4 @@ # Copyright (c) 2021-2024 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 dd5d9f4..3b13031 100644 --- a/tests/test_linter/test_collection.py +++ b/tests/test_linter/test_collection.py @@ -1,10 +1,13 @@ # Copyright (c) 2021-2024 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 @@ -14,16 +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 @@ -31,8 +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 @@ -43,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", @@ -69,18 +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]]], + *, + 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) @@ -90,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( @@ -110,15 +122,15 @@ def checks_side_effect(self, file_path, text): # True for the test file 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" @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"), @@ -127,8 +139,8 @@ def test__repr__(self, test_collection, test_other_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 diff --git a/tests/test_linter/test_error.py b/tests/test_linter/test_error.py index 495e302..47399c2 100644 --- a/tests/test_linter/test_error.py +++ b/tests/test_linter/test_error.py @@ -1,16 +1,23 @@ # Copyright (c) 2021-2024 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)"), ( @@ -19,11 +26,11 @@ ), ], ) -def test_to_rich(test_issue, verbose, expected): - """Test the Rich string formats for LinterError""" +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)) == 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..2f77c48 --- /dev/null +++ b/tests/test_linter/test_indenter/__init__.py @@ -0,0 +1,4 @@ +# Copyright (c) 2021-2024 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 f5d87d0..faa8878 100644 --- a/tests/test_linter/test_indenter/test_node.py +++ b/tests/test_linter/test_indenter/test_node.py @@ -1,7 +1,9 @@ # Copyright (c) 2021-2024 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 @@ -9,13 +11,15 @@ 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??? + def test_create_node(self) -> None: + """N/A.""" + # TODO: why is it not an __init__ method??? - def test_create_indentation_error(self): - """Test the Node.create_indentation_error 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, @@ -25,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 ef45a3e..7478fc8 100644 --- a/tests/test_linter/test_indenter/test_statement.py +++ b/tests/test_linter/test_indenter/test_statement.py @@ -1,21 +1,27 @@ # Copyright (c) 2021-2024 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 """ +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 +49,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 94afa90..a4b5f1f 100644 --- a/tests/test_linter/test_rule.py +++ b/tests/test_linter/test_rule.py @@ -95,7 +95,7 @@ def return_empty_list(*args: Any, **kwargs: Any) -> list[Any]: # noqa: ARG001 test_rule.checkline = return_empty_list else: # checkline > 0 - test_rule.checkline = lambda *_, line_no=0: [issue for i in range(checkline) for issue in make_issue_from_rule(test_rule)] # noqa: ARG005 + 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 Path(file_path["path"]).open(encoding="utf-8") as file_d: errors = test_rule.checkrule(file_path, file_d.read()) diff --git a/tests/test_rules/__init__.py b/tests/test_rules/__init__.py index 8d81cb3..70a2818 100644 --- a/tests/test_rules/__init__.py +++ b/tests/test_rules/__init__.py @@ -1,3 +1,4 @@ # Copyright (c) 2021-2024 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/utils.py b/tests/utils.py index 74c6672..b47a2f3 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -3,18 +3,23 @@ # that can be found in the LICENSE file. """utils.py - functions to assist with tests.""" -from collections.abc import Generator +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() -> Generator: +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() -> str: - """The description of the default rules.""" + """Return the description of the default rules.""" return ( "─────────────────────────── Rules in the Collection ────────────────────────────\n" "Origin: BUILT-IN\n" From a6e6fd3ed7dc86fbf08cafaecf3443ec2f61ffab Mon Sep 17 00:00:00 2001 From: gmuloc Date: Mon, 30 Sep 2024 23:44:04 +0200 Subject: [PATCH 08/14] WIP: More ruff, some pytest fixes --- tests/test_cli.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 4f48f8b..720ebb5 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -3,20 +3,21 @@ # that can be found in the LICENSE file. """Tests for j2lint.cli.py.""" +from __future__ import annotations + import logging import os import re from argparse import Namespace -from collections.abc import Generator from pathlib import Path -from typing import Any +from typing import TYPE_CHECKING, Any, Callable from unittest.mock import patch import pytest from rich.console import ConsoleDimensions +from j2lint import CONSOLE from j2lint.cli import ( - CONSOLE, create_parser, print_json_output, print_string_output, @@ -36,7 +37,11 @@ j2lint_default_rules_string, ) -# pylint: disable=fixme, too-many-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) @@ -112,7 +117,10 @@ def test_create_parser(default_namespace: Namespace, argv: list[str], namespace_ ], ) def test_sort_issues( - make_issues: pytest.Fixture, number_issues: int, issues_modifications: dict[int, dict[str, Any]], expected_sorted_issues_ids: list[tuple[Any]] + 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. @@ -151,7 +159,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), @@ -188,8 +196,8 @@ def test_sort_issues( ], ) def test_print_string_output( - capsys: pytest.Fixture, - make_issues: pytest.Fixture, + capsys: pytest.LogCaptureFixture, + make_issues: Callable[[int], list[LinterError]], options: Namespace, number_errors: int, number_warnings: int, From 25f943a519ca80157b7289a46d0c90d70b893b09 Mon Sep 17 00:00:00 2001 From: gmuloc Date: Tue, 1 Oct 2024 00:28:44 +0200 Subject: [PATCH 09/14] ci: Finish ruffing it --- j2lint/cli.py | 2 +- j2lint/utils.py | 6 +++--- tests/test_cli.py | 17 ++++++++++++----- tests/test_linter/test_collection.py | 6 +++--- tests/test_linter/test_error.py | 2 +- tests/test_rules/test_rules.py | 4 +--- tests/test_utils.py | 12 +++++++----- 7 files changed, 28 insertions(+), 21 deletions(-) diff --git a/j2lint/cli.py b/j2lint/cli.py index eeabf64..2ed4331 100644 --- a/j2lint/cli.py +++ b/j2lint/cli.py @@ -322,7 +322,7 @@ def run(args: list[str] | None = None) -> int: logger.debug("Lint options selected %s", options) stdin_filename = None - file_or_dir_names: list[Path] = list(set(options.files)) + 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(): diff --git a/j2lint/utils.py b/j2lint/utils.py index 5b096a1..a1d5595 100644 --- a/j2lint/utils.py +++ b/j2lint/utils.py @@ -76,7 +76,7 @@ def is_valid_file_type(filename: Path, extensions: list[str]) -> bool: return extension in extensions -def get_files(file_or_dir_names: list[str], extensions: list[str]) -> list[Path]: +def get_files(file_or_dir_names: list[Path], extensions: list[str]) -> list[Path]: """Get files from a directory recursively. Parameters @@ -98,9 +98,9 @@ def get_files(file_or_dir_names: list[str], extensions: list[str]) -> list[Path] raise TypeError(msg) for file_or_dir in file_or_dir_names: - file_or_dir_path = Path(file_or_dir) + file_or_dir_path = file_or_dir if file_or_dir_path.is_dir(): - for root, _, files in os.walk(file_or_dir_path): + for root, _, files in os.walk(str(file_or_dir_path)): root_path = Path(root) for file in files: file_path = root_path / file diff --git a/tests/test_cli.py b/tests/test_cli.py index 720ebb5..f5a31d6 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -6,7 +6,6 @@ from __future__ import annotations import logging -import os import re from argparse import Namespace from pathlib import Path @@ -372,19 +371,26 @@ def test_run( assert "DEBUG" in [record.levelname for record in caplog.records] -def test_run_stdin(capsys: pytest.Fixture) -> None: +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"): + import j2lint + + 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"]) @@ -397,6 +403,7 @@ def test_run_stdin(capsys: pytest.Fixture) -> None: re.MULTILINE, ) assert matches is not None - mocked_os_unlink.assert_called_with(matches.groups()[0]) - assert Path(matches.groups()[0]).exists() 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/test_collection.py b/tests/test_linter/test_collection.py index 3b13031..91208c1 100644 --- a/tests/test_linter/test_collection.py +++ b/tests/test_linter/test_collection.py @@ -83,7 +83,7 @@ def test_run( make_rules: Callable[[int], list[Rule]], make_issue_from_rule: Callable[[int], LinterError], file_path: Path, - expected_results: tuple[list[tuple[str, str]]], + expected_results: tuple[list[tuple[str, str]], list[tuple[str, str]]], *, verify_logs: bool, ) -> None: @@ -110,7 +110,7 @@ def checks_side_effect(self, file_path: Path, text: str) -> None: # type: ignor side_effect=checks_side_effect, autospec=True, ): - errors, warnings = test_collection.run(file_path) + 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] @@ -154,7 +154,7 @@ def test_create_from_directory(self, ignore_rules: list[str], warn_rules: list[s JinjaOperatorHasSpacesRule(), JinjaTemplateSyntaxErrorRule(), ] - collection = RulesCollection.create_from_directory("dummy", ignore_rules, warn_rules) + collection = RulesCollection.create_from_directory(Path("dummy"), ignore_rules, warn_rules) mocked_load_plugins.assert_called_once_with("dummy") assert collection.rules == mocked_load_plugins.return_value for rule in collection.rules: diff --git a/tests/test_linter/test_error.py b/tests/test_linter/test_error.py index 47399c2..4cdeb7f 100644 --- a/tests/test_linter/test_error.py +++ b/tests/test_linter/test_error.py @@ -28,7 +28,7 @@ ) 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)) == expected + assert str(test_issue.to_rich(verbose=verbose)) == expected def test_to_json(test_issue: LinterError) -> None: diff --git a/tests/test_rules/test_rules.py b/tests/test_rules/test_rules.py index b607731..2951a1b 100644 --- a/tests/test_rules/test_rules.py +++ b/tests/test_rules/test_rules.py @@ -175,10 +175,8 @@ def test_rules( expected_log a list of expected log tuples as defined per caplog.record_tuples """ - # with Path(filename).open() as f: - # print(f.read()) 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] diff --git a/tests/test_utils.py b/tests/test_utils.py index 6a7f42a..f28da08 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -5,7 +5,7 @@ from __future__ import annotations -import pathlib +from pathlib import Path from typing import TYPE_CHECKING, Any import pytest @@ -48,7 +48,7 @@ def test_load_plugins() -> None: ) 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(file_name, extensions) == expected + assert is_valid_file_type(Path(file_name), extensions) == expected @pytest.mark.parametrize( @@ -98,7 +98,8 @@ def test_is_valid_file_type(file_name: str, extensions: list[str], *, expected: 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: str) -> None: @@ -107,7 +108,7 @@ def test_get_files_dir(template_tmp_dir: str) -> None: # 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", @@ -116,7 +117,8 @@ def test_get_files_dir(template_tmp_dir: str) -> None: ] ) 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( From eee7caf263b92b2c314be47c3d11410f5982c1a6 Mon Sep 17 00:00:00 2001 From: gmuloc Date: Tue, 1 Oct 2024 09:22:09 +0200 Subject: [PATCH 10/14] ci: Update pre-commit while we are at it --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 305d66d..6ccc2ef 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -40,7 +40,7 @@ repos: - --no-extra-eol - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.6.7 + rev: v0.6.8 hooks: - id: ruff name: Run Ruff linter @@ -49,7 +49,7 @@ repos: name: Run Ruff formatter - repo: https://github.com/pycqa/pylint - rev: "v3.3.0" + rev: "v3.3.1" hooks: - id: pylint name: Check for Linting error on Python files From 0813e6be0dc0251d94ec1d3ea2301458c6aa1d55 Mon Sep 17 00:00:00 2001 From: gmuloc Date: Tue, 1 Oct 2024 09:30:20 +0200 Subject: [PATCH 11/14] Doc: Update README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From d52842ae8d9c422c58a6a9644145f45c9530a14f Mon Sep 17 00:00:00 2001 From: gmuloc Date: Tue, 1 Oct 2024 11:44:30 +0200 Subject: [PATCH 12/14] ci: Fix tox --- pyproject.toml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index ca3fdaf..cb1addb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -111,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 From b3af0e06fb6ed21f703e45934eb7f20d7ef63026 Mon Sep 17 00:00:00 2001 From: gmuloc Date: Tue, 1 Oct 2024 12:07:29 +0200 Subject: [PATCH 13/14] ci: Fixes --- j2lint/linter/collection.py | 2 +- j2lint/utils.py | 7 +++---- tests/test_cli.py | 3 +-- tests/test_linter/test_collection.py | 7 +++++-- tests/test_linter/test_indenter/test_statement.py | 2 ++ 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/j2lint/linter/collection.py b/j2lint/linter/collection.py index 16d49ad..b8ddc07 100644 --- a/j2lint/linter/collection.py +++ b/j2lint/linter/collection.py @@ -188,7 +188,7 @@ def create_from_directory(cls, rules_dir: Path, ignore_rules: list[str], warn_ru A RulesColleciton object containing the rules from rules_dir except for the ignored ones. """ result = cls() - result.rules = load_plugins(str(rules_dir.expanduser())) + 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 diff --git a/j2lint/utils.py b/j2lint/utils.py index a1d5595..9f5783b 100644 --- a/j2lint/utils.py +++ b/j2lint/utils.py @@ -21,12 +21,12 @@ Statement = tuple[str, int, int, str, str] -def load_plugins(directory: str) -> list[Rule]: +def load_plugins(directory: Path) -> list[Rule]: """Load and execute all the Rule modules from the specified directory. Parameters ---------- - directory : string + directory : Path Loads the modules a directory Returns @@ -36,8 +36,7 @@ def load_plugins(directory: str) -> list[Rule]: """ result = [] file_handle = None - directory_path = Path(directory) - for plugin_file in directory_path.glob(pattern="[A-Za-z_]*.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) diff --git a/tests/test_cli.py b/tests/test_cli.py index f5a31d6..cb2f4fd 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -15,6 +15,7 @@ import pytest from rich.console import ConsoleDimensions +import j2lint from j2lint import CONSOLE from j2lint.cli import ( create_parser, @@ -384,8 +385,6 @@ def test_run_stdin(capsys: pytest.LogCaptureFixture) -> None: In this test, the isatty answer is mocked. """ - import j2lint - 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, diff --git a/tests/test_linter/test_collection.py b/tests/test_linter/test_collection.py index 91208c1..bdd31d3 100644 --- a/tests/test_linter/test_collection.py +++ b/tests/test_linter/test_collection.py @@ -148,15 +148,18 @@ def test_create_from_directory(self, ignore_rules: list[str], warn_rules: list[s 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(Path("dummy"), ignore_rules, warn_rules) - mocked_load_plugins.assert_called_once_with("dummy") + 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: assert rule.ignore is True diff --git a/tests/test_linter/test_indenter/test_statement.py b/tests/test_linter/test_indenter/test_statement.py index 7478fc8..052dde8 100644 --- a/tests/test_linter/test_indenter/test_statement.py +++ b/tests/test_linter/test_indenter/test_statement.py @@ -8,6 +8,8 @@ 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 From c6d70d2cfb01d6d816dedd1aeb8fae0fe180ac50 Mon Sep 17 00:00:00 2001 From: gmuloc Date: Mon, 21 Oct 2024 22:18:50 +0200 Subject: [PATCH 14/14] ci: Update ruff --- .pre-commit-config.yaml | 4 ++-- pyproject.toml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bcac7be..554616a 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: @@ -43,7 +43,7 @@ repos: - --no-extra-eol - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.6.8 + rev: v0.7.0 hooks: - id: ruff name: Run Ruff linter diff --git a/pyproject.toml b/pyproject.toml index 5e4b5a6..41dceed 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,11 +44,11 @@ test = [ ] lint = [ "pylint>=3.0.0", - "ruff>=0.5.4,<0.7.0", + "ruff>=0.7.0,<0.8.0", "codespell>=2.2.6,<2.4.0", ] type = [ - "mypy==1.12.0", + "mypy==1.12.1", ] [project.urls]