LoginView has a typing issue

[ghost]

bug report

what's wrong

django's LoginView (django.contrib.auth.views.LoginView) violates the Liskov substitution principle. we see here that the view's form_valid method calls form.get_user(), meaning that form must be of type AuthenticationForm. however, FormMixin defines form as BaseForm, and attempting to type form properly results in a violation of the liskov substitution principle

the following minimal failing example overrides LoginView and copies the form_valid method exactly (with type hints added):

from django.contrib.auth.views import LoginView
from django.forms import BaseForm
from django.contrib.auth import login as auth_login
from django.http import HttpResponseRedirect
from django.contrib.auth.forms import AuthenticationForm

class MyLoginView(LoginView):
    def form_valid(self, form: AuthenticationForm) -> HttpResponseRedirect:
        """Security check complete. Log the user in."""
        auth_login(self.request, form.get_user())
        return HttpResponseRedirect(self.get_success_url())

it fails with the following error:

$ mypy testproj/views.py 
testproj/settings.py:28: error: Need type annotation for 'ALLOWED_HOSTS' (hint: "ALLOWED_HOSTS: List[<type>] = ...")
testproj/views.py:7: error: Argument 1 of "form_valid" is incompatible with supertype "FormMixin"; supertype defines the argument type as "BaseForm"
testproj/views.py:7: note: This violates the Liskov substitution principle
testproj/views.py:7: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
Found 2 errors in 2 files (checked 1 source file)

how is that should be

this shouldn't fail. i'm not exactly sure if this can be fixed in django-stubs or if this is something that needs to be fixed in django

clearly django's LoginView class expects form to be an AuthenticationForm due to the get_user method call, but typing it as such would violate the liskov principle

system information

• os: fedora 32
• python version: 3.8.6
• django version: 3.1.2
• mypy version: 0.790
• django-stubs version: 1.7.0

[ghost]

additional info:

a work-around is to override form_valid as follows:

def form_valid(self, form: BaseForm) -> HttpResponseRedirect:
    # insert any additional logic here
    return super().form_valid(form)

which is fine for my current use-case, but doesn't work if someone needs to completely override the method

i think that if this isn't possible to fix in django-stubs, there should be something in the documentation mentioning that workaround

[sobolevn]

Yes, I have seen this myself 😞

PRs are welcome!

[ghost]

i honestly have no idea how to even fix this, i don't think it's possible to do it on django-stubs' end. only thing i can think of would be to make an AuthenticationFormMixin that's just a copy of FormMixin except it takes an AuthenticationForm instead of a BaseForm.

maybe make FormMixin generic for the form class?

[sobolevn]

As a wild idea, we can try to make FormView generic on a form type 😆
But, I am not sure it will work.

[ghost]

since form_valid is defined in FormMixin, we'd have to make the entire subclass stack generic. here's what i came up with, mypy playground says it typechecks:

from typing import Generic, TypeVar

T = TypeVar('T', bound=BaseForm)

class ContextMixin: ...

class BaseForm: ...

class AuthenticationForm(BaseForm): ...

class FormMixin(Generic[T], ContextMixin):
    def form_valid(self, form: T): ...

class LoginView(FormView[T]): ...

class FormView(BaseFormView[T]): ...

class BaseFormView(FormMixin[T]): ...

class MyLoginView(LoginView[AuthenticationForm]):
    def form_valid(self, form: AuthenticationForm): ...

[ghost]

whoops i got my class hierarchy wrong. i think it should still work though

[ghost]

looks good, running tests one last time but i don't think there'll be any issues. i'll send the pr as soon as it finishes!