microsoft / vscode-isort

Import sorting for python using the isort library.
https://marketplace.visualstudio.com/items?itemName=ms-python.isort
MIT License
87 stars 20 forks source link

Undesired behavior on files with `# isort: skip_file` #415

Closed jicruz96 closed 2 months ago

jicruz96 commented 3 months ago

Diagnostic Data

Behavior

Expected Behavior

When I save a file with a # isort: skip_file comment, I expect the isort extension to:

  1. ignore the file and
  2. tell me why it ignored it with a simple warning such as Skipping file with 'skip_file' comment: <filename>.

Actual Behavior

  1. I get an unhelpful warning message with a nearly-illegible traceback.
Screenshot 2024-07-05 at 12 07 56 AM
  1. ... and the isort extension itself encounters a separate failure under the hood
Screenshot 2024-07-05 at 12 10 24 AM

Reproduction Steps:

  1. Ensure your isort extension is running in the current workspace and configured to organize imports on save.
  2. Create a .py file with the comment # isort: skip_file anywhere in the file. Anywhere is good. The file doesn't need code, even.
# tmp.py
# isort: skip_file
  1. Save the file to trigger a request to the isort extension.
  2. You will see an unhelpful warning message if your notifications are on and an UnboundLocalError in the isort logs.

Logs:

