flake8-return wrongly indicates R504

[xqt]

• Date you used flake8-return: 2020-06-18
• flake8-return version used, if any: 1.1.1
• Python version: 3.8.0
• Operating System: Windows 10

Description

flake8-return wrongly indicates R504 for the following samples:

    formatted = _USER_AGENT_FORMATTER.format(format_string, **values)
    # clean up after any blank components
    formatted = formatted.replace('()', '').replace('  ', ' ').strip()
    return formatted  # <<< wrongly indicated as R504 issue

def user_agent_username(username=None):

    if not username:
        return ''

    username = username.replace(' ', '_')  # Avoid spaces or %20.
    try:
        username.encode('ascii')  # just test, but not actually use it
    except UnicodeEncodeError:
        username = quote(username.encode('utf-8'))
    else:
        # % is legal in the default $wgLegalTitleChars
        # This is so that ops know the real pywikibot will not
        # allow a useragent in the username to allow through a hand-coded
        # percent-encoded value.
        if '%' in username:
            username = quote(username)
    return username  # <<< wrongly indicated as R504 issue

[afonasev]

Thx for issue! I will try fix it.

[akx]

Also seeing a false positive with a pattern like

    def resolve_from_url(self, url: str) -> dict:
        local_match = self.local_scope_re.match(url)
        if local_match:
            schema = get_schema(name=local_match.group(1))
            self.store[url] = schema
            return schema  # "R504 unecessary variable assignement before return statement."
        raise NotImplementedError(...)

where clearly schema is required as a local as it's assigned into the dict.

[simon-liebehenschel]

One more example:

my_dict = {}

def my_func()
        foo = calculate_foo()
        my_dict["foo_result"] = foo
        return foo

Of course my code looks different. I showed a simplified example.

[ericbn]

@xqt, your first example could be rewritten as:

    formatted = _USER_AGENT_FORMATTER.format(format_string, **values)
    # clean up after any blank components
    return formatted.replace('()', '').replace('  ', ' ').strip()

but I totally agree that from a style perspective the original code looks better.

Maybe R504 seems like a controversial rule.

[ericbn]

Actually, maybe someone would want the opposite of R504 to keep a consistent style: "only return a variable (or a constant?), never an expression".

[peterjochum]

Here is another simple example that triggers R504:

def get_bar_if_exists(obj):
    result = ""
    if hasattr(obj, "bar"):
        result = str(obj.bar)
    return result  # noqa: R504 - variable can be modified after declaration