function-redefined false positive

[sushobhit27]

Steps to reproduce

1. cat sample6.py

import csv

def create_csv(output_file, columns, rows, transform_func=None, as_dict=False):
    if transform_func is None:
        def transform_func(x):
            return x

    if not as_dict:
        with open(output_file, 'w') as f:
            writer = csv.writer(f)
            writer.writerow(columns)

            for row in rows:
                row = transform_func(row)
                if not isinstance(row, (tuple, list)):
                    row = (row,)
                writer.writerow(row)

2. pylint sample6.py

Current behavior

************* Module sample6
/home/sussolan/git_repos/pylint/sample6.py:5:8: E0102: function already defined line 3 (function-redefined)

---

Your code has been rated at 6.43/10

Expected behavior

There should not be function-redefined error as originally transform_func is an argument and moreover function is defined only when the argument's value is None. Therefore function is defined only once.

pylint --version output

pylint 2.2.0
astroid 2.0.1
Python 3.6.4 (default, Jan 7 2018, 15:53:53)
[GCC 6.4.0]

[PCManticore]

In this example we should most likely warn about shadowing a variable with our new function. Most likely we need to adapt redefined-argument-from-local to also check for this use case.

[sushobhit27]

In this example we should most likely warn about shadowing a variable with our new function. Most likely we need to adapt redefined-argument-from-local to also check for this use case.

make sense

[cdeil]

I got the same false positive here:

https://github.com/gammapy/gammapy/blob/cb993b580e8a54a776ed9fcaf534eddd320e79e4/gammapy/irf/psf_table.py#L541-L543

As mentioned above the error message is very confusing, it incorrectly states that a function was defined above.

I'm not even sure this should emit an error, passing callback=None and then defining a default callback if none is passed seems completely valid. That's the common and encouraged coding style for default mutable arguments, and I think (less common) for callbacks also, no?

Minimal test case to reproduce:

def f(a=None):
    if a is None:
        def a():
            print('hi')

    a()


f()
f(lambda _: print('ho'))

$ pylint -E test.py
************* Module test
test.py:3:8: E0102: function already defined line 1 (function-redefined)

[PCManticore]

@cdeil I definitely think this should trigger an error, albeit a better one than it does today. It feels cleaner to have the callback defined outside of the function, either a simple function or a staticmethod attached to the current class, and set it as a default value of the parameter, instead of having it defined in the body of the function under a condition.