Skip to content

Module name in decorator isn't renamed #90

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
orsinium opened this issue Jun 26, 2019 · 5 comments · May be fixed by #159
Open

Module name in decorator isn't renamed #90

orsinium opened this issue Jun 26, 2019 · 5 comments · May be fixed by #159
Assignees
Labels
good first issue Good for newcomers

Comments

@orsinium
Copy link

Input file:

import attr


@attr.s()
class A:
  ...

Code:

import bowler
bowler.Query('tmp.py').select_module('attr').rename('testme').execute(write=True, silent=True)

Output file:

import testme


@attr.s()
class A:
  ...

Attr was renamed in the import statement, but not in the decorator.

@thatch thatch added the good first issue Good for newcomers label Oct 5, 2020
@thatch
Copy link
Contributor

thatch commented Oct 5, 2020

A test for this should go near https://github.com/facebookincubator/Bowler/blob/master/bowler/tests/query.py#L187

To see what the lib2to3 pattern would look like, you can use python -m bowler dump [--selector-pattern] <filename>.

The bug is probably a missing piece of the pattern in https://github.com/facebookincubator/Bowler/blob/master/bowler/query.py#L120 that involves {dotted_name}

Getting started docs are at https://github.com/facebookincubator/Bowler/blob/master/CONTRIBUTING.md

@markrofail
Copy link

would like to contribute to this. Can I be assigned?

@markrofail
Copy link

I don't think the problem is in {dotted_name}

import testbefore

@testbefore()
class A:
    pass

also fails.

@amyreese
Copy link
Contributor

The selector will need to get more complicated/thorough to catch cases in decorators, since decorators (pre 3.9) have a restricted syntax and use different pieces of the grammar. You can see this by dumping the full parsed CST from your example:

(.venv) jreese@garbodor ~/workspace/bowler master± » cat foo.py
import testbefore

@testbefore()
class A:
    pass

@testbefore.foo()
def func():
    pass
(.venv) jreese@garbodor ~/workspace/bowler master± » bowler dump foo.py
foo.py
[file_input] ''
.  [simple_stmt] ''
.  .  [import_name] ''
.  .  .  [NAME] '' 'import'
.  .  .  [NAME] ' ' 'testbefore'
.  .  [NEWLINE] '' '\n'
.  [decorated] '\n'
.  .  [decorator] '\n'
.  .  .  [AT] '\n' '@'
.  .  .  [NAME] '' 'testbefore'
.  .  .  [LPAR] '' '('
.  .  .  [RPAR] '' ')'
.  .  .  [NEWLINE] '' '\n'
.  .  [classdef] ''
.  .  .  [NAME] '' 'class'
.  .  .  [NAME] ' ' 'A'
.  .  .  [COLON] '' ':'
.  .  .  [suite] ''
.  .  .  .  [NEWLINE] '' '\n'
.  .  .  .  [INDENT] '' '    '
.  .  .  .  [simple_stmt] ''
.  .  .  .  .  [NAME] '' 'pass'
.  .  .  .  .  [NEWLINE] '' '\n'
.  .  .  .  [DEDENT] '\n' ''
.  [decorated] ''
.  .  [decorator] ''
.  .  .  [AT] '' '@'
.  .  .  [dotted_name] ''
.  .  .  .  [NAME] '' 'testbefore'
.  .  .  .  [DOT] '' '.'
.  .  .  .  [NAME] '' 'foo'
.  .  .  [LPAR] '' '('
.  .  .  [RPAR] '' ')'
.  .  .  [NEWLINE] '' '\n'
.  .  [funcdef] ''
.  .  .  [NAME] '' 'def'
.  .  .  [NAME] ' ' 'func'
.  .  .  [parameters] ''
.  .  .  .  [LPAR] '' '('
.  .  .  .  [RPAR] '' ')'
.  .  .  [COLON] '' ':'
.  .  .  [suite] ''
.  .  .  .  [NEWLINE] '' '\n'
.  .  .  .  [INDENT] '' '    '
.  .  .  .  [simple_stmt] ''
.  .  .  .  .  [NAME] '' 'pass'
.  .  .  .  .  [NEWLINE] '' '\n'
.  .  .  .  [DEDENT] '' ''
.  [ENDMARKER] '' ''

Of note is that the selector will need to be expanded to cover use of both the dotted_name and decorator symbols with a single NAME leaf matching the target module. Python grammar is complicated. 😔

@amyreese
Copy link
Contributor

This is also a more general problem with a number of selectors in Bowler where they don't catch 100% of uses of the name, or where being 100% thorough is potentially catching cases where a local could be shadowing the global module name and Bowler doesn't actually know that.

Rupeshkotha added a commit to Rupeshkotha/Bowler that referenced this issue Mar 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants