afonasev / flake8-return

Flake8 plugin for return expressions checking.
MIT License
62 stars 69 forks source link

R504 false positive when variable is a function parameter #133

Open jakkdl opened 1 year ago

jakkdl commented 1 year ago

Description

def foo(a):
    if ...:
        a = 2
    return a
$ flake8 foo.py --select=R
foo.py:4:12: R504 unnecessary variable assignment before return statement.

variables that are function parameters should not be treated as unnecessary, cursory skim of https://github.com/afonasev/flake8-return/blob/master/flake8_return/mixins/unnecessary_assign.py looks like it doesn't look at FunctionDef or AsyncFunctionDef at all which explains it.

afonasev commented 1 year ago

Thank you for your issue! My personal opinion is that it is bad practice to change the value of an argument within a function, it is better to use a separate variable. But I will be glad to accept your contribution, which will correctly handle FunctionDef or AsyncFunctionDef.

jakkdl commented 1 year ago

Sure, it's got typing issues too - but the code that inspired the above example was actually an attempt to get rid of R504 for a more complex case - where the underlying complexity is in class attribute modifications where adding a new variable doesn't help:

class A:
    def foo(self, a):
        b = a
        if ...:
            b = function(self.class_var)
        self.class_var = None

        return b

this will also raise R504, and only way to get around that is with a different temporary variable (or even one per class variable you're caring about, which quickly becomes a vastly inferior solution, or in my case modifying a lot of state with a method call).

class A:
    def foo(self, a):
        tmp_var = self.class_var
        self.class_var = None

        if ...:
            return function(tmp_var)

        return a

I'm much less sure about how to go about attempting to get rid of that false positive though.