Do not infer a shadowed builtin as the builtin

[teddytennant]

Type of Changes

	Type
✓	🐛 Bug fix

Description

register_builtin_transform picks its tip by name text alone. A parameter called type is treated as builtins.type:

def convert_type(x, type):
    return type(x)

convert_type(12.34, str).split(".")

astroid turns that into ClassDef.float, and pylint reports E1101: Class 'float' has no 'split' member (pylint-dev/pylint#10994). Same hole for every transform registered this way (len, str, list, ...).

The fix is to resolve the name before running the transform, and fall back to normal inference when it is not the real builtin. Lookup goes through the name node (so later defs in the same scope do not count), with a small extra step for default args, which run in the enclosing scope. from builtins import str is treated as the builtin too.

Review follow-up

Addressed the cases Pierre flagged where the first version was too strict:

• from builtins import str / from builtins import *
• use before a later def in the same module or class body
• default and kw-only default values (def h(x, len=len([...])))

Also filled in the missing unshadowed controls for getattr, hasattr, property, and super.

On the cache note: the check stays inside the tip (same place as before) so the cheap name filter is unchanged. Restructuring so the tip could still be cached on a miss did not seem worth the churn for this fix; happy to revisit if it shows up in profiles.

Verification

tests/brain/test_builtin.py  57 passed
full suite                   2108 passed, 66 skipped, 15 xfailed

Closes pylint-dev/pylint#10994

[codspeed-hq]

Merging this PR will not alter performance

✅ 3 untouched benchmarks
⏩ 1 skipped benchmark¹

---

_{Comparing teddytennant:fix-shadowed-builtin-inference (6203fb3) with main (cec5b89)}

Footnotes

1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩

[codecov]

Codecov Report

❌ Patch coverage is 94.87179% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.66%. Comparing base (94b7090) to head (6203fb3).
⚠️ Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
astroid/brain/brain_builtin_inference.py	88.23%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3211   +/-   ##
=======================================
  Coverage   93.65%   93.66%           
=======================================
  Files          93       93           
  Lines       11613    11660   +47     
=======================================
+ Hits        10876    10921   +45     
- Misses        737      739    +2     

Flag	Coverage Δ
linux	93.53% <94.87%> (+0.01%)	⬆️
pypy	93.66% <94.87%> (+<0.01%)	⬆️
windows	93.63% <94.87%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
astroid/nodes/scoped_nodes/scoped_nodes.py	93.46% <100.00%> (+0.17%)	⬆️
astroid/brain/brain_builtin_inference.py	93.70% <88.23%> (-0.17%)	⬇️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Pierre-Sassoulas]

Thanks, this look promising !

I ran the pylint primer on the branch and found no new pylint false positives. But there's a set of cases where lookup() resolves differently from Python, so the tip now bails where it used to infer.

from builtins import str
str(42)

main infers Const('42'), this branch infers a plain Instance of builtins.str.

class Fruit:
    size = len("apple")   # Python: builtin, `len` is not bound in the class namespace yet
    def len(self):
        return 0

main infers 5, this branch raises InferenceError. Same at module level:

size = len([1, 2, 3])
def len(x):
    return "shadowed"

main infers 3, this branch raises InferenceError.

def h(x, len=len([1, 2, 3])):   # the default is evaluated in the *enclosing* scope
    return x

main infers 3, this branch infers Uninferable.

[Pierre-Sassoulas]

This prevents the use of _inference_tip_cached later, not sure if it's worth resturcturing for.

[teddytennant]

Yeah, fair point. I left the check in the tip so the cheap name filter stays as-is. Pulling it into the predicate (or otherwise making misses cacheable) felt like a bigger reshuffle than this bug needs. Easy to revisit if it shows up hot.

[teddytennant]

Thanks for the careful look. Pushed a follow-up that should cover the cases you hit:

• lookup from the name node (so later defs do not steal the builtin)
• from builtins import ... / star imports
• defaults and kw-only defaults evaluated in the enclosing scope
• the missing getattr / hasattr / property / super controls

Full suite is green on my side. Happy to tweak further if anything still looks off.

[Pierre-Sassoulas]

You were right that Name.lookup already handles statement order.

