Skip to content

feat: stubgen warn some modules are ignored #13919

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.

Sign up for GitHub

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

Open
tsbkw wants to merge 3 commits into
base: master
Choose a base branch
from
Open

feat: stubgen warn some modules are ignored #13919

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1104810
feat: stubgen warn some modules are ignored
tsbkw Oct 19, 2022
aab77a6
fix: Simplify condition, print reason of ignorant
tsbkw Oct 23, 2022
4f44401
test: Fix test case name and add assertions
tsbkw Oct 27, 2022
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
22 changes: 15 additions & 7 deletions mypy/stubgen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1406,10 +1406,13 @@ def get_qualified_name(o: Expression) -> str:
return ERROR_MARKER


def remove_blacklisted_modules(modules: list[StubSource]) -> list[StubSource]:
return [
def remove_blacklisted_modules(
modules: list[StubSource],
) -> tuple[list[StubSource], list[StubSource]]:
blacklisted_removed = [
module for module in modules if module.path is None or not is_blacklisted_path(module.path)
]
return blacklisted_removed, list(set(modules) - set(blacklisted_removed))


def is_blacklisted_path(path: str) -> bool:
Expand All @@ -1424,7 +1427,7 @@ def normalize_path_separators(path: str) -> str:

def collect_build_targets(
options: Options, mypy_opts: MypyOptions
) -> tuple[list[StubSource], list[StubSource]]:
) -> tuple[list[StubSource], list[StubSource], list[StubSource]]:
"""Collect files for which we need to generate stubs.

Return list of Python modules and C modules.
Expand All @@ -1449,9 +1452,9 @@ def collect_build_targets(
py_modules = [StubSource(m.module, m.path) for m in source_list]
c_modules = []

py_modules = remove_blacklisted_modules(py_modules)
py_modules, ignored_py_modules = remove_blacklisted_modules(py_modules)

return py_modules, c_modules
return py_modules, c_modules, ignored_py_modules


def find_module_paths_using_imports(
Expand Down Expand Up @@ -1683,7 +1686,7 @@ def collect_docs_signatures(doc_dir: str) -> tuple[dict[str, str], dict[str, str
def generate_stubs(options: Options) -> None:
"""Main entry point for the program."""
mypy_opts = mypy_options(options)
py_modules, c_modules = collect_build_targets(options, mypy_opts)
py_modules, c_modules, ignored_py_modules = collect_build_targets(options, mypy_opts)
sig_generators = get_sig_generators(options)
# Use parsed sources to generate stubs for Python modules.
generate_asts_for_modules(py_modules, options.parse_only, mypy_opts, options.verbose)
Expand Down Expand Up @@ -1713,12 +1716,17 @@ def generate_stubs(options: Options) -> None:
with generate_guarded(mod.module, target, options.ignore_errors, options.verbose):
generate_stub_for_c_module(mod.module, target, sig_generators=sig_generators)
num_modules = len(py_modules) + len(c_modules)
if not options.quiet and num_modules > 0:
if not options.quiet and (num_modules > 0 or len(ignored_py_modules) > 0):
print("Processed %d modules" % num_modules)
if len(files) == 1:
print(f"Generated {files[0]}")
else:
print(f"Generated files under {common_dir_prefix(files)}" + os.sep)
if ignored_py_modules:
print(f"Ignored {len(ignored_py_modules)} modules which regarded as vendored modules")
if options.verbose:
for ignored in ignored_py_modules:
print(f"\tignored: {ignored.path}")


HEADER = """%(prog)s [-h] [more options, see -h]
Expand Down
27 changes: 23 additions & 4 deletions mypy/test/teststubgen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,10 @@ def test_files_found(self) -> None:
self.make_file("subdir", "b.py")
os.mkdir(os.path.join("subdir", "pack"))
self.make_file("subdir", "pack", "__init__.py")
os.mkdir(os.path.join("subdir", "vendor"))
self.make_file("subdir", "vendor", "a.py")
opts = parse_options(["subdir"])
py_mods, c_mods = collect_build_targets(opts, mypy_options(opts))
py_mods, c_mods, ignored_py_mods = collect_build_targets(opts, mypy_options(opts))
assert_equal(c_mods, [])
files = {mod.path for mod in py_mods}
assert_equal(
Expand All @@ -72,6 +74,8 @@ def test_files_found(self) -> None:
os.path.join("subdir", "b.py"),
},
)
ignored_files = {mod.path for mod in ignored_py_mods}
assert_equal(ignored_files, {os.path.join("subdir", "vendor", "a.py")})
finally:
os.chdir(current)

