Repository: PyCQA/flake8

Issue:
Current implementation of `--diff` is kind of broken by design
`--diff` currently makes assumptions that code changes will introduce lint errors *at the position of the code change* which may have been true for `pycodestyle` which only concerns itself with whitespace / style issues -- but it is not true for pyflakes or generally true for flake8 plugins

here is a small concrete example:

```diff
 import os
 
 def main():
-    print(os.getcwd())
+    print('hi')
```

`flake8 t.py` reports F401 on line 1, but `flake8 --diff` silently passes (even though the diff _caused_ that new issue)

as such, I think the option's behaviour needs to be changed in an incompatible way, or removed entirely

if kept, I think the only reasonable behaviour would be to limit changes to the file which a diff touched -- rather than the lines which were touched

Hints from issue comments:
@sigmavirus24: > `--diff` currently makes assumptions that code changes will introduce lint errors _at the position of the code change_ which may have been true for `pycodestyle` which only concerns itself with whitespace / style issues -- but it is not true for pyflakes or generally true for flake8 plugins

Yeah this sounds right. 3.x reimplemented the features that 1.x and 2.x relied on which we used by importing and monkey-patching stuff in `pep8` directly. Even with it being a backwards incompatible release, I tried to keep as much backwards compatibility as possible.@fschulze: I created a replacement Python package for this functionality which also works with other tools like ruff: https://pypi.org/project/match-diff-lines/

Solution patch:
diff --git a/docs/source/internal/utils.rst b/docs/source/internal/utils.rst
index c745917..dc53378 100644
--- a/docs/source/internal/utils.rst
+++ b/docs/source/internal/utils.rst
@@ -66,11 +66,3 @@ The standard library's :func:`fnmatch.fnmatch` is excellent at deciding if a
 filename matches a single pattern. In our use case, however, we typically have
 a list of patterns and want to know if the filename matches any of them. This
 function abstracts that logic away with a little extra logic.
-
-.. autofunction:: flake8.utils.parse_unified_diff
-
-To handle usage of :option:`flake8 --diff`, |Flake8| needs to be able
-to parse the name of the files in the diff as well as the ranges indicated the
-sections that have been changed. This function either accepts the diff as an
-argument or reads the diff from standard-in. It then returns a dictionary with
-filenames as the keys and sets of line numbers as the value.
diff --git a/docs/source/user/options.rst b/docs/source/user/options.rst
index 170c1e7..f9e8b86 100644
--- a/docs/source/user/options.rst
+++ b/docs/source/user/options.rst
@@ -44,8 +44,6 @@ Index of Options
 
 - :option:`flake8 --count`
 
-- :option:`flake8 --diff`
-
 - :option:`flake8 --exclude`
 
 - :option:`flake8 --filename`
@@ -230,27 +228,6 @@ Options and their Descriptions
         count = True
 
 
-.. option:: --diff
-
-    :ref:`Go back to index <top>`
-
-    .. warning::
-
-        Due to hiding potential errors, this option is deprecated and will be
-        removed in a future version.
-
-    Use the unified diff provided on standard in to only check the modified
-    files and report errors included in the diff.
-
-    Command-line example:
-
-    .. prompt:: bash
-
-        git diff -u | flake8 --diff
-
-    This **can not** be specified in config files.
-
-
 .. option:: --exclude=<patterns>
 
     :ref:`Go back to index <top>`
diff --git a/src/flake8/api/legacy.py b/src/flake8/api/legacy.py
index 9635756..ba4339e 100644
--- a/src/flake8/api/legacy.py
+++ b/src/flake8/api/legacy.py
@@ -136,7 +136,6 @@ class StyleGuide:
                     stdin_display_name=self.options.stdin_display_name,
                     filename_patterns=self.options.filename,
                     exclude=self.options.exclude,
-                    is_running_from_diff=self.options.diff,
                 )
             )
             return not paths
