PyCQA / redbaron

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

Indentation error when adding multiple lines #150

Open sdh4 opened 6 years ago

sdh4 commented 6 years ago

RedBaron generates incorrect indentation when inserting multiple lines from indented code blocks into unindented code blocks, where there are blank lines present in the lines being inserted. Here is a testcase:

from redbaron import RedBaron
code1=RedBaron("\ndef foo():\n  fiddle()\n\n  faddle()\n")
code2=RedBaron("bar()\n")
code2.extend(code1[1].value)
print(code2.dumps())

And the actual results:

bar()
fiddle()

  faddle()

The faddle() line obviously should not be indented.

The problem seems to arise from LineProxyList's _generate_expected_list(), where eiither (a) the endl being transferred in ought to have the indentation spaces following it reset but doesn't. and/or (b) the .indentation field of the subsequent line (faddle(), above) should be reset, but this never happens.

sdh4 commented 6 years ago

Looking a little more closely, there seems to be a typo in the code for LineProxyList's _generate_expected_list() that should deal with this scenario.

Specifically the code block:

if previous and previous.type == "endl" and i[0].type != "endl" and previous.indentation != indentation:
                log("Previous is endl and current isn't endl and identation isn't correct, fix it")
                previous.indent = indentation

In this context "indentation" is the desired indentation for the current code block. "previous" is the immediately prior node and "previous.indentation" is a property that looks up the number of trailing spaces on the endl separator prior to previous (unless previous was an "endl" in which previous.indentation is None). i[0] is the node we are operating on.

In the assignment, previous.indent -- i.e. the number of spaces following the endl is set to the proper indentation, and the log message seems to imply that this is fixing up an indentation mismatch. But it is assigning previous.indent whereas the test in the if statement is on previous.indentation.

I think the conditional is a typo and was intended to be "previous.indent != indentation".

Fixing this (patch below) solves this issue. In addition, all of the testcases in the source tree pass.

--- base_nodes.py.orig  2017-01-02 15:59:58.000000000 -0600
+++ base_nodes.py   2017-11-16 07:45:47.716985388 -0600
@@ -1746,7 +1757,7 @@
             expected_list.append(i[0])
             log("-- current result: %s", ["".join(map(lambda x: x.dumps(), expected_list))])

-            if previous and previous.type == "endl" and i[0].type != "endl" and previous.indentation != indentation:
+            if previous and previous.type == "endl" and i[0].type != "endl" and previous.indent != indentation:
                 log("Previous is endl and current isn't endl and identation isn't correct, fix it")
                 previous.indent = indentation