PyCQA / redbaron

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

Can't insert_after a node inserted previously #82

Open andreparames opened 8 years ago

andreparames commented 8 years ago

Test case:

from redbaron import RedBaron
rb = RedBaron('''
  class A(object):
      a = 1
      b = 2
      c = 3
''')

node_b = rb.find('assignment', target=lambda x: x.dumps() == 'b')
node_b.insert_after('d = -1')
node_d = rb.find('assignment', target=lambda x: x.dumps() == 'd')
node_d.insert_after('o = 3')

Result:

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-6-681d36ee36d7> in <module>()
----> 1 node_d.insert_after('o = 3')

/usr/local/lib/python2.7/dist-packages/redbaron.py in insert_after(self, value, offset)
   1014 
   1015     def insert_after(self, value, offset=0):
-> 1016         self.parent.insert(self.index_on_parent + 1 + offset, value)
   1017 
   1018 

TypeError: unsupported operand type(s) for +: 'NoneType' and 'int'

Debugging the code, it seems the problem is that when d is inserted, the parent gets set to the node_list, instead of the ClassNode that the other attributes have as their parent. That prevents the index_on_parent from being calculated for that node.

The following patch seems to fix it, but I'm not sure it's safe:

diff --git a/redbaron.py b/redbaron.py
index 1890cf6..0aec875 100644
--- a/redbaron.py
+++ b/redbaron.py
@@ -1293,7 +1293,7 @@ class ProxyList(object):
         return len(self.data)

     def insert(self, index, value):
-        value = self._convert_input_to_node_object(value, parent=self.node_list, on_attribute=self.on_attribute)
+        value = self._convert_input_to_node_object(value, parent=self.parent, on_attribute=self.on_attribute)
         self.data.insert(index, value)
         self._diff_augmented_list()
JMCanning78 commented 7 years ago

I find similar behavior but it depends on how I initialize the nodelist object. Copied nodelists have the problem but not ones built by parsing code. Here's a trace to illustrate that:

>>> from redbaron import RedBaron
>>> test = RedBaron('#!python')
>>> test[-1].insert_after('# comment')
>>> test[-1].insert_after('# comment')
>>> test
0   #!python
1   # comment
2   # comment

>>> copy = RedBaron('#!python').copy()
>>> copy
0   #!python

>>> copy[-1].insert_after('# comment')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Library/Python/2.7/site-packages/redbaron/base_nodes.py", line 1095, in insert_after
    self.parent.insert(self.index_on_parent + 1 + offset, value)
AttributeError: 'NoneType' object has no attribute 'insert'

So I can insert multiple nodes in sequence, but it seems to depend on how the nodelist was constructed in the first place.

I tried using the patch @andreparames suggested on Dec 10, 2015, but that didn't fix my copied nodelist example.