Bowler icon indicating copy to clipboard operation
Bowler copied to clipboard

Module name in decorator isn't renamed

Open orsinium opened this issue 5 years ago • 5 comments

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.

orsinium avatar Jun 26 '19 16:06 orsinium

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

thatch avatar Oct 05 '20 23:10 thatch

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

markrofail avatar Oct 08 '20 11:10 markrofail

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

import testbefore

@testbefore()
class A:
    pass

also fails.

markrofail avatar Oct 18 '20 16:10 markrofail

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 avatar Oct 19 '20 23:10 amyreese

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.

amyreese avatar Oct 19 '20 23:10 amyreese