Isort: Exit code is still zero if there are no files to scan

[lntuition]

First of all, I always thank you for making a good library :+1:

I found weird working in my view. If user made wrong input and there is no file to check, exit code is still 0 the exit code is still 0. (Of course, this situation is not normal :disappointed: ) I'm not sure that this situation was intended design. In my opinion, exit code also should not be 0.

Belows are simple example of above situation

$ isort --check-only "not_exist"
WARNING: Unable to parse file not_exist due to [Errno 2] No such file or directory: '/home/jsh/not_exist'
$ echo $?
0

I'd like to hear others opinions. Thank you so much :smile:

[anirudnits]

I think its logical and can take this on, if nobody is looking into it that is.

[lntuition]

@anirudnits Thank you for opinions. I found some point about this problem (But i'm not sure, that is the right direction). Can i take this, if you haven't already started?

[anirudnits]

@lntuition sure thing 👍 that's why I asked to check if anybody has already started on this :)

[lntuition]

@anirudnits How kind of you :+1: Anyway, I think this problem should be a long way to me :cry:

[lntuition]

I thought of the following solutions.

1. sort_imports return None if warn exist.
2. So if all of return of sort_imports is None, it should exit with non zero.

So I fixed code like below. But it broken exist testsuite.
I'm noob on this project :( Can anyone help me?

diff --git a/isort/main.py b/isort/main.py
index cd71a40a..56462189 100644
--- a/isort/main.py
+++ b/isort/main.py
@@ -899,6 +899,7 @@ def main(argv: Optional[Sequence[str]] = None, stdin: Optional[TextIOWrapper] =
                 for file_name in file_names
             )

+        is_no_attempt = True
         for sort_attempt in attempt_iterator:
             if not sort_attempt:
                 continue  # pragma: no cover - shouldn't happen, satisfies type constraint
@@ -909,6 +910,10 @@ def main(argv: Optional[Sequence[str]] = None, stdin: Optional[TextIOWrapper] =
                 num_skipped += (
                     1  # pragma: no cover - shouldn't happen, due to skip in iter_source_code
                 )
+            is_no_attempt = False
+
+        if is_no_attempt:
+            sys.exit(1)

         num_skipped += len(skipped)
         if num_skipped and not arguments.get("quiet", False):

[anirudnits]

@lntuition what if there a number of files as input, some are valid files while some are missing like

isort existent.py non_existent.py

should the exit code be zero or non-zero?

[timothycrosley]

@anirudnits and @lntuition I think maybe for now:

1. Error only if all files are missing
2. Add a note to the existing warning that missing files will become errors in the future.
3. Tag this issue as an idea for the the next major release, and then when 6.0.0 is on the horizon we can think about changing the warning to an error if any files passed in are missing.

The reason I'm thinking this is that not allowing any missing files without returning an error code seems like too big a departure from the CLI interactions for anything but a major release.

[anirudnits]

@timothycrosley makes sense 👍

[lntuition]

@timothycrosley I agree :smile:

My previous solution was intented Error only if all files are missing. Hmm... Do I miss something? Maybe assumption that all of sort_attempt is None doesn't mean all files missing?

I'm sorry if it looks like a elementary question :(

[timothycrosley]

@lntuition the structure of your diff looks good! These are just clarifications if you indent to make a complete PR :) If you do plan on making a PR and are having trouble with tests in particular, my recommendation would be to make the PR with what you have against the repo adding [wip] to it, and then we can provide feedback on how to fix the test errors.

[lntuition]

I want to solve this problem 😅 (Also i don't know if it is possible on my own..) I made PR #1485. I'll try hard 💪

[lntuition]

Tag this issue as an idea for the the next major release, and then when 6.0.0 is on the horizon we can think about changing the warning to an error if any files passed in are missing.

This issue is partially solved by #1485. But I wouldn't close, because we dicussed above feature which will be added to 6.0.0 version. If you don't think so, can close this issue anytime :smile:

[timothycrosley]

Since I don't anticipate releasing 6.0.0 until a year from now at the earliest (as 5 was released relatively recently) I've been marking tickets with this label and then closing them to keep the open issues as manageable as possible while not missing any good ideas for the 6.0.0 release.