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

Don't ignore errors in files passed on the command line #14060

Merged
merged 4 commits into from Nov 10, 2022
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
10 changes: 7 additions & 3 deletions mypy/build.py
Expand Up @@ -1940,7 +1940,7 @@ def __init__(
raise
if follow_imports == "silent":
self.ignore_all = True
elif path and is_silent_import_module(manager, path):
elif path and is_silent_import_module(manager, path) and not root_source:
self.ignore_all = True
self.path = path
if path:
Expand Down Expand Up @@ -2629,7 +2629,7 @@ def find_module_and_diagnose(
else:
skipping_module(manager, caller_line, caller_state, id, result)
raise ModuleNotFound
if is_silent_import_module(manager, result):
if is_silent_import_module(manager, result) and not root_source:
follow_imports = "silent"
return (result, follow_imports)
else:
Expand Down Expand Up @@ -3024,7 +3024,11 @@ def load_graph(
for bs in sources:
try:
st = State(
id=bs.module, path=bs.path, source=bs.text, manager=manager, root_source=True
id=bs.module,
path=bs.path,
source=bs.text,
manager=manager,
root_source=not bs.followed,
)
except ModuleNotFound:
continue
Expand Down
14 changes: 7 additions & 7 deletions mypy/dmypy_server.py
Expand Up @@ -592,7 +592,7 @@ def fine_grained_increment_follow_imports(self, sources: list[BuildSource]) -> l
sources.extend(new_files)

# Process changes directly reachable from roots.
messages = fine_grained_manager.update(changed, [])
messages = fine_grained_manager.update(changed, [], followed=True)

# Follow deps from changed modules (still within graph).
worklist = changed[:]
Expand All @@ -609,13 +609,13 @@ def fine_grained_increment_follow_imports(self, sources: list[BuildSource]) -> l
sources2, graph, seen, changed_paths
)
self.update_sources(new_files)
messages = fine_grained_manager.update(changed, [])
messages = fine_grained_manager.update(changed, [], followed=True)
worklist.extend(changed)

t2 = time.time()

def refresh_file(module: str, path: str) -> list[str]:
return fine_grained_manager.update([(module, path)], [])
return fine_grained_manager.update([(module, path)], [], followed=True)

for module_id, state in list(graph.items()):
new_messages = refresh_suppressed_submodules(
Expand All @@ -632,10 +632,10 @@ def refresh_file(module: str, path: str) -> list[str]:
new_unsuppressed = self.find_added_suppressed(graph, seen, manager.search_paths)
if not new_unsuppressed:
break
new_files = [BuildSource(mod[1], mod[0]) for mod in new_unsuppressed]
new_files = [BuildSource(mod[1], mod[0], followed=True) for mod in new_unsuppressed]
sources.extend(new_files)
self.update_sources(new_files)
messages = fine_grained_manager.update(new_unsuppressed, [])
messages = fine_grained_manager.update(new_unsuppressed, [], followed=True)

for module_id, path in new_unsuppressed:
new_messages = refresh_suppressed_submodules(
Expand Down Expand Up @@ -717,15 +717,15 @@ def find_reachable_changed_modules(
for dep in state.dependencies:
if dep not in seen:
seen.add(dep)
worklist.append(BuildSource(graph[dep].path, graph[dep].id))
worklist.append(BuildSource(graph[dep].path, graph[dep].id, followed=True))
return changed, new_files

def direct_imports(
self, module: tuple[str, str], graph: mypy.build.Graph
) -> list[BuildSource]:
"""Return the direct imports of module not included in seen."""
state = graph[module[0]]
return [BuildSource(graph[dep].path, dep) for dep in state.dependencies]
return [BuildSource(graph[dep].path, dep, followed=True) for dep in state.dependencies]

def find_added_suppressed(
self, graph: mypy.build.Graph, seen: set[str], search_paths: SearchPaths
Expand Down
8 changes: 6 additions & 2 deletions mypy/modulefinder.py
Expand Up @@ -115,15 +115,19 @@ def __init__(
module: str | None,
text: str | None = None,
base_dir: str | None = None,
followed: bool = False,
) -> None:
self.path = path # File where it's found (e.g. 'xxx/yyy/foo/bar.py')
self.module = module or "__main__" # Module name (e.g. 'foo.bar')
self.text = text # Source code, if initially supplied, else None
self.base_dir = base_dir # Directory where the package is rooted (e.g. 'xxx/yyy')
self.followed = followed # Was this found by following imports?

def __repr__(self) -> str:
return "BuildSource(path={!r}, module={!r}, has_text={}, base_dir={!r})".format(
self.path, self.module, self.text is not None, self.base_dir
return (
"BuildSource(path={!r}, module={!r}, has_text={}, base_dir={!r}, followed={})".format(
self.path, self.module, self.text is not None, self.base_dir, self.followed
)
)


Expand Down
28 changes: 20 additions & 8 deletions mypy/server/update.py
Expand Up @@ -203,7 +203,10 @@ def __init__(self, result: BuildResult) -> None:
self.processed_targets: list[str] = []

def update(
self, changed_modules: list[tuple[str, str]], removed_modules: list[tuple[str, str]]
self,
changed_modules: list[tuple[str, str]],
removed_modules: list[tuple[str, str]],
followed: bool = False,
) -> list[str]:
"""Update previous build result by processing changed modules.

Expand All @@ -219,6 +222,7 @@ def update(
Assume this is correct; it's not validated here.
removed_modules: Modules that have been deleted since the previous update
or removed from the build.
followed: If True, the modules were found through following imports

Returns:
A list of errors.
Expand Down Expand Up @@ -256,7 +260,9 @@ def update(
self.blocking_error = None

while True:
result = self.update_one(changed_modules, initial_set, removed_set, blocking_error)
result = self.update_one(
changed_modules, initial_set, removed_set, blocking_error, followed
)
changed_modules, (next_id, next_path), blocker_messages = result

if blocker_messages is not None:
Expand Down Expand Up @@ -329,6 +335,7 @@ def update_one(
initial_set: set[str],
removed_set: set[str],
blocking_error: str | None,
followed: bool,
) -> tuple[list[tuple[str, str]], tuple[str, str], list[str] | None]:
"""Process a module from the list of changed modules.

Expand All @@ -355,7 +362,7 @@ def update_one(
)
return changed_modules, (next_id, next_path), None

result = self.update_module(next_id, next_path, next_id in removed_set)
result = self.update_module(next_id, next_path, next_id in removed_set, followed)
remaining, (next_id, next_path), blocker_messages = result
changed_modules = [(id, path) for id, path in changed_modules if id != next_id]
changed_modules = dedupe_modules(remaining + changed_modules)
Expand All @@ -368,7 +375,7 @@ def update_one(
return changed_modules, (next_id, next_path), blocker_messages

def update_module(
self, module: str, path: str, force_removed: bool
self, module: str, path: str, force_removed: bool, followed: bool
) -> tuple[list[tuple[str, str]], tuple[str, str], list[str] | None]:
"""Update a single modified module.

Expand All @@ -380,6 +387,7 @@ def update_module(
path: File system path of the module
force_removed: If True, consider module removed from the build even if path
exists (used for removing an existing file from the build)
followed: Was this found via import following?

Returns:
Tuple with these items:
Expand Down Expand Up @@ -417,7 +425,7 @@ def update_module(
manager.errors.reset()
self.processed_targets.append(module)
result = update_module_isolated(
module, path, manager, previous_modules, graph, force_removed
module, path, manager, previous_modules, graph, force_removed, followed
)
if isinstance(result, BlockedUpdate):
# Blocking error -- just give up
Expand Down Expand Up @@ -552,6 +560,7 @@ def update_module_isolated(
previous_modules: dict[str, str],
graph: Graph,
force_removed: bool,
followed: bool,
) -> UpdateResult:
"""Build a new version of one changed module only.

Expand All @@ -575,7 +584,7 @@ def update_module_isolated(
delete_module(module, path, graph, manager)
return NormalUpdate(module, path, [], None)

sources = get_sources(manager.fscache, previous_modules, [(module, path)])
sources = get_sources(manager.fscache, previous_modules, [(module, path)], followed)

if module in manager.missing_modules:
manager.missing_modules.remove(module)
Expand Down Expand Up @@ -728,12 +737,15 @@ def get_module_to_path_map(graph: Graph) -> dict[str, str]:


def get_sources(
fscache: FileSystemCache, modules: dict[str, str], changed_modules: list[tuple[str, str]]
fscache: FileSystemCache,
modules: dict[str, str],
changed_modules: list[tuple[str, str]],
followed: bool,
) -> list[BuildSource]:
sources = []
for id, path in changed_modules:
if fscache.isfile(path):
sources.append(BuildSource(path, id, None))
sources.append(BuildSource(path, id, None, followed=followed))
return sources


Expand Down
8 changes: 3 additions & 5 deletions mypy/test/testcmdline.py
Expand Up @@ -69,12 +69,10 @@ def test_python_cmdline(testcase: DataDrivenTestCase, step: int) -> None:
env["PYTHONPATH"] = PREFIX
if os.path.isdir(extra_path):
env["PYTHONPATH"] += os.pathsep + extra_path
cwd = os.path.join(test_temp_dir, custom_cwd or "")
args = [arg.replace("$CWD", os.path.abspath(cwd)) for arg in args]
process = subprocess.Popen(
fixed + args,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
cwd=os.path.join(test_temp_dir, custom_cwd or ""),
env=env,
fixed + args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=cwd, env=env
)
outb, errb = process.communicate()
result = process.returncode
Expand Down
65 changes: 65 additions & 0 deletions test-data/unit/cmdline.test
Expand Up @@ -1505,3 +1505,68 @@ def f():
[out]
a.py:2: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs
== Return code: 0

[case testCustomTypeshedDirFilePassedExplicitly]
# cmd: mypy --custom-typeshed-dir dir m.py dir/stdlib/foo.pyi
[file m.py]
1()
[file dir/stdlib/abc.pyi]
1() # Errors are not reported from typeshed by default
[file dir/stdlib/builtins.pyi]
class object: pass
class str(object): pass
class int(object): pass
[file dir/stdlib/sys.pyi]
[file dir/stdlib/types.pyi]
[file dir/stdlib/typing.pyi]
[file dir/stdlib/mypy_extensions.pyi]
[file dir/stdlib/typing_extensions.pyi]
[file dir/stdlib/foo.pyi]
1() # Errors are reported if the file was explicitly passed on the command line
[file dir/stdlib/VERSIONS]
[out]
dir/stdlib/foo.pyi:1: error: "int" not callable
m.py:1: error: "int" not callable

[case testFileInPythonPathPassedExplicitly1]
# cmd: mypy $CWD/pypath/foo.py
[file pypath/foo.py]
1()
[out]
pypath/foo.py:1: error: "int" not callable

[case testFileInPythonPathPassedExplicitly2]
# cmd: mypy pypath/foo.py
[file pypath/foo.py]
1()
[out]
pypath/foo.py:1: error: "int" not callable

[case testFileInPythonPathPassedExplicitly3]
# cmd: mypy -p foo
# cwd: pypath
[file pypath/foo/__init__.py]
1()
[file pypath/foo/m.py]
1()
[out]
foo/m.py:1: error: "int" not callable
foo/__init__.py:1: error: "int" not callable

[case testFileInPythonPathPassedExplicitly4]
# cmd: mypy -m foo
# cwd: pypath
[file pypath/foo.py]
1()
[out]
foo.py:1: error: "int" not callable

[case testFileInPythonPathPassedExplicitly5]
# cmd: mypy -m foo.m
# cwd: pypath
[file pypath/foo/__init__.py]
1() # TODO: Maybe this should generate errors as well? But how would we decide?
[file pypath/foo/m.py]
1()
[out]
foo/m.py:1: error: "int" not callable