Click here for detailed logs ``` 2024-07-04 23:46:22.559 [info] [Trace - 11:46:22 PM] Received notification 'window/logMessage'. 2024-07-04 23:46:22.559 [info] Params: { "type": 4, "message": "~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/bin/python -m isort - --check --filename ~/tmp/base/_typing.py" } 2024-07-04 23:46:22.559 [info] ~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/bin/python -m isort - --check --filename ~/tmp/base/_typing.py 2024-07-04 23:46:22.560 [info] [Trace - 11:46:22 PM] Received notification 'window/logMessage'. 2024-07-04 23:46:22.560 [info] Params: { "type": 4, "message": "CWD Linter: ~/tmp" } 2024-07-04 23:46:22.560 [info] CWD Linter: ~/tmp 2024-07-04 23:46:22.560 [info] [Trace - 11:46:22 PM] Received notification 'window/logMessage'. 2024-07-04 23:46:22.560 [info] Params: { "type": 2, "message": "Traceback (most recent call last):\n File \"~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/api.py\", line 210, in sort_stream\n changed = core.process(\n ^^^^^^^^^^^^^\n File \"~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/core.py\", line 162, in process\n raise FileSkipComment(\"Passed in content\")\nisort.exceptions.FileSkipComment: Passed in content contains a file skip comment and was skipped.\n\nDuring handling of the above exception, another exception occurred:\n\nTraceback (most recent call last):\n File \"~/.vscode/extensions/ms-python.isort-2023.10.1/bundled/tool/lsp_server.py\", line 644, in _run_tool_on_document\n result = utils.run_module(\n ^^^^^^^^^^^^^^^^^\n File \"~/.vscode/extensions/ms-python.isort-2023.10.1/bundled/tool/lsp_utils.py\", line 147, in run_module\n return _run_module(module, argv, use_stdin, source)\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n File \"~/.vscode/extensions/ms-python.isort-2023.10.1/bundled/tool/lsp_utils.py\", line 132, in _run_module\n runpy.run_module(module, run_name=\"__main__\")\n File \"\", line 229, in run_module\n File \"\", line 88, in _run_code\n File \"~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/__main__.py\", line 3, in \n main()\n File \"~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/main.py\", line 1132, in main\n incorrectly_sorted = not api.check_stream(\n ^^^^^^^^^^^^^^^^^\n File \"~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/api.py\", line 264, in check_stream\n changed: bool = sort_stream(\n ^^^^^^^^^^^^\n File \"~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/api.py\", line 218, in sort_stream\n raise FileSkipComment(content_source)\nisort.exceptions.FileSkipComment: ~/tmp/base/_typing.py contains a file skip comment and was skipped.\n" } 2024-07-04 23:46:22.560 [info] [Warn - 11:46:22 PM] Traceback (most recent call last): File "~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/api.py", line 210, in sort_stream changed = core.process( ^^^^^^^^^^^^^ File "~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/core.py", line 162, in process raise FileSkipComment("Passed in content") isort.exceptions.FileSkipComment: Passed in content contains a file skip comment and was skipped. During handling of the above exception, another exception occurred: Traceback (most recent call last): File "~/.vscode/extensions/ms-python.isort-2023.10.1/bundled/tool/lsp_server.py", line 644, in _run_tool_on_document result = utils.run_module( ^^^^^^^^^^^^^^^^^ File "~/.vscode/extensions/ms-python.isort-2023.10.1/bundled/tool/lsp_utils.py", line 147, in run_module return _run_module(module, argv, use_stdin, source) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "~/.vscode/extensions/ms-python.isort-2023.10.1/bundled/tool/lsp_utils.py", line 132, in _run_module runpy.run_module(module, run_name="__main__") File "", line 229, in run_module File "", line 88, in _run_code File "~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/__main__.py", line 3, in main() File "~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/main.py", line 1132, in main incorrectly_sorted = not api.check_stream( ^^^^^^^^^^^^^^^^^ File "~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/api.py", line 264, in check_stream changed: bool = sort_stream( ^^^^^^^^^^^^ File "~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/api.py", line 218, in sort_stream raise FileSkipComment(content_source) isort.exceptions.FileSkipComment: ~/tmp/base/_typing.py contains a file skip comment and was skipped. 2024-07-04 23:46:22.561 [info] [Trace - 11:46:22 PM] Received notification 'window/showMessage'. 2024-07-04 23:46:22.561 [info] Params: { "type": 2, "message": "Traceback (most recent call last):\n File \"~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/api.py\", line 210, in sort_stream\n changed = core.process(\n ^^^^^^^^^^^^^\n File \"~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/core.py\", line 162, in process\n raise FileSkipComment(\"Passed in content\")\nisort.exceptions.FileSkipComment: Passed in content contains a file skip comment and was skipped.\n\nDuring handling of the above exception, another exception occurred:\n\nTraceback (most recent call last):\n File \"~/.vscode/extensions/ms-python.isort-2023.10.1/bundled/tool/lsp_server.py\", line 644, in _run_tool_on_document\n result = utils.run_module(\n ^^^^^^^^^^^^^^^^^\n File \"~/.vscode/extensions/ms-python.isort-2023.10.1/bundled/tool/lsp_utils.py\", line 147, in run_module\n return _run_module(module, argv, use_stdin, source)\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n File \"~/.vscode/extensions/ms-python.isort-2023.10.1/bundled/tool/lsp_utils.py\", line 132, in _run_module\n runpy.run_module(module, run_name=\"__main__\")\n File \"\", line 229, in run_module\n File \"\", line 88, in _run_code\n File \"~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/__main__.py\", line 3, in \n main()\n File \"~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/main.py\", line 1132, in main\n incorrectly_sorted = not api.check_stream(\n ^^^^^^^^^^^^^^^^^\n File \"~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/api.py\", line 264, in check_stream\n changed: bool = sort_stream(\n ^^^^^^^^^^^^\n File \"~/Library/Caches/pypoetry/virtualenvs/puerto-rico-Ip6tKJaL-py3.11/lib/python3.11/site-packages/isort/api.py\", line 218, in sort_stream\n raise FileSkipComment(content_source)\nisort.exceptions.FileSkipComment: ~/tmp/base/_typing.py contains a file skip comment and was skipped.\n" } 2024-07-04 23:46:22.561 [info] [Trace - 11:46:22 PM] Received notification 'window/logMessage'. 2024-07-04 23:46:22.561 [info] Params: { "type": 1, "message": "isort check failed with error:\r\nTraceback (most recent call last):\n File \"~/.vscode/extensions/ms-python.isort-2023.10.1/bundled/tool/lsp_server.py\", line 106, in _linting_helper\n result = _run_tool_on_document(document, use_stdin=True, extra_args=[\"--check\"])\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n File \"~/.vscode/extensions/ms-python.isort-2023.10.1/bundled/tool/lsp_server.py\", line 657, in _run_tool_on_document\n if result.stderr:\n ^^^^^^\nUnboundLocalError: cannot access local variable 'result' where it is not associated with a value\n" } 2024-07-04 23:46:22.561 [info] [Error - 11:46:22 PM] isort check failed with error: Traceback (most recent call last): File "~/.vscode/extensions/ms-python.isort-2023.10.1/bundled/tool/lsp_server.py", line 106, in _linting_helper result = _run_tool_on_document(document, use_stdin=True, extra_args=["--check"]) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "~/.vscode/extensions/ms-python.isort-2023.10.1/bundled/tool/lsp_server.py", line 657, in _run_tool_on_document if result.stderr: ^^^^^^ UnboundLocalError: cannot access local variable 'result' where it is not associated with a value ```

Outcome When Attempting Debugging Steps:

Did running it from the command line work?

A SkipFileComment exception occurs, as expected.

...
isort.exceptions.FileSkipComment: tmp.py contains a file skip comment and was skipped.

But the UnboundLocalError obviously does not occur, as this is an error in the LSP server itself, not isort.

Extra Details

I suspect the UnboundLocalError in the LSP servier is an easy fix, just have to add a return None here.

For my actual concern, the unhelpful error message, I recommend that FileSkipComment print a helpful message.

        except isort.exceptions.SkipFileComment:
            log_warning(f"Skipping file with 'skip_file' comment: {document.path}")
            return None
        ...

I've offered a PR with these changes in #416

jicruz96 commented 3 months ago

I've provided a PR with my suggested fix. Feel free to make changes.