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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,53 @@ 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,
for example `pyproject.toml`:

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

or `mypy.ini`:

```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"))
2 changes: 1 addition & 1 deletion mypy_django_plugin/lib/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ def resolve_string_attribute_value(attr_expr: Expression, django_context: "Djang
member_name = attr_expr.name
if isinstance(attr_expr.expr, NameExpr) and attr_expr.expr.fullname == "django.conf.settings":
if hasattr(django_context.settings, member_name):
return getattr(django_context.settings, member_name) # type: ignore
return getattr(django_context.settings, member_name) # type: ignore[no-any-return]
return None


Expand Down
6 changes: 5 additions & 1 deletion mypy_django_plugin/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,11 @@ def get_attribute_hook(self, fullname: str) -> Optional[Callable[[AttributeConte

# Lookup of a settings variable
if class_name == fullnames.DUMMY_SETTINGS_BASE_CLASS:
return partial(settings.get_type_of_settings_attribute, django_context=self.django_context)
return partial(
settings.get_type_of_settings_attribute,
django_context=self.django_context,
plugin_config=self.plugin_config,
)

info = self._get_typeinfo_or_none(class_name)

Expand Down
12 changes: 11 additions & 1 deletion mypy_django_plugin/transformers/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from mypy.types import AnyType, Instance, TypeOfAny, TypeType
from mypy.types import Type as MypyType

from mypy_django_plugin.config import DjangoPluginConfig
from mypy_django_plugin.django.context import DjangoContext
from mypy_django_plugin.lib import helpers

Expand All @@ -19,7 +20,9 @@ def get_user_model_hook(ctx: FunctionContext, django_context: DjangoContext) ->
return TypeType(Instance(model_info, []))


def get_type_of_settings_attribute(ctx: AttributeContext, django_context: DjangoContext) -> MypyType:
def get_type_of_settings_attribute(
ctx: AttributeContext, django_context: DjangoContext, plugin_config: DjangoPluginConfig
) -> MypyType:
if not isinstance(ctx.context, MemberExpr):
return ctx.default_attr_type

Expand All @@ -42,5 +45,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 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")
39 changes: 39 additions & 0 deletions tests/typecheck/test_settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,42 @@
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

# Global:
reveal_type(settings.SECRET_KEY) # N: Revealed type is "builtins.str"

# Custom:
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

# Global:
reveal_type(settings.SECRET_KEY) # N: Revealed type is "builtins.str"

# Custom:
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