I think there's still a a scoped_nodes.py bug. _name_lookup_frame duplicates Lambda/FunctionDef.scope_lookup's default branch and calls the private _scope_lookup. Those test node in self.args.defaults, so a name inside a default never matches. If we fix this a comprehension or lambda in a default would be covered for free:

  def h(x, len=[len(y) for y in ([1, 2],)]):
  def h(x, len=lambda: len([1, 2])):

But it's pre-existing, feel free to not touch. Either way, annotations are the same family and neither version covers them (def h(x: str("a") = 1, str=None)), decorators likewise.

[teddytennant]

Went ahead and fixed it in scoped_nodes, you were right that it was the better place.

Lambda.scope_lookup and FunctionDef.scope_lookup now share a small helper that checks whether the node sits anywhere inside a default value, not just whether it is the default. Both of your cases work now:

def h(x, len=[len(y) for y in ([1, 2, 3],)]):
def h(x, len=lambda: len([1, 2]))

Both used to come back Uninferable on my branch and now infer 3, same as main.

That also let me delete _name_lookup_frame from the brain, it just calls lookup() now. Two of the three lines codecov was complaining about went with it and I covered the third, so patch coverage should be clean.

I wrote the helper to walk up from the node rather than call parent_of on each default, since scope_lookup is hot and the walk stops at the function. Curious what codspeed says.

Added tests in test_lookup.py for the lookup change itself and in test_builtin.py for the two inference cases. Full suite green locally.

You are right about annotations, they are still not covered. def h(x: str("a") = 1, str=None) is a different branch in scope_lookup and it felt like scope creep for this PR. Happy to open a separate issue for it. Decorators are actually already fine, Decorators.scope() skips the function frame, so @deco(str("a")) on def h(str=None) resolves correctly on both main and this branch.

[teddytennant]

Rebased on main. The only conflict was the ChangeLog, which is towncrier now, so the entry moved to doc/whatsnew/fragments/3211.bugfix and covers both the brain change and the scope_lookup one.

The red pylint job was test_functional[wrong_import_order_py315] complaining that astroid is classified first party instead of third party. It failed the same way on module-in-path-subpath-check two days ago and the runs since yesterday are green, so the rebase should sort it.

Local suite is 2088 passed, 2 failed, and both failures are my environment (no pkg_resources, and a typing_extensions mismatch). They fail identically on a clean origin/main.

[teddytennant]

Thanks for applying the fragment edit and adding the xfail tests directly, nothing left on my side. Happy to take the two xfail cases in this PR if you would rather not leave them as follow-ups.

[Pierre-Sassoulas]

Sorry I made #3232 and intended to merge it back here but got drowned in notifications. Fixing the xfail wxould make sense if you don't mind.

[teddytennant]

Rebased on main and fixed both xfails.

The import one was checking the alias instead of what was imported under it, so from builtins import int as str still looked like the real str. It asks real_name now.

The annotation one needed a small correction to the plan. Type parameters live in func.locals, so sending annotations to the enclosing scope broke def f[T](x: T). Annotations see the type parameters and defaults do not, so the helper now reports which of the two a node is in rather than just yes or no. Return annotations go with the annotations.

The xfail for the import case asserted Const(42), which was never going to happen since astroid infers int(42) as an instance. Changed the assertion to that.

Ran pylint's suite against this branch, same results as against main.

[Pierre-Sassoulas]

Thank you for fixing the two test case. I added two other test cases, The issue seems to be thatr brain_type.infer_type_sub lookup from scope, not name node:

-    node_scope, _ = node.scope().lookup("type")
+    node_scope, _ = node.lookup("type")

And for the other one we need to check is something is a bare annotation, there's probably a helper function for that.

[teddytennant]

The CodSpeed red looks like noise. The gate passed on 2ecc442, and the only commit since is your test-only one, which can't cost 10% on cold lint. main picked up the CodSpeedHQ action bump in between, and the report itself says it compared across different runtime environments.

codecov is down to the except AttributeInferenceError in _is_from_builtins_import. I don't think lookup() can reach it, since it only hands back an import that binds the name, so I'd rather mark it than invent a test for it.

Still on the two xfails. The bare annotation one is a bigger family than the single case: len = 5 followed by len: int has to keep shadowing, from builtins import str followed by str: object has to stay the builtin, and def f(): len: int makes the name local, so a function body should keep shadowing where a module or class body does not. I had a version that passed your test and quietly broke the first of those, so I want the whole set covered before I push.