Skip to content

Commit

Permalink
Don't ignore errors in files passed on the command line (#14060)
Browse files Browse the repository at this point in the history
#13768 had a bug so that errors were sometimes silenced in files that
were under a directory in `sys.path`. `sys.path` sometimes includes the 
current working directory, resulting in no errors reported at all. Fix it by 
always reporting errors if a file was passed on the command line (unless
*explicitly* silenced).

When using import following errors can still be ignored, which is 
questionable, but this didn't change recently so I'm not addressing it 
here.

Fixes #14042.
  • Loading branch information
JukkaL authored Nov 10, 2022
1 parent dbcbb3f commit 5cc1439
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 25 deletions.
10 changes: 7 additions & 3 deletions mypy/build.py
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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

0 comments on commit 5cc1439

Please sign in to comment.