diff --git a/reviewboard/diffviewer/chunk_generator.py b/reviewboard/diffviewer/chunk_generator.py index 5bdb8cf767acdc7bb03fdb1df99f1f23e5f03873..23e2b86a11178bafafadecf240edc52f6ce35ddc 100644 --- a/reviewboard/diffviewer/chunk_generator.py +++ b/reviewboard/diffviewer/chunk_generator.py @@ -6,8 +6,9 @@ import hashlib import logging import re +from collections.abc import Mapping from itertools import zip_longest -from typing import Any, Literal, Optional, TYPE_CHECKING, TypedDict +from typing import Any, Literal, TYPE_CHECKING, TypedDict import pygments.util from django.utils.encoding import force_str @@ -22,7 +23,10 @@ guess_lexer_for_filename) from reviewboard.codesafety import code_safety_checker_registry -from reviewboard.deprecation import RemovedInReviewBoard80Warning +from reviewboard.deprecation import ( + RemovedInReviewBoard80Warning, + RemovedInReviewBoard10_0Warning, +) from reviewboard.diffviewer.differ import DiffCompatVersion, get_differ from reviewboard.diffviewer.diffutils import ( DiffRegions, @@ -38,30 +42,52 @@ from reviewboard.diffviewer.settings import DiffSettings if TYPE_CHECKING: - from collections.abc import Iterator, Sequence + from collections.abc import Callable, Iterator, Sequence from django.http import HttpRequest from typing_extensions import TypeAlias - from reviewboard.diffviewer.differ import DiffOpcodeTag + from reviewboard.diffviewer.differ import Differ, DiffOpcodeTag from reviewboard.diffviewer.models.filediff import FileDiff + from reviewboard.diffviewer.opcode_generator import DiffOpcodeGenerator + + _HtmlFormatter = HtmlFormatter[str] +else: + _HtmlFormatter = HtmlFormatter logger = logging.getLogger(__name__) -class NoWrapperHtmlFormatter(HtmlFormatter): +class NoWrapperHtmlFormatter(_HtmlFormatter): """An HTML Formatter for Pygments that doesn't wrap items in a div.""" - def __init__(self, *args, **kwargs): - super(NoWrapperHtmlFormatter, self).__init__(*args, **kwargs) - - def _wrap_div(self, inner): - """Removes the div wrapper from formatted code. - - This is called by the formatter to wrap the contents of inner. - Inner is a list of tuples containing formatted code. If the first item - in the tuple is zero, then it's the div wrapper, so we should ignore - it. + + def _wrap_div( + self, + inner: Iterator[tuple[int, str]], + ) -> Iterator[tuple[int, str]]: + """Override for the _wrap_div method. + + The parent class uses this method to wrap raw contents in ``
`` + tags. This override prevents that, and filters out any wrapper nodes + which have already been added. + + Args: + inner (iterator): + The iterator of nodes. + + Yields: + tuple: + Each node to include. + + Tuple: + 0 (int): + 1, always. For the HTML formatter implementation, this + value is either 0 (indicating a wrapper node) or 1 + (indicating content). + + 1 (str): + The content of the node. """ for tup in inner: if tup[0]: @@ -89,7 +115,7 @@ int, # [1] Real line number in the original file. - Optional[int], + int | None, # [2] HTML markup of the original file line. str, @@ -98,7 +124,7 @@ DiffRegions, # [4] Real line number in the modified file. - Optional[int], + int | None, # [5] HTML markup of the modified file line. str, @@ -110,7 +136,7 @@ bool, # [8] Metadata for the line. - Optional[dict[str, Any]], + dict[str, Any] | None, ] @@ -140,7 +166,14 @@ numlines: int -class RawDiffChunkGenerator(object): +#: Type for information about a header in a file. +#: +#: Version Added: +#: 8.0 +HeaderInfo = Mapping[str, str] | None + + +class RawDiffChunkGenerator: """A generator for chunks for a diff that can be used for rendering. Each chunk represents an insert, delete, replace, or equal section. It @@ -159,25 +192,6 @@ Version Changed: 5.0: Added :py:attr:`all_code_safety_results`. - - Attributes: - all_code_safety_results (dict): - Code safety warnings were found while processing the diff. - - This is in the form of:: - - { - '': { - 'warnings': {'', ...}, - 'errors': {'', ...}, - }, - ... - } - - All keys are optional. - - Version Added: - 5.0 """ NEWLINES_RE = re.compile(r'\r?\n') @@ -198,6 +212,27 @@ # Instance variables # ###################### + #: Code safety warnings were found while processing the diff. + #: + #: This is in the form of:: + #: + #: { + #: '': { + #: 'warnings': {'', ...}, + #: 'errors': {'', ...}, + #: }, + #: ... + #: } + #: + #: All keys are optional. + #: + #: Version Added: + #: 5.0 + all_code_safety_results: dict[str, dict[str, set[str]]] + + #: The diff compatibility version. + diff_compat: int + #: Settings used for the generation of the diff. #: #: Version Added: @@ -207,6 +242,39 @@ #: reviewboard.diffviewer.settings.DiffSettings diff_settings: DiffSettings + #: The differ object. + differ: Differ | Any + + #: Whether to enable syntax highlighting + enable_syntax_highlighting: bool + + #: A list of file encodings to try. + encoding_list: Sequence[str] + + #: The old version of the file. + old: bytes | Sequence[bytes] + + #: The filename for the old version of the file. + orig_filename: str + + #: The filename for the new version of the file. + modified_filename: str + + #: The new version of the file. + new: bytes | Sequence[bytes] + + #: The current chunk being processed. + _chunk_index: int + + #: The most recently seen header information. + _last_header: tuple[ + HeaderInfo, + HeaderInfo, + ] + + #: The most recent header index used. + _last_header_index: list[int] + def __init__( self, old: bytes | Sequence[bytes] | None, @@ -289,8 +357,12 @@ assert diff_settings.tab_size self.diff_settings = diff_settings - self.old = old - self.new = new + if old is not None: + self.old = old + + if new is not None: + self.new = new + self.orig_filename = orig_filename self.modified_filename = modified_filename self.diff_settings = diff_settings @@ -302,12 +374,17 @@ self.all_code_safety_results = {} # Chunk processing state. - self._last_header = [None, None] + self._last_header = (None, None) self._last_header_index = [0, 0] self._chunk_index = 0 - def get_opcode_generator(self): - """Return the DiffOpcodeGenerator used to generate diff opcodes.""" + def get_opcode_generator(self) -> DiffOpcodeGenerator: + """Return the DiffOpcodeGenerator used to generate diff opcodes. + + Returns: + reviewboard.diffviewer.opcode_generator.DiffOpcodeGenerator: + The opcode generator. + """ return get_diff_opcode_generator(self.differ) def get_chunks( @@ -504,8 +581,13 @@ self.counts = counts - def check_line_code_safety(self, orig_line, modified_line, - extra_state={}, **kwargs): + def check_line_code_safety( + self, + orig_line: str | None, + modified_line: str | None, + extra_state: (dict[str, Any] | None) = None, + **kwargs, + ) -> list[tuple[str, dict[str, Any]]]: """Check the safety of a line of code. This will run the original and modified line through all registered @@ -536,25 +618,34 @@ A list of code safety results containing warnings or errors. Each item is a tuple containing: - 1. The registered checker ID. - 2. A dictionary with ``errors`` and/or ``warnings`` keys. + Tuple: + 0 (str): + The registered checker ID. + + 1 (dict): + A dictionary with ``errors`` and/or ``warnings`` keys. """ + if extra_state is None: + extra_state = {} + # Check for any unsafe/suspicious content on this line by passing # the raw source through any registered code safety checkers. results = [] to_check = [] if orig_line: - to_check.append(dict({ + to_check.append({ 'path': self.orig_filename, 'lines': [orig_line], - }, **extra_state)) + **extra_state, + }) if modified_line: - to_check.append(dict({ + to_check.append({ 'path': self.modified_filename, 'lines': [modified_line], - }, **extra_state)) + **extra_state, + }) if to_check: code_safety_configs = self.diff_settings.code_safety_configs @@ -612,7 +703,7 @@ s (bytes): The string to normalize. - encoding_list (list of unicode): + encoding_list (list of str): The list of encodings to try when converting the string to Unicode. @@ -691,18 +782,21 @@ for s in lines ] - def normalize_path_for_display(self, filename): + def normalize_path_for_display( + self, + filename: str, + ) -> str: """Normalize a file path for display to the user. By default, this returns the filename as-is. Subclasses can override the behavior to return a variant of the filename. Args: - filename (unicode): + filename (str): The filename to normalize. Returns: - unicode: + str: The normalized filename. """ return filename @@ -886,7 +980,8 @@ indentation_changes = meta.get('indentation_changes', {}) if line_pair[0] is not None and line_pair[1] is not None: - indentation_change = indentation_changes.get('%d-%d' % line_pair) + indentation_change = indentation_changes.get( + '{}-{}'.format(*line_pair)) # We check the ranges against (0, 0) for compatibility with a bug # present in Review Board 4.0.6 and older, where bad indentation @@ -954,7 +1049,11 @@ line_meta or None, ) - def _get_move_info(self, line_num, moved_meta): + def _get_move_info( + self, + line_num: int | None, + moved_meta: Mapping[int, int], + ) -> tuple[int, bool] | None: """Return information for a moved line. This will return a tuple containing the line number on the other end @@ -980,15 +1079,49 @@ return ( other_line_num, (line_num - 1 not in moved_meta or - other_line_num != moved_meta[line_num - 1] + 1) + other_line_num != moved_meta[line_num - 1] + 1), ) - def _highlight_indentation(self, old_markup, new_markup, is_indent, - raw_indent_len, norm_indent_len_diff): - """Highlights indentation in an HTML-formatted line. - - This will wrap the indentation in tags, and format it in - a way that makes it clear how many spaces or tabs were used. + def _highlight_indentation( + self, + old_markup: str, + new_markup: str, + is_indent: bool, + raw_indent_len: int, + norm_indent_len_diff: int, + ) -> tuple[str, str]: + """Highlight indentation in an HTML-formatted line. + + This will wrap the indentation in tags, and format it in + a way that makes it clear how many spaces or tabs were used. + + Args: + old_markup (str): + The markup for the old line. + + new_markup (str): + The markup for the new line. + + is_indent (bool): + If ``True``, the new line is indented compared to the old. If + ``False``, the new line is dedented. + + raw_indent_len (int): + The base amount of indentation in the line. + + norm_indent_len_diff (int): + The difference in indentation between the old and new lines. + + Returns: + tuple: + A 2-tuple of: + + Tuple: + 0 (str): + The changed markup for the old line. + + 1 (str): + The changed markup for the new line. """ if is_indent: new_markup = self._wrap_indentation_chars( @@ -1007,13 +1140,39 @@ return old_markup, new_markup - def _wrap_indentation_chars(self, class_name, markup, raw_indent_len, - norm_indent_len_diff, serializer): - """Wraps characters in a string with indentation markers. + def _wrap_indentation_chars( + self, + class_name: str, + markup: str, + raw_indent_len: int, + norm_indent_len_diff: int, + serializer: Callable[[str, int], tuple[str, str]], + ) -> str: + """Wrap characters in a string with indentation markers. This will insert the indentation markers and its wrapper in the markup string. It's careful not to interfere with any tags that may be used to highlight that line. + + Args: + class_name (str): + The class name to apply to the new element. + + markup (str): + The markup for the line. + + raw_indent_len (int): + The base amount of indentation in the line. + + norm_indent_len_diff (int): + The difference in indentation between the old and new lines. + + serializer (callable): + The method to call to serialize the indentation characters. + + Returns: + str: + The wrapped string. """ start_pos = 0 @@ -1039,13 +1198,17 @@ serialized, remainder = serializer(indentation, norm_indent_len_diff) - return '%s%s%s' % ( - markup[:start_pos], - class_name, - serialized, - remainder + markup[end_pos:]) + return ( + f'{markup[:start_pos]}' + f'{serialized}' + f'{remainder + markup[end_pos:]}' + ) - def _serialize_indentation(self, chars, norm_indent_len_diff): + def _serialize_indentation( + self, + chars: str, + norm_indent_len_diff: int, + ) -> tuple[str, str]: """Serializes an indentation string into an HTML representation. This will show every space as ``>``, and every tab as ``------>|``. @@ -1054,7 +1217,7 @@ boundary. Args: - chars (unicode): + chars (str): The indentation characters to serialize. norm_indent_len_diff (int): @@ -1064,8 +1227,12 @@ tuple: A 2-tuple containing: - 1. The serialized indentation string. - 2. The remaining indentation characters not serialized. + Tuple: + 0 (str): + The serialized indentation string. + + 1 (str): + The remaining indentation characters not serialized. """ assert chars @@ -1101,7 +1268,11 @@ return s, chars[j + 1:] - def _serialize_unindentation(self, chars, norm_indent_len_diff): + def _serialize_unindentation( + self, + chars: str, + norm_indent_len_diff: int, + ) -> tuple[str, str]: """Serialize an unindentation string into an HTML representation. This will show every space as ``<``, and every tab as ``|<------``. @@ -1110,7 +1281,7 @@ boundary. Args: - chars (unicode): + chars (str): The unindentation characters to serialize. norm_indent_len_diff (int): @@ -1120,8 +1291,12 @@ tuple: A 2-tuple containing: - 1. The serialized unindentation string. - 2. The remaining unindentation characters not serialized. + Tuple: + 0 (str): + The serialized unindentation string. + + 1 (str): + The remaining unindentation characters not serialized. """ assert chars @@ -1201,9 +1376,15 @@ meta = {} left_headers = list(self._get_interesting_headers( - all_lines, start, end - 1, False)) + lines=all_lines, + start=start, + end=end, + is_modified_file=False)) right_headers = list(self._get_interesting_headers( - all_lines, start, end - 1, True)) + lines=all_lines, + start=start, + end=end, + is_modified_file=True)) meta['left_headers'] = left_headers meta['right_headers'] = right_headers @@ -1211,7 +1392,11 @@ lines = all_lines[start:end] num_lines = len(lines) - compute_chunk_last_header(lines, num_lines, meta, self._last_header) + self._last_header = compute_chunk_last_header( + lines=lines, + numlines=num_lines, + meta=meta, + last_header=self._last_header) if (collapsible and end < len(all_lines) and @@ -1233,6 +1418,7 @@ def _get_interesting_headers( self, + *, lines: Sequence[DiffLine], start: int, end: int, @@ -1243,6 +1429,10 @@ This scans for all headers that fall within the specified range of the specified lines on both the original and modified files. + Version Changed: + 8.0: + Made the arguments keyword-only. + Args: lines (list of DiffLine): The lines in the chunk. @@ -1415,9 +1605,9 @@ self, request: HttpRequest, filediff: FileDiff, - interfilediff: Optional[FileDiff] = None, + interfilediff: (FileDiff | None) = None, force_interdiff: bool = False, - base_filediff: Optional[FileDiff] = None, + base_filediff: (FileDiff | None) = None, *, diff_settings: DiffSettings, ) -> None: @@ -1506,8 +1696,13 @@ return '-'.join(key) - def get_opcode_generator(self): - """Return the DiffOpcodeGenerator used to generate diff opcodes.""" + def get_opcode_generator(self) -> DiffOpcodeGenerator: + """Return the DiffOpcodeGenerator used to generate diff opcodes. + + Returns: + reviewboard.diffviewer.opcode_generator.DiffOpcodeGenerator: + The opcode generator. + """ diff = self.filediff.diff if self.interfilediff: @@ -1698,8 +1893,13 @@ total_line_count=(insert_count + delete_count + replace_count + equal_count)) - def check_line_code_safety(self, orig_line, modified_line, - extra_state={}, **kwargs): + def check_line_code_safety( + self, + orig_line: str, + modified_line: str, + extra_state: (dict[str, Any] | None) = None, + **kwargs, + ) -> list[tuple[str, dict[str, Any]]]: """Check the safety of a line of code. This will run the original and modified line through all registered @@ -1738,33 +1938,43 @@ 1. The registered checker ID. 2. A dictionary with ``errors`` and/or ``warnings`` keys. """ - return super(DiffChunkGenerator, self).check_line_code_safety( + if extra_state is None: + extra_state = {} + + return super().check_line_code_safety( orig_line=orig_line, modified_line=modified_line, - extra_state=dict({ + extra_state={ 'repository': self.repository, - }, **extra_state), + **extra_state, + }, **kwargs) - def normalize_path_for_display(self, filename): + def normalize_path_for_display( + self, + filename: str, + ) -> str: """Normalize a file path for display to the user. This uses the associated :py:class:`~reviewboard.scmtools.core.SCMTool` to normalize the filename. Args: - filename (unicode): + filename (str): The filename to normalize. Returns: - unicode: + str: The normalized filename. """ return self.tool.normalize_path_for_display( filename, extra_data=self.filediff.extra_data) - def _get_sha1(self, content): + def _get_sha1( + self, + content: bytes, + ) -> str: """Return a SHA1 hash for the provided content. Args: @@ -1772,12 +1982,15 @@ The content to generate the hash for. Returns: - unicode: + str: The resulting hash. """ return force_str(hashlib.sha1(content).hexdigest()) - def _get_sha256(self, content): + def _get_sha256( + self, + content: bytes, + ) -> str: """Return a SHA256 hash for the provided content. Args: @@ -1785,14 +1998,21 @@ The content to generate the hash for. Returns: - unicode: + str: The resulting hash. """ return force_str(hashlib.sha256(content).hexdigest()) -def compute_chunk_last_header(lines, numlines, meta, last_header=None): - """Computes information for the displayed function/class headers. +@deprecate_non_keyword_only_args(RemovedInReviewBoard10_0Warning) +def compute_chunk_last_header( + *, + lines: Sequence[DiffLine], + numlines: int, + meta: dict[str, Any], + last_header: (tuple[HeaderInfo, HeaderInfo] | None) = None, +) -> tuple[HeaderInfo, HeaderInfo]: + """Compute information for the displayed function/class headers. This will record the displayed headers, their line numbers, and expansion offsets relative to the header's collapsed line range. @@ -1800,24 +2020,59 @@ The last_header variable, if provided, will be modified, which is important when processing several chunks at once. It will also be returned as a convenience. + + Version Changed: + 8.0: + * Made arguments keyword-only. + * Changed to take in and return a 2-tuple instead of a 2-element list. + + Args: + lines (list of DiffLine): + The lines in the chunk. + + numlines (int): + The number of lines in the chuck. + + meta (dict): + Metadata for the chunk. + + last_header (tuple): + A 2-tuple of the most recent header information. + + Returns: + tuple: + A 2-tuple of: + + Tuple: + 0 (dict): + Header information for the original version of the file. + + 1 (dict): + Header information for the modified version of the file. """ - if last_header is None: - last_header = [None, None] - - line = lines[0] - - for i, (linenum, header_key) in enumerate([(line[1], 'left_headers'), - (line[4], 'right_headers')]): - headers = meta[header_key] - - if headers: - header = headers[-1] - last_header[i] = { - 'line': header[0], - 'text': header[1].strip(), - } - - return last_header + if last_header is not None: + left, right = last_header + else: + left = None + right = None + + if left_headers := meta['left_headers']: + header = left_headers[-1] + + left = { + 'line': header[0], + 'text': header[1].strip(), + } + + if right_headers := meta['right_headers']: + header = right_headers[-1] + + right = { + 'line': header[0], + 'text': header[1].strip(), + } + + return left, right _generator: type[DiffChunkGenerator] = DiffChunkGenerator diff --git a/reviewboard/diffviewer/differ.py b/reviewboard/diffviewer/differ.py index 678aad32a0f48a36fac06a9d602271d28b164202..0d3d100dde6dc546c4c00de15d19da809fb624eb 100644 --- a/reviewboard/diffviewer/differ.py +++ b/reviewboard/diffviewer/differ.py @@ -3,33 +3,39 @@ from __future__ import annotations import os -from typing import Any, Literal, Optional, TYPE_CHECKING +from typing import Any, Literal, TYPE_CHECKING + +from django.utils.translation import gettext as _ from reviewboard.diffviewer.errors import DiffCompatError from reviewboard.diffviewer.filetypes import (HEADER_REGEXES, HEADER_REGEX_ALIASES) if TYPE_CHECKING: - from collections.abc import Iterator + from collections.abc import Iterator, Sequence + from re import Pattern from typing_extensions import TypeAlias -# Compatibility versions: -# -class DiffCompatVersion(object): - # Python SequenceMatcher differ. +class DiffCompatVersion: + """Diff compatibility versions.""" + + #: Python SequenceMatcher differ. SMDIFFER = 0 - # Myers differ + #: Myers differ. MYERS = 1 - # Myers differ with bailing on a too high SMS cost - # (prevents very long diff times for certain files) + #: Myers differ with bailing on a too high SMS cost. + #: + #: This prevents very long diff times for certain files. MYERS_SMS_COST_BAIL = 2 + #: The default compatibility version to use. DEFAULT = MYERS_SMS_COST_BAIL + #: Versions that use the Myers diff algorithm. MYERS_VERSIONS = (MYERS, MYERS_SMS_COST_BAIL) @@ -69,13 +75,68 @@ int, # i2 int, # j1 int, # j2 - Optional[dict[str, Any]], # metadata + dict[str, Any] | None, # metadata ] -class Differ(object): +class Differ: """Base class for differs.""" - def __init__(self, a, b, ignore_space=False, compat_version=None): + + ###################### + # Instance variables # + ###################### + + #: The original version of the file, split into lines. + a: Sequence[str] + + #: The modified version of the file, split into lines. + b: Sequence[str] + + #: The diff compatibility version. + #: + #: Version Changed: + #: 8.0: + #: Changed to not allow ``None`` values. + compat_version: int + + #: Whether to ignore whitespace. + ignore_space: bool + + #: Regular expressions for finding interesting lines. + interesting_line_regexes: list[tuple[str, Pattern[str]]] + + #: Interesting lines in the file. + #: + #: Version Changed: + #: 8.0: + #: Changed from a 2-element list to a 2-tuple. + interesting_lines: tuple[ + dict[str, list[tuple[int, str]]], + dict[str, list[tuple[int, str]]], + ] + + def __init__( + self, + a: Sequence[str], + b: Sequence[str], + ignore_space: bool = False, + compat_version: int = DiffCompatVersion.DEFAULT, + ) -> None: + """Initialize the differ. + + Args: + a (list of str): + The original version of the file, split into lines. + + b (list of str): + The modified version of the file, split into lines. + + ignore_space (bool, optional): + Whether to ignore whitespace changes. + + compat_version (int, optional): + The diff compatibility version. + """ if type(a) is not type(b): raise TypeError @@ -84,32 +145,50 @@ self.ignore_space = ignore_space self.compat_version = compat_version self.interesting_line_regexes = [] - self.interesting_lines = [{}, {}] + self.interesting_lines = ({}, {}) - def add_interesting_line_regex(self, name, regex): - """Registers a regular expression used to look for interesting lines. + def add_interesting_line_regex( + self, + name: str, + regex: Pattern[str], + ) -> None: + """Register a regular expression used to look for interesting lines. All interesting lines found that match the regular expression will be stored and tagged with the given name. Callers can use get_interesting_lines to get the results. + + Args: + name (str): + The name of the regex to register. + + regex (re.Pattern): + The compiled regular expression. """ self.interesting_line_regexes.append((name, regex)) self.interesting_lines[0][name] = [] self.interesting_lines[1][name] = [] - def add_interesting_lines_for_headers(self, filename): - """Registers for interesting lines for headers based on filename. + def add_interesting_lines_for_headers( + self, + filename: str, + ) -> None: + """Register patterns for interesting lines for headers. This is a convenience over add_interesting_line_regex that will watch for headers (functions, classes, etc.) for the file type matching the given filename. + + Args: + filename (str): + The filename of the file being diffed. """ regexes = [] if filename in HEADER_REGEX_ALIASES: regexes = HEADER_REGEXES[HEADER_REGEX_ALIASES[filename]] else: - basename, ext = os.path.splitext(filename) + ext = os.path.splitext(filename)[1] if ext in HEADER_REGEXES: regexes = HEADER_REGEXES[ext] @@ -119,8 +198,27 @@ for regex in regexes: self.add_interesting_line_regex('header', regex) - def get_interesting_lines(self, name, is_modified_file): - """Returns the interesting lines tagged with the given name.""" + def get_interesting_lines( + self, + name: str, + is_modified_file: bool, + ) -> list[tuple[int, str]]: + """Return the interesting lines tagged with the given name. + + Args: + name (str): + The name of the type of interesting lines to get. + + is_modified_file (bool): + If ``True``, get interesting lines for the modified version of + the file. If ``False``, get interesting lines for the original + version of the file. + + Returns: + list of tuple: + A list of interesting lines in the file. Each item is a 2-tuple of + (line number, line). + """ if is_modified_file: index = 1 else: @@ -138,13 +236,38 @@ raise NotImplementedError -def get_differ(a, b, ignore_space=False, - compat_version=DiffCompatVersion.DEFAULT): - """Returns a differ for with the given settings. +def get_differ( + a: Sequence[str], + b: Sequence[str], + ignore_space: bool = False, + compat_version: int = DiffCompatVersion.DEFAULT, +) -> Differ: + """Return a differ for with the given settings. By default, this will return the MyersDiffer. Older differs can be used by specifying a compat_version, but this is only for *really* ancient diffs, currently. + + Args: + a (list of str): + The original file, split into lines. + + b (list of str): + The modified file, split into lines. + + ignore_space (bool): + Whether to ignore whitespace when performing the diff. + + compat_version (int): + The diff compatibility version. + + Returns: + Differ: + The new differ instance. + + Raises: + reviewboard.diffviewer.errors.DiffCompatError: + The compatibility version was not valid. """ cls = None @@ -156,7 +279,10 @@ cls = SMDiffer else: raise DiffCompatError( - 'Invalid diff compatibility version (%s) passed to Differ' % - compat_version) + _( + 'Invalid diff compatibility version ({compat_version}) passed ' + 'to Differ' + ).format(compat_version=compat_version) + ) return cls(a, b, ignore_space, compat_version=compat_version) diff --git a/reviewboard/diffviewer/opcode_generator.py b/reviewboard/diffviewer/opcode_generator.py index 1cc8adbac3d77bfa2343b3a92eb7f785d8d008be..7bfdee27676b88302f0270f86b8890110648fd96 100644 --- a/reviewboard/diffviewer/opcode_generator.py +++ b/reviewboard/diffviewer/opcode_generator.py @@ -4,45 +4,110 @@ import os import re -from typing import Optional, TYPE_CHECKING +from typing import TYPE_CHECKING from reviewboard.diffviewer.processors import (filter_interdiff_opcodes, post_process_filtered_equals) from reviewboard.diffviewer.settings import DiffSettings if TYPE_CHECKING: - from collections.abc import Iterable, Iterator + from collections.abc import Iterable, Iterator, Mapping + from typing import Any from django.http import HttpRequest from reviewboard.diffviewer.differ import ( Differ, DiffOpcode, + DiffOpcodeTag, DiffOpcodeWithMetadata, ) -class MoveRange(object): +class MoveRange: """Stores information on a move range. This will store the start and end of the range, and all groups that are a part of it. """ - def __init__(self, start, end, groups=[]): + + ###################### + # Instance variables # + ###################### + + #: The end index of the range. + #: + #: This indexes into the opcode generator's groups list. + end: int + + #: The opcodes in the range. + #: + #: This is a list of tuples where the first element is the opcode, and the + #: second is the index into the opcode generator's groups list. + groups: list[tuple[DiffOpcodeWithMetadata, int]] + + #: The start index of the range. + #: + #: This indexes into the opcode generator's groups list. + start: int + + def __init__( + self, + start: int, + end: int, + groups: (list[tuple[DiffOpcodeWithMetadata, int]] | None) = None, + ) -> None: + """Initialize the object. + + Args: + start (int): + The start index of the range. + + end (int): + The end index of the range. + + groups (list of tuple, optional): + The opcodes in the range. + """ self.start = start self.end = end - self.groups = groups + self.groups = groups or [] @property - def last_group(self): + def last_group(self) -> tuple[DiffOpcodeWithMetadata, int]: + """The last group in the move range. + + Raises: + IndexError: + The move range was empty. + """ return self.groups[-1] - def add_group(self, group, group_index): + def add_group( + self, + group: DiffOpcodeWithMetadata, + group_index: int, + ) -> None: + """Add a group to the range. + + Args: + group (reviewboard.diffviewer.differ.DiffOpcode): + The opcode. + + group_index (int): + The index of the group in the opcode generator's groups list. + """ if self.groups[-1] != group: self.groups.append((group, group_index)) - def __repr__(self): - return '' % (self.start, self.end, self.groups) + def __repr__(self) -> str: + """Return a string representation of the object. + + Returns: + str: + A string representation of the object. + """ + return f'' class DiffOpcodeGenerator: @@ -62,7 +127,7 @@ ###################### #: The raw contents for the diff. - diff: Optional[bytes] + diff: bytes | None #: The differ being used to generate the diff. differ: Differ @@ -76,20 +141,33 @@ #: The generated opcodes. groups: list[DiffOpcodeWithMetadata] + #: A list of all opcodes involving inserted lines. + #: + #: This will contain both pure inserts as well as replaces. + inserts: list[DiffOpcodeWithMetadata] + #: The raw contents of the interdiff range diff. - interdiff: Optional[bytes] + interdiff: bytes | None #: The HTTP request from the client. - request: Optional[HttpRequest] + request: HttpRequest | None + + #: A mapping for working with removed lines. + #: + #: This is a mapping from the line content of a removed line to a list of + #: potential opcodes relating to that removal. The values in that list are + #: tuples of the original line number, the opcode, and the index of that + #: opcode in the ``groups`` list. + removes: dict[str, list[tuple[int, DiffOpcodeWithMetadata, int]]] def __init__( self, differ: Differ, - diff: Optional[bytes] = None, - interdiff: Optional[bytes] = None, - request: Optional[HttpRequest] = None, + diff: (bytes | None) = None, + interdiff: (bytes | None) = None, + request: (HttpRequest | None) = None, *, - diff_settings: Optional[DiffSettings] = None, + diff_settings: (DiffSettings | None) = None, **kwargs, ) -> None: """Initialize the opcode generator. @@ -420,7 +498,11 @@ if prev_start_i != i2 or prev_start_j != j2: yield prev_start_i, i2, prev_start_j, j2, indentation_changes - def _compute_line_indentation(self, old_line, new_line): + def _compute_line_indentation( + self, + old_line: str, + new_line: str, + ) -> tuple[bool, int, int] | None: """Compute the indentation of a line. This will determine whether the indentation has changed in a line of @@ -432,21 +514,28 @@ "filtered-equal" lines. Args: - old_line (unicode): + old_line (str): The old line content. - new_line (unicode): + new_line (str): The new line content. Returns: tuple: A 3-tuple if indentation changes were found. This contains: - 1. Whether the content was indented (``True``) or unindented - (``False``). - 2. How many characters of indentation were added (if indenting) - or removed (if unindenting). - 3. The difference in indentation levels (between the two lines). + Tuple: + 0 (bool): + Whether the content was indented (``True``) or unindented + (``False``). + + 1 (int): + How many characters of indentation were added (if + indenting) or removed (if unindenting). + + 2 (int): + The difference in indentation levels (between the two + lines). If no indentation took place, or indentation logic is not appropriate for these lines, this will be ``None`` instead. @@ -538,9 +627,8 @@ raw_indent_len, abs(norm_old_line_indent_len - norm_new_line_indent_len)) - def _compute_moves(self): - # We now need to figure out all the moved locations. - # + def _compute_moves(self) -> None: + """Compute all the moved locations.""" # At this point, we know all the inserted groups, and all the # individually deleted lines. We'll be going through and finding # consecutive groups of matching inserts/deletes that represent a @@ -549,13 +637,21 @@ # The algorithm will be documented as we go in the code. # # We start by looping through all the inserted groups. - r_move_indexes_used = set() + r_move_indexes_used: set[int] = set() for insert in self.inserts: self._compute_move_for_insert(r_move_indexes_used, *insert) - def _compute_move_for_insert(self, r_move_indexes_used, itag, ii1, ii2, - ij1, ij2, imeta): + def _compute_move_for_insert( + self, + r_move_indexes_used: set[int], + itag: DiffOpcodeTag, + ii1: int, + ii2: int, + ij1: int, + ij2: int, + imeta: dict[str, Any] | None, + ) -> None: """Compute move information for a given insert-like chunk. Args: @@ -563,7 +659,7 @@ All remove indexes that have already been included in a move range. - itag (unicode): + itag (reviewboard.diffviewer.differ.DiffOpcodeTag): The chunk tag for the insert (``insert`` or ``replace``). ii1 (int): @@ -582,6 +678,8 @@ The metadata for the chunk for the modification, where the move ranges may be stored. """ + assert imeta is not None + # Store some state on the range we'll be working with inside this # insert group. @@ -597,7 +695,7 @@ # group for the line. The value is an instance of MoveRange. The values # in MoveRange are used to quickly locate deleted lines we've found # that match the inserted lines, so we can assemble ranges later. - r_move_ranges = {} # key -> (start, end, group) + r_move_ranges: dict[str, MoveRange] = {} # key -> (start, end, group) move_key = None is_replace = (itag == 'replace') @@ -635,13 +733,16 @@ if ri in r_move_indexes_used: continue - r_move_range = r_move_ranges.get(move_key) + if move_key: + r_move_range = r_move_ranges.get(move_key) + else: + r_move_range = None if not r_move_range or ri != r_move_range.end + 1: # We either didn't have a previous range, or this # group didn't immediately follow it, so we need # to start a new one. - move_key = '%s-%s-%s-%s' % rgroup[1:5] + move_key = '{}-{}-{}-{}'.format(*rgroup[1:5]) r_move_range = r_move_ranges.get(move_key) if r_move_range: @@ -662,6 +763,7 @@ # with the existing range, so it's time to build # one based on any removed lines we find that # match the inserted line. + assert move_key is not None r_move_ranges[move_key] = \ MoveRange(ri, ri, [(rgroup, rgroup_index)]) updated_range = True @@ -714,7 +816,7 @@ # range, so we need to try to find that group and # add it to the list of groups in the range, if it' # not already there. - last_group, last_group_index = r_move_range.last_group + last_group = r_move_range.last_group[0] if new_end_i >= last_group[2]: # This is in the next group, which hasn't been @@ -765,8 +867,10 @@ moved_to_ranges = dict(zip(r_range, i_range)) - for group, group_index in r_move_range.groups: + for group, _group_index in r_move_range.groups: rmeta = group[-1] + assert rmeta is not None + rmeta.setdefault('moved-to', {}).update( moved_to_ranges) @@ -786,7 +890,20 @@ i_move_range = MoveRange(i_move_cur, i_move_cur) r_move_ranges = {} - def _find_longest_move_range(self, r_move_ranges): + def _find_longest_move_range( + self, + r_move_ranges: Mapping[str, MoveRange], + ) -> MoveRange | None: + """Find the longest move range. + + Args: + r_move_ranges (dict): + A dictionary mapping positions to the move ranges. + + Returns: + MoveRange: + The longest move range found. + """ # Go through every range of lines we've found and find the longest. # # The longest move range wins. If we find two ranges that are equal, @@ -819,8 +936,11 @@ return r_move_range - def _determine_move_range(self, r_move_range): - """Determines if a move range is valid and should be included. + def _determine_move_range( + self, + r_move_range: MoveRange | None, + ) -> MoveRange | None: + """Determine if a move range is valid and should be included. This performs some tests to try to eliminate trivial changes that shouldn't have moves associated. @@ -831,6 +951,14 @@ If the move range is valid, any trailing whitespace-only lines will be stripped, ensuring it covers only a valid range of content. + + Args: + r_move_range (MoveRange): + The move range to check. + + Returns: + MoveRange: + The move range, if it is valid. ``None``, if it is not. """ if not r_move_range: return None @@ -853,6 +981,11 @@ if valid: break + if not valid: + return None + + assert new_end_i is not None + # Accept this if there's more than one line or if the first # line is long enough, in order to filter out small bits of garbage. valid = ( @@ -865,8 +998,6 @@ if not valid: return None - assert new_end_i is not None - return MoveRange(r_move_range.start, new_end_i, r_move_range.groups)