Skip to content

Commit

Permalink
Prevent dmypy from restarting if the configuration contains globs. (#…
Browse files Browse the repository at this point in the history
…7960)

If a configuration contains globs, then dmypy incorrectly detected
that the configration changed even though it did not.
This caused the daemon to always restart if a configuration contained globs.

Fixes #7576.
  • Loading branch information
syastrov authored and msullivan committed Nov 21, 2019
1 parent 4c77d16 commit 989626a
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 14 deletions.
27 changes: 14 additions & 13 deletions mypy/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class Options:

def __init__(self) -> None:
# Cache for clone_for_module()
self.per_module_cache = None # type: Optional[Dict[str, Options]]
self._per_module_cache = None # type: Optional[Dict[str, Options]]

# -- build options --
self.build_type = BuildType.STANDARD
Expand Down Expand Up @@ -222,7 +222,7 @@ def __init__(self) -> None:

# Per-module options (raw)
self.per_module_options = OrderedDict() # type: OrderedDict[str, Dict[str, object]]
self.glob_options = [] # type: List[Tuple[str, Pattern[str]]]
self._glob_options = [] # type: List[Tuple[str, Pattern[str]]]
self.unused_configs = set() # type: Set[str]

# -- development options --
Expand Down Expand Up @@ -280,7 +280,8 @@ def snapshot(self) -> object:
for k in get_class_descriptors(Options):
if hasattr(self, k) and k != "new_semantic_analyzer":
d[k] = getattr(self, k)
del d['per_module_cache']
# Remove private attributes from snapshot
d = {k: v for k, v in d.items() if not k.startswith('_')}
return d

def __repr__(self) -> str:
Expand All @@ -295,7 +296,7 @@ def apply_changes(self, changes: Dict[str, object]) -> 'Options':
return new_options

def build_per_module_cache(self) -> None:
self.per_module_cache = {}
self._per_module_cache = {}

# Config precedence is as follows:
# 1. Concrete section names: foo.bar.baz
Expand All @@ -320,7 +321,7 @@ def build_per_module_cache(self) -> None:
concrete = [k for k in structured_keys if not k.endswith('.*')]

for glob in unstructured_glob_keys:
self.glob_options.append((glob, self.compile_glob(glob)))
self._glob_options.append((glob, self.compile_glob(glob)))

# We (for ease of implementation) treat unstructured glob
# sections as used if any real modules use them or if any
Expand All @@ -333,7 +334,7 @@ def build_per_module_cache(self) -> None:
# on inheriting from parent configs.
options = self.clone_for_module(key)
# And then update it with its per-module options.
self.per_module_cache[key] = options.apply_changes(self.per_module_options[key])
self._per_module_cache[key] = options.apply_changes(self.per_module_options[key])

# Add the more structured sections into unused configs, since
# they only count as used if actually used by a real module.
Expand All @@ -345,14 +346,14 @@ def clone_for_module(self, module: str) -> 'Options':
NOTE: Once this method is called all Options objects should be
considered read-only, else the caching might be incorrect.
"""
if self.per_module_cache is None:
if self._per_module_cache is None:
self.build_per_module_cache()
assert self.per_module_cache is not None
assert self._per_module_cache is not None

# If the module just directly has a config entry, use it.
if module in self.per_module_cache:
if module in self._per_module_cache:
self.unused_configs.discard(module)
return self.per_module_cache[module]
return self._per_module_cache[module]

# If not, search for glob paths at all the parents. So if we are looking for
# options for foo.bar.baz, we search foo.bar.baz.*, foo.bar.*, foo.*,
Expand All @@ -363,15 +364,15 @@ def clone_for_module(self, module: str) -> 'Options':
path = module.split('.')
for i in range(len(path), 0, -1):
key = '.'.join(path[:i] + ['*'])
if key in self.per_module_cache:
if key in self._per_module_cache:
self.unused_configs.discard(key)
options = self.per_module_cache[key]
options = self._per_module_cache[key]
break

# OK and *now* we need to look for unstructured glob matches.
# We only do this for concrete modules, not structured wildcards.
if not module.endswith('.*'):
for key, pattern in self.glob_options:
for key, pattern in self._glob_options:
if pattern.match(module):
self.unused_configs.discard(key)
options = options.apply_changes(self.per_module_options[key])
Expand Down
3 changes: 2 additions & 1 deletion mypy/test/testdaemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

from mypy.test.config import test_temp_dir, PREFIX
from mypy.test.data import DataDrivenTestCase, DataSuite
from mypy.test.helpers import assert_string_arrays_equal
from mypy.test.helpers import assert_string_arrays_equal, normalize_error_messages

# Files containing test cases descriptions.
daemon_files = [
Expand Down Expand Up @@ -40,6 +40,7 @@ def test_daemon(testcase: DataDrivenTestCase) -> None:
cmd = cmd.replace('{python}', sys.executable)
sts, output = run_cmd(cmd)
output_lines = output.splitlines()
output_lines = normalize_error_messages(output_lines)
if sts:
output_lines.append('== Return code: %d' % sts)
assert_string_arrays_equal(expected_lines,
Expand Down
33 changes: 33 additions & 0 deletions test-data/unit/daemon.test
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,39 @@ from mypy.plugin import Plugin
class Dummy(Plugin): pass
def plugin(version): return Dummy

[case testDaemonRunRestartGlobs]
-- Ensure dmypy is not restarted if the configuration doesn't change and it contains globs
-- Note: Backslash path separator in output is replaced with forward slash so the same test succeeds on Windows as well
$ dmypy run -- foo --follow-imports=error --python-version=3.6
Daemon started
foo/lol.py:1: error: Name 'fail' is not defined
Found 1 error in 1 file (checked 3 source files)
== Return code: 1
$ dmypy run -- foo --follow-imports=error --python-version=3.6
foo/lol.py:1: error: Name 'fail' is not defined
Found 1 error in 1 file (checked 3 source files)
== Return code: 1
$ {python} -c "print('[mypy]')" >mypy.ini
$ {python} -c "print('ignore_errors=True')" >>mypy.ini
$ dmypy run -- foo --follow-imports=error --python-version=3.6
Restarting: configuration changed
Daemon stopped
Daemon started
Success: no issues found in 3 source files
$ dmypy stop
Daemon stopped
[file mypy.ini]
\[mypy]
ignore_errors = True
\[mypy-*.lol]
ignore_errors = False

[file foo/__init__.py]
[file foo/lol.py]
fail
[file foo/ok.py]
a: int = 1

[case testDaemonStatusKillRestartRecheck]
$ dmypy status
No status file found
Expand Down

0 comments on commit 989626a

Please sign in to comment.