diff --git a/src/flake8/checker.py b/src/flake8/checker.py
index d73349f..18b1ff6 100644
--- a/src/flake8/checker.py
+++ b/src/flake8/checker.py
@@ -97,8 +97,6 @@ class Manager:
         #   implementation issues
         # - the user provided stdin and that's not something we can handle
         #   well
-        # - we're processing a diff, which again does not work well with
-        #   multiprocessing and which really shouldn't require multiprocessing
         # - the user provided some awful input
 
         # class state is only preserved when using the `fork` strategy.
@@ -116,13 +114,6 @@ class Manager:
             )
             return 0
 
-        if self.options.diff:
-            LOG.warning(
-                "The --diff option was specified with --jobs but "
-                "they are not compatible. Ignoring --jobs arguments."
-            )
-            return 0
-
         jobs = self.options.jobs
 
         # If the value is "auto", we want to let the multiprocessing library
@@ -169,7 +160,6 @@ class Manager:
                 stdin_display_name=self.options.stdin_display_name,
                 filename_patterns=self.options.filename,
                 exclude=self.exclude,
-                is_running_from_diff=self.options.diff,
             )
         ]
         self.checkers = [c for c in self._all_checkers if c.should_process]
diff --git a/src/flake8/discover_files.py b/src/flake8/discover_files.py
index b8592c8..580d5fd 100644
--- a/src/flake8/discover_files.py
+++ b/src/flake8/discover_files.py
@@ -55,7 +55,6 @@ def expand_paths(
     stdin_display_name: str,
     filename_patterns: Sequence[str],
     exclude: Sequence[str],
-    is_running_from_diff: bool,
 ) -> Generator[str, None, None]:
     """Expand out ``paths`` from commandline to the lintable files."""
     if not paths:
@@ -75,24 +74,16 @@ def expand_paths(
             logger=LOG,
         )
 
-    def is_included(arg: str, fname: str) -> bool:
-        # while running from a diff, the arguments aren't _explicitly_
-        # listed so we still filter them
-        if is_running_from_diff:
-            return utils.fnmatch(fname, filename_patterns)
-        else:
-            return (
-                # always lint `-`
-                fname == "-"
-                # always lint explicitly passed (even if not matching filter)
-                or arg == fname
-                # otherwise, check the file against filtered patterns
-                or utils.fnmatch(fname, filename_patterns)
-            )
-
     return (
         filename
         for path in paths
         for filename in _filenames_from(path, predicate=is_excluded)
-        if is_included(path, filename)
+        if (
+            # always lint `-`
+            filename == "-"
+            # always lint explicitly passed (even if not matching filter)
+            or path == filename
+            # otherwise, check the file against filtered patterns
+            or utils.fnmatch(filename, filename_patterns)
+        )
     )
diff --git a/src/flake8/main/application.py b/src/flake8/main/application.py
index 5178abb..c76fb86 100644
--- a/src/flake8/main/application.py
+++ b/src/flake8/main/application.py
@@ -13,7 +13,6 @@ from flake8 import checker
 from flake8 import defaults
 from flake8 import exceptions
 from flake8 import style_guide
-from flake8 import utils
 from flake8.formatting.base import BaseFormatter
 from flake8.main import debug
 from flake8.main import options
@@ -66,9 +65,6 @@ class Application:
         #: with a non-zero status code
         self.catastrophic_failure = False
 
-        #: The parsed diff information
-        self.parsed_diff: dict[str, set[int]] = {}
-
     def parse_preliminary_options(
         self, argv: Sequence[str]
     ) -> tuple[argparse.Namespace, list[str]]:
@@ -158,13 +154,6 @@ class Application:
             print(json.dumps(info, indent=2, sort_keys=True))
             raise SystemExit(0)
 
-        if self.options.diff:
-            LOG.warning(
-                "the --diff option is deprecated and will be removed in a "
-                "future version."
-            )
-            self.parsed_diff = utils.parse_unified_diff()
-
         for loaded in self.plugins.all_plugins():
             parse_options = getattr(loaded.obj, "parse_options", None)
             if parse_options is None:
