Skip to content

Commit

Permalink
Merge pull request #1720 from PyCQA/remove-diff
Browse files Browse the repository at this point in the history
remove --diff option
  • Loading branch information
asottile committed Oct 27, 2022
2 parents 86268cb + fba6df8 commit 4272ade
Show file tree
Hide file tree
Showing 18 changed files with 11 additions and 526 deletions.
8 changes: 0 additions & 8 deletions docs/source/internal/utils.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
23 changes: 0 additions & 23 deletions docs/source/user/options.rst
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ Index of Options

- :option:`flake8 --count`

- :option:`flake8 --diff`

- :option:`flake8 --exclude`

- :option:`flake8 --filename`
Expand Down Expand Up @@ -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>`
Expand Down
1 change: 0 additions & 1 deletion src/flake8/api/legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ def excluded(path: str) -> bool:
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
Expand Down
10 changes: 0 additions & 10 deletions src/flake8/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ def _job_count(self) -> int:
# 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.
Expand All @@ -116,13 +114,6 @@ def _job_count(self) -> int:
)
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
Expand Down Expand Up @@ -169,7 +160,6 @@ def make_checkers(self, paths: list[str] | None = None) -> None:
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]
Expand Down
25 changes: 8 additions & 17 deletions src/flake8/discover_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -75,24 +74,16 @@ def is_excluded(arg: str) -> bool:
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)
)
)
23 changes: 1 addition & 22 deletions src/flake8/main/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
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
Expand Down Expand Up @@ -66,9 +65,6 @@ def __init__(self) -> None:
#: 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]]:
Expand Down Expand Up @@ -158,13 +154,6 @@ def parse_configuration_and_cli(
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:
Expand Down Expand Up @@ -194,9 +183,6 @@ def make_guide(self) -> None:
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
Expand All @@ -213,16 +199,9 @@ def run_checks(self) -> None:
: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:
Expand Down
8 changes: 0 additions & 8 deletions src/flake8/main/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ def register_default_options(option_manager: OptionManager) -> None:
- ``-q``/``--quiet``
- ``--color``
- ``--count``
- ``--diff``
- ``--exclude``
- ``--extend-exclude``
- ``--filename``
Expand Down Expand Up @@ -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",
Expand Down
27 changes: 1 addition & 26 deletions src/flake8/style_guide.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,18 +298,6 @@ def handle_error(
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."""
Expand All @@ -333,7 +321,6 @@ def __init__(
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."""
Expand Down Expand Up @@ -440,20 +427,8 @@ def handle_error(
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
67 changes: 0 additions & 67 deletions src/flake8/utils.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
"""Utility methods for flake8."""
from __future__ import annotations

import collections
import fnmatch as _fnmatch
import functools
import io
Expand All @@ -18,7 +17,6 @@

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"[-_.]+")
Expand Down Expand Up @@ -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.
Expand Down
33 changes: 0 additions & 33 deletions src/flake8/violation.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,36 +67,3 @@ def is_inline_ignored(self, disable_noqa: bool) -> bool:
"%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

0 comments on commit 4272ade

Please sign in to comment.