Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support --exclude more than once on command line #11329

Merged
merged 1 commit into from
Oct 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/source/command_line.rst
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ for full details, see :ref:`running-mypy`.
pass ``--exclude '/setup\.py$'``. Similarly, you can ignore discovering
directories with a given name by e.g. ``--exclude /build/`` or
those matching a subpath with ``--exclude /project/vendor/``. To ignore
multiple files / directories / paths, you can combine expressions with
``|``, e.g ``--exclude '/setup\.py$|/build/'``.
multiple files / directories / paths, you can provide the --exclude
flag more than once, e.g ``--exclude '/setup\.py$' --exclude '/build/'``.

Note that this flag only affects recursive directory tree discovery, that
is, when mypy is discovering files within a directory tree or submodules of
Expand Down
11 changes: 9 additions & 2 deletions docs/source/config_file.rst
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,19 @@ section of the command line docs.

.. confval:: exclude

:type: regular expression
:type: newline separated list of regular expressions

A regular expression that matches file names, directory names and paths
A newline list of regular expression that matches file names, directory names and paths
which mypy should ignore while recursively discovering files to check.
Use forward slashes on all platforms.

.. code-block:: ini

[mypy]
exclude =
^file1\.py$
^file2\.py$

For more details, see :option:`--exclude <mypy --exclude>`.

This option may only be set in the global section (``[mypy]``).
Expand Down
1 change: 1 addition & 0 deletions mypy/config_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ def check_follow_imports(choice: str) -> str:
'cache_dir': expand_path,
'python_executable': expand_path,
'strict': bool,
'exclude': lambda s: [p.strip() for p in s.split('\n') if p.strip()],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible root cause for #11825?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - seems likely. Good find. The config_parser uses very basic newlines to split apart different excludes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior is documented in the config_file.rst - about multiple patterns in a newline separated manner.

Didn't realize that multiline excludes like this were an old pattern
Suggestion on what to do to fix?

Copy link
Contributor

@posita posita Dec 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't realize that multiline excludes like this were an old pattern

To be fair, I don't know if that was ever official. It was more of a work-around that folks discovered because they (probably like you) were desperately seeking a fix for #10310.

config_parser.py is new to me, and I'm still trying to digest it. At a high level, I'm wondering if one can treat .toml files differently? It looks like there's a mechanism to do that, but I'm still trying to figure out if it's sufficient for what I have in mind, i.e., in a .toml file, if it's a string, just parse it as a single regex, but if it's an array, treat each item as its own regex? (Just thinking out loud here.)

Copy link
Contributor

@posita posita Dec 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking further, I think this might be as simple as appending the following here:

toml_config_types.update({
    # …
    'exclude': try_split,
})

UPDATE: Nope. No. That would still try to split strings along commas.

Dunno what testing looks like yet, though….

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I know you, you're probably ¾ done with a PR by now, but if you haven't started, I can take a stab….

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't started. Go for it!
Appreciate the compliment :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't looked at the details, but there's also this PR that was opened recently: #11746

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads-up @hauntsaninja! I don't think that one works. I'll comment there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#11828 is up. I commented on #11746.

}

# Reuse the ini_config_types and overwrite the diff
Expand Down
6 changes: 4 additions & 2 deletions mypy/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -864,11 +864,13 @@ def add_invertible_flag(flag: str,
group=code_group)
code_group.add_argument(
"--exclude",
action="append",
metavar="PATTERN",
default="",
default=[],
help=(
"Regular expression to match file names, directory names or paths which mypy should "
"ignore while recursively discovering files to check, e.g. --exclude '/setup\\.py$'"
"ignore while recursively discovering files to check, e.g. --exclude '/setup\\.py$'. "
"May be specified more than once, eg. --exclude a --exclude b"
)
)
code_group.add_argument(
Expand Down
17 changes: 11 additions & 6 deletions mypy/modulefinder.py
Original file line number Diff line number Diff line change
Expand Up @@ -500,16 +500,21 @@ def find_modules_recursive(self, module: str) -> List[BuildSource]:
return sources


def matches_exclude(subpath: str, exclude: str, fscache: FileSystemCache, verbose: bool) -> bool:
if not exclude:
def matches_exclude(subpath: str,
excludes: List[str],
fscache: FileSystemCache,
verbose: bool) -> bool:
if not excludes:
return False
subpath_str = os.path.relpath(subpath).replace(os.sep, "/")
if fscache.isdir(subpath):
subpath_str += "/"
if re.search(exclude, subpath_str):
if verbose:
print("TRACE: Excluding {}".format(subpath_str), file=sys.stderr)
return True
for exclude in excludes:
if re.search(exclude, subpath_str):
if verbose:
print("TRACE: Excluding {} (matches pattern {})".format(subpath_str, exclude),
file=sys.stderr)
return True
return False


Expand Down
2 changes: 1 addition & 1 deletion mypy/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def __init__(self) -> None:
# top-level __init__.py to your packages.
self.explicit_package_bases = False
# File names, directory names or subpaths to avoid checking
self.exclude: str = ""
self.exclude: List[str] = []

# disallow_any options
self.disallow_any_generics = False
Expand Down
57 changes: 31 additions & 26 deletions mypy/test/test_find_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ def test_find_sources_exclude(self) -> None:
}

