astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
32.29k stars 1.08k forks source link

A005 (builtin module shadowing) should ignore private modules #12949

Open jab opened 2 months ago

jab commented 2 months ago

I just updated from ruff 0.5.0 to 0.6.1, and had to add an ignore for A005 so that ruff check would continue to pass:

https://github.com/jab/bidict/commit/57a0d0b592c2db2d8114452bb6615dcb1d64876e

Without that ignore, it would fail as follows:

❯ pre-commit run --all-files --verbose --show-diff-on-failure ruff
ruff.....................................................................Failed
- hook id: ruff
- duration: 0.01s
- exit code: 1

bidict/_abc.py:1:1: A005 Module `_abc` is shadowing a Python builtin module
Found 1 error.

This check is failing because it treats even private builtin modules as not-to-be-shadowed, and Python provides a private _abc builtin:

❯ python -c 'import _abc'  # succeeds

I propose that private modules be excluded from this check.

charliermarsh commented 2 months ago

Note that you can suppress this via https://docs.astral.sh/ruff/settings/#lint_flake8-builtins_builtins-allowed-modules.

charliermarsh commented 2 months ago

I would say it's reasonable to flag these though. What do you think @AlexWaygood?

jab commented 2 months ago

While we're waiting for others' feedback, I realized I didn't yet provide rationale for why private modules should be excluded from this check, so here is some:

Quoting from ruff's own https://docs.astral.sh/ruff/rules/import-private-name/ rule:

PEP 8 states that names starting with an underscore are private. Thus, they are not intended to be used outside of the module in which they are defined. Further, as private imports are not considered part of the public API, they are prone to unexpected changes, especially outside of semantic versioning.

I'm not sure how much ruff tries to harmonize conflicting rules that users have enabled, but this could be an opportunity to improve harmony.

Also, a lot of the builtin private modules are like _abc (which is where I hit this), in that:

Another interesting observation is that, out of the dozens of private modules in the standard library, https://docs.python.org/3/py-modindex.html documents only two of them, _thread and _tkinter, further suggesting that the others are not intended to be directly imported, and outside contexts should also have those names available for internal use.

Click for list of builtin private modules in Python 3.9, in case it's helpful ``` _abc _aix_support _ast _asyncio _bisect _blake2 _bootlocale _bootsubprocess _bz2 _codecs _codecs_cn _codecs_hk _codecs_iso2022 _codecs_jp _codecs_kr _codecs_tw _collections _collections_abc _compat_pickle _compression _contextvars _crypt _csv _ctypes _ctypes_test _curses _curses_panel _datetime _dbm _decimal _elementtree _functools _hashlib _heapq _imp _io _json _locale _lsprof _lzma _markupbase _md5 _multibytecodec _multiprocessing _opcode _operator _osx_support _peg_parser _pickle _posixshmem _posixsubprocess _py_abc _pydecimal _pyio _queue _random _scproxy _sha1 _sha256 _sha3 _sha512 _signal _sitebuiltins _socket _sqlite3 _sre _ssl _stat _statistics _string _strptime _struct _symtable _sysconfigdata__darwin_darwin _testbuffer _testcapi _testimportmultiple _testinternalcapi _testmultiphase _thread _threading_local _tkinter _tracemalloc _uuid _virtualenv _warnings _weakref _weakrefset _xxsubinterpreters _xxtestfuzz _zoneinfo ```
jab commented 2 months ago

All that said, I'm fine with just keeping my...

[tool.ruff.lint.flake8-builtins]
builtins-allowed-modules = ["_abc"]

config if it's not worth making any changes here.