Prepend builtins base name

[andrecsilva]

Overview

find_base_name will now prepend builtins for builtin names.
A caveat of this change is that libcst considers any name that is not imported or originates in the module as builtin. This means that in:

foo()

foo is considered builtin.
Disregard this caveat, it's actually working correctly. I assume that it was behaving like this because of a test case I had to change.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (269469d) 96.26% compared to head (59a08af) 96.25%.
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #209      +/-   ##
==========================================
- Coverage   96.26%   96.25%   -0.01%     
==========================================
  Files          88       89       +1     
  Lines        4065     4089      +24     
==========================================
+ Hits         3913     3936      +23     
- Misses        152      153       +1     

Files	Coverage Δ
src/codemodder/codemods/utils_mixin.py	92.15% <100.00%> (+0.06%)	⬆️
src/codemodder/utils/utils.py	95.23% <ø> (-0.22%)	⬇️
src/core_codemods/remove_debug_breakpoint.py	95.65% <100.00%> (ø)

... and 2 files with indirect coverage changes

[drdavella]

@andrecsilva can you explain the motivation for this change? Given the caveat that you mentioned, I am not entirely convinced that this is the behavior we want.

[andrecsilva]

@andrecsilva can you explain the motivation for this change? Given the caveat that you mentioned, I am not entirely convinced that this is the behavior we want.

The goal is to be closer to the __module__ . __qualname__ naming pattern for Python objects. For builtin objects/functions, the __module__ equals builtins.
Until now, find_base_name was simply returning the function name for builtin functions. This means you need an additional check if the function is builtin. For example, if you had locally redefined a builtin name, say, print, find_base_name would still return print despite not being builtin.
The caveat is not a problem as long as you're acting on trees that comes from compilable python files and not small snippets of it. Even worse, the caveat still exists even without those changes (any name without import/local definition will receive the BuiltinAssignment from the ScopeProvider).
If you think this is a problem, I can add an extra check for inclusion in dir(__builtins__).
The above is wrong, see the edit to the description and comment below.

[clavedeluna]

Given the caveat, my personal preference would be to not accept this change.

[andrecsilva]

Given the caveat, my personal preference would be to not accept this change.

I'm happy to say that I was mistaken and it is actually working as intended. You can check for yourself with the following small example:
@drdavella, @clavedeluna

import libcst as cst
from libcst.codemod import Codemod, CodemodContext
from codemodder.codemods.utils_mixin import NameResolutionMixin


class A(Codemod, NameResolutionMixin):
    def transform_module_impl(self, tree):
        node = tree.body[0].body[0].value
        print(node)
        print(self.is_builtin_function(node))
        print(self.find_base_name(node))
        return tree

tree = cst.parse_module("foo()")
A(CodemodContext()).transform_module(tree)

[sonarqubecloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[clavedeluna]

nice! 👏