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
46 changes: 46 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
sobolevn marked this conversation as resolved.
Show resolved Hide resolved

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

or
sobolevn marked this conversation as resolved.
Show resolved Hide resolved

```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.
Expand Down
24 changes: 18 additions & 6 deletions mypy_django_plugin/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
intgr marked this conversation as resolved.
Show resolved Hide resolved


def exit_with_error(msg: str, is_toml: bool = False) -> NoReturn:
Expand All @@ -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:
Expand All @@ -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)
Expand All @@ -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:
Expand All @@ -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
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions mypy_django_plugin/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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":
Expand Down
9 changes: 8 additions & 1 deletion mypy_django_plugin/transformers/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
38 changes: 31 additions & 7 deletions tests/test_error_handling.py
Original file line number Diff line number Diff line change
Expand Up @@ -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: {}
"""
Expand All @@ -21,6 +22,7 @@
...
[tool.django-stubs]
django_settings_module = str (required)
strict_settings = bool (default: true)
...
(django-stubs) mypy: error: {}
"""
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -124,29 +140,37 @@ 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 = {}
""".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
Expand Up @@ -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"
sobolevn marked this conversation as resolved.
Show resolved Hide resolved
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