Increase priority of mypy internal Django settings import

[intgr]

As discussed in #2097 (reply in thread):

• django-stubs 5.0.0 introduces new cases of the "Import cycle from Django settings module prevents type inference" error that did not occur in 4.2.7.
• This was bisected down to the change in Use PRI_MYPY in get_additional_deps hook #2024.
• Updated the priority of internal settings import to mypy's PRI_HIGH level.

Related issues

• Tests: Add test case for settings import cycle #2098

[intgr]

I wonder if we can come up with another test case that produces the "Import cycle from Django settings module prevents type inference" error?

Would be nice to have test coverage for this code still.

[flaeppe]

Yeah, agreed but I'm not sure what that test could look like either.

If I've read the plugin code correctly it only interferes when the type checking haven't inferred a type of the settings module at the stage where plugin is invoked.

I tried importing like import django.conf, since that seemed to have lower priority. Then I get the error, but I'm not sure if that could be because get_additional_deps doesn't consider that style of importing properly.

[flaeppe]

Checking again I don't think it's anything with get_additional_deps. Perhaps that's enough to add for now at least.

All I did was to keep the test_setting_circular_import test case and change from django.conf import settings to import django.conf.

Perhaps you could add that case to the suite in this PR?

[intgr]

Indeed, updated the testcase as you suggested, thanks!

[intgr]

Well, I managed to coax a different error out of mypy.

[flaeppe]

Interesting, I think that's totally unresolvable for mypy since there's no type hint involved at all for the type checker

[flaeppe]

Oh, right, it used settings.IMMEDIATE_VALUE. Then there's actually a chance for it to figure it out

[asottile-sentry]

idk what to do with this but it seems to help my case at least

$ git diff
diff --git a/mypy_django_plugin/transformers/settings.py b/mypy_django_plugin/transformers/settings.py
index f821b971..61d50617 100644
--- a/mypy_django_plugin/transformers/settings.py
+++ b/mypy_django_plugin/transformers/settings.py
@@ -38,10 +38,7 @@ def get_type_of_settings_attribute(
             sym = module.names.get(setting_name)
             if sym is not None:
                 if sym.type is None:
-                    ctx.api.fail(
-                        f"Import cycle from Django settings module prevents type inference for {setting_name!r}",
-                        ctx.context,
-                    )
+                    typechecker_api.handle_cannot_determine_type(setting_name, ctx.context)
                     return ctx.default_attr_type
                 return sym.type
 

[flaeppe]

Do you think we should add the test I mentioned?

It's fine for me to merge this as is

[flaeppe]

We could investigate handle_cannot_determine_type further too. It looks interesting

I'm not familiar with it either though

[intgr]

Cool, indeed @asottile-sentry's suggestion fixes the root cause of the issue, by deferring the analysis to a later pass.

    def handle_cannot_determine_type(self, name: str, context: Context) -> None:
        node = self.scope.top_non_lambda_function()
        if self.pass_num < self.last_pass and isinstance(node, FuncDef):
            # Don't report an error yet. Just defer. Note that we don't defer
            # lambdas because they are coupled to the surrounding function
            # through the binder and the inferred type of the lambda, so it
            # would get messy.

But I think it's still useful to increase the priority of settings import, as this reduces the number of passes mypy needs to make and hopefully speeds things up. Apparently it's not a rare case.

For the sake of clear git history, let's merge this and then handle_cannot_determine_type as a separate PR.

[asottile-sentry]

fwiw I needed both of the improvements to make sentry's cycle warnings go away -- though we have some pretty badly cyclic stuff :/

[intgr]

It seems that handle_cannot_determine_type() only helps while analysing a function body. But when in module scope, it bails immediately.

[intgr]

UGH! We have a catch-22.

This actually regresses one of my projects where one settings file imports a "base" config file settings_env -> settings_base -> imports other stuff. Unfortunately, I noticed after I had already merged this. I wasn't able to boil this down to a simple test case though.

Changing PRI_HIGH to PRI_MED seems to solve all issues in my case. @asottile-sentry could you test if PRI_MED works for you as well?

[asottile-sentry]

seems fine with that yeah, PRI_MYPY is too low though