Expand All @@ -85,8 +89,11 @@ def test_packages_found(self) -> None:
self.make_file("pack", "__init__.py", content="from . import a, b")
self.make_file("pack", "a.py")
self.make_file("pack", "b.py")
os.mkdir(os.path.join("pack", "vendor"))
self.make_file("pack", "vendor", "__init__.py")
self.make_file("pack", "vendor", "a.py")
opts = parse_options(["-p", "pack"])
py_mods, c_mods = collect_build_targets(opts, mypy_options(opts))
py_mods, c_mods, ignored_py_mods = collect_build_targets(opts, mypy_options(opts))
assert_equal(c_mods, [])
files = {os.path.relpath(mod.path or "FAIL") for mod in py_mods}
assert_equal(
Expand All @@ -97,11 +104,19 @@ def test_packages_found(self) -> None:
os.path.join("pack", "b.py"),
},
)
ignored_files = {os.path.relpath(mod.path or "FAIL") for mod in ignored_py_mods}
Copy link
Member

@sobolevn sobolevn Oct 22, 2022

Choose a reason for hiding this comment

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

what is "FAIL"?

Copy link
Author

@tsbkw tsbkw Oct 24, 2022

Choose a reason for hiding this comment

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

In fact I wrote in the same way as files initialized here.

mypy/mypy/test/teststubgen.py

Line 98 in 1104810

files = {os.path.relpath(mod.path or "FAIL") for mod in py_mods}

StubSource.path may be None, so I guess the original code intend to express failure of module finding and also avoiding error which caused by passing None to os.path.relpath.
Maybe "NOT_FOUND" or just skipping mod without mod.path like below is better. Which do you think is better?

{os.path.relpath(mod.path) for mod in ignored_py_mods if mod.path is not None}

assert_equal(
ignored_files,
{
os.path.join("pack", "vendor", "__init__.py"),
os.path.join("pack", "vendor", "a.py"),
},
)
finally:
os.chdir(current)

@unittest.skipIf(sys.platform == "win32", "clean up fails on Windows")
def test_module_not_found(self) -> None:
def test_module_found(self) -> None:
current = os.getcwd()
captured_output = io.StringIO()
sys.stdout = captured_output
Expand All @@ -110,8 +125,12 @@ def test_module_not_found(self) -> None:
os.chdir(tmp)
self.make_file(tmp, "mymodule.py", content="import a")
opts = parse_options(["-m", "mymodule"])
py_mods, c_mods = collect_build_targets(opts, mypy_options(opts))
py_mods, c_mods, ignored_py_mods = collect_build_targets(opts, mypy_options(opts))
Copy link
Member

@sobolevn sobolevn Oct 26, 2022

Choose a reason for hiding this comment

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

ignored_py_mods var is not asserted here.
Do we have something to check here?

Copy link
Author

@tsbkw tsbkw Oct 27, 2022

Choose a reason for hiding this comment

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

Well I checked this test case and found its name test_module_not_found is confusing because module is found at this case.

I confirmed that I didn't make any unintentional changes by checking out master branch of mypy, and modified mypy/test/teststubgen.py around L103 like below and run pytest -k StubgenCmdLine -vvv.
The result of the assert_equal(c_mods, []) is fine but assert_equal(py_mods, []) reports error because mymodule was found.

    @unittest.skipIf(sys.platform == "win32", "clean up fails on Windows")
    def test_module_not_found(self) -> None:
        current = os.getcwd()
        captured_output = io.StringIO()
        sys.stdout = captured_output
        with tempfile.TemporaryDirectory() as tmp:
            try:
                os.chdir(tmp)
                self.make_file(tmp, "mymodule.py", content="import a")
                opts = parse_options(["-m", "mymodule"])
                py_mods, c_mods = collect_build_targets(opts, mypy_options(opts))
                assert captured_output.getvalue() == ""
                assert_equal(c_mods, [])
>               assert_equal(py_mods, [])
E               AssertionError: [<mypy.stubgen.StubSource object at 0x113393190>] != []

I fixed test case name and add assertions for variables py_mods, c_mods, ignored_py_mods by 4f44401.

assert captured_output.getvalue() == ""
files = {os.path.relpath(mod.path or "FAIL") for mod in py_mods}
assert_equal(files, {"mymodule.py"})
assert_equal(c_mods, [])
assert_equal(ignored_py_mods, [])
finally:
sys.stdout = sys.__stdout__
os.chdir(current)
Expand Down