@@ -194,9 +183,6 @@ class Application:
             self.options, self.formatter
         )
 
-        if self.options.diff:
-            self.guide.add_diff_ranges(self.parsed_diff)
-
     def make_file_checker_manager(self) -> None:
         """Initialize our FileChecker Manager."""
         assert self.guide is not None
@@ -213,16 +199,9 @@ class Application:
         :class:`~flake8.checker.Manger` instance run the checks it is
         managing.
         """
-        assert self.options is not None
         assert self.file_checker_manager is not None
-        if self.options.diff:
-            files: list[str] | None = sorted(self.parsed_diff)
-            if not files:
-                return
-        else:
-            files = None
 
-        self.file_checker_manager.start(files)
+        self.file_checker_manager.start()
         try:
             self.file_checker_manager.run()
         except exceptions.PluginExecutionFailed as plugin_failed:
diff --git a/src/flake8/main/options.py b/src/flake8/main/options.py
index 86a6cf8..891bae9 100644
--- a/src/flake8/main/options.py
+++ b/src/flake8/main/options.py
@@ -114,7 +114,6 @@ def register_default_options(option_manager: OptionManager) -> None:
     - ``-q``/``--quiet``
     - ``--color``
     - ``--count``
-    - ``--diff``
     - ``--exclude``
     - ``--extend-exclude``
     - ``--filename``
@@ -163,13 +162,6 @@ def register_default_options(option_manager: OptionManager) -> None:
         "all other output.",
     )
 
-    add_option(
-        "--diff",
-        action="store_true",
-        help="(DEPRECATED) Report changes only within line number ranges in "
-        "the unified diff provided on standard in by the user.",
-    )
-
     add_option(
         "--exclude",
         metavar="patterns",
diff --git a/src/flake8/style_guide.py b/src/flake8/style_guide.py
index 2fee0f3..94698f8 100644
--- a/src/flake8/style_guide.py
+++ b/src/flake8/style_guide.py
@@ -298,18 +298,6 @@ class StyleGuideManager:
             code, filename, line_number, column_number, text, physical_line
         )
 
-    def add_diff_ranges(self, diffinfo: dict[str, set[int]]) -> None:
-        """Update the StyleGuides to filter out information not in the diff.
-
-        This provides information to the underlying StyleGuides so that only
-        the errors in the line number ranges are reported.
-
-        :param diffinfo:
-            Dictionary mapping filenames to sets of line number ranges.
-        """
-        for guide in self.style_guides:
-            guide.add_diff_ranges(diffinfo)
-
 
 class StyleGuide:
     """Manage a Flake8 user's style guide."""
@@ -333,7 +321,6 @@ class StyleGuide:
         self.filename = filename
         if self.filename:
             self.filename = utils.normalize_path(self.filename)
-        self._parsed_diff: dict[str, set[int]] = {}
 
     def __repr__(self) -> str:
         """Make it easier to debug which StyleGuide we're using."""
@@ -440,20 +427,8 @@ class StyleGuide:
             self.should_report_error(error.code) is Decision.Selected
         )
         is_not_inline_ignored = error.is_inline_ignored(disable_noqa) is False
-        is_included_in_diff = error.is_in(self._parsed_diff)
-        if error_is_selected and is_not_inline_ignored and is_included_in_diff:
+        if error_is_selected and is_not_inline_ignored:
             self.formatter.handle(error)
             self.stats.record(error)
             return 1
         return 0
-
-    def add_diff_ranges(self, diffinfo: dict[str, set[int]]) -> None:
-        """Update the StyleGuide to filter out information not in the diff.
-
-        This provides information to the StyleGuide so that only the errors
-        in the line number ranges are reported.
-
-        :param diffinfo:
-            Dictionary mapping filenames to sets of line number ranges.
-        """
-        self._parsed_diff = diffinfo
diff --git a/src/flake8/utils.py b/src/flake8/utils.py
index 60555a9..afc3896 100644
--- a/src/flake8/utils.py
+++ b/src/flake8/utils.py
@@ -1,7 +1,6 @@
 """Utility methods for flake8."""
 from __future__ import annotations
 
