From dc3d69034d67d67972758ed0ef2c46f23add8f3a Mon Sep 17 00:00:00 2001 From: STerliakov Date: Sun, 12 Jun 2022 22:13:21 +0300 Subject: [PATCH 1/4] Fix manager types scope --- mypy.ini | 3 ++- mypy_django_plugin/lib/helpers.py | 9 ++++----- mypy_django_plugin/transformers/models.py | 6 +++++- tests/typecheck/managers/test_managers.yml | 19 +++++++++++++++++++ 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/mypy.ini b/mypy.ini index 2f756cf0f..3f1cbcaef 100644 --- a/mypy.ini +++ b/mypy.ini @@ -2,7 +2,8 @@ allow_redefinition = True check_untyped_defs = True ignore_missing_imports = True -incremental = True +# Incremental mode causes false-negatives +incremental = False strict_optional = True show_traceback = True warn_no_return = False diff --git a/mypy_django_plugin/lib/helpers.py b/mypy_django_plugin/lib/helpers.py index 9edfc7de6..1ce73fd8b 100644 --- a/mypy_django_plugin/lib/helpers.py +++ b/mypy_django_plugin/lib/helpers.py @@ -369,11 +369,10 @@ def bind_or_analyze_type(t: MypyType, api: SemanticAnalyzer, module_name: Option That should hopefully give a bound type.""" if isinstance(t, UnboundType) and module_name is not None: node = api.lookup_fully_qualified_or_none(module_name + "." + t.name) - if node is None: - return None - return node.type - else: - return api.anal_type(t) + if node is not None and node.type is not None: + return node.type + + return api.anal_type(t) def copy_method_to_another_class( diff --git a/mypy_django_plugin/transformers/models.py b/mypy_django_plugin/transformers/models.py index a6e405265..ccad0fc3a 100644 --- a/mypy_django_plugin/transformers/models.py +++ b/mypy_django_plugin/transformers/models.py @@ -212,7 +212,11 @@ def create_new_model_parametrized_manager(self, name: str, base_manager_info: Ty # replace self type with new class, if copying method if isinstance(sym.node, FuncDef): helpers.copy_method_to_another_class( - new_cls_def_context, self_type=custom_manager_type, new_method_name=name, method_node=sym.node + new_cls_def_context, + self_type=custom_manager_type, + new_method_name=name, + method_node=sym.node, + original_module_name=base_manager_info.module_name, ) continue diff --git a/tests/typecheck/managers/test_managers.yml b/tests/typecheck/managers/test_managers.yml index 2323b28ec..1083451d7 100644 --- a/tests/typecheck/managers/test_managers.yml +++ b/tests/typecheck/managers/test_managers.yml @@ -423,3 +423,22 @@ class MyModel(models.Model): objects = MyModelManager() + +- case: regression_manager_scope_foreign + main: | + from myapp.models import MyModel + reveal_type(MyModel.on_site) # N: Revealed type is "myapp.models.MyModel_CurrentSiteManager[myapp.models.MyModel]" + installed_apps: + - myapp + - django.contrib.sites + files: + - path: myapp/__init__.py + - path: myapp/models.py + content: | + from django.db import models + from django.contrib.sites.models import Site + from django.contrib.sites.managers import CurrentSiteManager + + class MyModel(models.Model): + site = models.ForeignKey(Site, on_delete=models.CASCADE) + on_site = CurrentSiteManager() From 46e8ac8966ceb42491aab4ed0364c83e0f51d7d6 Mon Sep 17 00:00:00 2001 From: STerliakov Date: Mon, 13 Jun 2022 11:09:42 +0300 Subject: [PATCH 2/4] Restore incremental mode and mention in developer docs --- CONTRIBUTING.md | 6 ++++++ mypy.ini | 3 +-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 821ea499a..9385a8bb7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -70,6 +70,12 @@ To execute the unit tests, simply run: pytest ``` +If you get some unexpected results or want to be sure that tests run is not affected by previous one, remove `mypy` cache: + +```bash +rm -r .mypy_cache +``` + We also test the stubs against Django's own test suite. This is done in CI but you can also do this locally. To execute the script run: diff --git a/mypy.ini b/mypy.ini index 3f1cbcaef..2f756cf0f 100644 --- a/mypy.ini +++ b/mypy.ini @@ -2,8 +2,7 @@ allow_redefinition = True check_untyped_defs = True ignore_missing_imports = True -# Incremental mode causes false-negatives -incremental = False +incremental = True strict_optional = True show_traceback = True warn_no_return = False From a3053b5721ec85becf6eeab10214e105355fd50e Mon Sep 17 00:00:00 2001 From: STerliakov Date: Tue, 14 Jun 2022 11:24:00 +0300 Subject: [PATCH 3/4] Separate dev mypy config and regular one --- .github/workflows/test.yml | 2 +- mypy.ini | 2 ++ mypy.ini.dev | 21 +++++++++++++++++++++ pytest.ini | 2 +- 4 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 mypy.ini.dev diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2f61b23d7..ee8387b85 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -46,7 +46,7 @@ jobs: pip install -r ./requirements.txt - name: Run tests - run: pytest + run: pytest --mypy-ini-file=mypy.ini typecheck: runs-on: ubuntu-latest diff --git a/mypy.ini b/mypy.ini index 2f756cf0f..c76042bf3 100644 --- a/mypy.ini +++ b/mypy.ini @@ -1,3 +1,5 @@ +# Regular configuration file (can be used as base in other projects, runs in CI) + [mypy] allow_redefinition = True check_untyped_defs = True diff --git a/mypy.ini.dev b/mypy.ini.dev new file mode 100644 index 000000000..f136350b3 --- /dev/null +++ b/mypy.ini.dev @@ -0,0 +1,21 @@ +# Configuration file to use during development + +[mypy] +allow_redefinition = True +check_untyped_defs = True +ignore_missing_imports = True +# Avoid caching between test runs +incremental = False +strict_optional = True +show_traceback = True +warn_no_return = False +warn_unused_ignores = True +warn_redundant_casts = True +warn_unused_configs = True +warn_unreachable = True + +plugins = + mypy_django_plugin.main + +[mypy.plugins.django-stubs] +django_settings_module = scripts.django_tests_settings diff --git a/pytest.ini b/pytest.ini index ae5aa2b0b..fb2028dd7 100644 --- a/pytest.ini +++ b/pytest.ini @@ -7,5 +7,5 @@ addopts = -s -v --cache-clear - --mypy-ini-file=./mypy.ini + --mypy-ini-file=./mypy.ini.dev --mypy-extension-hook=scripts.tests_extension_hook.django_plugin_hook From 74717f8644fbc2b03e24c4d29d9aac7bd2466910 Mon Sep 17 00:00:00 2001 From: STerliakov Date: Tue, 14 Jun 2022 17:39:43 +0300 Subject: [PATCH 4/4] Document config files usage --- mypy.ini | 5 +++++ mypy.ini.dev | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/mypy.ini b/mypy.ini index c76042bf3..e86c0eb46 100644 --- a/mypy.ini +++ b/mypy.ini @@ -1,5 +1,10 @@ # Regular configuration file (can be used as base in other projects, runs in CI) +# NOTE: this config file is not used by pytest locally. +# See comment in mypy.ini.dev for explanation. + +# WARNING: when changing this file, consider doing the same with mypy.ini.dev + [mypy] allow_redefinition = True check_untyped_defs = True diff --git a/mypy.ini.dev b/mypy.ini.dev index f136350b3..22f5e44ef 100644 --- a/mypy.ini.dev +++ b/mypy.ini.dev @@ -1,5 +1,14 @@ # Configuration file to use during development +# Changes of plugin code are not detected by mypy, so +# incremental mode can be harmful during development +# (mypy cache is not always invalidated). +# However, incremental mode has to be supported by django-stubs, +# thus another config (mypy.ini) is used in CI. +# Incremental mode is recommended for regular usage. + +# WARNING: when changing this file, consider doing the same with mypy.ini + [mypy] allow_redefinition = True check_untyped_defs = True