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

Add strict_settings option, allow runtime fallbacks for custom settings #1557

Merged
merged 15 commits into from
Jun 24, 2023
Merged
Next Next commit
Add strict_settings option, allow runtime fallbacks for custom sett…
…ings
sobolevn committed Jun 20, 2023

Verified

This commit was signed with the committer’s verified signature.
rm3l Armel Soro
commit 90ed760c2b310717b0493221d10231999c2d1fac
46 changes: 46 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -311,6 +311,52 @@ If you encounter this error in your own code, you can either cast the `Promise`

If this is reported on Django code, please report an issue or open a pull request to fix the type hints.

### How to use a custom library to handle Django settings?

Using something like [`django-split-settings`](https://github.com/wemake-services/django-split-settings) or [`django-configurations`](https://github.com/jazzband/django-configurations) will make it hard for mypy to infer your settings.

This might also be the case when using something like:

```python
try:
from .local_settings import *
except Exception:
pass
```

So, mypy would not like this code:

```python
from django.conf import settings

settings.CUSTOM_VALUE # E: 'Settings' object has no attribute 'CUSTOM_SETTING'
```

To handle this corner case we have a special setting `strict_settings` (`True` by default),
you can switch it to `False` to always return `Any` and not raise any errors if runtime settings module has the given value:

```toml
[tool.django-stubs]
strict_settings = false
```

or

```ini
[mypy.plugins.django-stubs]
strict_settings = false
```
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although there are just 2 settings for now, I would prefer it if all django-stubs settings were documented in one place. Maybe add another README section for settings and then link to it from here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can do this later. Good idea 👍


And then:

```python
# Works:
reveal_type(settings.EXISTS_IN_RUNTIME) # N: Any

# Errors:
reveal_type(settings.MISSING) # E: 'Settings' object has no attribute 'MISSING'
```

## Related projects

- [`awesome-python-typing`](https://github.com/typeddjango/awesome-python-typing) - Awesome list of all typing-related things in Python.
24 changes: 18 additions & 6 deletions mypy_django_plugin/config.py
Original file line number Diff line number Diff line change
@@ -14,21 +14,23 @@
(config)
...
[mypy.plugins.django-stubs]
django_settings_module: str (required)
django_settings_module = str (required)
strict_settings = bool (default: true)
...
"""
TOML_USAGE = """
(config)
...
[tool.django-stubs]
django_settings_module = str (required)
strict_settings = bool (default: true)
...
"""
INVALID_FILE = "mypy config file is not specified or found"
COULD_NOT_LOAD_FILE = "could not load configuration file"
MISSING_SECTION = "no section [{section}] found".format
MISSING_SECTION = "no section [{section}] found"
MISSING_DJANGO_SETTINGS = "missing required 'django_settings_module' config"
INVALID_SETTING = "invalid {key!r}: the setting must be a boolean".format
INVALID_BOOL_SETTING = "invalid {key!r}: the setting must be a boolean"


def exit_with_error(msg: str, is_toml: bool = False) -> NoReturn:
@@ -48,8 +50,9 @@ def exit_with_error(msg: str, is_toml: bool = False) -> NoReturn:


class DjangoPluginConfig:
__slots__ = ("django_settings_module",)
__slots__ = ("django_settings_module", "strict_settings")
django_settings_module: str
strict_settings: bool

def __init__(self, config_file: Optional[str]) -> None:
if not config_file:
@@ -75,7 +78,7 @@ def parse_toml_file(self, filepath: Path) -> None:
try:
config: Dict[str, Any] = data["tool"]["django-stubs"]
except KeyError:
toml_exit(MISSING_SECTION(section="tool.django-stubs"))
toml_exit(MISSING_SECTION.format(section="tool.django-stubs"))

if "django_settings_module" not in config:
toml_exit(MISSING_DJANGO_SETTINGS)
@@ -84,6 +87,10 @@ def parse_toml_file(self, filepath: Path) -> None:
if not isinstance(self.django_settings_module, str):
toml_exit("invalid 'django_settings_module': the setting must be a string")

self.strict_settings = config.get("strict_settings", True)
if not isinstance(self.strict_settings, bool):
toml_exit(INVALID_BOOL_SETTING.format(key="strict_settings"))

def parse_ini_file(self, filepath: Path) -> None:
parser = configparser.ConfigParser()
try:
@@ -94,9 +101,14 @@ def parse_ini_file(self, filepath: Path) -> None:

section = "mypy.plugins.django-stubs"
if not parser.has_section(section):
exit_with_error(MISSING_SECTION(section=section))
exit_with_error(MISSING_SECTION.format(section=section))

if not parser.has_option(section, "django_settings_module"):
exit_with_error(MISSING_DJANGO_SETTINGS)

self.django_settings_module = parser.get(section, "django_settings_module").strip("'\"")

try:
self.strict_settings = parser.getboolean(section, "strict_settings", fallback=True)
except ValueError:
exit_with_error(INVALID_BOOL_SETTING.format(key="strict_settings"))
7 changes: 4 additions & 3 deletions mypy_django_plugin/django/context.py
Original file line number Diff line number Diff line change
@@ -20,6 +20,7 @@
from mypy.types import AnyType, Instance, TypeOfAny, UnionType
from mypy.types import Type as MypyType

from mypy_django_plugin.config import DjangoPluginConfig
from mypy_django_plugin.exceptions import UnregisteredModelError
from mypy_django_plugin.lib import fullnames, helpers
from mypy_django_plugin.lib.fullnames import WITH_ANNOTATIONS_FULLNAME
@@ -78,10 +79,10 @@ class LookupsAreUnsupported(Exception):


class DjangoContext:
def __init__(self, django_settings_module: str) -> None:
self.django_settings_module = django_settings_module
def __init__(self, plugin_config: DjangoPluginConfig) -> None:
self.plugin_config = plugin_config

apps, settings = initialize_django(self.django_settings_module)
apps, settings = initialize_django(self.plugin_config.django_settings_module)
self.apps_registry = apps
self.settings = settings

6 changes: 3 additions & 3 deletions mypy_django_plugin/main.py
Original file line number Diff line number Diff line change
@@ -71,7 +71,7 @@ def __init__(self, options: Options) -> None:
sys.path.extend(mypy_path())
# Add paths from mypy_path config option
sys.path.extend(options.mypy_path)
self.django_context = DjangoContext(self.plugin_config.django_settings_module)
self.django_context = DjangoContext(self.plugin_config)

def _get_current_queryset_bases(self) -> Dict[str, int]:
model_sym = self.lookup_fully_qualified(fullnames.QUERYSET_CLASS_FULLNAME)
@@ -125,8 +125,8 @@ def _new_dependency(self, module: str) -> Tuple[int, str, int]:

def get_additional_deps(self, file: MypyFile) -> List[Tuple[int, str, int]]:
# for settings
if file.fullname == "django.conf" and self.django_context.django_settings_module:
return [self._new_dependency(self.django_context.django_settings_module)]
if file.fullname == "django.conf" and self.django_context.plugin_config.django_settings_module:
return [self._new_dependency(self.django_context.plugin_config.django_settings_module)]

# for values / values_list
if file.fullname == "django.db.models":
9 changes: 8 additions & 1 deletion mypy_django_plugin/transformers/settings.py
Original file line number Diff line number Diff line change
@@ -28,7 +28,7 @@ def get_type_of_settings_attribute(ctx: AttributeContext, django_context: Django
typechecker_api = helpers.get_typechecker_api(ctx)

# first look for the setting in the project settings file, then global settings
settings_module = typechecker_api.modules.get(django_context.django_settings_module)
settings_module = typechecker_api.modules.get(django_context.plugin_config.django_settings_module)
global_settings_module = typechecker_api.modules.get("django.conf.global_settings")
for module in [settings_module, global_settings_module]:
if module is not None:
@@ -42,5 +42,12 @@ def get_type_of_settings_attribute(ctx: AttributeContext, django_context: Django
return ctx.default_attr_type
return sym.type

# Now, we want to check if this setting really exist in runtime.
# If it does, we just return `Any`, not to raise any false-positives.
# But, we cannot reconstruct the exact runtime type.
# See https://github.com/typeddjango/django-stubs/pull/1163
if not django_context.plugin_config.strict_settings and hasattr(django_context.settings, setting_name):
return AnyType(TypeOfAny.implementation_artifact)

ctx.api.fail(f"'Settings' object has no attribute {setting_name!r}", ctx.context)
return ctx.default_attr_type
36 changes: 29 additions & 7 deletions tests/test_error_handling.py
Original file line number Diff line number Diff line change
@@ -11,7 +11,8 @@
(config)
...
[mypy.plugins.django-stubs]
django_settings_module: str (required)
django_settings_module = str (required)
strict_settings = bool (default: true)
...
(django-stubs) mypy: error: {}
"""
@@ -21,6 +22,7 @@
...
[tool.django-stubs]
django_settings_module = str (required)
strict_settings = bool (default: true)
...
(django-stubs) mypy: error: {}
"""
@@ -52,6 +54,11 @@ def write_to_file(file_contents: str, suffix: Optional[str] = None) -> Generator
"missing required 'django_settings_module' config",
id="no-settings-given",
),
pytest.param(
["[mypy.plugins.django-stubs]", "django_settings_module = some.module", "strict_settings = bad"],
"invalid 'strict_settings': the setting must be a boolean",
id="missing-settings-module",
),
],
)
def test_misconfiguration_handling(capsys: Any, config_file_contents: List[str], message_part: str) -> None:
@@ -113,6 +120,15 @@ def test_handles_filename(capsys: Any, filename: str) -> None:
"could not load configuration file",
id="invalid toml",
),
pytest.param(
"""
[tool.django-stubs]
django_settings_module = "some.module"
strict_settings = "a"
""",
"invalid 'strict_settings': the setting must be a boolean",
id="invalid strict_settings type",
),
],
)
def test_toml_misconfiguration_handling(capsys: Any, config_file_contents, message_part) -> None:
@@ -124,29 +140,35 @@ def test_toml_misconfiguration_handling(capsys: Any, config_file_contents, messa
assert error_message == capsys.readouterr().err


def test_correct_toml_configuration() -> None:
@pytest.mark.parametrize('boolean_value', ['true', 'false'])
def test_correct_toml_configuration(boolean_value: str) -> None:
config_file_contents = """
[tool.django-stubs]
some_other_setting = "setting"
django_settings_module = "my.module"
"""
strict_settings = {0}
""".format(boolean_value)

with write_to_file(config_file_contents, suffix=".toml") as filename:
config = DjangoPluginConfig(filename)

assert config.django_settings_module == "my.module"
assert config.strict_settings is (boolean_value == "true")


def test_correct_configuration() -> None:
@pytest.mark.parametrize('boolean_value', ['true', 'True', 'false', 'False'])
def test_correct_configuration(boolean_value) -> None:
"""Django settings module gets extracted given valid configuration."""
config_file_contents = "\n".join(
[
"[mypy.plugins.django-stubs]",
"\tsome_other_setting = setting",
"\tdjango_settings_module = my.module",
"some_other_setting = setting",
"django_settings_module = my.module",
f"strict_settings = {boolean_value}"
]
).expandtabs(4)
)
with write_to_file(config_file_contents) as filename:
config = DjangoPluginConfig(filename)

assert config.django_settings_module == "my.module"
assert config.strict_settings is (boolean_value.lower() == "true")
29 changes: 29 additions & 0 deletions tests/typecheck/test_settings.yml
Original file line number Diff line number Diff line change
@@ -58,3 +58,32 @@
main:4: error: 'Settings' object has no attribute 'NON_EXISTANT_SETTING'
main:5: error: 'Settings' object has no attribute 'NON_EXISTANT_SETTING'
main:5: note: Revealed type is "Any"
- case: settings_loaded_from_runtime_magic
disable_cache: true
main: |
from django.conf import settings
reveal_type(settings.A) # N: Revealed type is "Any"
reveal_type(settings.B) # E: 'Settings' object has no attribute 'B' # N: Revealed type is "Any"
custom_settings: |
# Some code that mypy cannot analyze, but values exist in runtime:
exec('A = 1')
mypy_config: |
[mypy.plugins.django-stubs]
django_settings_module = mysettings
strict_settings = false
- case: settings_loaded_from_runtime_magic_strict_default
disable_cache: true
main: |
from django.conf import settings
reveal_type(settings.A) # E: 'Settings' object has no attribute 'A' # N: Revealed type is "Any"
reveal_type(settings.B) # E: 'Settings' object has no attribute 'B' # N: Revealed type is "Any"
custom_settings: |
# Some code that mypy cannot analyze, but values exist in runtime:
exec('A = 1')
mypy_config: |
[mypy.plugins.django-stubs]
django_settings_module = mysettings