Skip to content

Commit

Permalink
Fix couple crashes in dmypy (#18098)
Browse files Browse the repository at this point in the history
Fixes #18019
Fixes #17775

These two are essentially variations of the same thing. Instead of
adding e.g. `types` to `SENSITIVE_INTERNAL_MODULES` (which would be
fragile and re-introduce same crashes whenever we add a new "core"
module) I add _all stdlib modules_. The only scenario when stdlib
changes is when a version of mypy changes, and in this case the daemon
will be (or should be) restarted anyway.

While adding tests for these I noticed a discrepancy in
`--follow-imports=normal` in the daemon: the files explicitly added on
the command line should be always treated as changed, since otherwise we
will not detect errors if a file was removed from command line in an
intermediate run.

Finally the tests also discovered a spurious error when cache is
disabled (via `/dev/null`).
  • Loading branch information
ilevkivskyi authored Nov 4, 2024
1 parent 1de4871 commit 976d105
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 21 deletions.
5 changes: 4 additions & 1 deletion mypy/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -1050,7 +1050,10 @@ def generate_deps_for_cache(manager: BuildManager, graph: Graph) -> dict[str, di
def write_plugins_snapshot(manager: BuildManager) -> None:
"""Write snapshot of versions and hashes of currently active plugins."""
snapshot = json_dumps(manager.plugins_snapshot)
if not manager.metastore.write(PLUGIN_SNAPSHOT_FILE, snapshot):
if (
not manager.metastore.write(PLUGIN_SNAPSHOT_FILE, snapshot)
and manager.options.cache_dir != os.devnull
):
manager.errors.set_file(_cache_dir_prefix(manager.options), None, manager.options)
manager.errors.report(0, 0, "Error writing plugins snapshot", blocker=True)

Expand Down
29 changes: 18 additions & 11 deletions mypy/dmypy_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,8 @@ def fine_grained_increment_follow_imports(
changed, new_files = self.find_reachable_changed_modules(
sources, graph, seen, changed_paths
)
# Same as in fine_grained_increment().
self.add_explicitly_new(sources, changed)
if explicit_export_types:
# Same as in fine_grained_increment().
add_all_sources_to_changed(sources, changed)
Expand Down Expand Up @@ -888,6 +890,22 @@ def _find_changed(
assert path
removed.append((source.module, path))

self.add_explicitly_new(sources, changed)

# Find anything that has had its module path change because of added or removed __init__s
last = {s.path: s.module for s in self.previous_sources}
for s in sources:
assert s.path
if s.path in last and last[s.path] != s.module:
# Mark it as removed from its old name and changed at its new name
removed.append((last[s.path], s.path))
changed.append((s.module, s.path))

return changed, removed

def add_explicitly_new(
self, sources: list[BuildSource], changed: list[tuple[str, str]]
) -> None:
# Always add modules that were (re-)added, since they may be detected as not changed by
# fswatcher (if they were actually not changed), but they may still need to be checked
# in case they had errors before they were deleted from sources on previous runs.
Expand All @@ -903,17 +921,6 @@ def _find_changed(
]
)

# Find anything that has had its module path change because of added or removed __init__s
last = {s.path: s.module for s in self.previous_sources}
for s in sources:
assert s.path
if s.path in last and last[s.path] != s.module:
# Mark it as removed from its old name and changed at its new name
removed.append((last[s.path], s.path))
changed.append((s.module, s.path))

return changed, removed

def cmd_inspect(
self,
show: str,
Expand Down
2 changes: 1 addition & 1 deletion mypy/metastore.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class MetadataStore:

@abstractmethod
def getmtime(self, name: str) -> float:
"""Read the mtime of a metadata entry..
"""Read the mtime of a metadata entry.
Raises FileNotFound if the entry does not exist.
"""
Expand Down
16 changes: 8 additions & 8 deletions mypy/server/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,7 @@
TypeInfo,
)
from mypy.options import Options
from mypy.semanal_main import (
core_modules,
semantic_analysis_for_scc,
semantic_analysis_for_targets,
)
from mypy.semanal_main import semantic_analysis_for_scc, semantic_analysis_for_targets
from mypy.server.astdiff import (
SymbolSnapshot,
compare_symbol_table_snapshots,
Expand All @@ -162,11 +158,12 @@
from mypy.server.target import trigger_to_target
from mypy.server.trigger import WILDCARD_TAG, make_trigger
from mypy.typestate import type_state
from mypy.util import module_prefix, split_target
from mypy.util import is_stdlib_file, module_prefix, split_target

MAX_ITER: Final = 1000

SENSITIVE_INTERNAL_MODULES = tuple(core_modules) + ("mypy_extensions", "typing_extensions")
# These are modules beyond stdlib that have some special meaning for mypy.
SENSITIVE_INTERNAL_MODULES = ("mypy_extensions", "typing_extensions")


class FineGrainedBuildManager:
Expand Down Expand Up @@ -406,7 +403,10 @@ def update_module(
# builtins and friends could potentially get triggered because
# of protocol stuff, but nothing good could possibly come from
# actually updating them.
if module in SENSITIVE_INTERNAL_MODULES:
if (
is_stdlib_file(self.manager.options.abs_custom_typeshed_dir, path)
or module in SENSITIVE_INTERNAL_MODULES
):
return [], (module, path), None

manager = self.manager
Expand Down
12 changes: 12 additions & 0 deletions mypy/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,18 @@ def is_typeshed_file(typeshed_dir: str | None, file: str) -> bool:
return False


def is_stdlib_file(typeshed_dir: str | None, file: str) -> bool:
if "stdlib" not in file:
# Fast path
return False
typeshed_dir = typeshed_dir if typeshed_dir is not None else TYPESHED_DIR
stdlib_dir = os.path.join(typeshed_dir, "stdlib")
try:
return os.path.commonpath((stdlib_dir, os.path.abspath(file))) == stdlib_dir
except ValueError: # Different drives on Windows
return False


def is_stub_package_file(file: str) -> bool:
# Use hacky heuristics to check whether file is part of a PEP 561 stub package.
if not file.endswith(".pyi"):
Expand Down
32 changes: 32 additions & 0 deletions test-data/unit/daemon.test
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,38 @@ mypy-daemon: error: Missing target module, package, files, or command.
$ dmypy stop
Daemon stopped

[case testDaemonRunTwoFilesFullTypeshed]
$ dmypy run x.py
Daemon started
Success: no issues found in 1 source file
$ dmypy run y.py
Success: no issues found in 1 source file
$ dmypy run x.py
Success: no issues found in 1 source file
[file x.py]
[file y.py]

[case testDaemonCheckTwoFilesFullTypeshed]
$ dmypy start
Daemon started
$ dmypy check foo.py
foo.py:3: error: Incompatible types in assignment (expression has type "str", variable has type "int") [assignment]
Found 1 error in 1 file (checked 1 source file)
== Return code: 1
$ dmypy check bar.py
Success: no issues found in 1 source file
$ dmypy check foo.py
foo.py:3: error: Incompatible types in assignment (expression has type "str", variable has type "int") [assignment]
Found 1 error in 1 file (checked 1 source file)
== Return code: 1
[file foo.py]
from bar import add
x: str = add("a", "b")
x_error: int = add("a", "b")
[file bar.py]
def add(a, b) -> str:
return a + b

[case testDaemonWarningSuccessExitCode-posix]
$ dmypy run -- foo.py --follow-imports=error --python-version=3.11
Daemon started
Expand Down

0 comments on commit 976d105

Please sign in to comment.