diff --git a/src/oca_pre_commit_hooks/base_checker.py b/src/oca_pre_commit_hooks/base_checker.py index 3932164..fd024ad 100644 --- a/src/oca_pre_commit_hooks/base_checker.py +++ b/src/oca_pre_commit_hooks/base_checker.py @@ -1,40 +1,52 @@ -from typing import Set, Union +from typing import List, NamedTuple, Set, Tuple, Union from colorama import Fore, Style -class CheckerError: - def __init__( - self, - code: str, - message: str, - filepath: str, - line: Union[int, None] = None, - column: Union[int, None] = None, - info: Union[str, None] = None, - ): - self.code = code - self.message = message - self.filepath = filepath - self.line = line - self.column = column - self.info = info +class FilePosition(NamedTuple): + filepath: str + line: int = 0 + column: int = 0 + + def to_string(self, separator: str = ":") -> str: + return separator.join(str(x) for x in iter(self) if x) + + def __str__(self): + return self.to_string() + + +class CheckerError(NamedTuple): + position: FilePosition + code: str + message: str + info: Union[str, None] = None + extra_positions: Union[List[FilePosition], None] = None def to_string(self): - res = Style.BRIGHT + Fore.RESET + self.filepath + Style.RESET_ALL - if self.line is not None: - res += Fore.CYAN + ":" + Style.RESET_ALL + str(self.line) - if self.column is not None: # pragma: no cover - res += Fore.CYAN + ":" + Style.RESET_ALL + str(self.column) - if not self.code and not self.message: # pragma: no cover - return res + # File position with a styled separator + res = Style.BRIGHT + Fore.RESET + res += self.position.to_string(separator=Fore.CYAN + ":" + Style.RESET_ALL) + res += Style.RESET_ALL + # Extra styled separator res += Fore.CYAN + ":" + Style.RESET_ALL - if self.code: - res += " " - res += Style.BRIGHT + Fore.RED + self.code + Style.RESET_ALL - if self.message: - res += " " - res += self.message + # Code + res += " " + res += Style.BRIGHT + Fore.RED + self.code + Style.RESET_ALL + # Message + res += " " + res += self.message + # Extra positions + if self.extra_positions: + res += "\n" + res += Fore.YELLOW + res += "\n".join(str(x) for x in iter(self.extra_positions)) + res += Style.RESET_ALL + # Optional info + if self.info: + res += "\n" + res += Style.DIM + res += self.info + res += Style.RESET_ALL return res def __str__(self): @@ -72,17 +84,17 @@ def register_error( code: str, message: str, filepath: str, - line: Union[int, None] = None, - column: Union[int, None] = None, + line: int = 0, + column: int = 0, info: Union[str, None] = None, + extra_positions: Union[List[Tuple[str, int, int]], None] = None, ): self.checks_errors.append( CheckerError( + position=FilePosition(filepath, line, column), code=code, message=message, - filepath=filepath, - line=line, - column=column, info=info, + extra_positions=[FilePosition(*x) for x in extra_positions] if extra_positions else None, ) ) diff --git a/src/oca_pre_commit_hooks/checks_odoo_module.py b/src/oca_pre_commit_hooks/checks_odoo_module.py index 9f95863..8de5346 100755 --- a/src/oca_pre_commit_hooks/checks_odoo_module.py +++ b/src/oca_pre_commit_hooks/checks_odoo_module.py @@ -206,11 +206,12 @@ def run(files_or_modules, enable=None, disable=None, no_verbose=False, no_exit=F all_check_errors.extend(checks_obj.checks_errors) exit_status = 1 # Sort errors by filepath, line, column and code - all_check_errors.sort(key=lambda x: (x.filepath, x.line or 0, x.column or 0, x.code)) + all_check_errors.sort() # Print errors if not no_verbose: for error in all_check_errors: print(error) + print("") if no_exit: return all_check_errors sys.exit(exit_status) diff --git a/src/oca_pre_commit_hooks/checks_odoo_module_csv.py b/src/oca_pre_commit_hooks/checks_odoo_module_csv.py index 76c9a71..b17d213 100644 --- a/src/oca_pre_commit_hooks/checks_odoo_module_csv.py +++ b/src/oca_pre_commit_hooks/checks_odoo_module_csv.py @@ -67,7 +67,7 @@ def check_csv(self): self.register_error( code="csv-duplicate-record-id", message=f"Duplicate CSV record `{record_id}`", - info="\n".join(f"{other_filepath}:{other_line}" for other_filepath, other_line, __ in records[1:]), filepath=filepath, line=line, + extra_positions=[(other_filepath, other_line) for other_filepath, other_line, __ in records[1:]], ) diff --git a/src/oca_pre_commit_hooks/checks_odoo_module_po.py b/src/oca_pre_commit_hooks/checks_odoo_module_po.py index 0f95696..ec4091f 100644 --- a/src/oca_pre_commit_hooks/checks_odoo_module_po.py +++ b/src/oca_pre_commit_hooks/checks_odoo_module_po.py @@ -397,11 +397,12 @@ def run(po_files, enable=None, disable=None, no_verbose=False, no_exit=False, li finally: del checks_po_obj # Sort errors by filepath, line, column and code - all_check_errors.sort(key=lambda x: (x.filepath, x.line or 0, x.column or 0, x.code)) + all_check_errors.sort() # Print errors if not no_verbose: for error in all_check_errors: print(error) + print("") if no_exit: return all_check_errors sys.exit(exit_status) diff --git a/src/oca_pre_commit_hooks/checks_odoo_module_xml.py b/src/oca_pre_commit_hooks/checks_odoo_module_xml.py index 7d4cbc7..b42adfe 100644 --- a/src/oca_pre_commit_hooks/checks_odoo_module_xml.py +++ b/src/oca_pre_commit_hooks/checks_odoo_module_xml.py @@ -173,9 +173,9 @@ def check_xml_records(self): self.register_error( code="xml-duplicate-record-id", message=f"Duplicate xml record id `{records[0].element.get('id')}`", - info="\n".join(f"{record.filename}:{record.element.sourceline}" for record in records[1:]), filepath=records[0].filename, line=records[0].element.sourceline, + extra_positions=[(record.filename, record.element.sourceline) for record in records[1:]], ) # fields_duplicated (empty dict if check is not enabled) @@ -185,9 +185,9 @@ def check_xml_records(self): self.register_error( code="xml-duplicate-fields", message=f"Duplicate xml field `{field_key[0]}`", - info="\n".join(f"{field[0]['filename_short']}:{field[1].sourceline}" for field in fields[1:]), filepath=fields[0][0]["filename_short"], line=fields[0][1].sourceline, + extra_positions=[(field[0]["filename_short"], field[1].sourceline) for field in fields[1:]], ) @utils.only_required_for_checks("xml-syntax-error") @@ -376,9 +376,9 @@ def check_xml_templates(self): self.register_error( code="xml-duplicate-template-id", message=f"Duplicate xml template id `{xmlid_key}`", - info="\n".join(f"{record.filename}:{record.element.sourceline}" for record in records[1:]), filepath=records[0].filename, line=records[0].element.sourceline, + extra_positions=[(record.filename, record.element.sourceline) for record in records[1:]], ) @utils.only_required_for_checks("xml-deprecated-data-node") diff --git a/tests/test_checks.py b/tests/test_checks.py index c0dce11..545ad75 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -111,9 +111,9 @@ def test_build_docstring(self): for code in sorted(all_check_errors_by_code): check_example_content += f"\n\n * {code}\n" for check_error in all_check_errors_by_code[code]: - msg = f"{check_error.filepath}" - if check_error.line: - msg += f"#L{check_error.line}" + msg = f"{check_error.position.filepath}" + if check_error.position.line: + msg += f"#L{check_error.position.line}" if check_error.message: msg += f" {check_error.message}" check_example_content += f"\n - https://github.com/OCA/odoo-pre-commit-hooks/blob/v{version}/{msg}" diff --git a/tests/test_checks_po.py b/tests/test_checks_po.py index 23856e4..82d43a5 100644 --- a/tests/test_checks_po.py +++ b/tests/test_checks_po.py @@ -111,9 +111,9 @@ def test_build_docstring(self): for code in sorted(all_check_errors_by_code): check_example_content += f"\n\n * {code}\n" for check_error in all_check_errors_by_code[code]: - msg = f"{check_error.filepath}" - if check_error.line: - msg += f"#L{check_error.line}" + msg = f"{check_error.position.filepath}" + if check_error.position.line: + msg += f"#L{check_error.position.line}" if check_error.message: msg += f" {check_error.message}" check_example_content += f"\n - https://github.com/OCA/odoo-pre-commit-hooks/blob/v{version}/{msg}"