From bc924bc5721330be5468a9d393ec6d1fa18f4059 Mon Sep 17 00:00:00 2001 From: Bernat Gabor Date: Thu, 7 Jun 2018 16:59:49 +0100 Subject: [PATCH] add tests for source tree and fix annotations --- .gitignore | 1 + mypy/build.py | 15 +++-- mypy/find_sources.py | 22 ++++--- mypy/main.py | 4 ++ mypy/options.py | 2 + mypy/stub_src_merge.py | 8 +-- mypy/test/test_source_finder.py | 108 +++++++++++++++++++++++++++++++ mypy/test/test_stub_src_merge.py | 51 ++++++++++----- runtests.py | 3 +- tox.ini | 2 +- 10 files changed, 178 insertions(+), 38 deletions(-) create mode 100644 mypy/test/test_source_finder.py diff --git a/.gitignore b/.gitignore index 84ed3eea9b718..be0b1ec0b0e14 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ dmypy.json # Packages *.egg *.egg-info +.eggs # IDEs .idea diff --git a/mypy/build.py b/mypy/build.py index 3c881c913ddf2..b3e1d494cb4aa 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -111,6 +111,12 @@ def __repr__(self) -> str: self.module, self.text is not None) + def __eq__(self, other: Any) -> bool: + return type(self) == type(other) and self.__dict__ == other.__dict__ + + def __ne__(self, other): + return not self == other + class BuildSourceSet: """Efficiently test a file's membership in the set of build sources.""" @@ -2562,11 +2568,10 @@ def load_graph(sources: List[BuildSource], manager: BuildManager, root_source=True) if bs.merge_with: mw = bs.merge_with - src_st = State(id=mw.module, path=mw.path, source=mw.text, - manager=manager, root_source=True) - src_st.parse_file() - src_st.merge_with(st, manager.errors) - st = src_st + stub_state = State(id=mw.module, path=mw.path, source=mw.text, + manager=manager, root_source=True) + stub_state.parse_file() + st.merge_with(stub_state, manager.errors) except ModuleNotFound: continue if st.id in graph: diff --git a/mypy/find_sources.py b/mypy/find_sources.py index 91557a2dd74c4..45abe284a0cb8 100644 --- a/mypy/find_sources.py +++ b/mypy/find_sources.py @@ -24,7 +24,7 @@ def create_source_list(files: Sequence[str], options: Options, Raises InvalidSourceList on errors. """ fscache = fscache or FileSystemCache() - finder = SourceFinder(fscache) + finder = SourceFinder(fscache, options.merge_stub_into_src) targets = [] for f in files: @@ -57,10 +57,11 @@ def keyfunc(name: str) -> Tuple[str, int]: class SourceFinder: - def __init__(self, fscache: FileSystemCache) -> None: + def __init__(self, fscache: FileSystemCache, merge_stub_into_src: bool) -> None: self.fscache = fscache # A cache for package names, mapping from directory path to module id and base dir self.package_cache = {} # type: Dict[str, Tuple[str, str]] + self.merge_stub_into_src = merge_stub_into_src def expand_dir(self, arg: str, mod_prefix: str = '') -> List[BuildSource]: """Convert a directory name to a list of sources to build.""" @@ -98,14 +99,15 @@ def expand_dir(self, arg: str, mod_prefix: str = '') -> List[BuildSource]: next_base, next_suffix = None, None else: next_base, next_suffix = os.path.splitext(name) - merge_with = None - if next_base is not None and next_base == base and name is not None: - merge_with = BuildSource(os.path.join(arg, name), - mod_prefix + next_base, - None, - base_dir) - src = BuildSource(path, mod_prefix + base, None, base_dir, - merge_with=merge_with) + src = BuildSource(path, mod_prefix + base, None, base_dir) + if self.merge_stub_into_src is True and next_base is not None \ + and next_base == base and name is not None: + merge_with = src + src = BuildSource(path=os.path.join(arg, name), + module=mod_prefix + next_base, + merge_with=merge_with, + text=None, + base_dir=base_dir) sources.append(src) return sources except StopIteration: diff --git a/mypy/main.py b/mypy/main.py index fe26b75fcf8e3..b00f2aeb38f8e 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -587,6 +587,10 @@ def add_invertible_flag(flag: str, other_group.add_argument( '--scripts-are-modules', action='store_true', help="script x becomes module x instead of __main__") + add_invertible_flag('--merge-stub-into-src', default=False, strict_flag=False, + help="when a stub and source file is in the same folder with same name " + "merge the stub file into the source file, and lint the source file", + group=other_group) if server_options: # TODO: This flag is superfluous; remove after a short transition (2018-03-16) diff --git a/mypy/options.py b/mypy/options.py index af9237d1807d5..cb5f17325ac81 100644 --- a/mypy/options.py +++ b/mypy/options.py @@ -200,6 +200,8 @@ def __init__(self) -> None: self.package_root = [] # type: List[str] self.cache_map = {} # type: Dict[str, Tuple[str, str]] + self.merge_stub_into_src = False + def snapshot(self) -> object: """Produce a comparable snapshot of this Option""" d = dict(self.__dict__) diff --git a/mypy/stub_src_merge.py b/mypy/stub_src_merge.py index 9981e7f15532d..aebd4b501a42c 100644 --- a/mypy/stub_src_merge.py +++ b/mypy/stub_src_merge.py @@ -10,7 +10,7 @@ Tuple, Type, Union, - cast + cast, ) from mypy.errors import Errors @@ -32,7 +32,7 @@ SymbolTable, TempNode, TypeInfo, - Var + Var, ) @@ -139,7 +139,7 @@ def merge_assignment(self, src: AssignmentStmt, stub: AssignmentStmt) -> None: # already made sure they match src.type = stub.type src.unanalyzed_type = stub.unanalyzed_type - stub_name = self.simple_assignment_name(stub) + stub_name = cast(str, self.simple_assignment_name(stub)) self.check_no_explicit_default(stub.rvalue, stub, stub_name) def merge_func_definition(self, src: FuncDef, stub: FuncDef) -> None: @@ -295,7 +295,7 @@ def decorator_argument_checks(self, src: CallExpr, stub: CallExpr) -> None: ) break for name, default_node in zip(stub.arg_names, stub.args): - self.check_no_explicit_default(default_node, src, name) + self.check_no_explicit_default(default_node, src, cast(str, name)) def check_no_explicit_default( self, default_node: Node, node: Node, name: str diff --git a/mypy/test/test_source_finder.py b/mypy/test/test_source_finder.py new file mode 100644 index 0000000000000..f1a04db8c44e3 --- /dev/null +++ b/mypy/test/test_source_finder.py @@ -0,0 +1,108 @@ +from pathlib import Path +from typing import Any, Callable, Dict, List, Tuple + +import pytest +from py._path.local import LocalPath + +from mypy.build import BuildSource +from mypy.find_sources import create_source_list +from mypy.options import Options + +MergeFinder = Callable[[Dict[str, Any], bool], Tuple[Path, List[BuildSource]]] + + +def create_files(folder: Path, content: Dict[str, Any]): + folder.mkdir(exist_ok=True) + for key, value in content.items(): + if key.endswith(".py") or key.endswith(".pyi"): + (folder / key).write_text(value) + elif isinstance(value, dict): + create_files(folder / key, value) + + +@pytest.fixture() +def merge_finder(tmpdir: LocalPath) -> MergeFinder: + def _checker( + content: Dict[str, Any], merge: bool + ) -> Tuple[Path, List[BuildSource]]: + options = Options() + options.merge_stub_into_src = merge + test_dir = str(tmpdir) + create_files(Path(test_dir), content) + targets = create_source_list(files=[test_dir], options=options) + return Path(str(tmpdir)), targets + + return _checker + + +def test_source_finder_merge(merge_finder: MergeFinder) -> None: + base_dir, found = merge_finder({"a.py": "", "a.pyi": ""}, True) + assert found == [ + BuildSource( + path=str(base_dir / "a.py"), + base_dir=str(base_dir), + module="a", + text=None, + merge_with=BuildSource( + path=str(base_dir / "a.pyi"), + base_dir=str(base_dir), + module="a", + text=None, + ), + ) + ] + + +def test_source_finder_merge_sub_folder(merge_finder: MergeFinder) -> None: + base_dir, found = merge_finder( + {"pkg": {"a.py": "", "a.pyi": "", "__init__.py": ""}}, True + ) + assert found == [ + BuildSource( + path=str(base_dir / "pkg" / "__init__.py"), + base_dir=str(base_dir), + module="pkg", + text=None, + ), + BuildSource( + path=str(base_dir / "pkg" / "a.py"), + base_dir=str(base_dir), + module="pkg.a", + text=None, + merge_with=BuildSource( + path=str(base_dir / "pkg" / "a.pyi"), + base_dir=str(base_dir), + module="pkg.a", + text=None, + ), + ), + ] + + +def test_source_finder_no_merge(merge_finder: MergeFinder) -> None: + base_dir, found = merge_finder({"a.py": "", "a.pyi": ""}, False) + assert found == [ + BuildSource( + path=str(base_dir / "a.pyi"), base_dir=str(base_dir), module="a", text=None + ) + ] + + +@pytest.mark.parametrize("merge", [True, False]) +def test_source_finder_merge_just_source(merge_finder, merge): + base_dir, found = merge_finder({"a.py": ""}, merge=merge) + assert found == [ + BuildSource( + path=str(base_dir / "a.py"), base_dir=str(base_dir), module="a", text=None + ) + ] + + +@pytest.mark.parametrize("merge", [True, False]) +def test_source_finder_merge_just_stub(merge_finder, merge): + base_dir, found = merge_finder({"a.pyi": ""}, merge=merge) + assert found == [ + BuildSource( + path=str(base_dir / "a.pyi"), base_dir=str(base_dir), module="a", text=None + ) + ] diff --git a/mypy/test/test_stub_src_merge.py b/mypy/test/test_stub_src_merge.py index c9b792ac648a9..3454c29dc42e4 100644 --- a/mypy/test/test_stub_src_merge.py +++ b/mypy/test/test_stub_src_merge.py @@ -1,6 +1,6 @@ import re import textwrap -from typing import Callable, Generator, List, Optional, Pattern, Tuple, cast +from typing import Callable, List, Optional, Pattern, Tuple, cast import pytest from _pytest.capture import CaptureFixture @@ -9,10 +9,10 @@ from mypy import build from mypy.build import BuildSource from mypy.defaults import PYTHON3_VERSION -from mypy.errors import Errors +from mypy.errors import ErrorInfo from mypy.options import Options -Checker = Callable[[str, str, Optional[str]], None] +Checker = Callable[[str, str, Optional[str]], List[ErrorInfo]] @pytest.fixture(name="checker_resource", scope="session") @@ -25,6 +25,7 @@ def checker_resource_fixture() -> Tuple[Pattern, Options]: options.dump_graph = True options.use_builtins_fixtures = True options.python_version = PYTHON3_VERSION + options.merge_stub_into_src = True return line_nr, options @@ -32,18 +33,18 @@ def checker_resource_fixture() -> Tuple[Pattern, Options]: @pytest.fixture(name="checker") def checker_fixture( checker_resource: Tuple[Pattern, Options], tmpdir: LocalPath, capsys: CaptureFixture -) -> Generator[Checker, None, None]: +) -> Checker: line_nr, options = checker_resource - def _do(src: str, stub: str, result: Optional[str] = None) -> List[Errors]: + def _do(src: str, stub: str, expected_src: Optional[str] = None) -> List[ErrorInfo]: src_path = str(tmpdir.join("t.py")) stub_path = str(tmpdir.join("t.pyi")) files = {src_path: src, stub_path: stub} - if result is not None: - result_path = str(tmpdir.join("result.py")) # type: Optional[str] - files[cast(str, result_path)] = result + if expected_src is not None: + expected_src_path = str(tmpdir.join("expected.py")) # type: Optional[str] + files[cast(str, expected_src_path)] = expected_src else: - result_path = None + expected_src_path = None for path, content in files.items(): with open(path, "wt") as f: @@ -51,26 +52,27 @@ def _do(src: str, stub: str, result: Optional[str] = None) -> List[Errors]: combined_ast, combined_res = _build_ast( BuildSource( - stub_path, None, None, merge_with=BuildSource(src_path, None, None) + src_path, None, None, merge_with=BuildSource(stub_path, None, None) ) ) - if result_path is not None: - result_ast, result_res = _build_ast(BuildSource(result_path, None, None)) - assert result_ast == combined_ast + if expected_src_path is not None: + expected_ast, expected_res = _build_ast( + BuildSource(expected_src_path, None, None) + ) + assert combined_ast == expected_ast capsys.readouterr() errors = combined_res.manager.errors.error_info_map return errors.get(src_path, []) def _build_ast(source): - path = source.merge_with.path if source.merge_with is not None else source.path res = build.build(sources=[source], options=options, alt_lib_path=tmpdir) - regex = re.compile(r"\s+{}\s+^".format(re.escape(path)), re.MULTILINE) + regex = re.compile(r"\s+{}\s+^".format(re.escape(source.path)), re.MULTILINE) ast_str = line_nr.sub( r"\1", regex.sub("\n", str(res.graph[source.module].tree)) ) return ast_str, res - yield _do + return _do def test_stub_src_type_does_not_match(checker: Checker) -> None: @@ -79,6 +81,7 @@ def test_stub_src_type_does_not_match(checker: Checker) -> None: a = 1""", """ def a() -> None: ...""", + None, ) assert [i.message for i in src_errors] == [ "conflict of src Var and stub:2 FuncDef definition" @@ -91,6 +94,7 @@ def test_merge_module_func_arg_conflict(checker: Checker) -> None: def a(arg): ...""", """ def a(arg1) -> None: ...""", + None, ) assert [i.message for i in src_errors] == [ "arg conflict of src ['arg'] and stub (line 2) ['arg1']" @@ -169,6 +173,7 @@ def __init__(self): """ class A: def __init__(self) -> None: ...""", + None, ) assert [i.message for i in src_errors] == ["no stub definition for class member a"] @@ -257,6 +262,7 @@ def a(): """ @d(p=1, r=None) def a() -> None: ...""", + None, ) assert [i.message for i in src_errors] == [ "stub should not contain default value, p has IntExpr", @@ -273,6 +279,7 @@ def a(): """ @d(p=..., r=...) def a() -> None: ...""", + None, ) assert [i.message for i in src_errors] == [ "conflict of src v and stub r decorator argument name" @@ -288,6 +295,7 @@ def a(): """ @d(p=...) def a() -> None: ...""", + None, ) assert [i.message for i in src_errors] == [ "conflict of src de and stub d decorator name" @@ -302,6 +310,7 @@ def a(): pass""", """ @de def a() -> None: ...""", + None, ) assert [i.message for i in src_errors] == [ "conflict of src d and stub de decorator name" @@ -311,12 +320,13 @@ def a() -> None: ...""", def test_decorator_source_less(checker: Checker) -> None: src_errors = checker( """ - @d + @d def a(): pass""", """ @d @de def a() -> None: ...""", + None, ) assert [i.message for i in src_errors] == [ "conflict of src NoneType and stub NameExpr decorator" @@ -332,6 +342,7 @@ def a(): pass""", """ @d def a() -> None: ...""", + None, ) assert [i.message for i in src_errors] == [ "conflict of src NameExpr and stub NoneType decorator" @@ -346,6 +357,7 @@ def a(): pass""", """ @d(pe=...) def a() -> None: ...""", + None, ) assert [i.message for i in src_errors] == [ "conflict of src p and stub pe decorator argument name" @@ -360,6 +372,7 @@ def a(): pass""", """ @d(p=1) def a() -> None: ...""", + None, ) assert [i.message for i in src_errors] == [ "stub should not contain default value, p has IntExpr" @@ -375,6 +388,7 @@ class A: class A: @classmethod def a(cls) -> None: ...""", + None, ) assert [i.message for i in src_errors] == ["no source for func a @stub:3"] @@ -387,6 +401,7 @@ class A: """ class A: def a(cls) -> None: ...""", + None, ) assert [i.message for i in src_errors] == ["no source for func a @stub:3"] @@ -399,6 +414,7 @@ class A: """ class A: a : int = ...""", + None, ) assert [i.message for i in src_errors] == ["no source for assign a @stub:3"] @@ -412,6 +428,7 @@ class A: """ class A: a, b = ..., ...""", + None, ) assert [i.message for i in src_errors] == [ "l-values must be simple name expressions, is TupleExpr", # src diff --git a/runtests.py b/runtests.py index 19f8a601c8311..f0fe2b84fae78 100755 --- a/runtests.py +++ b/runtests.py @@ -147,7 +147,8 @@ def test_path(*names: str) -> List[str]: 'testsolve', 'testsubtypes', 'testtypes', - 'test_stub_src_merge' + 'test_stub_src_merge', + 'test_source_finder' ) SLOW_FILES = test_path( diff --git a/tox.ini b/tox.ini index a9045e7bcb0fb..2a1ba0c295fc3 100644 --- a/tox.ini +++ b/tox.ini @@ -19,7 +19,7 @@ commands = mypy mypy --ignore-missing-imports --follow-imports=skip deps = diff_cover -r ./test-requirements.txt description = run stub tests and check diff coverage -commands = pytest mypy/test/test_stub_src_merge.py --cov mypy --cov-report=xml:{toxworkdir}/coverage.xml +commands = pytest mypy/test/test_stub_src_merge.py mypy/test/test_source_finder.py --cov mypy --cov-report=xml:{toxworkdir}/coverage.xml diff-cover --compare-branch upstream/master {toxworkdir}/coverage.xml