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

Fix file reloading in dmypy with --export-types #16359

Merged
merged 5 commits into from
Nov 1, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 45 additions & 7 deletions mypy/dmypy_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,15 +393,21 @@ def cmd_recheck(
t1 = time.time()
manager = self.fine_grained_manager.manager
manager.log(f"fine-grained increment: cmd_recheck: {t1 - t0:.3f}s")
self.options.export_types = export_types
old_export_types = self.options.export_types
self.options.export_types = self.options.export_types or export_types
if not self.following_imports():
messages = self.fine_grained_increment(sources, remove, update)
messages = self.fine_grained_increment(
sources, remove, update, explicit_export_types=export_types
)
else:
assert remove is None and update is None
messages = self.fine_grained_increment_follow_imports(sources)
messages = self.fine_grained_increment_follow_imports(
sources, explicit_export_types=export_types
)
res = self.increment_output(messages, sources, is_tty, terminal_width)
self.flush_caches()
self.update_stats(res)
self.options.export_types = old_export_types
return res

def check(
Expand All @@ -412,17 +418,21 @@ def check(
If is_tty is True format the output nicely with colors and summary line
(unless disabled in self.options). Also pass the terminal_width to formatter.
"""
self.options.export_types = export_types
old_export_types = self.options.export_types
self.options.export_types = self.options.export_types or export_types
if not self.fine_grained_manager:
res = self.initialize_fine_grained(sources, is_tty, terminal_width)
else:
if not self.following_imports():
messages = self.fine_grained_increment(sources)
messages = self.fine_grained_increment(sources, explicit_export_types=export_types)
else:
messages = self.fine_grained_increment_follow_imports(sources)
messages = self.fine_grained_increment_follow_imports(
sources, explicit_export_types=export_types
)
res = self.increment_output(messages, sources, is_tty, terminal_width)
self.flush_caches()
self.update_stats(res)
self.options.export_types = old_export_types
return res

def flush_caches(self) -> None:
Expand Down Expand Up @@ -535,6 +545,7 @@ def fine_grained_increment(
sources: list[BuildSource],
remove: list[str] | None = None,
update: list[str] | None = None,
explicit_export_types: bool = False,
) -> list[str]:
"""Perform a fine-grained type checking increment.

Expand All @@ -545,6 +556,8 @@ def fine_grained_increment(
sources: sources passed on the command line
remove: paths of files that have been removed
update: paths of files that have been changed or created
explicit_export_types: --export-type was passed in a check command
(as opposite to being set in dmypy start)
"""
assert self.fine_grained_manager is not None
manager = self.fine_grained_manager.manager
Expand All @@ -559,6 +572,10 @@ def fine_grained_increment(
# Use the remove/update lists to update fswatcher.
# This avoids calling stat() for unchanged files.
changed, removed = self.update_changed(sources, remove or [], update or [])
if explicit_export_types:
# If --export-types is given, we need to force full re-checking of all
# explicitly passed files, since we need to visit each expression.
add_all_sources_to_changed(sources, changed)
changed += self.find_added_suppressed(
self.fine_grained_manager.graph, set(), manager.search_paths
)
Expand All @@ -577,7 +594,9 @@ def fine_grained_increment(
self.previous_sources = sources
return messages

def fine_grained_increment_follow_imports(self, sources: list[BuildSource]) -> list[str]:
def fine_grained_increment_follow_imports(
self, sources: list[BuildSource], explicit_export_types: bool = False
) -> list[str]:
"""Like fine_grained_increment, but follow imports."""
t0 = time.time()

Expand All @@ -603,6 +622,9 @@ def fine_grained_increment_follow_imports(self, sources: list[BuildSource]) -> l
changed, new_files = self.find_reachable_changed_modules(
sources, graph, seen, changed_paths
)
if explicit_export_types:
# Same as in fine_grained_increment().
add_all_sources_to_changed(sources, changed)
sources.extend(new_files)

# Process changes directly reachable from roots.
Expand Down Expand Up @@ -1011,6 +1033,22 @@ def find_all_sources_in_build(
return result


def add_all_sources_to_changed(sources: list[BuildSource], changed: list[tuple[str, str]]) -> None:
"""Add all (explicit) sources to the list changed files in place.

Use this when re-processing of unchanged files is needed (e.g. for
the purpose of exporting types for inspections).
"""
changed_set = set(changed)
changed.extend(
[
(bs.module, bs.path)
for bs in sources
if bs.path and (bs.module, bs.path) not in changed_set
]
)


def fix_module_deps(graph: mypy.build.Graph) -> None:
"""After an incremental update, update module dependencies to reflect the new state.

Expand Down
3 changes: 2 additions & 1 deletion mypy/test/testfinegrained.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ def get_options(self, source: str, testcase: DataDrivenTestCase, build_cache: bo
options.use_fine_grained_cache = self.use_cache and not build_cache
options.cache_fine_grained = self.use_cache
options.local_partial_types = True
options.export_types = "inspect" in testcase.file
# Treat empty bodies safely for these test cases.
options.allow_empty_bodies = not testcase.name.endswith("_no_empty")
if re.search("flags:.*--follow-imports", source) is None:
Expand All @@ -163,7 +164,7 @@ def get_options(self, source: str, testcase: DataDrivenTestCase, build_cache: bo
return options

def run_check(self, server: Server, sources: list[BuildSource]) -> list[str]:
response = server.check(sources, export_types=True, is_tty=False, terminal_width=-1)
response = server.check(sources, export_types=False, is_tty=False, terminal_width=-1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm why do we have export_types=False here, but true above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Passing it explicitly on each run will force reloading files. Putting it in options from the start is IMO cleaner.

out = response["out"] or response["err"]
assert isinstance(out, str)
return out.splitlines()
Expand Down
27 changes: 27 additions & 0 deletions test-data/unit/daemon.test
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,33 @@ def bar() -> None:
x = foo('abc') # type: str
foo(arg='xyz')

[case testDaemonInspectCheck]
$ dmypy start
Daemon started
$ dmypy check foo.py
Success: no issues found in 1 source file
$ dmypy check foo.py --export-types
Success: no issues found in 1 source file
$ dmypy inspect foo.py:1:1
"int"
[file foo.py]
x = 1

[case testDaemonInspectRun]
$ dmypy run test1.py
Daemon started
Success: no issues found in 1 source file
$ dmypy run test2.py
Success: no issues found in 1 source file
$ dmypy run test1.py --export-types
Success: no issues found in 1 source file
$ dmypy inspect test1.py:1:1
"int"
[file test1.py]
a: int
[file test2.py]
a: str

[case testDaemonGetType]
$ dmypy start --log-file log.txt -- --follow-imports=error --no-error-summary --python-version 3.8
Daemon started
Expand Down