register_module_extender leaks _pydatetime / _collections_abc into qname() for star-import brains

[Chinmayrawat15]

Steps to reproduce

import astroid

for src in ("import datetime\ndatetime.datetime.utcnow",
            "import datetime\ndatetime.date.today",
            "import collections.abc\ncollections.abc.Iterable"):
    print(next(astroid.extract_node(src).infer()).qname())

Current behavior

_pydatetime.datetime.utcnow
_pydatetime.date.today
_collections_abc.Iterable

Expected behavior

datetime.datetime.utcnow
datetime.date.today
collections.abc.Iterable

The private implementation module leaks out of qname(), so anything that matches on qualified names has to know about _pydatetime and _collections_abc to work at all.

Where it comes from

register_module_extender copies the extension module's locals into the real module and reparents what it copied:

for name, objs in extension_module.locals.items():
    node.locals[name] = objs
    for obj in objs:
        if obj.parent is extension_module:
            obj.parent = node

That reparenting does the right thing for the ~30 brains that define their classes inline in the string_build source. It doesn't help the three that build the extension out of a star-import, because there extension_module.locals holds ImportFrom bindings rather than definitions. The binding gets reparented, inference then follows it into the private module, and qname() walks root() from a ClassDef that never moved.

The three are brain_datetime (from _pydatetime import *, 3.12+), brain_collections (from _collections_abc import *, 3.13+) and brain_decimal (from _pydecimal import *, only registered when _decimal is missing, so it rarely fires on CPython).

Why I'm raising it rather than just sending a patch

pylint currently depends on this behavior in two places, and I couldn't find a fix that doesn't have a cost worth discussing first.

On the pylint side, design_analysis.py lists 24 _collections_abc.* qnames in STDLIB_CLASSES_IGNORE_ANCESTOR and no public collections.abc.* equivalents, so fixing this changes too-many-ancestors. stdlib.py::_check_datetime matches {"_pydatetime.time", "datetime.time"}, which keeps working but leaves a dead entry. This came out of pylint-dev/pylint#11340, where @DanielNoord asked for the fix to live here instead of pylint, which I think is right.

I prototyped three approaches against main at 36179a4 on CPython 3.13.3. All three fix qname(). All three also passed the suite at 2071 passed with the same 4 pre-existing failures, except where noted.

One option is to return the private module directly, AstroidManager().ast_from_module_name("_pydatetime"), which is one line per brain. Because the whole module gets copied rather than just its __all__, datetime.locals goes from 16 entries to 58. datetime._MAXORDINAL and datetime._check_date_fields stop being no-member, which is a new false negative. It also reparents the cached _pydatetime nodes, so analysing that module afterwards reports them as living in datetime.

A second is to parse the private module's source and let the existing reparenting handle it. Same 16-to-58 namespace growth, no mutation of the cached module, but it duplicates a 2639-line AST and needs __file__, which isn't guaranteed.

A third is to resolve the wildcard inside register_module_extender and reparent the resolved definitions. This keeps datetime.locals at 16 with no private names, which is the behavior I'd want. But reparenting a ClassDef out of its own module cuts its scope chain, so _pydatetime.date can no longer see module-level helpers like _check_date_fields, and datetime.datetime.now() stops inferring. My attempt broke 12 tests. A narrower reparenting guard might save it, I'm not sure yet.

So the tension is between keeping the public namespace honest and keeping the private module's own scope intact. I'd rather not guess which one you'd accept.

Happy to write whichever you prefer, including the pylint side, since that needs to land in the same cycle.

python -c "from astroid import __pkginfo__; print(__pkginfo__.version)" output

4.4.0-dev0 (main at 36179a4). Also reproduced on 4.2.0b5 and 4.3.1.

[Chinmayrawat15]

Correcting one thing in the description, and adding a fourth approach that didn't work out either.

The correction is on the third approach, resolving the wildcard inside register_module_extender. The 12 test failures are real. But I also said datetime.datetime.now() stops inferring, and that was wrong. It was a formatting bug in my own probe, which choked on UninferableBase.__format__ and I read the traceback as an inference failure. Inference results there are identical to main. Sorry for the noise.

The fourth approach is to keep the star-import, so only __all__ gets bound and the namespace stays at 16 entries, and rename the cached private module instead of moving any node:

manager = AstroidManager()
module = AstroidBuilder(manager).string_build("from _pydatetime import *")
manager.ast_from_module_name("_pydatetime").name = "datetime"
return module

For datetime alone this looks like what I wanted. qname() is fixed, datetime.locals stays at 16 with no private helpers, _pydatetime.date keeps its own scope so _check_date_fields still resolves, and inference output matches main exactly. Suite is 2071 passed with only the 4 failures that are already on main here (namespace-package, symlink resolution, TypedDict).

Then I applied the same thing to brain_collections and it fell apart. 10 new failures across CollectionsBrain, TypingBrain and test_dataclasses. Worse, collections.abc.Iterable gave me collections.abc.Iterable in an isolated probe and _collections_abc.Iterable on a plain run of the same code, so the outcome depends on what got loaded first. That order-dependence is inherent to mutating a module the manager has already cached and handed out, which makes me think it isn't safe for datetime either, just currently lucky.

So all four approaches are still on the table only as sketches, and I'd rather have a steer from someone who knows the manager's caching contract before I turn any of them into a PR. The one thing I'm now fairly confident about is that mutating the cached private module in place, whether by reparenting its nodes or renaming it, is the wrong family of fix.