wemake-services / wemake-python-styleguide

The strictest and most opinionated python linter ever!
https://wemake-python-styleguide.rtfd.io
MIT License
2.49k stars 381 forks source link

Forbid name shadowing #426

Closed sobolevn closed 5 years ago

sobolevn commented 5 years ago

Rule request

Thesis

We can shadow names when using nested functions. Like in this example:

def test():
    test = 1

    def other(test):
        print(test)

    other(2)

test()

Can you tell what will be printed from the first sight?

Reasoning

Name shadowing can be very tricky. And it will cause different errors in your code. We need to prevent that.

kxepal commented 5 years ago

That's easy - 2. You just start from local context and getting through outer ones until hit globals. Your example is too syntetic, but consider this one:

def any(thing):
  thing = some(thing)
  def no(thing):
    pass
  return no(thing)

We want to explicitly pass thing to inner no function, but it still named the same because it means the same outer one. Is that wrong?

sobolevn commented 5 years ago

Yes, it is. There's no need to:

  1. Use nested functions generally (we only allow factories and decorators)
  2. Use explicit parameters instead of closures
  3. Naming it the same way as original parameter, since it is already another thing, because it now acts in different context
kxepal commented 5 years ago

Well, if you forbid point 1, I don't see any cases when this issue may exists. However, there would be need to detect decorator-functions. But their pattern +/- quite oblivious.

sobolevn commented 5 years ago

@kxepal we already forbid most nested functions except decorator and factory functions. See https://wemake-python-styleguide.readthedocs.io/en/latest/pages/violations/best_practices.html#wemake_python_styleguide.violations.best_practices.NestedFunctionViolation

kxepal commented 5 years ago

Awesome! Than that this issue shouldn't be an issue at all. What's the case?

sobolevn commented 5 years ago

For decorator and factory functions that can be nested 🙂

And also someone can use # noqa to disable the nested function violation. But, we still will be able to find other possible errors.

kxepal commented 5 years ago

But only for one (simple decorators) or two levels (paratetrized decorators). Nothing deeper and there closure is used to by pass arguments, so no shadowing happens.

kxepal commented 5 years ago

Do you have rule to forbid # noqa commetaries? That should be helpful.

sobolevn commented 5 years ago

Yes, I have a plan to forbid too many noqa comments.

sobolevn commented 5 years ago

The same should also work for this case:

def factory(factory):
      def factory():
            factory = 1

There are three violations here:

sobolevn commented 5 years ago

We can also shadow attributes and methods:

class Test(object):
     some = 1

     def some():
          ...
sobolevn commented 5 years ago

Import shadowing is already handled by flake8.

sobolevn commented 5 years ago

Related #399

I will close this issue in order to work on #399 with bigger scope.