Add Django 1.10+ request.user attribute fixer

[Alurith]

Fixes #419

[adamchainz]

Thanks for starting on this one! There’s still an unused import, and I’ve left one comment on an issue I spotted.

Please also add a changelog note crediting yourself and documentation in the style of the other fixers.

[adamchainz]

Find the opening parenthesis and delete from the opening to closing only, rather than j+1. There may be space, even newlines and comments, before the parenthesis!

Add these tests to see what I mean:

request . user . is_authenticated ( )

if (
    request
    .user
    .is_authenticated  # bla
    ()
):
    ...

[Alurith]

Thanks for the feedback, I also found a whitespace in the fixer's filename that I removed.

I am working on a fix for the additional cases you suggested but I encountered an issue.
I've changed the rewrite function like this:

def rewrite_user_is_auth(tokens: list[Token], i: int) -> None:
    j = find(tokens, i, name=NAME, src="is_authenticated")
    y = find(tokens, j, name=OP, src="(")
    z = find(tokens, y, name=OP, src=")")
    del tokens[z]
    del tokens[y]

but I can't make it work with this case

def test_spaces_and_comments():
    check_transformed(
        """\
        if (
            request
            .user
            .is_authenticated  # bla
            ()
        ):
            ...
        """,
        """\
        if (
            request
            .user
            .is_authenticated  # bla
            
        ):
            ...
        """,
        settings,
    )

This is the output of pytest

>       assert fixed == dedented_after
E       AssertionError: assert 'if (\n    request\n    .user\n    .is_authenticated  # bla\n    \n):\n    ...\n' == 'if (\n    request\n    .user\n    .is_authenticated  # bla\n    ()\n):\n    ...\n'
E
E           if (
E               request
E               .user
E               .is_authenticated  # bla
E         -     ()
E         ?     --
E         +     
E           ):
E               ...

tests\fixers\tools.py:25: AssertionError

It seems the only different between the two string are some whitespaces after the comment, so I tought it was because of my formatter but even disabling it or thinkering with the whitespaces didn't solve the issue.

I am sure I am missing something here, can you point me in the right direction?

[adamchainz]

The problem is gonna be because either Black or textwrap.dedent() trims trailing whitespace in the file, whilst the fixer leaves it. We’re fine leaving the whitespace, django-upgrade leaves formatting to Black or whatever.

This version of the test should work:

def test_spaces_and_comments():
    check_transformed(
        """\
        if (
            request
            .user
            .is_authenticated  # bla
            () # bla
        ):
            ...
        """,
        """\
        if (
            request
            .user
            .is_authenticated  # bla
             # bla
        ):
            ...
        """,
        settings,
    )

[adamchainz]

Ah, doing two del operations will also break things. After deleting a token, the laters ones shift leftwards in position, decreasing their indexes. Best to delete a range instead - leaving a suggestion.

[Alurith]

I was deleting first the closing parenthesis w/ del tokens[z] and the the opening one w/ del tokens[y] to not shift positions. Doing as you suggest will remove the space between the parentheses in this case.

def test_spaces_between():
    check_transformed(
        "request . user . is_authenticated ( )",
        "request . user . is_authenticated  ",
        settings,
    )

Is that fine?

[adamchainz]

Yes, that is more consistent IMO. It will remove a comment in this obscure case:

request.user.is_anonymous(  # something
)

But I think that’s fine.

[adamchainz]

I read the release notes section and noted that is_anonymous was also changed. Let’s make the fixer cover both (suggestions below), rename the fixer file to request_user_attributes.py, and the tests to test_request_user_attributes.py.

I will action these changes now to speed things along.

[adamchainz]

Ah, doing two del operations will also break things. After deleting a token, the laters ones shift leftwards in position, decreasing their indexes. Best to delete a range instead - leaving a suggestion.

[adamchainz]

Thank you for your contribution! Released in 1.16.0.