# file name
options.exclude = r"/f\.py$"
options.exclude = [r"/f\.py$"]
fscache = FakeFSCache(files)
assert find_sources(["/"], options, fscache) == [
("a2", "/pkg"),
Expand All @@ -309,7 +309,7 @@ def test_find_sources_exclude(self) -> None:
assert find_sources(["/pkg/a2/b/f.py"], options, fscache) == [('a2.b.f', '/pkg')]

# directory name
options.exclude = "/a1/"
options.exclude = ["/a1/"]
fscache = FakeFSCache(files)
assert find_sources(["/"], options, fscache) == [
("a2", "/pkg"),
Expand All @@ -323,13 +323,13 @@ def test_find_sources_exclude(self) -> None:
with pytest.raises(InvalidSourceList):
find_sources(["/pkg/a1/b"], options, fscache)

options.exclude = "/a1/$"
options.exclude = ["/a1/$"]
assert find_sources(["/pkg/a1"], options, fscache) == [
('e', '/pkg/a1/b/c/d'), ('f', '/pkg/a1/b')
]

# paths
options.exclude = "/pkg/a1/"
options.exclude = ["/pkg/a1/"]
fscache = FakeFSCache(files)
assert find_sources(["/"], options, fscache) == [
("a2", "/pkg"),
Expand All @@ -339,15 +339,17 @@ def test_find_sources_exclude(self) -> None:
with pytest.raises(InvalidSourceList):
find_sources(["/pkg/a1"], options, fscache)

options.exclude = "/(a1|a3)/"
fscache = FakeFSCache(files)
assert find_sources(["/"], options, fscache) == [
("a2", "/pkg"),
("a2.b.c.d.e", "/pkg"),
("a2.b.f", "/pkg"),
]
# OR two patterns together
for orred in [["/(a1|a3)/"], ["a1", "a3"], ["a3", "a1"]]:
options.exclude = orred
fscache = FakeFSCache(files)
assert find_sources(["/"], options, fscache) == [
("a2", "/pkg"),
("a2.b.c.d.e", "/pkg"),
("a2.b.f", "/pkg"),
]

options.exclude = "b/c/"
options.exclude = ["b/c/"]
fscache = FakeFSCache(files)
assert find_sources(["/"], options, fscache) == [
("a2", "/pkg"),
Expand All @@ -356,19 +358,22 @@ def test_find_sources_exclude(self) -> None:
]

# nothing should be ignored as a result of this
options.exclude = "|".join((
big_exclude1 = [
"/pkg/a/", "/2", "/1", "/pk/", "/kg", "/g.py", "/bc", "/xxx/pkg/a2/b/f.py"
"xxx/pkg/a2/b/f.py",
))
fscache = FakeFSCache(files)
assert len(find_sources(["/"], options, fscache)) == len(files)

files = {
"pkg/a1/b/c/d/e.py",
"pkg/a1/b/f.py",
"pkg/a2/__init__.py",
"pkg/a2/b/c/d/e.py",
"pkg/a2/b/f.py",
}
fscache = FakeFSCache(files)
assert len(find_sources(["."], options, fscache)) == len(files)
]
big_exclude2 = ["|".join(big_exclude1)]
for big_exclude in [big_exclude1, big_exclude2]:
options.exclude = big_exclude
fscache = FakeFSCache(files)
assert len(find_sources(["/"], options, fscache)) == len(files)

files = {
"pkg/a1/b/c/d/e.py",
"pkg/a1/b/f.py",
"pkg/a2/__init__.py",
"pkg/a2/b/c/d/e.py",
"pkg/a2/b/f.py",
}
fscache = FakeFSCache(files)
assert len(find_sources(["."], options, fscache)) == len(files)
54 changes: 54 additions & 0 deletions test-data/unit/cmdline.test
Original file line number Diff line number Diff line change
Expand Up @@ -1309,3 +1309,57 @@ pkg.py:1: error: "int" not callable
1()
[out]
pkg.py:1: error: "int" not callable

[case testCmdlineExclude]
# cmd: mypy --exclude abc .
[file abc/apkg.py]
1()
[file b/bpkg.py]
1()
[file c/cpkg.py]
1()
[out]
c/cpkg.py:1: error: "int" not callable
b/bpkg.py:1: error: "int" not callable

[case testCmdlineMultipleExclude]
# cmd: mypy --exclude abc --exclude b/ .
[file abc/apkg.py]
1()
[file b/bpkg.py]
1()
[file c/cpkg.py]
1()
[out]
c/cpkg.py:1: error: "int" not callable

[case testCmdlineCfgExclude]
# cmd: mypy .
[file mypy.ini]
\[mypy]
exclude = abc
[file abc/apkg.py]
1()
[file b/bpkg.py]
1()
[file c/cpkg.py]
1()
[out]
c/cpkg.py:1: error: "int" not callable
b/bpkg.py:1: error: "int" not callable

[case testCmdlineCfgMultipleExclude]
# cmd: mypy .
[file mypy.ini]
\[mypy]
exclude =
abc
b
[file abc/apkg.py]
1()
[file b/bpkg.py]
1()
[file c/cpkg.py]
1()
[out]
c/cpkg.py:1: error: "int" not callable