A005 stdlib-module-shadowing warns that utils.logging shadows python logging module

[dwt]

This rule warns for me that utils.logging shadows stdlib logging.

Is this intentional? The whole point why our logging module is in utils is so the names do not collide. Am I missing something?

[MichaReiser]

Hi. Thanks for opening the question.

This seems to be matching the upstream behavior (more as a note to myself rather than this being a justification why):

https://github.com/gforcada/flake8-builtins/blob/5484162ace8d9546513be710d70e9c148010d9cc/flake8_builtins.py#L292

I'm not 100% sure if this is the right behavior or if the rule is too eager to warn. IMO, the rule should warn if your module is named logging because it then shadows the stdlib module. Naming it utils.logging seems less problematic to me. @AlexWaygood am I overlooking something?

[mattmess1221]

I think this rule should only trigger for modules if there isn't an __init__.py file in the directory OR any module in the package is executable. e.g. it has a shebang or if __name__ == '__main__': check.

This is the only way I can see module shadowing being possible, as doing so prepends the parent directory to sys.path, overriding the stdlib.

[MichaReiser]

I discussed this with @AlexWaygood in our 1:1 and we think that we should change the default to only flag modules that have the exact same full qualified name as a standard library module and instead, add an option to enable the more strict linting that flags modules that only have the same name.

The motivation for an option is that some projects might want stricter behavior to guarantee that modules don't shadow builtins when running a module from a subdirectory (e.g. cd into the utils directory and run a script from there).

[InSyncWithFoo]

Somehow I can't replicate the problem.

1. Library layout:

   .
   ├── pyproject.toml
   └── src
       └── test
           ├── __init__.py
           └── utils
               └── logging.py
   

   $ uvx ruff check --isolated --select A005 src
   All checks passed!

2. App layout:

   .
   ├── hello.py
   ├── pyproject.toml
   └── utils
       └── logging.py
   

   $ uvx ruff check --isolated --select A005
   All checks passed!

This is perhaps because package is None is such cases.

---

One thing to note is that package is always Root and never Nested, even for this straight-out-of-the-docs case where baz/__init__.py is passed directly to Ruff:

.
├── README.md
├── pyproject.toml
└── src
    └── foo
        ├── __init__.py
        └── bar
            └── baz
                └── __init__.py

Meanwhile, test_contents uses a hardcoded Root call.

Is this a bigger bug?

[martina-oefelein]

It triggers if util is a regular package (contains an __init__.py):

.
├── pyproject.toml
└── src
    └── test
        ├── __init__.py
        └── utils
            ├── __init__.py
            └── logging.py

$ uvx ruff check --isolated --select A005 src
src/test/utils/logging.py:1:1: A005 Module `logging` shadows a Python standard-library module

[InSyncWithFoo]

Thanks. In that case, this has something to do with the "never-Nested" problem above.

[scott-boost]

├── pyproject.toml
└── AcmeOrg
    └── http
        ├── __init__.py
        └── utils
            ├── __init__.py
            └── logging.py

interesting that flake8 didnt trigger A005 in this setup BUT ruff did
(notice that AcmeOrg doesnt contain a __init__.py file)

[rh-sp]

Opinion

I think that the current rule is too strict, but "only fully qualified names" sounds like it could be too loose, depending on how it's interpreted. In my opinion, any root-level packages that are named the same as any stdlib root-level package should be blocked, along with all their child packages.

Example

• abc ❌
• collections ❌
  • abc ❌
  • foobar ❌
• foobar ✅
  • abc ✅
  • collections ✅
    • abc ✅
• urlparse ✅

Reasoning

I haven't been around then, but from what I've heard, in the olden days, it wasn't well-defined what import abc does when in a package. Does it refer to the root-level package abc, or the abc defined at the current package level? So in that sense, for older Pythons, it made sense to flag foobar.abc as potentially shadowing abc.

However, in modern Python, this issue has been resolved. import abc always refers to the root-level package, never foobar.abc (or collections.abc). At the same time, .abc (which can stand for foobar.abc or collections.abc, depending on context) can never shadow abc. So, foobar.abc does not need to trigger the rule.

Additionally, collections.foobar should also trigger the rule, even though the fully qualified name doesn't exist in stdlib. This is because if you import collections.foobar, while the import statement itself can only mean one thing, from what I understand in order for it to work, collections must be shadowed, so the problem is still there.

Hope that makes sense :)

[flying-sheep]

import abc always refers to the root-level package, never foobar.abc (or collections.abc). At the same time, .abc (which can stand for foobar.abc or collections.abc, depending on context) can never shadow abc. So, foobar.abc does not need to trigger the rule.

Yeah definitely. This rule has a lot of false positives.