PyCQA / redbaron

Bottom-up approach to refactoring in python
http://redbaron.pycqa.org/
694 stars 74 forks source link

Indentation of except clause changes during insertion #173

Open julienrhodes opened 6 years ago

julienrhodes commented 6 years ago

Please consider adding the pytest attached. I can get my code to be refactored correctly when commenting out line 1861 in base_nodes.py (master as of today) (it's a call to modify_last_indentation in _generate_expected_list). I'm not sure what it is supposed to accomplish, so obviously I'm not suggesting it as a solution.

test_indentation_except.py:

import redbaron

SRC = '''
class One(object):
    def foo(self):
        try:
            print 'hi'
        except ValueError:
            pass
'''

def testInsert():
    r1 = redbaron.RedBaron(SRC)

    my_class = r1.find('class', name='One')
    x1 = my_class.find('except')
    indentation_before = x1.indentation
    my_class.insert(0, 'def __init__(self):\n    pass')

    assert indentation_before == x1.indentation
julienrhodes commented 5 years ago

After some more troubleshooting, I found that in my code-base the following lines needed to be removed. works_for_me.patch.zip

jrs65 commented 5 years ago

@julienrhodes I've been having exactly the same problem as you. I just tried applying your patch to the latest master and it fails a bunch of the tests in tests/test_insert.py. I'm feeling motivated today so maybe I'll try and revise it to something that passes the tests.

julienrhodes commented 5 years ago

Darn, sorry to hear that. I was patching master commit: 165d51f2f7da08dc04076e07179a94cf41ba221d and was unconcerned with any breaking tests at the time.

jrs65 commented 5 years ago

Thanks. I've tried patching that commit and it still doesn't pass the full set of unit tests. There few which fail look like tests of corner cases in the indentation so I guess it's not surprising they fail, presumably the code you removed was there to make them work!

ChillarAnand commented 5 years ago

While refactoring some code, I stumbled on this issue.

from redbaron import RedBaron

code = '''
class Foo:

    def get_nh_tenant(self):
        try:
            x = 1
        except Exception as e:
            x = 2
'''

red = RedBaron(code)
class_ = red.find('ClassNode', name='Foo')
to_json = 'x = 3'
class_.value.append(to_json)
print(red.dumps())

It prints

class Foo:

    def get_nh_tenant(self):
        try:
            x = 1
    except Exception as e:
            x = 2
x = 1

Indentation for except statement and newly added statements are broken.

kenseehart commented 5 years ago

I'm running into the same problem. In my use case, my refactoring code can modify or remove lines, but, as it turns out, it will never (intentionally) change the indentation of existing lines. These changes sometimes case redbaron to incorrectly modify indentation of other lines in an undesirable way, specifically on 'if', 'elif', 'else' blocks in my use case. Unfortunately, I didn't have time to locate and fix the root cause. So based on these assumptions, my hack workaround was this:

def backup_indentation(x):
    d = {}
    ifa = x.find_all(['if', 'elif', 'else'])
    for z in ifa:
        if len(z.indentation) > 0:
            d[id(z)] = len(z.indentation)
    return d

def restore_indentation(x, d):
    ifa = x.find_all(['if', 'elif', 'else'])
    for z in ifa:
        if len(z.indentation) < d[id(z)]:
            z.increase_indentation(d[id(z)]-len(z.indentation))
        if len(z.indentation) > d[id(z)]:
            z.decrease_indentation(len(z.indentation)-d[id(z)])

...
saved_indentation = backup_indentation(node)
... change stuff
restore_indentation(node, saved_indentation)

This is just a hack that I found helpful. Your mileage may vary, and you might need to change the assumptions to suit your needs. Also, this hack is not suitable for a general-purpose tool, but may be okay for narrow use cases.