-import collections
 import fnmatch as _fnmatch
 import functools
 import io
@@ -18,7 +17,6 @@ from typing import Sequence
 
 from flake8 import exceptions
 
-DIFF_HUNK_REGEXP = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@.*$")
 COMMA_SEPARATED_LIST_RE = re.compile(r"[,\s]")
 LOCAL_PLUGIN_LIST_RE = re.compile(r"[,\t\n\r\f\v]")
 NORMALIZE_PACKAGE_NAME_RE = re.compile(r"[-_.]+")
@@ -202,71 +200,6 @@ def stdin_get_lines() -> list[str]:
     return list(io.StringIO(stdin_get_value()))
 
 
-def parse_unified_diff(diff: str | None = None) -> dict[str, set[int]]:
-    """Parse the unified diff passed on stdin.
-
-    :returns:
-        dictionary mapping file names to sets of line numbers
-    """
-    # Allow us to not have to patch out stdin_get_value
-    if diff is None:
-        diff = stdin_get_value()
-
-    number_of_rows = None
-    current_path = None
-    parsed_paths: dict[str, set[int]] = collections.defaultdict(set)
-    for line in diff.splitlines():
-        if number_of_rows:
-            if not line or line[0] != "-":
-                number_of_rows -= 1
-            # We're in the part of the diff that has lines starting with +, -,
-            # and ' ' to show context and the changes made. We skip these
-            # because the information we care about is the filename and the
-            # range within it.
-            # When number_of_rows reaches 0, we will once again start
-            # searching for filenames and ranges.
-            continue
-
-        # NOTE(sigmavirus24): Diffs that we support look roughly like:
-        #    diff a/file.py b/file.py
-        #    ...
-        #    --- a/file.py
-        #    +++ b/file.py
-        # Below we're looking for that last line. Every diff tool that
-        # gives us this output may have additional information after
-        # ``b/file.py`` which it will separate with a \t, e.g.,
-        #    +++ b/file.py\t100644
-        # Which is an example that has the new file permissions/mode.
-        # In this case we only care about the file name.
-        if line[:3] == "+++":
-            current_path = line[4:].split("\t", 1)[0]
-            # NOTE(sigmavirus24): This check is for diff output from git.
-            if current_path[:2] == "b/":
-                current_path = current_path[2:]
-            # We don't need to do anything else. We have set up our local
-            # ``current_path`` variable. We can skip the rest of this loop.
-            # The next line we will see will give us the hung information
-            # which is in the next section of logic.
-            continue
-
-        hunk_match = DIFF_HUNK_REGEXP.match(line)
-        # NOTE(sigmavirus24): pep8/pycodestyle check for:
-        #    line[:3] == '@@ '
-        # But the DIFF_HUNK_REGEXP enforces that the line start with that
-        # So we can more simply check for a match instead of slicing and
-        # comparing.
-        if hunk_match:
-            (row, number_of_rows) = (
-                1 if not group else int(group) for group in hunk_match.groups()
-            )
-            assert current_path is not None
-            parsed_paths[current_path].update(range(row, row + number_of_rows))
-
-    # We have now parsed our diff into a dictionary that looks like:
-    #    {'file.py': set(range(10, 16), range(18, 20)), ...}
-    return parsed_paths
-
-
 def is_using_stdin(paths: list[str]) -> bool:
     """Determine if we're going to read from stdin.
 
diff --git a/src/flake8/violation.py b/src/flake8/violation.py
index 45834b2..96161d4 100644
--- a/src/flake8/violation.py
+++ b/src/flake8/violation.py
@@ -67,36 +67,3 @@ class Violation(NamedTuple):
             "%r is not ignored inline with ``# noqa: %s``", self, codes_str
         )
         return False
-
-    def is_in(self, diff: dict[str, set[int]]) -> bool:
-        """Determine if the violation is included in a diff's line ranges.
-
-        This function relies on the parsed data added via
-        :meth:`~StyleGuide.add_diff_ranges`. If that has not been called and
-        we are not evaluating files in a diff, then this will always return
-        True. If there are diff ranges, then this will return True if the
-        line number in the error falls inside one of the ranges for the file
-        (and assuming the file is part of the diff data). If there are diff
-        ranges, this will return False if the file is not part of the diff
-        data or the line number of the error is not in any of the ranges of
-        the diff.
-
-        :returns:
-            True if there is no diff or if the error is in the diff's line
-            number ranges. False if the error's line number falls outside
-            the diff's line number ranges.
-        """
-        if not diff:
-            return True
-
-        # NOTE(sigmavirus24): The parsed diff will be a defaultdict with
-        # a set as the default value (if we have received it from
-        # flake8.utils.parse_unified_diff). In that case ranges below
-        # could be an empty set (which is False-y) or if someone else
-        # is using this API, it could be None. If we could guarantee one
-        # or the other, we would check for it more explicitly.
-        line_numbers = diff.get(self.filename)
-        if not line_numbers:
-            return False
-
-        return self.line_number in line_numbers


Test patch:
diff --git a/tests/integration/test_main.py b/tests/integration/test_main.py
index dfa0e0b..db60f6b 100644
--- a/tests/integration/test_main.py
+++ b/tests/integration/test_main.py
@@ -13,42 +13,6 @@ from flake8.main import cli
 from flake8.options import config
 
 
-def test_diff_option(tmpdir, capsys):
-    """Ensure that `flake8 --diff` works."""
-    t_py_contents = """\
-import os
-import sys  # unused but not part of diff
-
-print('(to avoid trailing whitespace in test)')
-print('(to avoid trailing whitespace in test)')
-print(os.path.join('foo', 'bar'))
-
-y  # part of the diff and an error
-"""
-
-    diff = """\
-diff --git a/t.py b/t.py
-index d64ac39..7d943de 100644
---- a/t.py
-+++ b/t.py
-@@ -4,3 +4,5 @@ import sys  # unused but not part of diff
- print('(to avoid trailing whitespace in test)')
- print('(to avoid trailing whitespace in test)')
- print(os.path.join('foo', 'bar'))
-+
-+y  # part of the diff and an error
-"""
-
-    with mock.patch.object(utils, "stdin_get_value", return_value=diff):
-        with tmpdir.as_cwd():
-            tmpdir.join("t.py").write(t_py_contents)
-            assert cli.main(["--diff"]) == 1
-
-    out, err = capsys.readouterr()
-    assert out == "t.py:8:1: F821 undefined name 'y'\n"
-    assert err == ""
-
-
 def test_form_feed_line_split(tmpdir, capsys):
     """Test that form feed is treated the same for stdin."""
     src = "x=1\n\f\ny=1\n"
diff --git a/tests/unit/test_checker_manager.py b/tests/unit/test_checker_manager.py
index 32bc9a4..b75bf6d 100644
--- a/tests/unit/test_checker_manager.py
+++ b/tests/unit/test_checker_manager.py
@@ -14,12 +14,7 @@ from flake8.plugins import finder
 
 def style_guide_mock():
     """Create a mock StyleGuide object."""
-    return mock.MagicMock(
-        **{
-            "options.diff": False,
-            "options.jobs": JobsArgument("4"),
-        }
-    )
+    return mock.MagicMock(**{"options.jobs": JobsArgument("4")})
 
 
 def _parallel_checker_manager():
diff --git a/tests/unit/test_discover_files.py b/tests/unit/test_discover_files.py
index edf047d..ca945c2 100644
--- a/tests/unit/test_discover_files.py
+++ b/tests/unit/test_discover_files.py
@@ -125,7 +125,6 @@ def _expand_paths(
     stdin_display_name="stdin",
     filename_patterns=("*.py",),
     exclude=(),
-    is_running_from_diff=False,
 ):
     return set(
         expand_paths(
@@ -133,7 +132,6 @@ def _expand_paths(
             stdin_display_name=stdin_display_name,
             filename_patterns=filename_patterns,
             exclude=exclude,
-            is_running_from_diff=is_running_from_diff,
         )
     )
 
@@ -166,11 +164,3 @@ def test_alternate_stdin_name_is_filtered():
 def test_filename_included_even_if_not_matching_include(tmp_path):
     some_file = str(tmp_path.joinpath("some/file"))
     assert _expand_paths(paths=(some_file,)) == {some_file}
-
-
-def test_diff_filenames_filtered_by_patterns(tmp_path):
-    f1 = str(tmp_path.joinpath("f1"))
-    f2 = str(tmp_path.joinpath("f2.py"))
-
-    ret = _expand_paths(paths=(f1, f2), is_running_from_diff=True)
-    assert ret == {f2}
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index 98a5a5d..82eef63 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -183,44 +183,6 @@ def test_fnmatch(filename, patterns, expected):
     assert utils.fnmatch(filename, patterns) is expected
 
 
-def read_diff_file(filename):
-    """Read the diff file in its entirety."""
-    with open(filename) as fd:
-        content = fd.read()
-    return content
-
-
-SINGLE_FILE_DIFF = read_diff_file("tests/fixtures/diffs/single_file_diff")
-SINGLE_FILE_INFO = {
-    "flake8/utils.py": set(range(75, 83)).union(set(range(84, 94))),
-}
-TWO_FILE_DIFF = read_diff_file("tests/fixtures/diffs/two_file_diff")
-TWO_FILE_INFO = {
-    "flake8/utils.py": set(range(75, 83)).union(set(range(84, 94))),
-    "tests/unit/test_utils.py": set(range(115, 128)),
-}
-MULTI_FILE_DIFF = read_diff_file("tests/fixtures/diffs/multi_file_diff")
-MULTI_FILE_INFO = {
-    "flake8/utils.py": set(range(75, 83)).union(set(range(84, 94))),
-    "tests/unit/test_utils.py": set(range(115, 129)),
-    "tests/fixtures/diffs/single_file_diff": set(range(1, 28)),
-    "tests/fixtures/diffs/two_file_diff": set(range(1, 46)),
-}
-
-
-@pytest.mark.parametrize(
-    "diff, parsed_diff",
-    [
-        (SINGLE_FILE_DIFF, SINGLE_FILE_INFO),
-        (TWO_FILE_DIFF, TWO_FILE_INFO),
-        (MULTI_FILE_DIFF, MULTI_FILE_INFO),
-    ],
-)
-def test_parse_unified_diff(diff, parsed_diff):
-    """Verify that what we parse from a diff matches expectations."""
-    assert utils.parse_unified_diff(diff) == parsed_diff
-
-
 def test_stdin_get_value_crlf():
     """Ensure that stdin is normalized from crlf to lf."""
     stdin = io.TextIOWrapper(io.BytesIO(b"1\r\n2\r\n"), "UTF-8")
diff --git a/tests/unit/test_violation.py b/tests/unit/test_violation.py
index a4a43da..1b4852b 100644
--- a/tests/unit/test_violation.py
+++ b/tests/unit/test_violation.py
@@ -51,22 +51,3 @@ def test_disable_is_inline_ignored():
         assert error.is_inline_ignored(True) is False
 
     assert getline.called is False
-
-
-@pytest.mark.parametrize(
-    "violation_file,violation_line,diff,expected",
-    [
-        ("file.py", 10, {}, True),
-        ("file.py", 1, {"file.py": range(1, 2)}, True),
-        ("file.py", 10, {"file.py": range(1, 2)}, False),
-        ("file.py", 1, {"other.py": range(1, 2)}, False),
-        ("file.py", 10, {"other.py": range(1, 2)}, False),
-    ],
-)
-def test_violation_is_in_diff(violation_file, violation_line, diff, expected):
-    """Verify that we find violations within a diff."""
-    violation = Violation(
-        "E001", violation_file, violation_line, 1, "warning", "line"
-    )
-
-    assert violation.is_in(diff) is expected