PyCQA / isort

A Python utility / library to sort imports.
https://pycqa.github.io/isort/
MIT License
6.49k stars 580 forks source link

--lines-before-import inconsistent between writing to a stream compared to a file in some cases #2156

Open Aphosis opened 1 year ago

Aphosis commented 1 year ago

Hi,

First of all I'm sorry if this issue is a bit of a mess, I dug into isort quite a bit to try to get to the bottom of the issue, and the process might not be as straightforward as it could.

The symptom of the issue is that there is a small difference in behavior between running isort when writing to a file directly, compared to writing to a stream.

In my testing, it's at least easily reproducible with the --lines-before-import option: it removes some newlines when streaming, in some cases.

I did not test this on 5.12 as I'm stuck with Python 3.7 for now, but I didn't see anything mentioning this issue in the changelog. So for this test, I was on 5.11.5.

Here are the steps to recreate the environment I tested this in:

mkdir isort-sandbox
cd isort-sandbox/
python3 -m venv venv
source venv/bin/activate
pip install isort

Here is a minimal script to reproduce the issue, called script.py in later sections:

def my_function():
    def inner_function():
        return 1

    # A comment
    return inner_function()

The line before # A comment gets removed when running in stream mode, for exemple the following command:

isort --lines-before-import=1 --diff - --file script.py < script.py

...produces this diff:

--- script.py:before    2023-07-13 14:09:33.457270
+++ script.py:after     2023-07-13 14:10:35.409390
@@ -1,6 +1,5 @@
 def my_function():
     def inner_function():
         return 1
-
     # A comment
     return inner_function()

Whereas running this command:

isort --lines-before-import=1 --diff script.py

...does not produce any output.

Furthermore, running isort on the file directly, but with the --stdout flag, results in the same issue: the newline disappears.

isort --lines-before-import=1 --stdout script.py 

...produces this output:

def my_function():
    def inner_function():
        return 1
    # A comment
    return inner_function()

After digging into isort.api and isort.core a bit, I think this may be because when streaming directly to output, we don't have the chance to ensure we actually changed anything relevant in import blocks, whereas when we write to file after the fact (the regular isort script.py call), we first check if we made any changes to import blocks before computing a diff and applying it to the file.

As for why the newline disappears in the first place, I think the issue is caused by the different indentation levels: for example, if I add a pass just before the comment then there is no longer a diff when running in stream mode:

def my_function():
    def inner_function():
        return 1

    pass
    # A comment
    return inner_function()

From debugging the code, without the pass statement the comment was considered an import_section, which made isort ignore my new line.

Here's the relevant condition: https://github.com/PyCQA/isort/blob/5.11.5/isort/core.py#L284

Which has the consequence of not writing lines above the comment (in my example, a newline): https://github.com/PyCQA/isort/blob/5.11.5/isort/core.py#L365

So, there is really to separate issues:

  1. isort may produce an output that is not clean in some cases (especially since the example script did not have any import, I would expect no difference between the input and output)
  2. Writing directly to a stream does not allow us to safeguard against such edge cases.

I don't know if there is a good solution for both those issues, but solving 1. kind of makes 2. irrelevant.

For what it's worth, in this particular case using the --treat-all-comment-as-code flag allows us to keep the newline.

Python version: 3.7.7 isort version: 5.11.5

EDIT: Possibly the same root cause as #2095 ?

yosida95 commented 1 year ago

I encountered this issue on the latest version of isort (5.12.0) and Python 3.10.