flake8-return icon indicating copy to clipboard operation
flake8-return copied to clipboard

R504 false positive when variable is a function parameter

Open jakkdl opened this issue 2 years ago • 2 comments

  • Date you used flake8-return: 2023-03-16
  • flake8-return version used, if any: 1.2.0
  • Python version, if any: 3.10.9
  • Operating System: Linux

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.

jakkdl avatar Mar 16 '23 11:03 jakkdl

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.

afonasev avatar Mar 16 '23 12:03 afonasev

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.

jakkdl avatar Mar 16 '23 13:03 jakkdl